Validated that PCI alias has proper ids

Either the vendor_id and product_id needs to be set or the
resource_class needs to be set in each alias. This is now validated when
the alias is parsed to avoid late failure during placement
allocation_candidates query.

Conflicts:
      nova/conf/pci.py due to I62ec475988eab8de948498f50d8d4c0d47321102
      not on stable/2025.1

Closes-Bug: #2111440
Change-Id: I7fd43b3d6faac8c4098b0983e8adc596414823a1
(cherry picked from commit acc6221)

Author: gibizer

doc/source/admin/pci-passthrough.rst

@@ -494,7 +494,9 @@ configuration option supports requesting devices by Placement resource class
 name via the ``resource_class`` field and also support requesting traits to
 be present on the selected devices via the ``traits`` field in the alias. If
 the ``resource_class`` field is not specified in the alias then it is defaulted
-by nova to ``CUSTOM_PCI_<vendor_id>_<product_id>``.
+by nova to ``CUSTOM_PCI_<vendor_id>_<product_id>``. Either the ``product_id``
+and ``vendor_id`` or the ``resource_class`` field must be provided in each
+alias.
 
 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/conf/pci.py

@@ -70,7 +70,8 @@
     alias = {
       "name": "A16_16A",
       "device_type": "type-VF",
-      resource_class: "CUSTOM_A16_16A",
+      "resource_class": "GPU_VF",
+      "traits": "blue, big"
     }
 
   Valid key values are :
@@ -108,6 +109,8 @@
     in the alias is matched against the ``resource_class`` defined in the
     ``[pci]device_spec``. This field can only be used only if
     ``[filter_scheduler]pci_in_placement`` is enabled.
+    Either the product_id and vendor_id or the resource_class field must be
+    provided in each alias.
 
   ``traits``
     An optional comma separated list of Placement trait names requested to be

nova/pci/request.py

@@ -121,10 +121,7 @@
 }
 
 
-def _validate_aliases(aliases):
-    """Checks the parsed aliases for common mistakes and raise easy to parse
-    error messages
-    """
+def _validate_multispec(aliases):
     if CONF.filter_scheduler.pci_in_placement:
         alias_with_multiple_specs = [
             name for name, spec in aliases.items() if len(spec[1]) > 1]
@@ -138,6 +135,44 @@ def _validate_aliases(aliases):
                 % ",".join(alias_with_multiple_specs))
 
 
