Move get_pci_mapping_for_migration to MigrationContext

In order to fix Bug #1809095, it is required to update
PCI related VIFs with the original PCI address on the source
host to allow virt driver to properly unplug the VIF from hypervisor,
e.g allow the proper VF representor to be unplugged
from the integration bridge in case of a hardware offloaded OVS.

To do so, some preliminary work is needed to allow code-sharing
between nova.network.neutronv2 and nova.compute.manager

This change:
- Moves common logic to retrieve the PCI mapping between
  the source and destination node from nova.network.neutronv2
  to objects.migration_context.
- Makes code adjustments to methods in nova.network.neutronv2
  to accomodate the former.

Change-Id: I9a5118373548c525b2b1c2271e7d210cc92e4f4c
Partial-Bug: #1809095
(cherry picked from commit 5a1c385)

Author: adrianchiris

nova/network/neutronv2/api.py

@@ -3253,45 +3253,14 @@ def cleanup_instance_network_on_host(self, context, instance, host):
         # device_id field on the port which is not what we'd want for shelve.
         pass
 
-    def _get_pci_devices_from_migration_context(self, migration_context,
-                                                migration):
-        if migration and migration.get('status') == 'reverted':
-            # In case of revert, swap old and new devices to
-            # update the ports back to the original devices.
-            return (migration_context.new_pci_devices,
-                    migration_context.old_pci_devices)
-        return (migration_context.old_pci_devices,
-                migration_context.new_pci_devices)
-
-    def _get_pci_mapping_for_migration(self, context, instance, migration):
-        """Get the mapping between the old PCI devices and the new PCI
-        devices that have been allocated during this migration.  The
-        correlation is based on PCI request ID which is unique per PCI
-        devices for SR-IOV ports.
-
-        :param context:  The request context.
-        :param instance: Get PCI mapping for this instance.
-        :param migration: The migration for this instance.
-        :Returns: dictionary of mapping {'<old pci address>': <New PciDevice>}
-        """
-        migration_context = instance.migration_context
-        if not migration_context:
+    def _get_pci_mapping_for_migration(self, instance, migration):
+        if not instance.migration_context:
             return {}
-
-        old_pci_devices, new_pci_devices = \
-            self._get_pci_devices_from_migration_context(migration_context,
-                                                         migration)
-        if old_pci_devices and new_pci_devices:
-            LOG.debug("Determining PCI devices mapping using migration "
-                      "context: old_pci_devices: %(old)s, "
-                      "new_pci_devices: %(new)s",
-                      {'old': [dev for dev in old_pci_devices],
-                       'new': [dev for dev in new_pci_devices]})
-            return {old.address: new
-                    for old in old_pci_devices
-                        for new in new_pci_devices
-                            if old.request_id == new.request_id}
-        return {}
+        # In case of revert, swap old and new devices to
+        # update the ports back to the original devices.
+        revert = (migration and
+                  migration.get('status') == 'reverted')
+        return instance.migration_context.get_pci_mapping_for_migration(revert)
 
     def _update_port_binding_for_instance(self, context, instance, host,
                                           migration=None):
