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.

Partial-Bug: #1809095

Change-Id: I9a5118373548c525b2b1c2271e7d210cc92e4f4c
(cherry picked from commit 84bb00a86da539183211364961ada2c1b1bb5edc)
(cherry picked from commit 28e7be8c8be5609ee33ff3eedacd20a70b9a409d)

4 files changed, 85 insertions, 81 deletions

diff --git a/nova/network/neutronv2/api.py b/nova/network/neutronv2/api.py
index 47b7ac7dcd..ba2911ae06 100644
--- a/nova/network/neutronv2/api.py
+++ b/nova/network/neutronv2/api.py
@@ -2647,45 +2647,14 @@ class API(base_api.NetworkAPI):
         # 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):
@@ -2724,7 +2693,7 @@ class API(base_api.NetworkAPI):
             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')
diff --git a/nova/objects/migration_context.py b/nova/objects/migration_context.py
index 9774eeada5..e882ef97d8 100644
--- a/nova/objects/migration_context.py
+++ b/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 import exception
 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 @@ class MigrationContext(base.NovaPersistentObject, base.NovaObject):
             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 {}
diff --git a/nova/tests/unit/network/test_neutronv2.py b/nova/tests/unit/network/test_neutronv2.py
index 907aba7310..d24ac6a31e 100644
--- a/nova/tests/unit/network/test_neutronv2.py
+++ b/nova/tests/unit/network/test_neutronv2.py
@@ -4143,56 +4143,29 @@ class TestNeutronv2WithMock(test.TestCase):
     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(
diff --git a/nova/tests/unit/objects/test_migration_context.py b/nova/tests/unit/objects/test_migration_context.py
index b1c2d7bd72..d5e96e7a13 100644
--- a/nova/tests/unit/objects/test_migration_context.py
+++ b/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 @@ class _TestMigrationContext(object):
 
 
 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,