Reject mixed VF rc and trait config

If two VFs from the same PF are configured by two separate
[pci]device_spec entries then it is possible to define contradicting
resource classes or traits. This patch detects and rejects such
configuration.

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

doc/source/admin/pci-passthrough.rst

@@ -367,6 +367,11 @@ resource class can be customized via the ``resource_class`` tag in the
 tag in that configuration that allows specifying a list of placement traits to
 be added to the resource provider representing the matching PCI devices.
 
+.. note::
+   Having different resource class or traits configuration for VFs under the
+   same parent PF is not supported and the nova-compute service will refuse to
+   start with such configuration.
+
 .. important::
    While nova supported configuring both the PF and its children VFs for PCI
    passthrough in the past, it only allowed consuming either the parent PF or

nova/compute/pci_placement_translator.py

@@ -131,7 +131,25 @@ class PciResourceProvider:
             )
 
         rc = _get_rc_for_dev(dev, dev_spec_tags)
+        if self.resource_class and rc != self.resource_class:
+            raise exception.PlacementPciMixedResourceClassException(
+                new_rc=rc,
+                new_dev=dev.address,
+                current_rc=self.resource_class,
+                current_devs=",".join(
+                    dev.address for dev in self.children_devs)
+            )
+
         traits = _get_traits_for_dev(dev_spec_tags)
+        if self.traits is not None and self.traits != traits:
+            raise exception.PlacementPciMixedTraitsException(
+                new_traits=",".join(sorted(traits)),
+                new_dev=dev.address,
+                current_traits=",".join(sorted(self.traits)),
+                current_devs=",".join(
+                    dev.address for dev in self.children_devs),
+            )
+
         self.children_devs.append(dev)
         self.resource_class = rc
         self.traits = traits

nova/exception.py

@@ -2440,3 +2440,19 @@ class PlacementPciDependentDeviceException(PlacementPciException):
         "[pci]device_spec is not supported. Either the parent PF or its "
         "children VFs can be configured."
     )
+
+
+class PlacementPciMixedResourceClassException(PlacementPciException):
+    msg_fmt = _(
+        "VFs from the same PF cannot be configured with different "
+        "'resource_class' values in [pci]device_spec. We got %(new_rc)s "
+        "for %(new_dev)s and %(current_rc)s for %(current_devs)s."
+    )
+
+
+class PlacementPciMixedTraitsException(PlacementPciException):
+    msg_fmt = _(
+        "VFs from the same PF cannot be configured with different set "
+        "of 'traits' in [pci]device_spec. We got %(new_traits)s for "
+        "%(new_dev)s and %(current_traits)s for %(current_devs)s."
+    )

nova/tests/functional/libvirt/test_pci_in_placement.py

@@ -216,3 +216,71 @@ class PlacementPCIReportingTests(test_pci_sriov_servers._PCIServersTestBase):
             "[pci]device_spec is not supported",
             str(ex)
         )
+
+    def test_sibling_vfs_with_contradicting_resource_classes_rejected(self):
+        # The fake libvirt will emulate on the host:
+        # * one type-PF dev in slot 0 with two type-VF under it
+        pci_info = fakelibvirt.HostPCIDevicesInfo(
+            num_pci=0, num_pfs=1, num_vfs=2)
+        # the config matches the two VFs separately and tries to configure
+        # them with different resource class
+        device_spec = self._to_device_spec_conf(
+            [
+                {
+                    "address": "0000:81:00.1",
+                    "resource_class": "vf1"
+                },
+                {
+                    "address": "0000:81:00.2",
+                    "resource_class": "vf2"
+                },
+            ]
+        )
+        self.flags(group='pci', device_spec=device_spec)
+
+        ex = self.assertRaises(
+            exception.PlacementPciMixedResourceClassException,
+            self.start_compute,
+            hostname="compute1",
+            pci_info=pci_info
+        )
+        self.assertIn(
+            "VFs from the same PF cannot be configured with different "
+            "'resource_class' values in [pci]device_spec. We got "
+            "CUSTOM_VF2 for 0000:81:00.2 and CUSTOM_VF1 for 0000:81:00.1.",
+            str(ex)
+        )
+
+    def test_sibling_vfs_with_contradicting_traits_rejected(self):
+        # The fake libvirt will emulate on the host:
+        # * one type-PF dev in slot 0 with two type-VF under it
+        pci_info = fakelibvirt.HostPCIDevicesInfo(
+            num_pci=0, num_pfs=1, num_vfs=2)
+        # the config matches the two VFs separately and tries to configure
+        # them with different trait list
+        device_spec = self._to_device_spec_conf(
+            [
+                {
+                    "address": "0000:81:00.1",
+                    "traits": "foo",
+                },
+                {
+                    "address": "0000:81:00.2",
+                    "traits": "bar",
+                },
+            ]
+        )
+        self.flags(group='pci', device_spec=device_spec)
+
+        ex = self.assertRaises(
+            exception.PlacementPciMixedTraitsException,
+            self.start_compute,
+            hostname="compute1",
+            pci_info=pci_info
+        )
+        self.assertIn(
+            "VFs from the same PF cannot be configured with different set of "
+            "'traits' in [pci]device_spec. We got CUSTOM_BAR for 0000:81:00.2 "
+            "and CUSTOM_FOO for 0000:81:00.1.",
+            str(ex)
+        )

nova/tests/unit/compute/test_pci_placement_translator.py

@@ -163,3 +163,46 @@ class TestTranslator(test.NoDBTestCase):
             "children VFs can be configured.",
             str(ex),
         )
+
+    def test_mixed_rc_for_sibling_vfs(self):
+        pv = ppt.PlacementView("fake-node")
+        vf1, vf2, vf3, vf4 = [
+            pci_device.PciDevice(
+                address="0000:81:00.%d" % f,
+                parent_addr="0000:71:00.0",
+                dev_type=fields.PciDeviceType.SRIOV_VF)
+            for f in range(0, 4)
+        ]
+
+        pv.add_dev(vf1, {"resource_class": "a", "traits": "foo,bar,baz"})
+        # order is irrelevant
+        pv.add_dev(vf2, {"resource_class": "a", "traits": "foo,baz,bar"})
+        # but missing trait is rejected
+        ex = self.assertRaises(
+            exception.PlacementPciMixedTraitsException,
+            pv.add_dev,
+            vf3,
+            {"resource_class": "a", "traits": "foo,bar"},
+        )
+        self.assertEqual(
+            "VFs from the same PF cannot be configured with different set of "
+            "'traits' in [pci]device_spec. We got CUSTOM_BAR,CUSTOM_FOO for "
+            "0000:81:00.2 and CUSTOM_BAR,CUSTOM_BAZ,CUSTOM_FOO for "
+            "0000:81:00.0,0000:81:00.1.",
+            str(ex),
+        )
+        # as well as additional trait
+        ex = self.assertRaises(
+            exception.PlacementPciMixedTraitsException,
+            pv.add_dev,
+            vf4,
+            {"resource_class": "a", "traits": "foo,bar,baz,extra"}
+        )
+        self.assertEqual(
+            "VFs from the same PF cannot be configured with different set of "
+            "'traits' in [pci]device_spec. We got "
+            "CUSTOM_BAR,CUSTOM_BAZ,CUSTOM_EXTRA,CUSTOM_FOO for 0000:81:00.3 "
+            "and CUSTOM_BAR,CUSTOM_BAZ,CUSTOM_FOO for "
+            "0000:81:00.0,0000:81:00.1.",
+            str(ex),
+        )