@@ -3337,7 +3306,7 @@ def _update_port_binding_for_instance(self, context, instance, host,
             if (vnic_type in network_model.VNIC_TYPES_SRIOV
                     and migration is not None):
                 if not pci_mapping:
-                    pci_mapping = self._get_pci_mapping_for_migration(context,
+                    pci_mapping = self._get_pci_mapping_for_migration(
                         instance, migration)
 
                 pci_slot = binding_profile.get('pci_slot')

nova/objects/migration_context.py

@@ -12,6 +12,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from oslo_log import log as logging
 from oslo_serialization import jsonutils
 from oslo_utils import versionutils
 
@@ -20,6 +21,8 @@
 from nova.objects import base
 from nova.objects import fields
 
+LOG = logging.getLogger(__name__)
+
 
 @base.NovaObjectRegistry.register
 class MigrationContext(base.NovaPersistentObject, base.NovaObject):
@@ -80,3 +83,32 @@ def get_by_instance_uuid(cls, context, instance_uuid):
             return None
 
         return cls.obj_from_db_obj(db_extra['migration_context'])
+
+    def get_pci_mapping_for_migration(self, revert):
+        """Get the mapping between the old PCI devices and the new PCI
+        devices that have been allocated during this migration.  The
+        correlation is based on PCI request ID which is unique per PCI
+        devices for SR-IOV ports.
+
+        :param revert: If True, return a reverse mapping i.e
+               mapping between new PCI devices and old PCI devices.
+        :returns: dictionary of PCI mapping.
+                  if revert==False:
+                      {'<old pci address>': <New PciDevice>}
+                  if revert==True:
+                      {'<new pci address>': <Old PciDevice>}
+        """
+        step = -1 if revert else 1
+        current_pci_devs, updated_pci_devs = (self.old_pci_devices,
+                                              self.new_pci_devices)[::step]
+        if current_pci_devs and updated_pci_devs:
+            LOG.debug("Determining PCI devices mapping using migration "
+                      "context: current_pci_devs: %(cur)s, "
+                      "updated_pci_devs: %(upd)s",
+                      {'cur': [dev for dev in current_pci_devs],
+                       'upd': [dev for dev in updated_pci_devs]})
+            return {curr_dev.address: upd_dev
+                    for curr_dev in current_pci_devs
+                        for upd_dev in updated_pci_devs
+                            if curr_dev.request_id == upd_dev.request_id}
+        return {}

nova/tests/unit/network/test_neutronv2.py

@@ -4681,56 +4681,29 @@ def test_update_port_bindings_for_instance_with_diff_host_unbound_vif_type(
     def test_get_pci_mapping_for_migration(self):
         instance = fake_instance.fake_instance_obj(self.context)
         instance.migration_context = objects.MigrationContext()
-        old_pci_devices = objects.PciDeviceList(
-            objects=[objects.PciDevice(vendor_id='1377',
-                                       product_id='0047',
-                                       address='0000:0a:00.1',
-                                       compute_node_id=1,
-                                       request_id='1234567890')])
-
-        new_pci_devices = objects.PciDeviceList(
-            objects=[objects.PciDevice(vendor_id='1377',
-                                       product_id='0047',
-                                       address='0000:0b:00.1',
-                                       compute_node_id=2,
-                                       request_id='1234567890')])
-
-        instance.migration_context.old_pci_devices = old_pci_devices
-        instance.migration_context.new_pci_devices = new_pci_devices
-        instance.pci_devices = instance.migration_context.old_pci_devices
         migration = {'status': 'confirmed'}
 
-        pci_mapping = self.api._get_pci_mapping_for_migration(
-            self.context, instance, migration)
-        self.assertEqual(
-            {old_pci_devices[0].address: new_pci_devices[0]}, pci_mapping)
+        with mock.patch.object(instance.migration_context,
+                               'get_pci_mapping_for_migration') as map_func:
+            self.api._get_pci_mapping_for_migration(instance, migration)
+            map_func.assert_called_with(False)
 
     def test_get_pci_mapping_for_migration_reverted(self):
         instance = fake_instance.fake_instance_obj(self.context)
         instance.migration_context = objects.MigrationContext()
-        old_pci_devices = objects.PciDeviceList(
-            objects=[objects.PciDevice(vendor_id='1377',
-                                       product_id='0047',
-                                       address='0000:0a:00.1',
-                                       compute_node_id=1,
-                                       request_id='1234567890')])
-
-        new_pci_devices = objects.PciDeviceList(
-            objects=[objects.PciDevice(vendor_id='1377',
-                                       product_id='0047',
-                                       address='0000:0b:00.1',
-                                       compute_node_id=2,
-                                       request_id='1234567890')])
-
-        instance.migration_context.old_pci_devices = old_pci_devices
-        instance.migration_context.new_pci_devices = new_pci_devices
-        instance.pci_devices = instance.migration_context.old_pci_devices
         migration = {'status': 'reverted'}
 
+        with mock.patch.object(instance.migration_context,
+                               'get_pci_mapping_for_migration') as map_func:
+            self.api._get_pci_mapping_for_migration(instance, migration)
+            map_func.assert_called_with(True)
+
+    def test_get_pci_mapping_for_migration_no_migration_context(self):
+        instance = fake_instance.fake_instance_obj(self.context)
+        instance.migration_context = None
         pci_mapping = self.api._get_pci_mapping_for_migration(
-            self.context, instance, migration)
-        self.assertEqual(
-            {new_pci_devices[0].address: old_pci_devices[0]}, pci_mapping)
+            instance, None)
+        self.assertDictEqual({}, pci_mapping)
 
     @mock.patch.object(neutronapi, 'get_client', return_value=mock.Mock())
     def test_update_port_profile_for_migration_teardown_false(

nova/tests/unit/objects/test_migration_context.py

@@ -52,6 +52,23 @@ def get_fake_migration_context_obj(ctxt):
     return obj
 
 
+def get_fake_migration_context_with_pci_devs(ctxt=None):
+    obj = get_fake_migration_context_obj(ctxt)
+    obj.old_pci_devices = objects.PciDeviceList(
+        objects=[objects.PciDevice(vendor_id='1377',
+                                   product_id='0047',
+                                   address='0000:0a:00.1',
+                                   compute_node_id=1,
+                                   request_id=uuids.pcidev)])
+    obj.new_pci_devices = objects.PciDeviceList(
+        objects=[objects.PciDevice(vendor_id='1377',
+                                   product_id='0047',
+                                   address='0000:0b:00.1',
+                                   compute_node_id=2,
+                                   request_id=uuids.pcidev)])
+    return obj
+
+
 class _TestMigrationContext(object):
 
     def _test_get_by_instance_uuid(self, db_data):
@@ -104,7 +121,20 @@ def test_get_by_instance_uuid_missing(self, mock_get):
 
 
 class TestMigrationContext(test_objects._LocalTest, _TestMigrationContext):
-    pass
+
+    def test_pci_mapping_for_migration(self):
+        mig_ctx = get_fake_migration_context_with_pci_devs()
+        pci_mapping = mig_ctx.get_pci_mapping_for_migration(False)
+        self.assertDictEqual(
+            {mig_ctx.old_pci_devices[0].address: mig_ctx.new_pci_devices[0]},
+            pci_mapping)
+
+    def test_pci_mapping_for_migration_revert(self):
+        mig_ctx = get_fake_migration_context_with_pci_devs()
+        pci_mapping = mig_ctx.get_pci_mapping_for_migration(True)
+        self.assertDictEqual(
+            {mig_ctx.new_pci_devices[0].address: mig_ctx.old_pci_devices[0]},
+            pci_mapping)
 
 
 class TestMigrationContextRemote(test_objects._RemoteTest,