Merge "libvirt: Add support for virtio-based input devices"

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"
 }

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
+If set, either the ``hw_input_bus`` or ``hw_pointer_model`` image metadata
-this configuration option.
+properties will take precedence over this configuration option.
 
 Related options:
 

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}

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()
 

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(),

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': [],

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':

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(

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',

nova/tests/unit/virt/libvirt/test_driver.py

@@ -5796,25 +5796,38 @@ class LibvirtConnTestCase(test.NoDBTestCase,
 
             cfg = self._get_guest_config_with_graphics()
 
+            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(
-                cfg.devices[6], vconfig.LibvirtConfigGuestRng)
+                    next(devices), vconfig.LibvirtConfigGuestInput)
+
             self.assertIsInstance(
-                cfg.devices[7], vconfig.LibvirtConfigGuestUSBHostController)
+                next(devices), vconfig.LibvirtConfigGuestRng)
             self.assertIsInstance(
-                cfg.devices[8], vconfig.LibvirtConfigMemoryBalloon)
+                next(devices), vconfig.LibvirtConfigGuestUSBHostController)
+            self.assertIsInstance(
+                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')
 

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)
-
+            self._guest_add_keyboard_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)
 
         # 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
+        if pointer_bus:
-        # something other than a USB tablet, there's nothing to do
+            pointer_model = 'tablet'
-        if pointer_model is None and CONF.pointer_model in (None, 'ps2mouse'):
+            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()
+        pointer = vconfig.LibvirtConfigGuestInput()
-        tablet.type = 'tablet'
+        pointer.type = pointer_model
-        tablet.bus = 'usb'
+        pointer.bus = pointer_bus
-        guest.add_device(tablet)
+        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,

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.