diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 5f5d5e8a8f4f..8dd8c2de1354 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -7462,14 +7462,7 @@ class ComputeManager(manager.Manager): else: if pci_device: self.rt.unclaim_pci_devices(context, pci_device, instance) - - # remove pci device from instance - instance.pci_devices.objects.remove(pci_device) - - # remove pci request from instance - instance.pci_requests.requests = [ - pci_req for pci_req in instance.pci_requests.requests - if pci_req.request_id != pci_device.request_id] + instance.remove_pci_device_and_request(pci_device) if port_allocation: # Deallocate the resources in placement that were used by the @@ -7509,17 +7502,19 @@ class ComputeManager(manager.Manager): :param requested_networks: the objects.NetworkRequestList describing the requested interface. The requested_networks.objects list shall have a single item. - :raises ValueError: if there is more than one interface requested - :raises InterfaceAttachFailed: if the PCI device claim fails + :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.objects) != 1: - raise ValueError( - "Only support one interface per interface attach request") + 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.objects[0] + requested_network = requested_networks[0] pci_numa_affinity_policy = hardware.get_pci_numa_policy_constraint( instance.flavor, instance.image_meta) @@ -7534,8 +7529,10 @@ class ComputeManager(manager.Manager): return None if len(pci_reqs.requests) > 1: - raise ValueError( - "Only support one interface per interface attach request") + 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] @@ -7552,9 +7549,7 @@ class ComputeManager(manager.Manager): # Update the requested network with the pci request id requested_network.pci_request_id = pci_req.request_id - - instance.pci_requests.requests.append(pci_req) - instance.pci_devices.objects.append(device) + instance.add_pci_device_and_request(device, pci_req) return device @@ -7707,15 +7702,14 @@ class ComputeManager(manager.Manager): raise exception.PortNotFound(_("Port %s is not " "attached") % port_id) - pci_device = None pci_req = pci_req_module.get_instance_pci_request_from_vif( context, instance, condemned) pci_device = None if pci_req: pci_devices = [pci_device - for pci_device in instance.pci_devices.objects - if pci_device.request_id == pci_req.request_id] + for pci_device in instance.pci_devices.objects + if pci_device.request_id == pci_req.request_id] if not pci_devices: LOG.warning( diff --git a/nova/compute/resource_tracker.py b/nova/compute/resource_tracker.py index c777a4a01626..33926e0d477b 100644 --- a/nova/compute/resource_tracker.py +++ b/nova/compute/resource_tracker.py @@ -1913,7 +1913,8 @@ class ResourceTracker(object): @utils.synchronized(COMPUTE_RESOURCE_SEMAPHORE, fair=True) def claim_pci_devices( - self, context, pci_requests, instance_numa_topology=None): + self, context, pci_requests, instance_numa_topology=None + ): """Claim instance PCI resources :param context: security context diff --git a/nova/objects/instance.py b/nova/objects/instance.py index 69d69e6e5f1a..6a82f12c4f36 100644 --- a/nova/objects/instance.py +++ b/nova/objects/instance.py @@ -1217,6 +1217,18 @@ class Instance(base.NovaPersistentObject, base.NovaObject, return objects.BlockDeviceMappingList.get_by_instance_uuid( self._context, self.uuid) + def remove_pci_device_and_request(self, pci_device): + """Remove the PciDevice and the related InstancePciRequest""" + if pci_device in self.pci_devices.objects: + self.pci_devices.objects.remove(pci_device) + self.pci_requests.requests = [ + pci_req for pci_req in self.pci_requests.requests + if pci_req.request_id != pci_device.request_id] + + 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) + def _make_instance_list(context, inst_list, db_inst_list, expected_attrs): get_fault = expected_attrs and 'fault' in expected_attrs diff --git a/nova/tests/functional/libvirt/test_pci_sriov_servers.py b/nova/tests/functional/libvirt/test_pci_sriov_servers.py index 994521dfada3..e661c67266c9 100644 --- a/nova/tests/functional/libvirt/test_pci_sriov_servers.py +++ b/nova/tests/functional/libvirt/test_pci_sriov_servers.py @@ -250,8 +250,7 @@ class SRIOVAttachDetachTest(_PCIServersTestBase): def _attach_port(self, instance_uuid, port_id): self.api.attach_interface( instance_uuid, - {'interfaceAttachment': { - 'port_id': port_id}}) + {'interfaceAttachment': {'port_id': port_id}}) fake_notifier.wait_for_versioned_notifications( 'instance.interface_attach.end') diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index ec387ac85dfd..641dcb2a3cca 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -10274,9 +10274,10 @@ class ComputeAPITestCase(BaseTestCase): mock.patch.dict(self.compute.driver.capabilities, supports_attach_interface=True), mock.patch('oslo_concurrency.lockutils.lock'), - mock.patch.object(self.compute, - '_claim_pci_device_for_interface_attach', - return_value=None) + mock.patch.object( + self.compute, + "_claim_pci_device_for_interface_attach", + return_value=None) ) as (cap, mock_lock, mock_claim_pci): vif = self.compute.attach_interface(self.context, instance, @@ -10329,9 +10330,8 @@ class ComputeAPITestCase(BaseTestCase): mock.patch.object(self.compute.rt, 'allocate_pci_devices_for_instance'), mock.patch.object(instance, 'save') - ) as ( - mock_capabilities, mock_create_resource_req, mock_claim_pci, - mock_allocate_pci, mock_save): + ) as (mock_capabilities, mock_create_resource_req, mock_claim_pci, + mock_allocate_pci, mock_save): pci_req = objects.InstancePCIRequest(request_id=uuids.pci_req) pci_device = objects.PciDevice(request_id=pci_req.request_id) @@ -10461,9 +10461,8 @@ class ComputeAPITestCase(BaseTestCase): 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): + ) as (mock_notify, mock_attach, mock_allocate, mock_deallocate, + mock_dict, mock_claim_pci): mock_allocate.return_value = nwinfo mock_attach.side_effect = exception.NovaException("attach_failed") diff --git a/nova/tests/unit/objects/test_instance.py b/nova/tests/unit/objects/test_instance.py index 1ea97ef07af7..63e82eab5aa9 100644 --- a/nova/tests/unit/objects/test_instance.py +++ b/nova/tests/unit/objects/test_instance.py @@ -1631,6 +1631,27 @@ class TestInstanceObject(test_objects._LocalTest, self._test_save_objectfield_fk_constraint_fails( 'other_foreign_key', db_exc.DBReferenceError) + def test_remove_pci_device_and_request(self): + instance = fake_instance.fake_instance_obj(self.context) + pci_device = objects.PciDevice(request_id=uuids.req) + pci_request = objects.InstancePCIRequest(request_id=uuids.req) + other_device = objects.PciDevice(request_id=uuids.other_req) + other_request = objects.InstancePCIRequest(request_id=uuids.other_req) + instance.add_pci_device_and_request(pci_device, pci_request) + instance.add_pci_device_and_request(other_device, other_request) + + self.assertIn(pci_device, instance.pci_devices) + self.assertIn(pci_request, instance.pci_requests.requests) + self.assertIn(other_device, instance.pci_devices) + self.assertIn(other_request, instance.pci_requests.requests) + + instance.remove_pci_device_and_request(pci_device) + + self.assertNotIn(pci_device, instance.pci_devices) + self.assertNotIn(pci_request, instance.pci_requests.requests) + self.assertIn(other_device, instance.pci_devices) + self.assertIn(other_request, instance.pci_requests.requests) + class TestRemoteInstanceObject(test_objects._RemoteTest, _TestInstanceObject):