Reject devname based device_spec config

We agreed not to support 'devname' based [pci]device_spec configuration
in the new PCI Placement tracking code. So this patch adds a check to
reject those.

blueprint: pci-device-tracking-in-placement
Change-Id: Ifa0dd34506ebc25cfe5bafd6952b72b0008fc741

doc/source/admin/pci-passthrough.rst

@@ -386,4 +386,12 @@ be added to the resource provider representing the matching PCI devices.
    enforce the same rule for the configuration as well and will refuse to
    start if both the parent PF and its VFs are configured.
 
+.. important::
+   While nova supported configuring PCI devices by device name via the
+   ``devname`` parameter in :oslo.config:option:`pci.device_spec` in the past,
+   this proved to be problematic as the netdev name of a PCI device could
+   change for multiple reasons during hypervisor reboot. So since nova 26.0.0
+   (Zed) the nova-compute service will refuse to start with such configuration.
+   It is suggested to use the PCI address of the device instead.
+
 For deeper technical details please read the `nova specification. <https://specs.openstack.org/openstack/nova-specs/specs/zed/approved/pci-device-tracking-in-placement.html>`_

nova/compute/pci_placement_translator.py

@@ -22,6 +22,7 @@ from nova import exception
 from nova.i18n import _
 from nova.objects import fields
 from nova.objects import pci_device
+from nova.pci import devspec
 from nova.pci import manager as pci_manager
 
 
@@ -278,6 +279,18 @@ class PlacementView:
             rp.update_provider_tree(provider_tree)
 
 
+def ensure_no_dev_spec_with_devname(dev_specs: ty.List[devspec.PciDeviceSpec]):
+    for dev_spec in dev_specs:
+        if dev_spec.dev_spec_conf.get("devname"):
+            msg = _(
+                "Invalid [pci]device_spec configuration. PCI Placement "
+                "reporting does not support 'devname' based device "
+                "specification but we got %(dev_spec)s. "
+                "Please use PCI address in the configuration instead."
+            ) % {"dev_spec": dev_spec.dev_spec_conf}
+            raise exception.PlacementPciException(error=msg)
+
+
 def update_provider_tree_for_pci(
     provider_tree: provider_tree.ProviderTree,
     nodename: str,
@@ -317,6 +330,8 @@ def update_provider_tree_for_pci(
         # If tracking is not enabled we just return without touching anything
         return False
 
+    ensure_no_dev_spec_with_devname(pci_tracker.dev_filter.specs)
+
     LOG.debug(
         'Collecting PCI inventories and allocations to track them in Placement'
     )

nova/pci/devspec.py

@@ -12,6 +12,7 @@
 #    under the License.
 
 import abc
+import copy
 import re
 import string
 import typing as ty
@@ -267,6 +268,10 @@ class WhitelistPciAddress(object):
 
 class PciDeviceSpec(PciAddressSpec):
     def __init__(self, dev_spec: ty.Dict[str, str]) -> None:
+        # stored for better error reporting
+        self.dev_spec_conf = copy.deepcopy(dev_spec)
+        # the non tag fields (i.e. address, devname) will be removed by
+        # _init_dev_details
         self.tags = dev_spec
         self._init_dev_details()
 

nova/tests/functional/libvirt/test_pci_in_placement.py

@@ -51,6 +51,12 @@ class PlacementPCIReportingTests(test_pci_sriov_servers._PCIServersTestBase):
         # These tests should not depend on the host's sysfs
         self.useFixture(
             fixtures.MockPatch('nova.pci.utils.is_physical_function'))
+        self.useFixture(
+            fixtures.MockPatch(
+                'nova.pci.utils.get_function_by_ifname',
+                return_value=(None, False)
+            )
+        )
 
     @staticmethod
     def _to_device_spec_conf(spec_list):
@@ -309,3 +315,26 @@ class PlacementPCIReportingTests(test_pci_sriov_servers._PCIServersTestBase):
             inventories={},
             traits={},
         )
+
+    def test_devname_based_dev_spec_rejected(self):
+        device_spec = self._to_device_spec_conf(
+            [
+                {
+                    "devname": "eth0",
+                },
+            ]
+        )
+        self.flags(group='pci', device_spec=device_spec)
+
+        ex = self.assertRaises(
+            exception.PlacementPciException,
+            self.start_compute,
+            hostname="compute1",
+        )
+        self.assertIn(
+            " Invalid [pci]device_spec configuration. PCI Placement reporting "
+            "does not support 'devname' based device specification but we got "
+            "{'devname': 'eth0'}. Please use PCI address in the configuration "
+            "instead.",
+            str(ex)
+        )

nova/tests/unit/compute/test_pci_placement_translator.py

@@ -50,6 +50,7 @@ class TestTranslator(test.NoDBTestCase):
         )
         # So we have a device but there is no spec for it
         pci_tracker.dev_filter.get_devspec = mock.Mock(return_value=None)
+        pci_tracker.dev_filter.specs = []
         # we expect that the provider_tree is not touched as the device without
         # spec is skipped, we assert that with the NonCallableMock
         provider_tree = mock.NonCallableMock()