From 7961cc3428b14c92c80e0c435ae8a8ca1d9ea806 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Tue, 2 Feb 2021 15:54:53 +0000 Subject: [PATCH] libvirt: Always enable USB controller on PPC64 In change I5b16d9ddc9cad56b367d1ad94abba95d675840b0, we started disabling the USB controller whenever we deemed it unnecessary. We did this by scanning through the requested devices and identifying those that used the USB bus, only adding the controller if one or more matching devices were found. It transpires that just as QEMU adds PS2 input devices for x86, libvirt automatically adds USB input devices for PPC64 [1]. These devices won't appear in the XML we provide to libvirt, since we, the user, are not the ones requesting them, but the disabling of the instance will result in the instance failing to boot and libvirt emitting the following error message: unsupported configuration: USB is disabled for this domain, but USB devices are present in the domain XML Resolve this issue by bypassing the devices check and leaving configuration of the USB controller to libvirt, as before. [1] https://github.com/libvirt/libvirt/blob/3d42a5766/src/qemu/qemu_domain.c#L3559-L3560 Change-Id: Iefcb4bbe1ae31dcf79a3ba88372b4971d97ad2ea Signed-off-by: Stephen Finucane Closes-Bug: #1914259 --- nova/tests/unit/virt/libvirt/test_driver.py | 15 +++++++++++++++ nova/virt/libvirt/driver.py | 18 ++++++++++++++---- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 77b1073ce7..57496a7b7e 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -8420,6 +8420,21 @@ class LibvirtConnTestCase(test.NoDBTestCase, else: self.fail('Did not find a USB host controller') + # while PPC64 should always get a USB controller, regardless of config + self.mock_get_arch.return_value = fields.Architecture.PPC64 + 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) + for device in cfg.devices: + if isinstance(device, vconfig.LibvirtConfigGuestUSBHostController): + # the model property should be unset, meaning auto-configured + self.assertIsNone(device.model) + break + else: + self.fail('Did not find a USB host controller') + @mock.patch.object(libvirt_driver, 'LOG') @mock.patch.object( fakelibvirt, 'VIR_PERF_PARAM_CPU_CLOCK', 'cpu_clock', create=True) diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index d91b653517..42807f0234 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -5747,6 +5747,10 @@ class LibvirtDriver(driver.ComputeDriver): s390x_archs = (fields.Architecture.S390, fields.Architecture.S390X) return libvirt_utils.get_arch(image_meta) in s390x_archs + def _is_ppc64_guest(self, image_meta): + archs = (fields.Architecture.PPC64, fields.Architecture.PPC64LE) + return libvirt_utils.get_arch(image_meta) in archs + def _create_consoles_qemu_kvm(self, guest_cfg, instance, flavor, image_meta): char_dev_cls = vconfig.LibvirtConfigGuestSerial @@ -5884,8 +5888,12 @@ class LibvirtDriver(driver.ComputeDriver): cpu_config.features.add(xf) return cpu_config - def _guest_needs_usb(self, guest): + def _guest_needs_usb(self, guest, image_meta): """Evaluate devices currently attached to the guest.""" + if self._is_ppc64_guest(image_meta): + # PPC64 guests get a USB keyboard and mouse automatically + return True + for dev in guest.devices: if isinstance(dev, vconfig.LibvirtConfigGuestDisk): if dev.target_bus == 'usb': @@ -5897,7 +5905,7 @@ class LibvirtDriver(driver.ComputeDriver): return False - def _guest_add_usb_root_controller(self, guest): + def _guest_add_usb_root_controller(self, guest, image_meta): """Add USB root controller, if necessary. Note that these are added by default on x86-64. We add the controller @@ -5908,7 +5916,9 @@ class LibvirtDriver(driver.ComputeDriver): usbhost.index = 0 # an unset model means autodetect, while 'none' means don't add a # controller (x86 gets one by default) - usbhost.model = None if self._guest_needs_usb(guest) else 'none' + usbhost.model = None + if not self._guest_needs_usb(guest, image_meta): + usbhost.model = 'none' guest.add_device(usbhost) def _guest_add_pcie_root_ports(self, guest): @@ -6096,7 +6106,7 @@ class LibvirtDriver(driver.ComputeDriver): if self._guest_needs_pcie(guest, caps): self._guest_add_pcie_root_ports(guest) - self._guest_add_usb_root_controller(guest) + self._guest_add_usb_root_controller(guest, image_meta) self._guest_add_pci_devices(guest, instance)