From 672b288324c1463a6e5b924e89e99aeb1b86e12e Mon Sep 17 00:00:00 2001 From: Balazs Gibizer <balazs.gibizer@est.tech> Date: Thu, 8 Oct 2020 16:43:01 +0200 Subject: [PATCH] Refactor _claim_pci_device_for_interface_attach to prepare for qos This removes some of the non PCI specific part of the _claim_pci_device_for_interface_attach() to reuse them later for the qos handling. The goal is to have the create_resource_requests separated from the actual PCI claim. Change-Id: Id19702da05cac25192f6aa21f8f4449602ef7729 Blueprint: support-interface-attach-with-qos-ports --- nova/compute/manager.py | 79 ++++++++++----------- nova/objects/instance.py | 1 + nova/tests/unit/compute/test_compute.py | 28 ++++++-- nova/tests/unit/compute/test_compute_mgr.py | 7 +- 4 files changed, 66 insertions(+), 49 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index c68fa5c417bd..06e2032261e2 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -7518,68 +7518,36 @@ class ComputeManager(manager.Manager): self, context: nova.context.RequestContext, instance: 'objects.Instance', - requested_networks: 'objects.NetworkRequestsList' + pci_reqs: 'objects.InstancePCIRequests', ) -> ty.Optional['objects.PciDevice']: - """Claim PCI device if the requested interface needs it - - If a PCI device is claimed then the requested_networks is updated - with the pci request id and the pci_requests and pci_devices fields - of the instance is also updated accordingly. + """Claim PCI devices if there are PCI requests :param context: nova.context.RequestContext :param instance: the objects.Instance to where the interface is being attached - :param requested_networks: the objects.NetworkRequestList describing - the requested interface. The requested_networks.objects list shall - have a single item. + :param pci_reqs: A InstancePCIRequests object describing the + needed PCI devices :raises InterfaceAttachPciClaimFailed: if the PCI device claim fails - :raises InterfaceAttachFailed: if more than one interface is requested :returns: An objects.PciDevice describing the claimed PCI device for the interface or None if no device is requested """ - if len(requested_networks) != 1: - LOG.warning( - "Interface attach only supports one interface per attach " - "request", instance=instance) - raise exception.InterfaceAttachFailed(instance_uuid=instance.uuid) - - requested_network = requested_networks[0] - - pci_numa_affinity_policy = hardware.get_pci_numa_policy_constraint( - instance.flavor, instance.image_meta) - pci_reqs = objects.InstancePCIRequests( - requests=[], instance_uuid=instance.uuid) - self.network_api.create_resource_requests( - context, requested_networks, pci_reqs, - affinity_policy=pci_numa_affinity_policy) - if not pci_reqs.requests: - # The requested port does not require a PCI device so nothing to do return None - if len(pci_reqs.requests) > 1: - LOG.warning( - "Interface attach only supports one interface per attach " - "request", instance=instance) - raise exception.InterfaceAttachFailed(instance_uuid=instance.uuid) - - pci_req = pci_reqs.requests[0] - devices = self.rt.claim_pci_devices( context, pci_reqs, instance.numa_topology) if not devices: LOG.info('Failed to claim PCI devices during interface attach ' - 'for PCI request %s', pci_req, instance=instance) + 'for PCI request %s', pci_reqs, instance=instance) raise exception.InterfaceAttachPciClaimFailed( instance_uuid=instance.uuid) + # NOTE(gibi): We assume that maximum one PCI devices is attached per + # interface attach request. device = devices[0] - - # Update the requested network with the pci request id - requested_network.pci_request_id = pci_req.request_id - instance.add_pci_device_and_request(device, pci_req) + instance.pci_devices.objects.append(device) return device @@ -7643,8 +7611,35 @@ class ComputeManager(manager.Manager): ] ) - pci_device = self._claim_pci_device_for_interface_attach( - context, instance, requested_networks) + if len(requested_networks) != 1: + LOG.warning( + "Interface attach only supports one interface per attach " + "request", instance=instance) + raise exception.InterfaceAttachFailed(instance_uuid=instance.uuid) + + pci_numa_affinity_policy = hardware.get_pci_numa_policy_constraint( + instance.flavor, instance.image_meta) + pci_reqs = objects.InstancePCIRequests( + requests=[], instance_uuid=instance.uuid) + self.network_api.create_resource_requests( + context, requested_networks, pci_reqs, + affinity_policy=pci_numa_affinity_policy) + + # We only support one port per attach request so we at most have one + # pci request + pci_req = None + if pci_reqs.requests: + pci_req = pci_reqs.requests[0] + requested_networks[0].pci_request_id = pci_req.request_id + instance.pci_requests.requests.append(pci_req) + + try: + pci_device = self._claim_pci_device_for_interface_attach( + context, instance, pci_reqs) + except exception.InterfaceAttachPciClaimFailed: + with excutils.save_and_reraise_exception(): + if pci_req: + instance.pci_requests.requests.remove(pci_req) network_info = self.network_api.allocate_for_instance( context, diff --git a/nova/objects/instance.py b/nova/objects/instance.py index 6a82f12c4f36..26813b7fbca1 100644 --- a/nova/objects/instance.py +++ b/nova/objects/instance.py @@ -1225,6 +1225,7 @@ class Instance(base.NovaPersistentObject, base.NovaObject, pci_req for pci_req in self.pci_requests.requests if pci_req.request_id != pci_device.request_id] + # TODO(gibi): remove this as it is unused def add_pci_device_and_request(self, pci_device, pci_request): self.pci_requests.requests.append(pci_request) self.pci_devices.objects.append(pci_device) diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index 8f6cae721edd..dbe3b17b7341 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -10145,11 +10145,12 @@ class ComputeAPITestCase(BaseTestCase): mock.patch.dict(self.compute.driver.capabilities, supports_attach_interface=True), mock.patch('oslo_concurrency.lockutils.lock'), + mock.patch("nova.network.neutron.API.create_resource_requests"), mock.patch.object( self.compute, "_claim_pci_device_for_interface_attach", return_value=None) - ) as (cap, mock_lock, mock_claim_pci): + ) as (cap, mock_lock, mock_create_resource_req, mock_claim_pci): vif = self.compute.attach_interface(self.context, instance, network_id, @@ -10175,7 +10176,12 @@ class ComputeAPITestCase(BaseTestCase): mock.ANY, delay=mock.ANY, do_log=mock.ANY, fair=mock.ANY, semaphores=mock.ANY) mock_claim_pci.assert_called_once_with( - self.context, instance, network_requests) + self.context, instance, + test.MatchType(objects.InstancePCIRequests)) + pci_reqs = mock_claim_pci.mock_calls[0][1][2] + self.assertEqual(instance.uuid, pci_reqs.instance_uuid) + self.assertEqual([], pci_reqs.requests) + return nwinfo, port_id @mock.patch.object(compute_utils, 'notify_about_instance_action') @@ -10262,10 +10268,11 @@ class ComputeAPITestCase(BaseTestCase): mock.patch.dict(self.compute.driver.capabilities, supports_attach_interface=True, supports_tagged_attach_interface=True), + mock.patch("nova.network.neutron.API.create_resource_requests"), mock.patch.object(self.compute, '_claim_pci_device_for_interface_attach', return_value=None) - ) as (mock_capabilities, mock_claim_pci): + ) as (mock_capabilities, mock_create_resource_req, mock_claim_pci): vif = self.compute.attach_interface(self.context, instance, network_id, @@ -10289,7 +10296,11 @@ class ComputeAPITestCase(BaseTestCase): mock.call(self.context, instance, self.compute.host, action='interface_attach', phase='end')]) mock_claim_pci.assert_called_once_with( - self.context, instance, network_requests) + self.context, instance, + test.MatchType(objects.InstancePCIRequests)) + pci_reqs = mock_claim_pci.mock_calls[0][1][2] + self.assertEqual(instance.uuid, pci_reqs.instance_uuid) + self.assertEqual([], pci_reqs.requests) return nwinfo, port_id @@ -10329,11 +10340,12 @@ class ComputeAPITestCase(BaseTestCase): 'deallocate_port_for_instance'), mock.patch.dict(self.compute.driver.capabilities, supports_attach_interface=True), + mock.patch("nova.network.neutron.API.create_resource_requests"), mock.patch.object(self.compute, '_claim_pci_device_for_interface_attach', return_value=None), ) as (mock_notify, mock_attach, mock_allocate, mock_deallocate, - mock_dict, mock_claim_pci): + mock_dict, mock_create_resource_req, mock_claim_pci): mock_allocate.return_value = nwinfo mock_attach.side_effect = exception.NovaException("attach_failed") @@ -10361,7 +10373,11 @@ class ComputeAPITestCase(BaseTestCase): exception=mock_attach.side_effect, phase='error')]) mock_claim_pci.assert_called_once_with( - self.context, instance, network_requests) + self.context, instance, + test.MatchType(objects.InstancePCIRequests)) + pci_reqs = mock_claim_pci.mock_calls[0][1][2] + self.assertEqual(instance.uuid, pci_reqs.instance_uuid) + self.assertEqual([], pci_reqs.requests) def test_attach_sriov_interface_failed_in_driver(self): new_type = flavors.get_flavor_by_flavor_id('4') diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py index 3651489ae669..3f99a498b89a 100644 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -2543,8 +2543,11 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, f_instance = objects.Instance._from_db_object(self.context, objects.Instance(), db_instance) + f_instance.flavor = objects.Flavor() + f_instance.system_metadata = {} e = exception.InterfaceAttachFailed(instance_uuid=f_instance.uuid) + @mock.patch('nova.network.neutron.API.create_resource_requests') @mock.patch.object(self.compute, '_claim_pci_device_for_interface_attach', return_value=None) @@ -2555,7 +2558,9 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase, 'allocate_for_instance', side_effect=e) @mock.patch.object(self.compute, '_instance_update', side_effect=lambda *a, **k: {}) - def do_test(update, meth, add_fault, notify, event, mock_claim_pci): + def do_test( + update, meth, add_fault, notify, event, mock_claim_pci, + mock_create_resource_req): self.assertRaises(exception.InterfaceAttachFailed, self.compute.attach_interface, self.context, f_instance, uuids.network_id,