+def _validate_required_ids(aliases):
+    if CONF.filter_scheduler.pci_in_placement:
+        alias_without_ids_or_rc = set()
+        for name, alias in aliases.items():
+            for spec in alias[1]:
+                ids = "vendor_id" in spec and "product_id" in spec
+                rc = "resource_class" in spec
+                if not ids and not rc:
+                    alias_without_ids_or_rc.add(name)
+
+        if alias_without_ids_or_rc:
+            raise exception.PciInvalidAlias(
+                "The PCI alias(es) %s does not have vendor_id and product_id "
+                "fields set or resource_class field set."
+                % ",".join(sorted(alias_without_ids_or_rc)))
+    else:
+        alias_without_ids = set()
+        for name, alias in aliases.items():
+            for spec in alias[1]:
+                ids = "vendor_id" in spec and "product_id" in spec
+                if not ids:
+                    alias_without_ids.add(name)
+
+        if alias_without_ids:
+            raise exception.PciInvalidAlias(
+                "The PCI alias(es) %s does not have vendor_id and product_id "
+                "fields set."
+                % ",".join(sorted(alias_without_ids)))
+
+
+def _validate_aliases(aliases):
+    """Checks the parsed aliases for common mistakes and raise easy to parse
+    error messages
+    """
+    _validate_multispec(aliases)
+    _validate_required_ids(aliases)
+
+
 def _get_alias_from_config() -> Alias:
     """Parse and validate PCI aliases from the nova config.
 

nova/tests/functional/regressions/test_bug_2111440.py

@@ -0,0 +1,48 @@
+# Licensed under the Apache License, Version 2.0 (the "License"); you may
+# not use this file except in compliance with the License. You may obtain
+# a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+from nova.tests.functional.api import client
+from nova.tests.functional.libvirt import test_pci_in_placement as base
+
+
+class MissingRCAndIdAliasWithPCIInPlacementTest(
+    base.PlacementPCIReportingTests
+):
+
+    def test_alias_without_rc_or_vendor_product_id_is_not_supported(self):
+        self.flags(group='filter_scheduler', pci_in_placement=True)
+
+        pci_alias = [
+            {
+                "device_type": "type-VF",
+                "name": "a-vf",
+                "traits": "foo"
+            },
+        ]
+        self.flags(
+            group="pci",
+            alias=self._to_list_of_json_str(pci_alias),
+        )
+        extra_spec = {"pci_passthrough:alias": "a-vf:1"}
+        flavor_id = self._create_flavor(extra_spec=extra_spec)
+
+        exc = self.assertRaises(
+            client.OpenStackApiException,
+            self._create_server,
+            flavor_id=flavor_id,
+            networks=[],
+        )
+        self.assertEqual(400, exc.response.status_code)
+        self.assertIn(
+            "The PCI alias(es) a-vf does not have vendor_id and product_id "
+            "fields set or resource_class field set.",
+            exc.response.text)

nova/tests/unit/pci/test_request.py

@@ -226,6 +226,7 @@ def test_get_alias_from_config_valid_rc_and_traits(self):
             "resource_class": "foo",
             "traits": "bar,baz",
         })
+        self.flags(pci_in_placement=True, group='filter_scheduler')
         self.flags(alias=[fake_alias], group='pci')
         aliases = request._get_alias_from_config()
         self.assertIsNotNone(aliases)
@@ -278,6 +279,69 @@ def test_get_alias_from_config_conflicting_numa_policy(self):
             exception.PciInvalidAlias,
             request._get_alias_from_config)
 
+    def test_get_alias_from_config_missing_ids(self):
+        a1 = jsonutils.dumps({
+            "name": "a1",
+            "product_id": "4444",
+        })
+        a2 = jsonutils.dumps({
+            "name": "a2",
+            "vendor_id": "4444",
+        })
+        a3 = jsonutils.dumps({
+            "name": "a3",
+        })
+        a4 = jsonutils.dumps({
+            "name": "a4",
+            # ignored as PCI in Placement is not enabled
+            "resource_class": "foo",
+        })
+        a5 = jsonutils.dumps({
+            "name": "a5",
+            "vendor_id": "4444",
+            "product_id": "4444",
+        })
+        self.flags(alias=[a1, a2, a3, a4, a5], group='pci')
+
+        ex = self.assertRaises(
+            exception.PciInvalidAlias, request._get_alias_from_config)
+        self.assertEqual(
+            "The PCI alias(es) a1,a2,a3,a4 does not have vendor_id and "
+            "product_id fields set.",
+            str(ex))
+
+    def test_get_alias_from_config_missing_ids_or_rc_pci_in_placement(self):
+        a1 = jsonutils.dumps({
+            "name": "a1",
+            "product_id": "4444",
+        })
+        a2 = jsonutils.dumps({
+            "name": "a2",
+            "vendor_id": "4444",
+        })
+        a3 = jsonutils.dumps({
+            "name": "a3",
+        })
+        a4 = jsonutils.dumps({
+            "name": "a4",
+            "resource_class": "foo",
+        })
+        a5 = jsonutils.dumps({
+            "name": "a5",
+            "vendor_id": "4444",
+            "product_id": "4444",
+        })
+
+        self.flags(pci_in_placement=True, group='filter_scheduler')
+        self.flags(alias=[a1, a2, a3, a4, a5], group='pci')
+
+        ex = self.assertRaises(
+            exception.PciInvalidAlias, request._get_alias_from_config)
+        self.assertEqual(
+            "The PCI alias(es) a1,a2,a3 does not have vendor_id and "
+            "product_id fields set or resource_class field set.",
+            str(ex))
+
     def _verify_result(self, expected, real):
         exp_real = zip(expected, real)
         for exp, real in exp_real: