Merge "Move get_pci_mapping_for_migration to MigrationContext" into stable/stein

Author: Zuul

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,