diff --git a/nova/tests/fixtures/libvirt.py b/nova/tests/fixtures/libvirt.py index 5ccf01e40f9..c7692b3100a 100644 --- a/nova/tests/fixtures/libvirt.py +++ b/nova/tests/fixtures/libvirt.py @@ -2052,6 +2052,17 @@ def device_lookup_by_name(self, dev_name): return self.pci_info.get_device_by_name(dev_name) def nodeDeviceLookupByName(self, name): + # See bug https://bugs.launchpad.net/nova/+bug/2098892 + # We don't test this by importing the libvirt module because the + # libvirt module is forbidden to be imported into our test + # environment. It is excluded from test-requirements.txt and we + # also use the ImportModulePoisonFixture in nova/test.py to prevent + # use of modules such as libvirt. + if not isinstance(name, str) and name is not None: + raise TypeError( + 'virNodeDeviceLookupByName() argument 2 must be str or ' + f'None, not {type(name)}') + if name.startswith('mdev'): return self.mdev_info.get_device_by_name(name) diff --git a/nova/tests/unit/virt/libvirt/test_host.py b/nova/tests/unit/virt/libvirt/test_host.py index 6fb6f98281c..3cf252ecc12 100644 --- a/nova/tests/unit/virt/libvirt/test_host.py +++ b/nova/tests/unit/virt/libvirt/test_host.py @@ -2128,6 +2128,48 @@ def test_tpool_list_all_connections(self): self.assertIsInstance(domain, tpool.Proxy) self.assertIn(domain.UUIDString(), (uuids.vm1, uuids.vm2)) + def _add_fake_host_devices(self): + self.conn._obj.pci_info = fakelibvirt.HostPCIDevicesInfo( + num_pci=1, num_pfs=2, num_vfs=2, num_mdevcap=3) + mdevs = { + 'mdev_4b20d080_1b54_4048_85b3_a6a62d165c01': + fakelibvirt.FakeMdevDevice( + dev_name='mdev_4b20d080_1b54_4048_85b3_a6a62d165c01', + type_id=fakelibvirt.NVIDIA_11_VGPU_TYPE, + parent=fakelibvirt.MDEVCAP_DEV1_PCI_ADDR), + } + self.conn._obj.mdev_info = fakelibvirt.HostMdevDevicesInfo( + devices=mdevs) + + def test_tpool_list_all_devices(self): + self._add_fake_host_devices() + devs = self.host.list_all_devices( + fakelibvirt.VIR_CONNECT_LIST_NODE_DEVICES_CAP_PCI_DEV) + self.assertEqual(8, len(devs)) + for dev in devs: + self.assertIsInstance(dev, tpool.Proxy) + + def test_tpool_list_pci_devices(self): + self._add_fake_host_devices() + dev_names = self.host.list_pci_devices() + self.assertEqual(8, len(dev_names)) + for name in dev_names: + self.assertIsInstance(name, str) + + def test_tpool_list_mdev_capable_devices(self): + self._add_fake_host_devices() + dev_names = self.host.list_mdev_capable_devices() + self.assertEqual(3, len(dev_names)) + for name in dev_names: + self.assertIsInstance(name, str) + + def test_tpool_list_mediated_devices(self): + self._add_fake_host_devices() + dev_names = self.host.list_mediated_devices() + self.assertEqual(1, len(dev_names)) + for name in dev_names: + self.assertIsInstance(name, str) + class LoadersTestCase(test.NoDBTestCase): diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index fef9d5083e3..55c470b166c 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -7959,13 +7959,27 @@ def _get_pci_passthrough_devices(self): dev.name(): dev for dev in self._host.list_all_devices(flags=dev_flags) } - net_devs = [dev for dev in devices.values() if "net" in dev.listCaps()] + + # NOTE(mnaser): The listCaps() function can raise an exception if the + # device disappeared while we're looping, this method + # returns an empty list rather than raising an exception + # which will remove the device for Nova's resource + # tracker, but that is OK since the device disappeared. + def _safe_list_caps(dev): + try: + return dev.listCaps() + except libvirt.libvirtError: + return [] + + net_devs = [ + dev for dev in devices.values() if "net" in _safe_list_caps(dev) + ] vdpa_devs = [ - dev for dev in devices.values() if "vdpa" in dev.listCaps() + dev for dev in devices.values() if "vdpa" in _safe_list_caps(dev) ] pci_devs = { name: dev for name, dev in devices.items() - if "pci" in dev.listCaps()} + if "pci" in _safe_list_caps(dev)} pci_info = [ self._host._get_pcidev_info( name, dev, net_devs, diff --git a/nova/virt/libvirt/host.py b/nova/virt/libvirt/host.py index 6c1eab17cb5..fa94860db11 100644 --- a/nova/virt/libvirt/host.py +++ b/nova/virt/libvirt/host.py @@ -1486,31 +1486,31 @@ def get_vdpa_device_path( nodedev = self.get_vdpa_nodedev_by_address(pci_address) return nodedev.vdpa_capability.dev_path - def list_pci_devices(self, flags=0): + def list_pci_devices(self, flags: int = 0) -> ty.List[str]: """Lookup pci devices. - :returns: a list of virNodeDevice instance + :returns: a list of strings, names of the virNodeDevice instances """ return self._list_devices("pci", flags=flags) - def list_mdev_capable_devices(self, flags=0): + def list_mdev_capable_devices(self, flags: int = 0) -> ty.List[str]: """Lookup devices supporting mdev capabilities. - :returns: a list of virNodeDevice instance + :returns: a list of strings, names of the virNodeDevice instances """ return self._list_devices("mdev_types", flags=flags) - def list_mediated_devices(self, flags=0): + def list_mediated_devices(self, flags: int = 0) -> ty.List[str]: """Lookup mediated devices. - :returns: a list of strings with the name of the instance + :returns: a list of strings, names of the virNodeDevice instances """ return self._list_devices("mdev", flags=flags) - def _list_devices(self, cap, flags=0): + def _list_devices(self, cap, flags: int = 0) -> ty.List[str]: """Lookup devices. - :returns: a list of virNodeDevice instance + :returns: a list of strings, names of the virNodeDevice instances """ try: return self.get_connection().listDevices(cap, flags) @@ -1533,7 +1533,10 @@ def list_all_devices( :returns: a list of virNodeDevice xml strings. """ try: - return self.get_connection().listAllDevices(flags) or [] + alldevs = [ + self._wrap_libvirt_proxy(dev) + for dev in self.get_connection().listAllDevices(flags)] or [] + return alldevs except libvirt.libvirtError as ex: LOG.warning(ex) return [] diff --git a/releasenotes/notes/unproxied-libvirt-list-devices-7cd218c1a33535c9.yaml b/releasenotes/notes/unproxied-libvirt-list-devices-7cd218c1a33535c9.yaml new file mode 100644 index 00000000000..eaf7b9b1ca8 --- /dev/null +++ b/releasenotes/notes/unproxied-libvirt-list-devices-7cd218c1a33535c9.yaml @@ -0,0 +1,11 @@ +fixes: + - | + `Bug #2091033`_: Fixed calls to libvirt ``listDevices()`` and + ``listAllDevices()`` from potentially blocking all other greenthreads + in ``nova-compute``. Under certain circumstances, it was possible for + the ``nova-compute`` service to freeze with all other greenthreads + blocked and unable to perform any other activities including logging. + This issue has been fixed by wrapping the libvirt ``listDevices()`` + and ``listAllDevices()`` calls with ``eventlet.tpool.Proxy``. + + .. _Bug #2091033: https://bugs.launchpad.net/nova/+bug/2091033