Merge "Follow up for I67504a37b0fe2ae5da3cba2f3122d9d0e18b9481"

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)
-
+                instance.remove_pci_device_and_request(pci_device)
-                # 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]
 
             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 InterfaceAttachPciClaimFailed: if the PCI device claim fails
-        :raises InterfaceAttachFailed: 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:
+        if len(requested_networks) != 1:
-            raise ValueError(
+            LOG.warning(
-                "Only support one interface per interface attach request")
+                "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(
+            LOG.warning(
-                "Only support one interface per interface attach request")
+                "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.add_pci_device_and_request(device, pci_req)
-        instance.pci_requests.requests.append(pci_req)
-        instance.pci_devices.objects.append(device)
 
         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
+                           for pci_device in instance.pci_devices.objects
-                            if pci_device.request_id == pci_req.request_id]
+                           if pci_device.request_id == pci_req.request_id]
 
             if not pci_devices:
                 LOG.warning(

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

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

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': {
+            {'interfaceAttachment': {'port_id': port_id}})
-                'port_id': port_id}})
         fake_notifier.wait_for_versioned_notifications(
             'instance.interface_attach.end')
 

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,
+            mock.patch.object(
-            '_claim_pci_device_for_interface_attach',
+                self.compute,
-            return_value=None)
+                "_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 (
+        ) as (mock_capabilities, mock_create_resource_req, mock_claim_pci,
-                mock_capabilities, mock_create_resource_req, mock_claim_pci,
+              mock_allocate_pci, mock_save):
-                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 (
+        ) as (mock_notify, mock_attach, mock_allocate, mock_deallocate,
-                mock_notify, mock_attach, mock_allocate, mock_deallocate,
+              mock_dict, mock_claim_pci):
-                mock_dict, mock_claim_pci):
 
             mock_allocate.return_value = nwinfo
             mock_attach.side_effect = exception.NovaException("attach_failed")

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):