From acc6221660d5113314c66361cace9e3982fa76bf Mon Sep 17 00:00:00 2001 From: Balazs Gibizer Date: Wed, 28 May 2025 14:32:54 +0200 Subject: [PATCH] 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. Closes-Bug: #2111440 Change-Id: I7fd43b3d6faac8c4098b0983e8adc596414823a1 --- doc/source/admin/pci-passthrough.rst | 4 +- nova/conf/pci.py | 5 +- nova/pci/request.py | 43 +++++++++++-- .../regressions/test_bug_2111440.py | 48 ++++++++++++++ nova/tests/unit/pci/test_request.py | 64 +++++++++++++++++++ 5 files changed, 158 insertions(+), 6 deletions(-) create mode 100644 nova/tests/functional/regressions/test_bug_2111440.py diff --git a/doc/source/admin/pci-passthrough.rst b/doc/source/admin/pci-passthrough.rst index 4789b7932c62..bf7f0e1d32a9 100644 --- a/doc/source/admin/pci-passthrough.rst +++ b/doc/source/admin/pci-passthrough.rst @@ -499,7 +499,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__``. +by nova to ``CUSTOM_PCI__``. 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. `_ diff --git a/nova/conf/pci.py b/nova/conf/pci.py index 6ed7a6aa3105..f1666d9483f8 100644 --- a/nova/conf/pci.py +++ b/nova/conf/pci.py @@ -69,7 +69,8 @@ Possible Values: alias = { "name": "A16_16A", "device_type": "type-VF", - "resource_class": "CUSTOM_A16_16A", + "resource_class": "GPU_VF", + "traits": "blue, big" } Valid key values are : @@ -107,6 +108,8 @@ Possible Values: 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 diff --git a/nova/pci/request.py b/nova/pci/request.py index 6ef87df551a9..d7c9e79243d2 100644 --- a/nova/pci/request.py +++ b/nova/pci/request.py @@ -121,10 +121,7 @@ _ALIAS_SCHEMA = { } -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. diff --git a/nova/tests/functional/regressions/test_bug_2111440.py b/nova/tests/functional/regressions/test_bug_2111440.py new file mode 100644 index 000000000000..44b858724e7b --- /dev/null +++ b/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) diff --git a/nova/tests/unit/pci/test_request.py b/nova/tests/unit/pci/test_request.py index fcd14d117c1c..d6b759f59149 100644 --- a/nova/tests/unit/pci/test_request.py +++ b/nova/tests/unit/pci/test_request.py @@ -226,6 +226,7 @@ class PciRequestTestCase(test.NoDBTestCase): "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 @@ class PciRequestTestCase(test.NoDBTestCase): 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: