From cab8139498c7ea6b05cfdc8b4997276051b943fc Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Fri, 14 Jul 2017 17:00:27 +0100 Subject: [PATCH] conf: Deprecate 'keymap' options 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. Users should instead rely on their clients and guests to correctly configure this. This is the second part of the three part deprecation cycle for these options. At this point, they are retained but deprecated them and their defaults modified to be unset. This allows us to warn users with libvirt hypervisors that have configured the options about the pitfalls of the option and give them time to prepare migration strategies, if necessary. A replacement option is added to the VMWare group to allow us to retain this functionality for that hypervisor. Combined with the above, this will allow us to remove the options in a future release. [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: I9a50a111ff4911f4364a1b24d646095c72af3d2c Closes-Bug: #1682020 --- nova/conf/spice.py | 7 +++- nova/conf/vmware.py | 14 ++++++++ nova/conf/vnc.py | 11 +++++- .../unit/virt/libvirt/test_fakelibvirt.py | 4 +-- .../tests/unit/virt/vmwareapi/test_vm_util.py | 15 ++++++-- nova/virt/libvirt/driver.py | 27 +++++++++------ nova/virt/vmwareapi/driver.py | 6 ++++ nova/virt/vmwareapi/vm_util.py | 5 ++- ...ecate-keymap-options-b41ad9f33a5923e1.yaml | 34 +++++++++++++++++++ 9 files changed, 105 insertions(+), 18 deletions(-) create mode 100644 releasenotes/notes/deprecate-keymap-options-b41ad9f33a5923e1.yaml diff --git a/nova/conf/spice.py b/nova/conf/spice.py index b640ae59a22c..f68f72941941 100644 --- a/nova/conf/spice.py +++ b/nova/conf/spice.py @@ -143,7 +143,12 @@ Related options: ``server_listen`` using the value of this option. """), cfg.StrOpt('keymap', - default='en-us', + deprecated_for_removal=True, + deprecated_since='18.0.0', + deprecated_reason=""" +Configuring this option forces QEMU to do keymap conversions. These conversions +are lossy and can result in significant issues for users of non en-US +keyboards. Refer to bug #1682020 for more information.""", help=""" A keyboard layout which is supported by the underlying hypervisor on this node. diff --git a/nova/conf/vmware.py b/nova/conf/vmware.py index d0f1bba09930..9dc73fd68a2b 100644 --- a/nova/conf/vmware.py +++ b/nova/conf/vmware.py @@ -180,6 +180,20 @@ Below options should be set to enable VNC client. default=10000, help=""" Total number of VNC ports. +"""), + cfg.StrOpt('vnc_keymap', + default='en-us', + help=""" +Keymap for VNC. + +The keyboard mapping (keymap) determines which keyboard layout a VNC +session should use by default. + +Possible values: + +* A keyboard layout which is supported by the underlying hypervisor on + this node. This is usually an 'IETF language tag' (for example + 'en-us'). """), cfg.BoolOpt('use_linked_clone', default=True, diff --git a/nova/conf/vnc.py b/nova/conf/vnc.py index 608fd55808a3..4446a68791ea 100644 --- a/nova/conf/vnc.py +++ b/nova/conf/vnc.py @@ -38,9 +38,15 @@ Guests will get created with graphical devices to support this. Clients cfg.StrOpt( 'keymap', - default='en-us', deprecated_group='DEFAULT', deprecated_name='vnc_keymap', + deprecated_for_removal=True, + deprecated_since='18.0.0', + deprecated_reason=""" +Configuring this option forces QEMU to do keymap conversions. These conversions +are lossy and can result in significant issues for users of non en-US +keyboards. You should instead use a VNC client that supports Extended Key Event +messages, such as noVNC 1.0.0. Refer to bug #1682020 for more information.""", help=""" Keymap for VNC. @@ -100,6 +106,9 @@ This option sets the public base URL to which client systems will connect. noVNC clients can use this address to connect to the noVNC instance and, by extension, the VNC sessions. +If using noVNC >= 1.0.0, you should use ``vnc_lite.html`` instead of +``vnc_auto.html``. + Related options: * novncproxy_host diff --git a/nova/tests/unit/virt/libvirt/test_fakelibvirt.py b/nova/tests/unit/virt/libvirt/test_fakelibvirt.py index 57acfb7c78ba..5bc603922204 100644 --- a/nova/tests/unit/virt/libvirt/test_fakelibvirt.py +++ b/nova/tests/unit/virt/libvirt/test_fakelibvirt.py @@ -53,8 +53,8 @@ def get_vm_xml(name="testname", uuid=None, source_type='file', - - + + ''' % {'name': name, 'uuid_tag': uuid_tag, diff --git a/nova/tests/unit/virt/vmwareapi/test_vm_util.py b/nova/tests/unit/virt/vmwareapi/test_vm_util.py index 839780f7d01b..f2b2fff1406e 100644 --- a/nova/tests/unit/virt/vmwareapi/test_vm_util.py +++ b/nova/tests/unit/virt/vmwareapi/test_vm_util.py @@ -442,7 +442,7 @@ class VMwareVMUtilTestCase(test.NoDBTestCase): self.assertEqual(0, unit_number) self.assertEqual(1, controller_spec.device.busNumber) - def test_get_vnc_config_spec(self): + def _test_get_vnc_config_spec(self, keymap): fake_factory = fake.FakeFactory() result = vm_util.get_vnc_config_spec(fake_factory, 7) @@ -460,12 +460,23 @@ class VMwareVMUtilTestCase(test.NoDBTestCase): expected.extraConfig.append(remote_display_vnc_port) remote_display_vnc_keymap = fake_factory.create('ns0:OptionValue') - remote_display_vnc_keymap.value = 'en-us' + remote_display_vnc_keymap.value = keymap remote_display_vnc_keymap.key = 'RemoteDisplay.vnc.keyMap' expected.extraConfig.append(remote_display_vnc_keymap) self.assertEqual(expected, result) + def test_get_vnc_config_spec(self): + # TODO(stephenfin): Fold this back in and stop overridding the keymap + # option once we remove the '[vnc] keymap' option + self.flags(vnc_keymap='en-ie', group='vmware') + self._test_get_vnc_config_spec('en-ie') + + def test_get_vnc_config_spec__legacy_keymap(self): + self.flags(keymap='en-uk', group='vnc') + self.flags(vnc_keymap='en-ie', group='vmware') + self._test_get_vnc_config_spec('en-uk') + def _create_fake_vms(self): fake_vms = fake.FakeRetrieveResult() OptionValue = collections.namedtuple('OptionValue', ['key', 'value']) diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 08f0e06826c6..c1dd57925520 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -462,12 +462,11 @@ class LibvirtDriver(driver.ComputeDriver): conf.driver_cache = cache_mode def _do_quality_warnings(self): - """Warn about untested driver configurations. + """Warn about potential configuration issues. - This will log a warning message about untested driver or host arch - configurations to indicate to administrators that the quality is - unknown. Currently, only qemu or kvm on intel 32- or 64-bit systems - is tested upstream. + This will log a warning message for things such as untested driver or + host arch configurations in order to indicate potential issues to + administrators. """ caps = self._host.get_capabilities() hostarch = caps.host.cpu.arch @@ -481,6 +480,18 @@ class LibvirtDriver(driver.ComputeDriver): 'nova/latest/user/support-matrix.html', {'type': CONF.libvirt.virt_type, 'arch': hostarch}) + if CONF.vnc.keymap: + LOG.warning('The option "[vnc] keymap" has been deprecated ' + 'in favor of configuration within the guest. ' + 'Update nova.conf to address this change and ' + 'refer to bug #1682020 for more information.') + + if CONF.spice.keymap: + LOG.warning('The option "[spice] keymap" has been deprecated ' + 'in favor of configuration within the guest. ' + 'Update nova.conf to address this change and ' + 'refer to bug #1682020 for more information.') + def _handle_conn_event(self, enabled, reason): LOG.info("Connection event '%(enabled)d' reason '%(reason)s'", {'enabled': enabled, 'reason': reason}) @@ -5168,9 +5179,6 @@ class LibvirtDriver(driver.ComputeDriver): graphics = vconfig.LibvirtConfigGuestGraphics() graphics.type = "vnc" 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.server_listen guest.add_device(graphics) @@ -5179,9 +5187,6 @@ class LibvirtDriver(driver.ComputeDriver): graphics = vconfig.LibvirtConfigGuestGraphics() graphics.type = "spice" 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) diff --git a/nova/virt/vmwareapi/driver.py b/nova/virt/vmwareapi/driver.py index b7b3bb558cbe..15cb2d376a70 100644 --- a/nova/virt/vmwareapi/driver.py +++ b/nova/virt/vmwareapi/driver.py @@ -92,6 +92,12 @@ class VMwareVCDriver(driver.ComputeDriver): raise Exception(_("Must specify host_ip, host_username and " "host_password to use vmwareapi.VMwareVCDriver")) + if CONF.vnc.keymap: + LOG.warning('The option "[vnc] keymap" has been deprecated in ' + 'favor of the VMWare-specific "[vmware] vnc_keymap" ' + 'option. Please update nova.conf to address this ' + 'change') + self._datastore_regex = None if CONF.vmware.datastore_regex: try: diff --git a/nova/virt/vmwareapi/vm_util.py b/nova/virt/vmwareapi/vm_util.py index bdcf65ab46bc..8ad2b1f8009e 100644 --- a/nova/virt/vmwareapi/vm_util.py +++ b/nova/virt/vmwareapi/vm_util.py @@ -1001,7 +1001,10 @@ def get_vnc_config_spec(client_factory, port): opt_port.value = port opt_keymap = client_factory.create('ns0:OptionValue') opt_keymap.key = "RemoteDisplay.vnc.keyMap" - opt_keymap.value = CONF.vnc.keymap + if CONF.vnc.keymap: + opt_keymap.value = CONF.vnc.keymap + else: + opt_keymap.value = CONF.vmware.vnc_keymap extras = [opt_enabled, opt_port, opt_keymap] diff --git a/releasenotes/notes/deprecate-keymap-options-b41ad9f33a5923e1.yaml b/releasenotes/notes/deprecate-keymap-options-b41ad9f33a5923e1.yaml new file mode 100644 index 000000000000..72a3bc9f4ae7 --- /dev/null +++ b/releasenotes/notes/deprecate-keymap-options-b41ad9f33a5923e1.yaml @@ -0,0 +1,34 @@ +--- +deprecations: + - | + Two keymap-related configuration options have been deprecated: + + - ``[vnc] keymap`` + - ``[spice] keymap`` + + The VNC option affects the libvirt and VMWare virt drivers, while the SPICE + option only affects libvirt. For the libvirt driver, configuring these + options resulted in lossy keymap conversions for the given graphics method. + It is recommended that users should unset these options and configure their + guests as necessary instead. In the case of noVNC, noVNC 1.0.0 should be + used as this provides support for QEMU's Extended Key Event messages. Refer + to `bug #1682020`__ and the `QEMU RFB pull request`__ for more information. + + For the VMWare driver, only the VNC option applies. However, this option is + deprecated and will not affect any other driver in the future. A new option + has been added to the ``[vmware]`` group to replace this: + + - ``[vmware] vnc_keymap`` + + The ``[vnc] keymap`` and ``[spice] keymap`` options will be removed in a + future release. + + __ https://bugs.launchpad.net/nova/+bug/1682020 + __ https://github.com/novnc/noVNC/pull/596 +upgrade: + - | + noVNC 1.0.0 introduced a breaking change in the URLs used to access the + console. Previously, the ``vnc_auto.html`` path was used but it is now + necessary to use the ``vnc_lite.html`` path. When noVNC is updated to + 1.0.0, ``[vnc] novncproxy_base_url`` configuration value must be updated on + each compute node to reflect this change.