From c34a17db6f1ed4469d963a4e6426594602939912 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Tue, 6 Oct 2020 18:06:15 +0100 Subject: [PATCH] libvirt: Only add a USB controller if it's necessary Libvirt will add a USB controller by default on x86-64 hosts. This is legacy behavior due to QEMU's historical behaviour of not providing a way to disable them. However, a USB controller is both unnecessary and in some cases (real-time) actively harmful if there are no guest devices that rely on the USB bus. The libvirt documentation suggests that there are a number of devices that can use a USB bus: input devices, various types of disk device, hub devices, USB serial ports, sound devices, and host passthrough and redirected USB devices. Of these, nova supports only the first two. Add a check for these devices and if present, add a controller, else disable the controller. Change-Id: I5b16d9ddc9cad56b367d1ad94abba95d675840b0 Signed-off-by: Stephen Finucane --- nova/tests/unit/virt/libvirt/test_driver.py | 247 +++++++++++++----- nova/virt/libvirt/driver.py | 34 ++- ...rt-no-usb-controller-2556e3f0881a538f.yaml | 7 + 3 files changed, 213 insertions(+), 75 deletions(-) create mode 100644 releasenotes/notes/libvirt-no-usb-controller-2556e3f0881a538f.yaml diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 6396da57954a..20b09920f13e 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -2738,7 +2738,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertEqual(cfg.os_type, fields.VMMode.HVM) self.assertEqual(cfg.os_boot_dev, ["hd"]) self.assertIsNone(cfg.os_root) - self.assertEqual(len(cfg.devices), 10) + self.assertEqual(len(cfg.devices), 11) self.assertIsInstance(cfg.devices[0], vconfig.LibvirtConfigGuestDisk) self.assertIsInstance(cfg.devices[1], @@ -2758,6 +2758,8 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertIsInstance(cfg.devices[8], vconfig.LibvirtConfigGuestRng) self.assertIsInstance(cfg.devices[9], + vconfig.LibvirtConfigGuestUSBHostController) + self.assertIsInstance(cfg.devices[10], vconfig.LibvirtConfigMemoryBalloon) self.assertEqual(len(cfg.metadata), 1) self.assertIsInstance(cfg.metadata[0], @@ -2919,13 +2921,15 @@ class LibvirtConnTestCase(test.NoDBTestCase, cfg.os_cmdline) self.assertEqual("OpenStack Nova", cfg.os_init_env['product_name']) self.assertIsNone(cfg.os_root) - self.assertEqual(3, len(cfg.devices)) + self.assertEqual(4, len(cfg.devices)) self.assertIsInstance(cfg.devices[0], vconfig.LibvirtConfigGuestFilesys) self.assertIsInstance(cfg.devices[1], vconfig.LibvirtConfigGuestInterface) self.assertIsInstance(cfg.devices[2], vconfig.LibvirtConfigGuestConsole) + self.assertIsInstance(cfg.devices[3], + vconfig.LibvirtConfigGuestUSBHostController) def test_get_guest_config_lxc_with_id_maps(self): self.flags(virt_type='lxc', group='libvirt') @@ -2945,13 +2949,15 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertEqual("console=tty0 console=ttyS0 console=hvc0", cfg.os_cmdline) self.assertIsNone(cfg.os_root) - self.assertEqual(3, len(cfg.devices)) + self.assertEqual(4, len(cfg.devices)) self.assertIsInstance(cfg.devices[0], vconfig.LibvirtConfigGuestFilesys) self.assertIsInstance(cfg.devices[1], vconfig.LibvirtConfigGuestInterface) self.assertIsInstance(cfg.devices[2], vconfig.LibvirtConfigGuestConsole) + self.assertIsInstance(cfg.devices[3], + vconfig.LibvirtConfigGuestUSBHostController) self.assertEqual(len(cfg.idmaps), 2) self.assertIsInstance(cfg.idmaps[0], vconfig.LibvirtConfigGuestUIDMap) @@ -5131,7 +5137,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertEqual(cfg.os_type, fields.VMMode.HVM) self.assertEqual(cfg.os_boot_dev, ["hd"]) self.assertIsNone(cfg.os_root) - self.assertEqual(len(cfg.devices), 10) + self.assertEqual(len(cfg.devices), 11) self.assertIsInstance(cfg.devices[0], vconfig.LibvirtConfigGuestDisk) self.assertIsInstance(cfg.devices[1], @@ -5151,6 +5157,8 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertIsInstance(cfg.devices[8], vconfig.LibvirtConfigGuestRng) self.assertIsInstance(cfg.devices[9], + vconfig.LibvirtConfigGuestUSBHostController) + self.assertIsInstance(cfg.devices[10], vconfig.LibvirtConfigMemoryBalloon) def test_get_guest_config_with_root_device_name(self): @@ -5173,13 +5181,15 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertEqual(cfg.os_type, "uml") self.assertEqual(cfg.os_boot_dev, []) self.assertEqual(cfg.os_root, '/dev/vdb') - self.assertEqual(len(cfg.devices), 3) + self.assertEqual(len(cfg.devices), 4) self.assertIsInstance(cfg.devices[0], vconfig.LibvirtConfigGuestDisk) self.assertIsInstance(cfg.devices[1], vconfig.LibvirtConfigGuestDisk) self.assertIsInstance(cfg.devices[2], vconfig.LibvirtConfigGuestConsole) + self.assertIsInstance(cfg.devices[3], + vconfig.LibvirtConfigGuestUSBHostController) def test_has_uefi_support_not_supported_arch(self): drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) @@ -5607,7 +5617,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, cfg = self._get_guest_config_with_graphics() - self.assertEqual(len(cfg.devices), 7) + self.assertEqual(len(cfg.devices), 8) self.assertIsInstance(cfg.devices[0], vconfig.LibvirtConfigGuestDisk) self.assertIsInstance(cfg.devices[1], @@ -5621,6 +5631,8 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertIsInstance(cfg.devices[5], vconfig.LibvirtConfigGuestRng) self.assertIsInstance(cfg.devices[6], + vconfig.LibvirtConfigGuestUSBHostController) + self.assertIsInstance(cfg.devices[7], vconfig.LibvirtConfigMemoryBalloon) self.assertEqual(cfg.devices[3].type, 'vnc') @@ -5636,7 +5648,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, cfg = self._get_guest_config_with_graphics() - self.assertEqual(len(cfg.devices), 8) + self.assertEqual(len(cfg.devices), 9) self.assertIsInstance(cfg.devices[0], vconfig.LibvirtConfigGuestDisk) self.assertIsInstance(cfg.devices[1], @@ -5652,6 +5664,8 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertIsInstance(cfg.devices[6], vconfig.LibvirtConfigGuestRng) self.assertIsInstance(cfg.devices[7], + vconfig.LibvirtConfigGuestUSBHostController) + self.assertIsInstance(cfg.devices[8], vconfig.LibvirtConfigMemoryBalloon) self.assertEqual(cfg.devices[3].type, "tablet") @@ -5669,7 +5683,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, cfg = self._get_guest_config_with_graphics() - self.assertEqual(len(cfg.devices), 8) + self.assertEqual(len(cfg.devices), 9) self.assertIsInstance(cfg.devices[0], vconfig.LibvirtConfigGuestDisk) self.assertIsInstance(cfg.devices[1], @@ -5685,6 +5699,8 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertIsInstance(cfg.devices[6], vconfig.LibvirtConfigGuestRng) self.assertIsInstance(cfg.devices[7], + vconfig.LibvirtConfigGuestUSBHostController) + self.assertIsInstance(cfg.devices[8], vconfig.LibvirtConfigMemoryBalloon) self.assertEqual(cfg.devices[3].type, 'tablet') @@ -5706,7 +5722,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, expect = {"ppc": "vga", "ppc64": "vga", "ppc64le": "vga", "aarch64": "virtio"} video_type = expect.get(blockinfo.libvirt_utils.get_arch({}), "qxl") - self.assertEqual(len(cfg.devices), 8) + self.assertEqual(len(cfg.devices), 9) self.assertIsInstance(cfg.devices[0], vconfig.LibvirtConfigGuestDisk) self.assertIsInstance(cfg.devices[1], @@ -5722,6 +5738,8 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertIsInstance(cfg.devices[6], vconfig.LibvirtConfigGuestRng) self.assertIsInstance(cfg.devices[7], + vconfig.LibvirtConfigGuestUSBHostController) + self.assertIsInstance(cfg.devices[8], vconfig.LibvirtConfigMemoryBalloon) self.assertEqual(cfg.devices[3].target_name, "com.redhat.spice.0") @@ -5813,7 +5831,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, cfg = drvr._get_guest_config(instance_ref, [], image_meta, disk_info) - self.assertEqual(8, len(cfg.devices)) + self.assertEqual(9, len(cfg.devices)) self.assertIsInstance(cfg.devices[0], vconfig.LibvirtConfigGuestDisk) self.assertIsInstance(cfg.devices[1], @@ -5829,6 +5847,8 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertIsInstance(cfg.devices[6], vconfig.LibvirtConfigGuestRng) self.assertIsInstance(cfg.devices[7], + vconfig.LibvirtConfigGuestUSBHostController) + self.assertIsInstance(cfg.devices[8], vconfig.LibvirtConfigMemoryBalloon) self.assertEqual("tcp", cfg.devices[2].type) @@ -5849,7 +5869,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, cfg = drvr._get_guest_config(instance_ref, [], image_meta, disk_info) - self.assertEqual(10, len(cfg.devices)) + self.assertEqual(11, len(cfg.devices)) self.assertIsInstance(cfg.devices[0], vconfig.LibvirtConfigGuestDisk) self.assertIsInstance(cfg.devices[1], @@ -5869,6 +5889,8 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertIsInstance(cfg.devices[8], vconfig.LibvirtConfigGuestRng) self.assertIsInstance(cfg.devices[9], + vconfig.LibvirtConfigGuestUSBHostController) + self.assertIsInstance(cfg.devices[10], vconfig.LibvirtConfigMemoryBalloon) self.assertEqual("tcp", cfg.devices[2].type) @@ -5909,7 +5931,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, image_meta) cfg = drvr._get_guest_config(instance_ref, [], image_meta, disk_info) - self.assertEqual(10, len(cfg.devices), cfg.devices) + self.assertEqual(11, len(cfg.devices), cfg.devices) self.assertIsInstance(cfg.devices[0], vconfig.LibvirtConfigGuestDisk) self.assertIsInstance(cfg.devices[1], @@ -5929,6 +5951,8 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertIsInstance(cfg.devices[8], vconfig.LibvirtConfigGuestRng) self.assertIsInstance(cfg.devices[9], + vconfig.LibvirtConfigGuestUSBHostController) + self.assertIsInstance(cfg.devices[10], vconfig.LibvirtConfigMemoryBalloon) self.assertEqual("tcp", cfg.devices[2].type) @@ -6145,7 +6169,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, image_meta) cfg = drvr._get_guest_config(instance_ref, [], image_meta, disk_info) - self.assertEqual(len(cfg.devices), 6) + self.assertEqual(len(cfg.devices), 7) self.assertIsInstance(cfg.devices[0], vconfig.LibvirtConfigGuestDisk) self.assertIsInstance(cfg.devices[1], @@ -6157,6 +6181,8 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertIsInstance(cfg.devices[4], vconfig.LibvirtConfigGuestVideo) self.assertIsInstance(cfg.devices[5], + vconfig.LibvirtConfigGuestUSBHostController) + self.assertIsInstance(cfg.devices[6], vconfig.LibvirtConfigMemoryBalloon) self.assertEqual(cfg.devices[3].type, "vnc") @@ -6281,7 +6307,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, image_meta) cfg = drvr._get_guest_config(instance_ref, [], image_meta, disk_info) - self.assertEqual(len(cfg.devices), 10) + self.assertEqual(len(cfg.devices), 11) self.assertIsInstance(cfg.devices[0], vconfig.LibvirtConfigGuestDisk) self.assertIsInstance(cfg.devices[1], @@ -6301,6 +6327,8 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertIsInstance(cfg.devices[8], vconfig.LibvirtConfigGuestRng) self.assertIsInstance(cfg.devices[9], + vconfig.LibvirtConfigGuestUSBHostController) + self.assertIsInstance(cfg.devices[10], vconfig.LibvirtConfigMemoryBalloon) self.assertEqual(cfg.devices[3].type, "tablet") @@ -6323,7 +6351,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, image_meta) cfg = drvr._get_guest_config(instance_ref, [], image_meta, disk_info) - self.assertEqual(len(cfg.devices), 9) + self.assertEqual(len(cfg.devices), 10) self.assertIsInstance(cfg.devices[0], vconfig.LibvirtConfigGuestDisk) self.assertIsInstance(cfg.devices[1], @@ -6339,11 +6367,13 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertIsInstance(cfg.devices[6], vconfig.LibvirtConfigGuestRng) self.assertIsInstance(cfg.devices[7], - vconfig.LibvirtConfigGuestWatchdog) + vconfig.LibvirtConfigGuestUSBHostController) self.assertIsInstance(cfg.devices[8], + vconfig.LibvirtConfigGuestWatchdog) + self.assertIsInstance(cfg.devices[9], vconfig.LibvirtConfigMemoryBalloon) - self.assertEqual("none", cfg.devices[7].action) + self.assertEqual("none", cfg.devices[8].action) def _test_get_guest_usb_tablet(self, vnc_enabled, spice_enabled, os_type, agent_enabled=False, image_meta=None): @@ -6477,7 +6507,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, cfg = drvr._get_guest_config(instance_ref, [], image_meta, disk_info) - self.assertEqual(9, len(cfg.devices)) + self.assertEqual(10, len(cfg.devices)) self.assertIsInstance(cfg.devices[0], vconfig.LibvirtConfigGuestDisk) self.assertIsInstance(cfg.devices[1], @@ -6493,11 +6523,13 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertIsInstance(cfg.devices[6], vconfig.LibvirtConfigGuestRng) self.assertIsInstance(cfg.devices[7], - vconfig.LibvirtConfigGuestWatchdog) + vconfig.LibvirtConfigGuestUSBHostController) self.assertIsInstance(cfg.devices[8], + vconfig.LibvirtConfigGuestWatchdog) + self.assertIsInstance(cfg.devices[9], vconfig.LibvirtConfigMemoryBalloon) - self.assertEqual("none", cfg.devices[7].action) + self.assertEqual("none", cfg.devices[8].action) def test_get_guest_config_with_watchdog_overrides_flavor(self): self.flags(virt_type='kvm', group='libvirt') @@ -6517,7 +6549,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, cfg = drvr._get_guest_config(instance_ref, [], image_meta, disk_info) - self.assertEqual(9, len(cfg.devices)) + self.assertEqual(10, len(cfg.devices)) self.assertIsInstance(cfg.devices[0], vconfig.LibvirtConfigGuestDisk) self.assertIsInstance(cfg.devices[1], @@ -6533,10 +6565,12 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertIsInstance(cfg.devices[6], vconfig.LibvirtConfigGuestRng) self.assertIsInstance(cfg.devices[7], - vconfig.LibvirtConfigGuestWatchdog) + vconfig.LibvirtConfigGuestUSBHostController) self.assertIsInstance(cfg.devices[8], + vconfig.LibvirtConfigGuestWatchdog) + self.assertIsInstance(cfg.devices[9], vconfig.LibvirtConfigMemoryBalloon) - self.assertEqual("pause", cfg.devices[7].action) + self.assertEqual("pause", cfg.devices[8].action) def test_get_guest_config_with_video_driver_image_meta(self): self.flags(virt_type='kvm', group='libvirt') @@ -6547,45 +6581,47 @@ class LibvirtConnTestCase(test.NoDBTestCase, "disk_format": "raw", "properties": {"hw_video_model": "vmvga"}}) - 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) - self.assertEqual(len(cfg.devices), 8) - self.assertIsInstance(cfg.devices[0], - vconfig.LibvirtConfigGuestDisk) - self.assertIsInstance(cfg.devices[1], - vconfig.LibvirtConfigGuestDisk) - self.assertIsInstance(cfg.devices[2], - vconfig.LibvirtConfigGuestSerial) - self.assertIsInstance(cfg.devices[3], - vconfig.LibvirtConfigGuestInput) - self.assertIsInstance(cfg.devices[4], - vconfig.LibvirtConfigGuestGraphics) - self.assertIsInstance(cfg.devices[5], - vconfig.LibvirtConfigGuestVideo) - self.assertIsInstance(cfg.devices[6], - vconfig.LibvirtConfigGuestRng) - self.assertIsInstance(cfg.devices[7], - vconfig.LibvirtConfigMemoryBalloon) - - self.assertEqual(cfg.devices[4].type, "vnc") - self.assertEqual(cfg.devices[5].type, "vmvga") - - def test_get_guest_config_with_qga_through_image_meta(self): - self.flags(virt_type='kvm', group='libvirt') - - drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) - instance_ref = objects.Instance(**self.test_instance) - image_meta = objects.ImageMeta.from_dict({ - "disk_format": "raw", - "properties": {"hw_qemu_guest_agent": "yes"}}) - 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) self.assertEqual(len(cfg.devices), 9) + self.assertIsInstance(cfg.devices[0], + vconfig.LibvirtConfigGuestDisk) + self.assertIsInstance(cfg.devices[1], + vconfig.LibvirtConfigGuestDisk) + self.assertIsInstance(cfg.devices[2], + vconfig.LibvirtConfigGuestSerial) + self.assertIsInstance(cfg.devices[3], + vconfig.LibvirtConfigGuestInput) + self.assertIsInstance(cfg.devices[4], + vconfig.LibvirtConfigGuestGraphics) + self.assertIsInstance(cfg.devices[5], + vconfig.LibvirtConfigGuestVideo) + self.assertIsInstance(cfg.devices[6], + vconfig.LibvirtConfigGuestRng) + self.assertIsInstance(cfg.devices[7], + vconfig.LibvirtConfigGuestUSBHostController) + self.assertIsInstance(cfg.devices[8], + vconfig.LibvirtConfigMemoryBalloon) + + self.assertEqual(cfg.devices[4].type, "vnc") + self.assertEqual(cfg.devices[5].type, "vmvga") + + def test_get_guest_config_with_qga_through_image_meta(self): + self.flags(virt_type='kvm', group='libvirt') + + drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True) + instance_ref = objects.Instance(**self.test_instance) + image_meta = objects.ImageMeta.from_dict({ + "disk_format": "raw", + "properties": {"hw_qemu_guest_agent": "yes"}}) + + 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) + self.assertEqual(10, len(cfg.devices)) self.assertIsInstance(cfg.devices[0], vconfig.LibvirtConfigGuestDisk) self.assertIsInstance(cfg.devices[1], @@ -6603,6 +6639,8 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertIsInstance(cfg.devices[7], vconfig.LibvirtConfigGuestRng) self.assertIsInstance(cfg.devices[8], + vconfig.LibvirtConfigGuestUSBHostController) + self.assertIsInstance(cfg.devices[9], vconfig.LibvirtConfigMemoryBalloon) self.assertEqual(cfg.devices[3].type, "tablet") @@ -6645,7 +6683,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, disk_info = blockinfo.get_disk_info( CONF.libvirt.virt_type, instance, image_meta) cfg = drvr._get_guest_config(instance, [], image_meta, disk_info) - self.assertEqual(len(cfg.devices), 9) + self.assertEqual(10, len(cfg.devices)) self.assertIsInstance( cfg.devices[0], vconfig.LibvirtConfigGuestDisk) self.assertIsInstance( @@ -6663,7 +6701,9 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertIsInstance( cfg.devices[7], vconfig.LibvirtConfigGuestVTPM) self.assertIsInstance( - cfg.devices[8], vconfig.LibvirtConfigMemoryBalloon) + cfg.devices[8], vconfig.LibvirtConfigGuestUSBHostController) + self.assertIsInstance( + cfg.devices[9], vconfig.LibvirtConfigMemoryBalloon) self.assertEqual(cfg.devices[3].type, 'tablet') self.assertEqual(cfg.devices[4].type, 'vnc') @@ -6692,7 +6732,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, cfg = drvr._get_guest_config(instance_ref, [], image_meta, disk_info) - self.assertEqual(len(cfg.devices), 8) + self.assertEqual(9, len(cfg.devices)) self.assertIsInstance(cfg.devices[0], vconfig.LibvirtConfigGuestDisk) self.assertIsInstance(cfg.devices[1], @@ -6708,6 +6748,8 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertIsInstance(cfg.devices[6], vconfig.LibvirtConfigGuestRng) self.assertIsInstance(cfg.devices[7], + vconfig.LibvirtConfigGuestUSBHostController) + self.assertIsInstance(cfg.devices[8], vconfig.LibvirtConfigMemoryBalloon) self.assertEqual(cfg.devices[4].type, "spice") @@ -6864,7 +6906,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, instance_ref, image_meta) cfg = drvr._get_guest_config(instance_ref, [], image_meta, disk_info) - self.assertEqual(len(cfg.devices), 8) + self.assertEqual(len(cfg.devices), 9) self.assertIsInstance(cfg.devices[0], vconfig.LibvirtConfigGuestDisk) self.assertIsInstance(cfg.devices[1], @@ -6880,6 +6922,8 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertIsInstance(cfg.devices[6], vconfig.LibvirtConfigGuestRng) self.assertIsInstance(cfg.devices[7], + vconfig.LibvirtConfigGuestUSBHostController) + self.assertIsInstance(cfg.devices[8], vconfig.LibvirtConfigMemoryBalloon) self.assertEqual(cfg.devices[3].type, "tablet") @@ -6900,7 +6944,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, cfg = drvr._get_guest_config(instance_ref, [], image_meta, disk_info) - self.assertEqual(len(cfg.devices), 7) + self.assertEqual(len(cfg.devices), 8) self.assertIsInstance(cfg.devices[0], vconfig.LibvirtConfigGuestDisk) self.assertIsInstance(cfg.devices[1], @@ -6914,6 +6958,8 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertIsInstance(cfg.devices[5], vconfig.LibvirtConfigGuestRng) self.assertIsInstance(cfg.devices[6], + vconfig.LibvirtConfigGuestUSBHostController) + self.assertIsInstance(cfg.devices[7], vconfig.LibvirtConfigMemoryBalloon) self.assertEqual(cfg.devices[5].model, 'random') @@ -6937,7 +6983,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, cfg = drvr._get_guest_config(instance_ref, [], image_meta, disk_info) - self.assertEqual(len(cfg.devices), 6) + self.assertEqual(len(cfg.devices), 7) self.assertIsInstance(cfg.devices[0], vconfig.LibvirtConfigGuestDisk) self.assertIsInstance(cfg.devices[1], @@ -6949,6 +6995,8 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertIsInstance(cfg.devices[4], vconfig.LibvirtConfigGuestVideo) self.assertIsInstance(cfg.devices[5], + vconfig.LibvirtConfigGuestUSBHostController) + self.assertIsInstance(cfg.devices[6], vconfig.LibvirtConfigMemoryBalloon) def test_get_guest_config_with_rng_limits(self): @@ -6968,7 +7016,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, cfg = drvr._get_guest_config(instance_ref, [], image_meta, disk_info) - self.assertEqual(len(cfg.devices), 7) + self.assertEqual(len(cfg.devices), 8) self.assertIsInstance(cfg.devices[0], vconfig.LibvirtConfigGuestDisk) self.assertIsInstance(cfg.devices[1], @@ -6982,7 +7030,9 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertIsInstance(cfg.devices[5], vconfig.LibvirtConfigGuestRng) self.assertIsInstance(cfg.devices[6], - vconfig.LibvirtConfigMemoryBalloon) + vconfig.LibvirtConfigGuestUSBHostController) + self.assertIsInstance(cfg.devices[7], + vconfig.LibvirtConfigMemoryBalloon) self.assertEqual(cfg.devices[5].model, 'random') self.assertEqual(cfg.devices[5].backend, '/dev/urandom') @@ -7009,7 +7059,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, cfg = drvr._get_guest_config(instance_ref, [], image_meta, disk_info) - self.assertEqual(len(cfg.devices), 7) + self.assertEqual(len(cfg.devices), 8) self.assertIsInstance(cfg.devices[0], vconfig.LibvirtConfigGuestDisk) self.assertIsInstance(cfg.devices[1], @@ -7023,6 +7073,8 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertIsInstance(cfg.devices[5], vconfig.LibvirtConfigGuestRng) self.assertIsInstance(cfg.devices[6], + vconfig.LibvirtConfigGuestUSBHostController) + self.assertIsInstance(cfg.devices[7], vconfig.LibvirtConfigMemoryBalloon) self.assertEqual(cfg.devices[5].model, 'random') @@ -8598,6 +8650,55 @@ class LibvirtConnTestCase(test.NoDBTestCase, break self.assertTrue(no_exist) + def test_get_guest_usb_controller(self): + self.flags(enabled=True, group='vnc') + + 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) + + # we don't have any input manually configured and our disks are using + # the sata bus, but having VNC enabled means a USB tablet will be added + # by default + 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') + + # disabling VNC means we should no longer have a video device present, + # meaning no USB controller is necessary now + self.flags(enabled=False, group='vnc') + + cfg = drvr._get_guest_config(instance_ref, [], image_meta, disk_info) + for device in cfg.devices: + if isinstance(device, vconfig.LibvirtConfigGuestUSBHostController): + self.assertEqual('none', device.model) + break + else: + self.fail('Did not find a USB host controller') + + # but adding a USB-based disk device should necessitate a USB + # controller once again + image_meta = objects.ImageMeta.from_dict( + {'properties': {'hw_disk_bus': 'usb'}}) + 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) @@ -20299,7 +20400,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertEqual(instance_ref.flavor.vcpus, cfg.vcpus) self.assertEqual(fields.VMMode.HVM, cfg.os_type) self.assertIsNone(cfg.os_root) - self.assertEqual(6, len(cfg.devices)) + self.assertEqual(7, len(cfg.devices)) self.assertIsInstance(cfg.devices[0], vconfig.LibvirtConfigGuestDisk) self.assertEqual(cfg.devices[0].driver_format, "ploop") @@ -20313,6 +20414,8 @@ class LibvirtConnTestCase(test.NoDBTestCase, vconfig.LibvirtConfigGuestGraphics) self.assertIsInstance(cfg.devices[5], vconfig.LibvirtConfigGuestVideo) + self.assertIsInstance(cfg.devices[6], + vconfig.LibvirtConfigGuestUSBHostController) def test_get_guest_config_parallels_ct_rescue(self): self._test_get_guest_config_parallels_ct(rescue=True) @@ -20347,9 +20450,9 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertEqual("/sbin/init", cfg.os_init_path) self.assertIsNone(cfg.os_root) if rescue: - self.assertEqual(5, len(cfg.devices)) + self.assertEqual(6, len(cfg.devices)) else: - self.assertEqual(4, len(cfg.devices)) + self.assertEqual(5, len(cfg.devices)) self.assertIsInstance(cfg.devices[0], vconfig.LibvirtConfigGuestFilesys) @@ -20372,6 +20475,8 @@ class LibvirtConnTestCase(test.NoDBTestCase, vconfig.LibvirtConfigGuestGraphics) self.assertIsInstance(cfg.devices[device_index + 3], vconfig.LibvirtConfigGuestVideo) + self.assertIsInstance(cfg.devices[device_index + 4], + vconfig.LibvirtConfigGuestUSBHostController) def _test_get_guest_config_parallels_volume(self, vmmode, devices): self.flags(virt_type='parallels', group='libvirt') @@ -20428,8 +20533,8 @@ class LibvirtConnTestCase(test.NoDBTestCase, self.assertTrue(disk_found) def test_get_guest_config_parallels_volume(self): - self._test_get_guest_config_parallels_volume(fields.VMMode.EXE, 4) - self._test_get_guest_config_parallels_volume(fields.VMMode.HVM, 6) + self._test_get_guest_config_parallels_volume(fields.VMMode.EXE, 5) + self._test_get_guest_config_parallels_volume(fields.VMMode.HVM, 7) @mock.patch('nova.virt.libvirt.driver.LibvirtDriver.' '_guest_add_accel_pci_devices') diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index f1d16ce71aa1..840080006ce9 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -5916,6 +5916,33 @@ class LibvirtDriver(driver.ComputeDriver): cpu_config.features.add(xf) return cpu_config + def _guest_needs_usb(self, guest): + """Evaluate devices currently attached to the guest.""" + for dev in guest.devices: + if isinstance(dev, vconfig.LibvirtConfigGuestDisk): + if dev.target_bus == 'usb': + return True + + if isinstance(dev, vconfig.LibvirtConfigGuestInput): + if dev.bus == 'usb': + return True + + return False + + def _guest_add_usb_root_controller(self, guest): + """Add USB root controller, if necessary. + + Note that these are added by default on x86-64. We add the controller + here explicitly so that we can _disable_ it (by setting the model to + 'none') if it's not necessary. + """ + usbhost = vconfig.LibvirtConfigGuestUSBHostController() + 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' + guest.add_device(usbhost) + def _guest_add_pcie_root_ports(self, guest): """Add PCI Express root ports. @@ -5983,10 +6010,6 @@ class LibvirtDriver(driver.ComputeDriver): keyboard.bus = "usb" guest.add_device(keyboard) - usbhost = vconfig.LibvirtConfigGuestUSBHostController() - usbhost.index = 0 - guest.add_device(usbhost) - 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): @@ -6105,6 +6128,8 @@ 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_pci_devices(guest, instance) pci_arq_list = [] @@ -6125,6 +6150,7 @@ class LibvirtDriver(driver.ComputeDriver): 'But got these unsupported types: %s.', instance.uuid, supported_types_set, ah_types_set.difference(supported_types_set)) + self._guest_add_accel_pci_devices(guest, pci_arq_list) self._guest_add_watchdog_action(guest, flavor, image_meta) diff --git a/releasenotes/notes/libvirt-no-usb-controller-2556e3f0881a538f.yaml b/releasenotes/notes/libvirt-no-usb-controller-2556e3f0881a538f.yaml new file mode 100644 index 000000000000..b05e5368b0c4 --- /dev/null +++ b/releasenotes/notes/libvirt-no-usb-controller-2556e3f0881a538f.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Previously, when using the libvirt driver on x86 hosts, a USB controller + was added by default to all instances even if no guest device actually + required this controller. This has been resolved. A USB controller will + now only be added if an input or disk device requires a USB bus.