Merge "Ignore PCI devices with 32bit domain" into stable/victoria

Author: Zuul

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
 -------------

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)
 ------------------------

nova/conf/pci.py

@@ -116,7 +116,13 @@
 
   ``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

nova/pci/manager.py

@@ -117,8 +117,42 @@ def update_devices_from_hypervisor_resources(self, devices_json):
 
         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

nova/tests/unit/pci/test_manager.py

@@ -22,7 +22,6 @@
 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 @@ def test_update_devices_from_hypervisor_resources(self, _mock_dev_assign):
         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 @@ def test_update_devices_from_hypervisor_resources_32bit_domain(self):
         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')