openstack/nova
Code Issues Proposed changes

Merge "Add managed flag to PCI device specification"

Browse Source
This commit is contained in:
Zuul
2025-02-28 01:43:20 +00:00
committed by Gerrit Code Review
parent 97dad74476 12905dade3
commit d9d34aa0e6
7 changed files with 222 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
nova/objects/pci_device.py
View File

@@ -177,11 +177,22 @@ class PciDevice(base.NovaPersistentObject, base.NovaObject):
# - "parent_ifname": the netdev name of the parent (PF)
# device of a VF
# - "mac_address": the MAC address of the PF
# - "managed": "true"/"false" if the device is managed by
# hypervisor
extra_info = self.extra_info
data = v if isinstance(v, str) else jsonutils.dumps(v)
extra_info.update({k: data})
self.extra_info = extra_info
# Remove the "managed" key if it was set previously
# As with the previous case, we must explicitly assign to
# self.extra_info so that obj_what_changed detects the modification
# and triggers a save later.
if "managed" not in dev_dict and "managed" in self.extra_info:
extra_info = self.extra_info
del extra_info["managed"]
self.extra_info = extra_info
def __init__(self, *args, **kwargs):
super(PciDevice, self).__init__(*args, **kwargs)

14
nova/pci/devspec.py
View File

@@ -317,6 +317,15 @@ class PciDeviceSpec(PciAddressSpec):
address_obj = self._address_obj()
self._remote_managed = strutils.bool_from_string(
self.tags.get(PCI_REMOTE_MANAGED_TAG))
if self.tags.get("managed", None) is not None:
try:
self.tags["managed"] = (
"true" if strutils.bool_from_string(
self.tags.get("managed"), strict=True) else "false"
)
except ValueError as e:
raise exception.PciConfigInvalidSpec(reason=str(e))
if self._remote_managed:
if address_obj is None:
# Note that this will happen if a netdev was specified in the
@@ -399,3 +408,8 @@ class PciDeviceSpec(PciAddressSpec):
def get_tags(self) -> ty.Dict[str, str]:
return self.tags
def enhanced_pci_device_with_spec_tags(self, dev: ty.Dict[str, ty.Any]):
managed = self.tags.get("managed")
if managed is not None:
dev.update({'managed': managed})

7
nova/pci/manager.py
View File

@@ -131,7 +131,12 @@ class PciDevTracker(object):
devices = []
for dev in jsonutils.loads(devices_json):
try:
if self.dev_filter.device_assignable(dev):
pci_dev_spec = self.dev_filter.device_assignable(dev)
if pci_dev_spec is not None:
# Since some configuration parameters cannot be
# discovered by the driver, we need to add them from
# the device specification provided by the operator.
pci_dev_spec.enhanced_pci_device_with_spec_tags(dev)
devices.append(dev)
except exception.PciConfigInvalidSpec as e:
# The raised exception is misleading as the problem is not with

13
nova/pci/whitelist.py
View File

