diff options
author | Balazs Gibizer <balazs.gibizer@est.tech> | 2020-10-08 14:27:44 +0200 |
---|---|---|
committer | Stephen Finucane <stephenfin@redhat.com> | 2021-05-17 16:35:50 +0100 |
commit | 90ffc553d7f4152a6a4a8708787150d3c3c40b03 (patch) | |
tree | a2aa07779d721f946023e6b8340c9fc04cdaca62 /nova | |
parent | 0354d4d9f47354e2b4fc0b2343c27e734fe2e494 (diff) | |
download | nova-90ffc553d7f4152a6a4a8708787150d3c3c40b03.tar.gz |
Ignore PCI devices with 32bit domain
Nova and QEMU[1] supports PCI devices with a PCI address that has 16 bit
domain. However there are hypervisors that reports PCI addresses with
32 bit domain. While today we cannot assign these to guests this should
not prevent the nova-compute service to start.
This patch changes the PCI manager to ignore such PCI devices.
Please note that this patch does not change fact that nova does not
allow specifying PCI addresses with 32bit domain in the
[pci]/passthrough_whitelist configuration. Such configuration is still
rejected at nova-compute service startup.
Closes-Bug: #1897528
[1] https://github.com/qemu/qemu/blob/f2a1cf9180f63e88bb38ff21c169da97c3f2bad5/hw/core/qdev-properties.c#L993
Change-Id: I59a0746b864610b6a314078cf5661d3d2b84b1d4
(cherry picked from commit 8c9d6fc8f073cde78b79ae259c9915216f5d59b0)
Diffstat (limited to 'nova')
-rw-r--r-- | nova/conf/pci.py | 8 | ||||
-rw-r--r-- | nova/pci/manager.py | 38 | ||||
-rw-r--r-- | nova/tests/unit/pci/test_manager.py | 23 |
3 files changed, 53 insertions, 16 deletions
diff --git a/nova/conf/pci.py b/nova/conf/pci.py index b812b39676..b383d0a69f 100644 --- a/nova/conf/pci.py +++ b/nova/conf/pci.py @@ -116,7 +116,13 @@ Possible values: ``address`` PCI address of the device. Both traditional glob style and regular - expression syntax is supported. + expression syntax is supported. Please note that the address fields are + restricted to the following maximum values: + + * domain - 0xFFFF + * bus - 0xFF + * slot - 0x1F + * function - 0x7 ``devname`` Device name of the device (for e.g. interface name). Not all PCI devices diff --git a/nova/pci/manager.py b/nova/pci/manager.py index 05930b0beb..c55e732c01 100644 --- a/nova/pci/manager.py +++ b/nova/pci/manager.py @@ -117,8 +117,42 @@ class PciDevTracker(object): devices = [] for dev in jsonutils.loads(devices_json): - if self.dev_filter.device_assignable(dev): - devices.append(dev) + try: + if self.dev_filter.device_assignable(dev): + devices.append(dev) + except exception.PciConfigInvalidWhitelist as e: + # The raised exception is misleading as the problem is not with + # the whitelist config but with the host PCI device reported by + # libvirt. The code that matches the host PCI device to the + # withelist spec reuses the WhitelistPciAddress object to parse + # the host PCI device address. That parsing can fail if the + # PCI address has a 32 bit domain. But this should not prevent + # processing the rest of the devices. So we simply skip this + # device and continue. + # Please note that this except block does not ignore the + # invalid whitelist configuration. The whitelist config has + # already been parsed or rejected in case it was invalid. At + # this point the self.dev_filter representes the parsed and + # validated whitelist config. + LOG.debug( + 'Skipping PCI device %s reported by the hypervisor: %s', + {k: v for k, v in dev.items() + if k in ['address', 'parent_addr']}, + # NOTE(gibi): this is ugly but the device_assignable() call + # uses the PhysicalPciAddress class to parse the PCI + # addresses and that class reuses the code from + # PciAddressSpec that was originally designed to parse + # whitelist spec. Hence the raised exception talks about + # whitelist config. This is misleading as in our case the + # PCI address that we failed to parse came from the + # hypervisor. + # TODO(gibi): refactor the false abstraction to make the + # code reuse clean from the false assumption that we only + # parse whitelist config with + # devspec.PciAddressSpec._set_pci_dev_info() + str(e).replace( + 'Invalid PCI devices Whitelist config:', 'The')) + self._set_hvdevs(devices) @staticmethod diff --git a/nova/tests/unit/pci/test_manager.py b/nova/tests/unit/pci/test_manager.py index 683f307929..c1b26d9726 100644 --- a/nova/tests/unit/pci/test_manager.py +++ b/nova/tests/unit/pci/test_manager.py @@ -22,7 +22,6 @@ from oslo_utils.fixture import uuidsentinel import nova from nova.compute import vm_states from nova import context -from nova import exception from nova import objects from nova.objects import fields from nova.pci import manager @@ -237,7 +236,9 @@ class PciDevTrackerTestCase(test.NoDBTestCase): tracker.update_devices_from_hypervisor_resources(fake_pci_devs_json) self.assertEqual(2, len(tracker.pci_devs)) - def test_update_devices_from_hypervisor_resources_32bit_domain(self): + @mock.patch("nova.pci.manager.LOG.debug") + def test_update_devices_from_hypervisor_resources_32bit_domain( + self, mock_debug): self.flags( group='pci', passthrough_whitelist=[ @@ -261,17 +262,13 @@ class PciDevTrackerTestCase(test.NoDBTestCase): fake_pci_devs_json = jsonutils.dumps(fake_pci_devs) tracker = manager.PciDevTracker(self.fake_context) # We expect that the device with 32bit PCI domain is ignored - # tracker.update_devices_from_hypervisor_resources(fake_pci_devs_json) - # self.assertEqual(0, len(tracker.pci_devs)) - # - # This is the bug 1897528 - ex = self.assertRaises( - exception.PciConfigInvalidWhitelist, - tracker.update_devices_from_hypervisor_resources, - fake_pci_devs_json) - self.assertEqual( - 'Invalid PCI devices Whitelist config: property domain (10000) is ' - 'greater than the maximum allowable value (FFFF).', str(ex)) + tracker.update_devices_from_hypervisor_resources(fake_pci_devs_json) + self.assertEqual(0, len(tracker.pci_devs)) + mock_debug.assert_called_once_with( + 'Skipping PCI device %s reported by the hypervisor: %s', + {'address': '10000:00:02.0', 'parent_addr': None}, + 'The property domain (10000) is greater than the maximum ' + 'allowable value (FFFF).') def test_set_hvdev_new_dev(self): fake_pci_3 = dict(fake_pci, address='0000:00:00.4', vendor_id='v2') |