Merge "Ignore PCI devices with 32bit domain" into stable/ussuri21.2.2

5 files changed, 78 insertions, 16 deletions

diff --git a/doc/source/admin/networking.rst b/doc/source/admin/networking.rst
index 407a43aafe..626bb89592 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
 <admin/config-sriov.html>`
 
+.. 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 663fdbaf51..c3eb76e68e 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 <https://bugs.launchpad.net/nova/+bug/1512880>`_ 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 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..17792dded4 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 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')