diff --git a/doc/notification_samples/common_payloads/ImageMetaPropsPayload.json b/doc/notification_samples/common_payloads/ImageMetaPropsPayload.json index 3435730b1c10..c970a4c45df8 100644 --- a/doc/notification_samples/common_payloads/ImageMetaPropsPayload.json +++ b/doc/notification_samples/common_payloads/ImageMetaPropsPayload.json @@ -4,5 +4,5 @@ "hw_architecture": "x86_64" }, "nova_object.name": "ImageMetaPropsPayload", - "nova_object.version": "1.6" + "nova_object.version": "1.7" } diff --git a/nova/conf/compute.py b/nova/conf/compute.py index df68f0d82a55..e787424977ac 100644 --- a/nova/conf/compute.py +++ b/nova/conf/compute.py @@ -296,8 +296,8 @@ Generic property to specify the pointer type. Input devices allow interaction with a graphical framebuffer. For example to provide a graphic tablet for absolute cursor movement. -If set, the 'hw_pointer_model' image property takes precedence over -this configuration option. +If set, either the ``hw_input_bus`` or ``hw_pointer_model`` image metadata +properties will take precedence over this configuration option. Related options: diff --git a/nova/notifications/objects/image.py b/nova/notifications/objects/image.py index 4b673ab56652..d35627eb3d2b 100644 --- a/nova/notifications/objects/image.py +++ b/nova/notifications/objects/image.py @@ -123,7 +123,8 @@ class ImageMetaPropsPayload(base.NotificationPayloadBase): # Version 1.4: Added 'mixed' to hw_cpu_policy field # Version 1.5: Added 'hw_tpm_model' and 'hw_tpm_version' fields # Version 1.6: Added 'socket' to hw_pci_numa_affinity_policy - VERSION = '1.6' + # Version 1.7: Added 'hw_input_bus' field + VERSION = '1.7' SCHEMA = { k: ('image_meta_props', k) for k in image_meta.ImageMetaProps.fields} diff --git a/nova/objects/fields.py b/nova/objects/fields.py index 4d03456848d3..8c4fc299aab6 100644 --- a/nova/objects/fields.py +++ b/nova/objects/fields.py @@ -465,6 +465,14 @@ class ImageSignatureKeyType(BaseNovaEnum): ) +class InputBus(BaseNovaEnum): + + USB = 'usb' + VIRTIO = 'virtio' + + ALL = (USB, VIRTIO) + + class MigrationType(BaseNovaEnum): MIGRATION = 'migration' # cold migration @@ -793,7 +801,7 @@ class PointerModelType(BaseNovaEnum): USBTABLET = "usbtablet" - ALL = (USBTABLET) + ALL = (USBTABLET,) class NotificationPriority(BaseNovaEnum): @@ -1236,6 +1244,10 @@ class ImageSignatureKeyTypeField(BaseEnumField): AUTO_TYPE = ImageSignatureKeyType() +class InputBusField(BaseEnumField): + AUTO_TYPE = InputBus() + + class MigrationTypeField(BaseEnumField): AUTO_TYPE = MigrationType() diff --git a/nova/objects/image_meta.py b/nova/objects/image_meta.py index 184c37b862ac..c07b358647c4 100644 --- a/nova/objects/image_meta.py +++ b/nova/objects/image_meta.py @@ -177,14 +177,17 @@ class ImageMetaProps(base.NovaObject): # Version 1.26: Added 'mixed' to 'hw_cpu_policy' field # Version 1.27: Added 'hw_tpm_model' and 'hw_tpm_version' fields # Version 1.28: Added 'socket' to 'hw_pci_numa_affinity_policy' + # Version 1.29: Added 'hw_input_bus' field # NOTE(efried): When bumping this version, the version of # ImageMetaPropsPayload must also be bumped. See its docstring for details. - VERSION = '1.28' + VERSION = '1.29' def obj_make_compatible(self, primitive, target_version): super(ImageMetaProps, self).obj_make_compatible(primitive, target_version) target_version = versionutils.convert_version_to_tuple(target_version) + if target_version < (1, 29): + primitive.pop('hw_input_bus', None) if target_version < (1, 28): policy = primitive.get('hw_pci_numa_affinity_policy', None) if policy == fields.PCINUMAAffinityPolicy.SOCKET: @@ -330,6 +333,9 @@ class ImageMetaProps(base.NovaObject): # This indicates the guest needs UEFI firmware 'hw_firmware_type': fields.FirmwareTypeField(), + # name of the input bus type to use, e.g. usb, virtio + 'hw_input_bus': fields.InputBusField(), + # boolean - used to trigger code to inject networking when booting a CD # image with a network boot image 'hw_ipxe_boot': fields.FlexibleBooleanField(), diff --git a/nova/tests/functional/notification_sample_tests/test_instance.py b/nova/tests/functional/notification_sample_tests/test_instance.py index 3af9f8720567..21c922845427 100644 --- a/nova/tests/functional/notification_sample_tests/test_instance.py +++ b/nova/tests/functional/notification_sample_tests/test_instance.py @@ -1233,7 +1233,7 @@ class TestInstanceNotificationSample( 'nova_object.data': {}, 'nova_object.name': 'ImageMetaPropsPayload', 'nova_object.namespace': 'nova', - 'nova_object.version': u'1.6', + 'nova_object.version': '1.7', }, 'image.size': 58145823, 'image.tags': [], @@ -1329,7 +1329,7 @@ class TestInstanceNotificationSample( 'nova_object.data': {}, 'nova_object.name': 'ImageMetaPropsPayload', 'nova_object.namespace': 'nova', - 'nova_object.version': u'1.6', + 'nova_object.version': '1.7', }, 'image.size': 58145823, 'image.tags': [], diff --git a/nova/tests/unit/notifications/objects/test_notification.py b/nova/tests/unit/notifications/objects/test_notification.py index 2cb97f8a2d7a..a55bbc0b9eb1 100644 --- a/nova/tests/unit/notifications/objects/test_notification.py +++ b/nova/tests/unit/notifications/objects/test_notification.py @@ -387,7 +387,7 @@ notification_object_data = { # ImageMetaProps, so when you see a fail here for that reason, you must # *also* bump the version of ImageMetaPropsPayload. See its docstring for # more information. - 'ImageMetaPropsPayload': '1.6-2be4d0bdd1d19a541c46a0d69e244d3f', + 'ImageMetaPropsPayload': '1.7-652a6048036c0d0cb8740ea62521c459', 'InstanceActionNotification': '1.0-a73147b93b520ff0061865849d3dfa56', 'InstanceActionPayload': '1.8-4fa3da9cbf0761f1f700ae578f36dc2f', 'InstanceActionRebuildNotification': diff --git a/nova/tests/unit/objects/test_image_meta.py b/nova/tests/unit/objects/test_image_meta.py index 4238d056a785..a4b7a33fa826 100644 --- a/nova/tests/unit/objects/test_image_meta.py +++ b/nova/tests/unit/objects/test_image_meta.py @@ -349,6 +349,22 @@ class TestImageMetaProps(test.NoDBTestCase): self.assertRaises(exception.ObjectActionError, obj.obj_to_primitive, '1.0') + def test_obj_make_compatible_input_bus(self): + """Check 'hw_input_bus' compatibility.""" + # assert that 'hw_input_bus' is supported on a suitably new version + obj = objects.ImageMetaProps( + hw_input_bus=objects.fields.InputBus.VIRTIO, + ) + primitive = obj.obj_to_primitive('1.29') + self.assertIn('hw_input_bus', primitive['nova_object.data']) + self.assertEqual( + objects.fields.InputBus.VIRTIO, + primitive['nova_object.data']['hw_input_bus']) + + # and is absent on older versions + primitive = obj.obj_to_primitive('1.28') + self.assertNotIn('hw_input_bus', primitive['nova_object.data']) + def test_obj_make_compatible_video_model(self): # assert that older video models are preserved. obj = objects.ImageMetaProps( diff --git a/nova/tests/unit/objects/test_objects.py b/nova/tests/unit/objects/test_objects.py index 8c80c64d817b..38761463f4dc 100644 --- a/nova/tests/unit/objects/test_objects.py +++ b/nova/tests/unit/objects/test_objects.py @@ -1074,7 +1074,7 @@ object_data = { 'HyperVLiveMigrateData': '1.4-e265780e6acfa631476c8170e8d6fce0', 'IDEDeviceBus': '1.0-29d4c9f27ac44197f01b6ac1b7e16502', 'ImageMeta': '1.8-642d1b2eb3e880a367f37d72dd76162d', - 'ImageMetaProps': '1.28-a55674868f9e319a6b49d688e558d0aa', + 'ImageMetaProps': '1.29-5c02bd7b1e050ef39513d3fca728e543', 'Instance': '2.7-d187aec68cad2e4d8b8a03a68e4739ce', 'InstanceAction': '1.2-9a5abc87fdd3af46f45731960651efb5', 'InstanceActionEvent': '1.4-5b1f361bd81989f8bb2c20bb7e8a4cb4', diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index b84fabb960a2..553203b8734d 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -5796,25 +5796,38 @@ class LibvirtConnTestCase(test.NoDBTestCase, cfg = self._get_guest_config_with_graphics() - self.assertEqual(len(cfg.devices), 9) + if guestarch != fields.Architecture.AARCH64: + self.assertEqual(len(cfg.devices), 9) + else: + # AArch64 adds a keyboard by default + self.assertEqual(len(cfg.devices), 10) + + devices = iter(cfg.devices) + self.assertIsInstance( - cfg.devices[0], vconfig.LibvirtConfigGuestDisk) + next(devices), vconfig.LibvirtConfigGuestDisk) self.assertIsInstance( - cfg.devices[1], vconfig.LibvirtConfigGuestDisk) + next(devices), vconfig.LibvirtConfigGuestDisk) self.assertIsInstance( - cfg.devices[2], vconfig.LibvirtConfigGuestSerial) + next(devices), vconfig.LibvirtConfigGuestSerial) self.assertIsInstance( - cfg.devices[3], vconfig.LibvirtConfigGuestChannel) + next(devices), vconfig.LibvirtConfigGuestChannel) self.assertIsInstance( - cfg.devices[4], vconfig.LibvirtConfigGuestGraphics) + next(devices), vconfig.LibvirtConfigGuestGraphics) self.assertIsInstance( - cfg.devices[5], vconfig.LibvirtConfigGuestVideo) + next(devices), vconfig.LibvirtConfigGuestVideo) + + if guestarch == fields.Architecture.AARCH64: + # AArch64 adds a keyboard by default + self.assertIsInstance( + next(devices), vconfig.LibvirtConfigGuestInput) + self.assertIsInstance( - cfg.devices[6], vconfig.LibvirtConfigGuestRng) + next(devices), vconfig.LibvirtConfigGuestRng) self.assertIsInstance( - cfg.devices[7], vconfig.LibvirtConfigGuestUSBHostController) + next(devices), vconfig.LibvirtConfigGuestUSBHostController) self.assertIsInstance( - cfg.devices[8], vconfig.LibvirtConfigMemoryBalloon) + next(devices), vconfig.LibvirtConfigMemoryBalloon) self.assertEqual(cfg.devices[3].target_name, "com.redhat.spice.0") self.assertEqual(cfg.devices[3].type, 'spicevmc') @@ -6400,6 +6413,18 @@ class LibvirtConnTestCase(test.NoDBTestCase, dev = self._test_guest_add_pointer_device(image_meta) self.assertIsNotNone(dev) + image_meta = {'properties': {'hw_input_bus': 'usb'}} + + dev = self._test_guest_add_pointer_device(image_meta) + self.assertEqual('tablet', dev.type) + self.assertEqual('usb', dev.bus) + + image_meta = {'properties': {'hw_input_bus': 'virtio'}} + + dev = self._test_guest_add_pointer_device(image_meta) + self.assertEqual('tablet', dev.type) + self.assertEqual('virtio', dev.bus) + @mock.patch.object(libvirt_driver.LOG, 'warning') def test_guest_add_pointer_device__fail_with_spice_agent(self, mock_warn): self.flags(enabled=False, group='vnc') @@ -6437,6 +6462,48 @@ class LibvirtConnTestCase(test.NoDBTestCase, str(mock_warn.call_args[0]), ) + def _test_guest_add_keyboard_device(self, image_meta, arch=None): + self.mock_uname.return_value = fakelibvirt.os_uname( + 'Linux', '', '5.4.0-0-generic', '', + arch or fields.Architecture.X86_64) + + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) + guest = vconfig.LibvirtConfigGuest() + guest.os_type = fields.VMMode.HVM + image_meta = objects.ImageMeta.from_dict({'properties': image_meta}) + return drvr._guest_add_keyboard_device(guest, image_meta) + + def test_guest_add_keyboard_device(self): + props = {} + + dev = self._test_guest_add_keyboard_device(props) + self.assertIsNone(dev) + + props = {'hw_input_bus': 'usb'} + + dev = self._test_guest_add_keyboard_device(props) + self.assertEqual('usb', dev.bus) + self.assertEqual('keyboard', dev.type) + + props = {'hw_input_bus': 'virtio'} + + dev = self._test_guest_add_keyboard_device(props) + self.assertEqual('virtio', dev.bus) + self.assertEqual('keyboard', dev.type) + + props = {'hw_architecture': fields.Architecture.AARCH64} + + dev = self._test_guest_add_keyboard_device(props) + self.assertEqual('usb', dev.bus) + self.assertEqual('keyboard', dev.type) + + props = {} + + dev = self._test_guest_add_keyboard_device( + props, arch=fields.Architecture.AARCH64) + self.assertEqual('usb', dev.bus) + self.assertEqual('keyboard', dev.type) + def test_get_guest_config_with_watchdog_action_flavor(self): self.flags(virt_type='kvm', group='libvirt') diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 8490687cc941..d48d0ecbad93 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -6197,17 +6197,6 @@ class LibvirtDriver(driver.ComputeDriver): return True return False - def _guest_add_usb_host_keyboard(self, guest): - """Add USB Host controller and keyboard for graphical console use. - - Add USB keyboard as PS/2 support may not be present on non-x86 - architectures. - """ - keyboard = vconfig.LibvirtConfigGuestInput() - keyboard.type = "keyboard" - keyboard.bus = "usb" - guest.add_device(keyboard) - def _get_guest_config(self, instance, network_info, image_meta, disk_info, rescue=None, block_device_info=None, context=None, mdevs=None, accel_info=None): @@ -6307,14 +6296,7 @@ class LibvirtDriver(driver.ComputeDriver): self._add_video_driver(guest, image_meta, flavor) self._guest_add_pointer_device(guest, image_meta) - - # We want video == we want graphical console. Some architectures - # do not have input devices attached in default configuration. - # Let then add USB Host controller and USB keyboard. - # x86(-64) and ppc64 have usb host controller and keyboard - # s390x does not support USB - if caps.host.cpu.arch == fields.Architecture.AARCH64: - self._guest_add_usb_host_keyboard(guest) + self._guest_add_keyboard_device(guest, image_meta) # Some features are only supported 'qemu' and 'kvm' hypervisor if virt_type in ('qemu', 'kvm'): @@ -6568,11 +6550,25 @@ class LibvirtDriver(driver.ComputeDriver): return add_video_driver def _guest_add_pointer_device(self, guest, image_meta): + """Build the pointer device to add to the instance. + + The configuration is determined by examining the 'hw_input_bus' image + metadata property, the 'hw_pointer_model' image metadata property, and + the '[DEFAULT] pointer_model' config option in that order. + """ + pointer_bus = image_meta.properties.get('hw_input_bus') pointer_model = image_meta.properties.get('hw_pointer_model') - # If the user hasn't requested anything and the host config says to use - # something other than a USB tablet, there's nothing to do - if pointer_model is None and CONF.pointer_model in (None, 'ps2mouse'): + if pointer_bus: + pointer_model = 'tablet' + pointer_bus = pointer_bus + elif pointer_model or CONF.pointer_model == 'usbtablet': + # Handle the legacy 'hw_pointer_model' image metadata property + pointer_model = 'tablet' + pointer_bus = 'usb' + else: + # If the user hasn't requested anything and the host config says to + # use something other than a USB tablet, there's nothing to do return # For backward compatibility, we don't want to error out if the host @@ -6599,12 +6595,37 @@ class LibvirtDriver(driver.ComputeDriver): 'configuration.') return - tablet = vconfig.LibvirtConfigGuestInput() - tablet.type = 'tablet' - tablet.bus = 'usb' - guest.add_device(tablet) + pointer = vconfig.LibvirtConfigGuestInput() + pointer.type = pointer_model + pointer.bus = pointer_bus + guest.add_device(pointer) + # returned for unit testing purposes - return tablet + return pointer + + def _guest_add_keyboard_device(self, guest, image_meta): + """Add keyboard for graphical console use.""" + bus = image_meta.properties.get('hw_input_bus') + + if not bus: + # AArch64 doesn't provide a default keyboard so we explicitly add + # one; for everything else we rely on default (e.g. for x86, + # libvirt will automatically add a PS2 keyboard) + # TODO(stephenfin): We might want to do this for other non-x86 + # architectures + arch = libvirt_utils.get_arch(image_meta) + if arch != fields.Architecture.AARCH64: + return None + + bus = 'usb' + + keyboard = vconfig.LibvirtConfigGuestInput() + keyboard.type = 'keyboard' + keyboard.bus = bus + guest.add_device(keyboard) + + # returned for unit testing purposes + return keyboard def _get_guest_xml(self, context, instance, network_info, disk_info, image_meta, rescue=None, diff --git a/releasenotes/notes/add-hw_input_bus-image-metadata-prop-059bea459dec618e.yaml b/releasenotes/notes/add-hw_input_bus-image-metadata-prop-059bea459dec618e.yaml new file mode 100644 index 000000000000..d06499abb8c9 --- /dev/null +++ b/releasenotes/notes/add-hw_input_bus-image-metadata-prop-059bea459dec618e.yaml @@ -0,0 +1,10 @@ +--- +features: + - | + A new image metadata property, ``hw_input_bus``, has been added. This + allows you to specify the bus used for input devices - a pointer and + keyboard - which are attached to the instance when graphics are enabled + on compute nodes using the libvirt virt driver. Two values are currently + accepted: ``usb`` and ``virtio``. This image metadata property effectively + replaced the ``hw_pointer_model`` image metadata property, which is + nontheless retained for backwards compatibility purposes.