openstack/nova
Code Issues Proposed changes

conf: Allow users to unset 'keymap' options

Browse Source 
Defining the 'keymap' option in libvirt results in the '-k' option being
passed through to QEMU [1][2]. This QEMU option has some uses, primarily
for users interacting with QEMU via stdin on the text console. However,
for users interacting with QEMU via VNC or Spice, like nova users do, it
is strongly recommended to never add the "-k" option. Doing so will
force QEMU to do keymap conversions which are known to be lossy. This
disproportionately affects users with non-US keyboard layouts, who would
be better served by relying on the guest OS to manage this.

In the long term, we would like to deprecate these options. However, we
must do this in three parts. This part allows users to unset the options
and warns users who have them set about the side effects. This change is
intended to be backported. A future change will fully deprecate the
options. Finally, after the deprecation cycle has passed, we can remove
these options in their entirety.

[1] https://github.com/libvirt/libvirt/blob/v1.2.9-maint/src/qemu/qemu_command.c#L6985-L6986
[2] https://github.com/libvirt/libvirt/blob/v1.2.9-maint/src/qemu/qemu_command.c#L7215-L7216

Change-Id: I6b1d719db0537b0f53768dbb00a5b4d01c85ba3a
Related-Bug: #1682020
This commit is contained in:
Stephen Finucane 2017-08-23 11:15:17 +01:00
parent 6119f6ff51
commit d983234288
3 changed files with 65 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

53
nova/tests/unit/virt/libvirt/test_driver.py
View File

@ -3566,7 +3566,10 @@ class LibvirtConnTestCase(test.NoDBTestCase,
mock_save.assert_called_with()
def test_get_guest_config_with_vnc(self):
self.flags(enabled=True, group='vnc')
self.flags(enabled=True,
vncserver_listen='10.0.0.1',
keymap='en-ie',
group='vnc')
self.flags(virt_type='kvm', group='libvirt')
self.flags(pointer_model='ps2mouse')
self.flags(enabled=False, group='spice')
@ -3596,7 +3599,9 @@ class LibvirtConnTestCase(test.NoDBTestCase,
self.assertIsInstance(cfg.devices[6],
vconfig.LibvirtConfigMemoryBalloon)
self.assertEqual(cfg.devices[4].type, "vnc")
self.assertEqual(cfg.devices[4].type, 'vnc')
self.assertEqual(cfg.devices[4].keymap, 'en-ie')
self.assertEqual(cfg.devices[4].listen, '10.0.0.1')
def test_get_guest_config_with_vnc_and_tablet(self):
self.flags(enabled=True, group='vnc')
@ -3642,6 +3647,8 @@ class LibvirtConnTestCase(test.NoDBTestCase,
group='libvirt')
self.flags(enabled=True,
agent_enabled=False,
server_listen='10.0.0.1',
keymap='en-ie',
group='spice')
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
@ -3671,8 +3678,10 @@ class LibvirtConnTestCase(test.NoDBTestCase,
self.assertIsInstance(cfg.devices[7],
vconfig.LibvirtConfigMemoryBalloon)
self.assertEqual(cfg.devices[4].type, "tablet")
self.assertEqual(cfg.devices[5].type, "spice")
self.assertEqual(cfg.devices[4].type, 'tablet')
self.assertEqual(cfg.devices[5].type, 'spice')
self.assertEqual(cfg.devices[5].keymap, 'en-ie')
self.assertEqual(cfg.devices[5].listen, '10.0.0.1')
def test_get_guest_config_with_spice_and_agent(self):
self.flags(enabled=False, group='vnc')
@ -3715,6 +3724,42 @@ class LibvirtConnTestCase(test.NoDBTestCase,
self.assertEqual(cfg.devices[5].type, "spice")
self.assertEqual(cfg.devices[6].type, "qxl")
def _test_get_guest_config_with_graphics(self):
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
instance_ref = objects.Instance(**self.test_instance)
image_meta = objects.ImageMeta.from_dict(self.test_image_meta)
disk_info = blockinfo.get_disk_info(CONF.libvirt.virt_type,
instance_ref,
image_meta)
cfg = drvr._get_guest_config(instance_ref, [],
image_meta, disk_info)
return cfg.devices
def test_get_guest_config_with_vnc_no_keymap(self):
self.flags(virt_type='kvm', group='libvirt')
self.flags(enabled=True, keymap=None, group='vnc')
self.flags(enabled=False, group='spice')
devices = self._test_get_guest_config_with_graphics()
for device in devices:
if device.root_name == 'graphics':
self.assertIsInstance(device,
vconfig.LibvirtConfigGuestGraphics)
self.assertEqual('vnc', device.type)
self.assertIsNone(device.keymap)
def test_get_guest_config_with_spice_no_keymap(self):
self.flags(virt_type='kvm', group='libvirt')
self.flags(enabled=True, keymap=None, group='spice')
self.flags(enabled=False, group='vnc')
devices = self._test_get_guest_config_with_graphics()
for device in devices:
if device.root_name == 'graphics':
self.assertIsInstance(device,
vconfig.LibvirtConfigGuestGraphics)
self.assertEqual('spice', device.type)
self.assertIsNone(device.keymap)
@mock.patch.object(host.Host, 'get_guest')
@mock.patch.object(libvirt_driver.LibvirtDriver,
'_get_serial_ports_from_guest')

12
nova/virt/libvirt/driver.py
View File

@ -4950,14 +4950,22 @@ class LibvirtDriver(driver.ComputeDriver):
if CONF.vnc.enabled and guest.virt_type not in ('lxc', 'uml'):
graphics = vconfig.LibvirtConfigGuestGraphics()
graphics.type = "vnc"
graphics.keymap = CONF.vnc.keymap
if CONF.vnc.keymap:
# TODO(stephenfin): There are some issues here that may
# necessitate deprecating this option entirely in the future.
# Refer to bug #1682020 for more information.
graphics.keymap = CONF.vnc.keymap
graphics.listen = CONF.vnc.vncserver_listen
guest.add_device(graphics)
add_video_driver = True
if CONF.spice.enabled and guest.virt_type not in ('lxc', 'uml', 'xen'):
graphics = vconfig.LibvirtConfigGuestGraphics()
graphics.type = "spice"
graphics.keymap = CONF.spice.keymap
if CONF.spice.keymap:
# TODO(stephenfin): There are some issues here that may
# necessitate deprecating this option entirely in the future.
# Refer to bug #1682020 for more information.
graphics.keymap = CONF.spice.keymap
graphics.listen = CONF.spice.server_listen
guest.add_device(graphics)
add_video_driver = True

6
releasenotes/notes/unsettable-keymap-settings-fa831c02e4158507.yaml Normal file
View File

@ -0,0 +1,6 @@
---
fixes:
- |
It is now possible to unset the ``[vnc]keymap`` and ``[spice]keymap``
configuration options. These were known to cause issues for some users
with non-US keyboards and may be deprecated in the future.