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

Author: Balazs Gibizer

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