@@ -82,15 +82,20 @@ class Whitelist(object):
return specs
def device_assignable(self, dev: ty.Dict[str, ty.Any]) -> bool:
"""Check if a device can be assigned to a guest.
def device_assignable(
self, dev: ty.Dict[str, ty.Any]
) -> ty.Optional[devspec.PciDeviceSpec]:
"""Check if a device is part of pci device_spec (whitelist) and so
can be assigned to a guest.
If yes return the spec, else return None
:param dev: A dictionary describing the device properties
:return: A devspec.PciDeviceSpec or None
"""
for spec in self.specs:
if spec.match(dev):
return True
return False
return spec
return None
def get_devspec(
self, pci_dev: 'objects.PciDevice',

33
nova/tests/unit/objects/test_pci_device.py
View File

@@ -190,6 +190,39 @@ class _TestPciDeviceObject(object):
self.assertEqual(self.pci_device.obj_what_changed(),
set(['vendor_id', 'product_id', 'parent_addr']))
def test_update_and_remove_extra_info_key(self):
self.dev_dict = copy.copy(dev_dict)
self.dev_dict['managed'] = "true"
self.pci_device = pci_device.PciDevice.create(None, self.dev_dict)
self.pci_device.obj_reset_changes()
changes = {'managed': 'no'}
self.pci_device.update_device(changes)
self.assertEqual(
self.pci_device.obj_what_changed(),
set(
[
"parent_addr",
"extra_info"
]
),
)
self.assertEqual(self.pci_device.extra_info, {"managed": "no"})
self.pci_device.obj_reset_changes()
changes = {}
self.pci_device.update_device(changes)
self.assertEqual(
self.pci_device.obj_what_changed(),
set(
[
"parent_addr",
"extra_info"
]
),
)
self.assertNotIn("managed", self.pci_device.extra_info)
def test_update_device_same_value(self):
self.pci_device = pci_device.PciDevice.create(None, dev_dict)
self.pci_device.obj_reset_changes()

96
nova/tests/unit/pci/test_manager.py
View File

@@ -56,6 +56,18 @@ fake_pci_5 = dict(fake_pci, address='0000:00:02.2',
dev_type=fields.PciDeviceType.SRIOV_VF,
parent_addr='0000:00:01.1',
vendor_id='v2', product_id='p2', numa_node=None)
fake_pci_6 = dict(fake_pci, address='0000:00:02.2',
dev_type=fields.PciDeviceType.SRIOV_VF,
parent_addr='0000:00:00.1',
vendor_id='10c9', product_id='8086', numa_node=None)
fake_pci_7 = dict(fake_pci, address='0000:00:02.3',
dev_type=fields.PciDeviceType.SRIOV_VF,
parent_addr='0000:00:00.1',
vendor_id='10c9', product_id='8086', numa_node=None)
fake_pci_8 = dict(fake_pci, address='0000:00:02.4',
dev_type=fields.PciDeviceType.SRIOV_VF,
parent_addr='0000:00:00.1',
vendor_id='10c9', product_id='8086', numa_node=None)
fake_pci_devs_tree = [fake_pci_3, fake_pci_4, fake_pci_5]
fake_db_dev = {
@@ -220,16 +232,92 @@ class PciDevTrackerTestCase(test.NoDBTestCase):
self.assertIsNone(vf.parent_device)
self.assertEqual([], vf.child_devices)
@mock.patch('nova.pci.whitelist.Whitelist.device_assignable',
return_value=True)
def test_update_devices_from_hypervisor_resources(self, _mock_dev_assign):
fake_pci_devs = [copy.deepcopy(fake_pci_4), copy.deepcopy(fake_pci_5)]
# Mocking as a vf to avoid
# os.path.isdir on /sys/bus/pci/devices/0000:00:02.2
# which is not appropriate for tests
@mock.patch("nova.pci.utils.is_physical_function", return_value=False)
def test_update_devices_from_hypervisor_resources(self, mock_vf):
self.flags(
group="pci",
device_spec=[
'{"product_id":"8086", "vendor_id":"10c9", '
'"address":"0000:00:02.2"}',
'{"product_id":"8086", "vendor_id":"10c9", '
'"address":"0000:00:02.3"}',
],
)
# fake_pci_4 and fake_pci_5 vendor id 'v2' is not valid so use
# fake_pci_6 and fake_pci_7 instead
fake_pci_devs = [copy.deepcopy(fake_pci_6), copy.deepcopy(fake_pci_7)]
fake_pci_devs_json = jsonutils.dumps(fake_pci_devs)
tracker = manager.PciDevTracker(
self.fake_context, objects.ComputeNode(id=1, numa_topology=None))
tracker.update_devices_from_hypervisor_resources(fake_pci_devs_json)
self.assertEqual(5, len(tracker.pci_devs))
@mock.patch("nova.pci.utils.is_physical_function", return_value=False)
def test_update_devices_from_hypervisor_resources_with_managed(
self, _mock_vf
):
self.flags(
group="pci",
device_spec=[
'{"product_id":"8086", "vendor_id":"10c9", '
'"address":"0000:00:02.2", "managed":"no"}',
'{"product_id":"8086", "vendor_id":"10c9", '
'"address":"0000:00:02.3", "managed":"yes"}',
],
)
fake_pci_devs = [copy.deepcopy(fake_pci_6), copy.deepcopy(fake_pci_7)]
fake_pci_devs_json = jsonutils.dumps(fake_pci_devs)
tracker = manager.PciDevTracker(
self.fake_context, objects.ComputeNode(id=1, numa_topology=None)
)
tracker.update_devices_from_hypervisor_resources(fake_pci_devs_json)
self.assertEqual(5, len(tracker.pci_devs))
pci_addr_extra_info = {
dev.address: dev.extra_info for dev in tracker.pci_devs
}
self.assertEqual(
pci_addr_extra_info["0000:00:02.2"]["managed"], "false"
)
self.assertEqual(
pci_addr_extra_info["0000:00:02.3"]["managed"], "true"
)
@mock.patch("nova.pci.manager.LOG.debug")
@mock.patch("nova.pci.utils.is_physical_function", return_value=False)
def test_update_devices_from_hypervisor_resources_with_managed_invalid(
self, _mock_vf, mock_debug
):
self.flags(
group="pci",
device_spec=[
'{"product_id":"8086", "vendor_id":"10c9", '
'"address":"0000:00:02.2", "managed":"no"}',
'{"product_id":"8086", "vendor_id":"10c9", '
'"address":"0000:00:02.3", "managed":"yes"}',
'{"product_id":"8086", "vendor_id":"10c9", '
'"address":"0000:00:02.4", "managed":"invalid"}',
],
)
exc = self.assertRaises(
exception.PciConfigInvalidSpec,
manager.PciDevTracker,
self.fake_context,
objects.ComputeNode(id=1, numa_topology=None),
)
self.assertEqual(
"Invalid [pci]device_spec config: Unrecognized value 'invalid', "
"acceptable values are: '0', '1', 'f', 'false', 'n', 'no', 'off', "
"'on', 't', 'true', 'y', 'yes'",
str(exc)
)
@mock.patch("nova.pci.manager.LOG.debug")
def test_update_devices_from_hypervisor_resources_32bit_domain(
self, mock_debug):

57
nova/tests/unit/pci/test_whitelist.py
View File

@@ -13,6 +13,7 @@
# License for the specific language governing permissions and limitations
# under the License.
from nova import exception
from nova.pci import whitelist
from nova import test
@@ -81,3 +82,59 @@ class WhitelistTestCase(test.NoDBTestCase):
white_list = '{"product_id":"0001", "vendor_id":"8086"}'
parsed = whitelist.Whitelist([white_list])
self.assertTrue(parsed.device_assignable(dev_dict))
def test_device_managed(self):
white_list = (
'{"product_id":"0001", "vendor_id":"8086", "managed": "yes"}'
)
parsed = whitelist.Whitelist([white_list])
self.assertEqual(1, len(parsed.specs))
self.assertTrue(parsed.specs[0].tags["managed"], "true")
def test_device_managed_true(self):
white_list = (
'{"product_id":"0001", "vendor_id":"8086", "managed": "true"}'
)
parsed = whitelist.Whitelist([white_list])
self.assertEqual(1, len(parsed.specs))
self.assertTrue(parsed.specs[0].tags["managed"], "true")
def test_device_managed_int(self):
white_list = (
'{"product_id":"0001", "vendor_id":"8086", "managed": 1}'
)
parsed = whitelist.Whitelist([white_list])
self.assertEqual(1, len(parsed.specs))
self.assertTrue(parsed.specs[0].tags["managed"], "true")
def test_device_not_managed(self):
white_list = (
'{"product_id":"0001", "vendor_id":"8086", "managed": "no"}'
)
parsed = whitelist.Whitelist([white_list])
self.assertEqual(1, len(parsed.specs))
self.assertTrue(parsed.specs[0].tags["managed"], "false")
def test_device_managed_not_set(self):
white_list = (
'{"product_id":"0001", "vendor_id":"8086"}'
)
parsed = whitelist.Whitelist([white_list])
self.assertEqual(1, len(parsed.specs))
self.assertNotIn("managed", parsed.specs[0].tags)
def test_device_managed_invalid_value(self):
white_list = (
'{"product_id":"0001", "vendor_id":"8086", "managed": "invalid"}'
)
exc = self.assertRaises(
exception.PciConfigInvalidSpec, whitelist.Whitelist, [white_list]
)
self.assertEqual(
"Invalid [pci]device_spec config: Unrecognized value 'invalid', "
"acceptable values are: '0', '1', 'f', 'false', 'n', 'no', 'off', "
"'on', 't', 'true', 'y', 'yes'",
str(exc)
)