From 6c3175d3ee8ffe323630f57655318ba3da9fe8b5 Mon Sep 17 00:00:00 2001 From: Artom Lifshitz Date: Thu, 4 Mar 2021 09:41:51 -0500 Subject: [PATCH] pci manager: replace node_id parameter with compute_node To implement the `socket` PCI NUMA affinity policy, we'll need to track the host NUMA topology in the PCI stats code. To achieve this, PCI stats will need to know the compute node it's running on. Prepare for this by replacing the node_id parameter with compute_node. Node_id was previously optional, but that looks to have been only to facilitate testing, as that's the only place where it was not passed it. We use compute_node (instead of just making node_id mandatory) because it allows for an optimization later on wherein the PCI manager does not need to pull the ComputeNode object from the database needlessly. Implements: blueprint pci-socket-affinity Change-Id: Idc839312d1449e9327ee7e3793d53ed080a44d0c --- nova/compute/resource_tracker.py | 3 +- nova/pci/manager.py | 17 ++++------ nova/tests/unit/compute/test_claims.py | 7 +++- .../unit/compute/test_resource_tracker.py | 25 +++++++++++--- nova/tests/unit/pci/test_manager.py | 33 ++++++++----------- 5 files changed, 47 insertions(+), 38 deletions(-) diff --git a/nova/compute/resource_tracker.py b/nova/compute/resource_tracker.py index f64f8c948082..565822d20c63 100644 --- a/nova/compute/resource_tracker.py +++ b/nova/compute/resource_tracker.py @@ -763,8 +763,7 @@ class ResourceTracker(object): def _setup_pci_tracker(self, context, compute_node, resources): if not self.pci_tracker: - n_id = compute_node.id - self.pci_tracker = pci_manager.PciDevTracker(context, node_id=n_id) + self.pci_tracker = pci_manager.PciDevTracker(context, compute_node) if 'pci_passthrough_devices' in resources: dev_json = resources.pop('pci_passthrough_devices') self.pci_tracker.update_devices_from_hypervisor_resources( diff --git a/nova/pci/manager.py b/nova/pci/manager.py index 0c823934faf7..acf712f3d5fe 100644 --- a/nova/pci/manager.py +++ b/nova/pci/manager.py @@ -51,25 +51,22 @@ class PciDevTracker(object): are saved. """ - def __init__(self, context, node_id=None): + def __init__(self, context, compute_node): """Create a pci device tracker. - If a node_id is passed in, it will fetch pci devices information - from database, otherwise, it will create an empty devices list - and the resource tracker will update the node_id information later. + :param context: The request context. + :param compute_node: The object.ComputeNode whose PCI devices we're + tracking. """ super(PciDevTracker, self).__init__() self.stale = {} - self.node_id = node_id + self.node_id = compute_node.id self.dev_filter = whitelist.Whitelist(CONF.pci.passthrough_whitelist) self.stats = stats.PciDeviceStats(dev_filter=self.dev_filter) self._context = context - if node_id: - self.pci_devs = objects.PciDeviceList.get_by_compute_node( - context, node_id) - else: - self.pci_devs = objects.PciDeviceList(objects=[]) + self.pci_devs = objects.PciDeviceList.get_by_compute_node( + context, self.node_id) self._build_device_tree(self.pci_devs) self._initial_instance_usage() diff --git a/nova/tests/unit/compute/test_claims.py b/nova/tests/unit/compute/test_claims.py index b4fc716343c8..a13bd4355e32 100644 --- a/nova/tests/unit/compute/test_claims.py +++ b/nova/tests/unit/compute/test_claims.py @@ -56,7 +56,12 @@ class DummyTracker(object): def new_pci_tracker(self): ctxt = context.RequestContext('testuser', 'testproject') - self.pci_tracker = pci_manager.PciDevTracker(ctxt) + with mock.patch.object( + objects.PciDeviceList, 'get_by_compute_node', + return_value=objects.PciDeviceList() + ): + self.pci_tracker = pci_manager.PciDevTracker( + ctxt, objects.ComputeNode(id=1)) class ClaimTestCase(test.NoDBTestCase): diff --git a/nova/tests/unit/compute/test_resource_tracker.py b/nova/tests/unit/compute/test_resource_tracker.py index 1b8d18d19e92..d47e92ffb131 100644 --- a/nova/tests/unit/compute/test_resource_tracker.py +++ b/nova/tests/unit/compute/test_resource_tracker.py @@ -2113,7 +2113,12 @@ class TestInstanceClaim(BaseTestCase): # TODO(jaypipes): Remove once the PCI tracker is always created # upon the resource tracker being initialized... - self.rt.pci_tracker = pci_manager.PciDevTracker(mock.sentinel.ctx) + with mock.patch.object( + objects.PciDeviceList, 'get_by_compute_node', + return_value=objects.PciDeviceList() + ): + self.rt.pci_tracker = pci_manager.PciDevTracker( + mock.sentinel.ctx, _COMPUTE_NODE_FIXTURES[0]) pci_dev = pci_device.PciDevice.create( None, fake_pci_device.dev_dict) @@ -2478,7 +2483,6 @@ class TestResize(BaseTestCase): pci_get_by_instance_mock, is_bfv_mock, revert=False): - self.flags(reserved_host_disk_mb=0, reserved_host_memory_mb=0) virt_resources = copy.deepcopy(_VIRT_DRIVER_AVAIL_RESOURCES) @@ -2652,7 +2656,8 @@ class TestResize(BaseTestCase): # TODO(jaypipes): Remove once the PCI tracker is always created # upon the resource tracker being initialized... - self.rt.pci_tracker = pci_manager.PciDevTracker(mock.sentinel.ctx) + self.rt.pci_tracker = pci_manager.PciDevTracker( + mock.sentinel.ctx, _COMPUTE_NODE_FIXTURES[0]) pci_dev = pci_device.PciDevice.create( None, fake_pci_device.dev_dict) @@ -2743,7 +2748,12 @@ class TestResize(BaseTestCase): # TODO(jaypipes): Remove once the PCI tracker is always created # upon the resource tracker being initialized... - self.rt.pci_tracker = pci_manager.PciDevTracker(mock.sentinel.ctx) + with mock.patch.object( + objects.PciDeviceList, 'get_by_compute_node', + return_value=objects.PciDeviceList() + ): + self.rt.pci_tracker = pci_manager.PciDevTracker( + mock.sentinel.ctx, cn) pci_dev = pci_device.PciDevice.create( None, fake_pci_device.dev_dict) @@ -3030,7 +3040,12 @@ class TestLiveMigration(BaseTestCase): migration = objects.Migration(id=42, migration_type='live-migration', status='accepted') image_meta = objects.ImageMeta(properties=objects.ImageMetaProps()) - self.rt.pci_tracker = pci_manager.PciDevTracker(mock.sentinel.ctx) + with mock.patch.object( + objects.PciDeviceList, 'get_by_compute_node', + return_value=objects.PciDeviceList() + ): + self.rt.pci_tracker = pci_manager.PciDevTracker( + mock.sentinel.ctx, _COMPUTE_NODE_FIXTURES[0]) with test.nested( mock.patch.object(objects.ImageMeta, 'from_instance', return_value=image_meta), diff --git a/nova/tests/unit/pci/test_manager.py b/nova/tests/unit/pci/test_manager.py index 004cae380b62..eb8e1c054e9c 100644 --- a/nova/tests/unit/pci/test_manager.py +++ b/nova/tests/unit/pci/test_manager.py @@ -19,7 +19,6 @@ import mock from oslo_serialization import jsonutils from oslo_utils.fixture import uuidsentinel -import nova from nova.compute import vm_states from nova import context from nova import objects @@ -144,7 +143,8 @@ class PciDevTrackerTestCase(test.NoDBTestCase): def _create_tracker(self, fake_devs): self.fake_devs = fake_devs - self.tracker = manager.PciDevTracker(self.fake_context, 1) + self.tracker = manager.PciDevTracker( + self.fake_context, objects.ComputeNode(id=1)) def setUp(self): super(PciDevTrackerTestCase, self).setUp() @@ -216,25 +216,15 @@ class PciDevTrackerTestCase(test.NoDBTestCase): self.assertIsNone(vf.parent_device) self.assertEqual([], vf.child_devices) - @mock.patch.object(nova.objects.PciDeviceList, 'get_by_compute_node') - def test_pcidev_tracker_create_no_nodeid(self, mock_get_cn): - self.tracker = manager.PciDevTracker(self.fake_context) - self.assertEqual(len(self.tracker.pci_devs), 0) - self.assertFalse(mock_get_cn.called) - - @mock.patch.object(nova.objects.PciDeviceList, 'get_by_compute_node') - def test_pcidev_tracker_create_with_nodeid(self, mock_get_cn): - self.tracker = manager.PciDevTracker(self.fake_context, node_id=1) - mock_get_cn.assert_called_once_with(self.fake_context, 1) - @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), copy.deepcopy(fake_pci_2)] + fake_pci_devs = [copy.deepcopy(fake_pci_4), copy.deepcopy(fake_pci_5)] fake_pci_devs_json = jsonutils.dumps(fake_pci_devs) - tracker = manager.PciDevTracker(self.fake_context) + tracker = manager.PciDevTracker( + self.fake_context, objects.ComputeNode(id=1)) tracker.update_devices_from_hypervisor_resources(fake_pci_devs_json) - self.assertEqual(2, len(tracker.pci_devs)) + self.assertEqual(5, len(tracker.pci_devs)) @mock.patch("nova.pci.manager.LOG.debug") def test_update_devices_from_hypervisor_resources_32bit_domain( @@ -260,10 +250,12 @@ class PciDevTrackerTestCase(test.NoDBTestCase): fake_pci_devs = [fake_pci] fake_pci_devs_json = jsonutils.dumps(fake_pci_devs) - tracker = manager.PciDevTracker(self.fake_context) - # We expect that the device with 32bit PCI domain is ignored + tracker = manager.PciDevTracker( + self.fake_context, objects.ComputeNode(id=1)) + # We expect that the device with 32bit PCI domain is ignored, so we'll + # have only the 3 original fake devs tracker.update_devices_from_hypervisor_resources(fake_pci_devs_json) - self.assertEqual(0, len(tracker.pci_devs)) + self.assertEqual(3, len(tracker.pci_devs)) mock_debug.assert_called_once_with( 'Skipping PCI device %s reported by the hypervisor: %s', {'address': '10000:00:02.0', 'parent_addr': None}, @@ -433,7 +425,8 @@ class PciDevTrackerTestCase(test.NoDBTestCase): fake_db_dev_3 = dict(fake_db_dev_1, id=4, address='0000:00:00.4') fake_devs_numa = copy.deepcopy(fake_db_devs) fake_devs_numa.append(fake_db_dev_3) - self.tracker = manager.PciDevTracker(1) + self.tracker = manager.PciDevTracker( + mock.sentinel.context, objects.ComputeNode(id=1)) self.tracker._set_hvdevs(fake_devs_numa) pci_requests = copy.deepcopy(fake_pci_requests)[:1] pci_requests[0]['count'] = 2