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
This commit is contained in:

committed by
Dan Smith

parent
c3f392dd8e
commit
acc6221660
@ -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_<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>`_
|
||||
|
||||
|
@ -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
|
||||
|
@ -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.
|
||||
|
||||
|
48
nova/tests/functional/regressions/test_bug_2111440.py
Normal file
48
nova/tests/functional/regressions/test_bug_2111440.py
Normal file
@ -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)
|
@ -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:
|
||||
|
Reference in New Issue
Block a user