From 22daca3e7f33e89b937f03a89db63cf25489e3c9 Mon Sep 17 00:00:00 2001 From: Balazs Gibizer Date: Thu, 8 Oct 2020 14:27:44 +0200 Subject: [PATCH] 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. Changes: nova/pci/manager.py NOTE(stephenfin): We need to replace a use of 'str(exc)' with 'six.text_type(exc)' to keep pep8/Python 2.7 happy. 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) (cherry picked from commit 90ffc553d7f4152a6a4a8708787150d3c3c40b03) --- doc/source/admin/networking.rst | 12 +++++++++ doc/source/admin/pci-passthrough.rst | 12 +++++++++ nova/conf/pci.py | 8 +++++- nova/pci/manager.py | 39 ++++++++++++++++++++++++++-- nova/tests/unit/pci/test_manager.py | 23 +++++++--------- 5 files changed, 78 insertions(+), 16 deletions(-) diff --git a/doc/source/admin/networking.rst b/doc/source/admin/networking.rst index 407a43aafeac..626bb8959226 100644 --- a/doc/source/admin/networking.rst +++ b/doc/source/admin/networking.rst @@ -24,6 +24,18 @@ A full guide on configuring and using SR-IOV is provided in the :neutron-doc:`OpenStack Networking service documentation ` +.. note:: + + Nova only supports PCI addresses where the fields are restricted to the + following maximum value: + + * domain - 0xFFFF + * bus - 0xFF + * slot - 0x1F + * function - 0x7 + + Nova will ignore PCI devices reported by the hypervisor if the address is + outside of these ranges. NUMA Affinity ------------- diff --git a/doc/source/admin/pci-passthrough.rst b/doc/source/admin/pci-passthrough.rst index 663fdbaf518f..c3eb76e68e1b 100644 --- a/doc/source/admin/pci-passthrough.rst +++ b/doc/source/admin/pci-passthrough.rst @@ -37,6 +37,18 @@ devices with potentially different capabilities. supported until the 14.0.0 Newton release, see `bug 1512800 `_ for details. +.. note:: + + Nova only supports PCI addresses where the fields are restricted to the + following maximum value: + + * domain - 0xFFFF + * bus - 0xFF + * slot - 0x1F + * function - 0x7 + + Nova will ignore PCI devices reported by the hypervisor if the address is + outside of these ranges. Configure host (Compute) ------------------------ diff --git a/nova/conf/pci.py b/nova/conf/pci.py index b812b39676a5..b383d0a69fc4 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 05930b0bebb0..17792dded419 100644 --- a/nova/pci/manager.py +++ b/nova/pci/manager.py @@ -19,6 +19,7 @@ import collections from oslo_config import cfg from oslo_log import log as logging from oslo_serialization import jsonutils +import six from nova import exception from nova import objects @@ -117,8 +118,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() + six.text_type(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 683f30792992..c1b26d9726cb 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')