Merge "Actually pass the migration data object down to the virt drivers"

nova/compute/manager.py

@@ -5110,7 +5110,10 @@ def pre_live_migration(self, context, instance, block_migration, disk,
                                        migrate_data)
         if isinstance(pre_live_migration_data,
                       migrate_data_obj.LiveMigrateData):
-            pre_live_migration_data = pre_live_migration_data.to_legacy_dict()
+            pre_live_migration_data = pre_live_migration_data.to_legacy_dict(
+                pre_migration_result=True)
+            pre_live_migration_data = pre_live_migration_data[
+                'pre_live_migration_result']
 
         # NOTE(tr3buchet): setup networks on destination host
         self.network_api.setup_networks_on_host(context, instance,
@@ -5131,6 +5134,21 @@ def pre_live_migration(self, context, instance, block_migration, disk,
 
         return pre_live_migration_data
 
+    def _get_migrate_data_obj(self):
+        # FIXME(danms): A couple patches from now, we'll be able to
+        # avoid this failure _if_ we get a new-style call with the
+        # object.
+        if CONF.compute_driver.startswith('libvirt'):
+            return objects.LibvirtLiveMigrateData()
+        elif CONF.compute_driver.startswith('xenapi'):
+            return objects.XenapiLiveMigrateData()
+        else:
+            LOG.error(_('Older RPC caller and unsupported virt driver in '
+                        'use. Unable to handle this!'))
+            raise exception.MigrationError(
+                _('Unknown compute driver while providing compatibility '
+                  'with older RPC formats'))
+
     def _do_live_migration(self, context, dest, instance, block_migration,
                            migration, migrate_data):
         # NOTE(danms): We should enhance the RT to account for migrations
@@ -5154,7 +5172,8 @@ def _do_live_migration(self, context, dest, instance, block_migration,
                 context, instance,
                 block_migration, disk, dest, migrate_data)
             migrate_data['pre_live_migration_result'] = pre_migration_data
-
+            migrate_data_obj = self._get_migrate_data_obj()
+            migrate_data_obj.from_legacy_dict(migrate_data)
         except Exception:
             with excutils.save_and_reraise_exception():
                 LOG.exception(_LE('Pre live migration failed at %s'),
@@ -5165,12 +5184,12 @@ def _do_live_migration(self, context, dest, instance, block_migration,
 
         self._set_migration_status(migration, 'running')
 
-        migrate_data['migration'] = migration
+        migrate_data_obj.migration = migration
         try:
             self.driver.live_migration(context, instance, dest,
                                        self._post_live_migration,
                                        self._rollback_live_migration,
-                                       block_migration, migrate_data)
+                                       block_migration, migrate_data_obj)
         except Exception:
             # Executing live migration
             # live_migration might raises exceptions, but
@@ -5227,11 +5246,9 @@ def _live_migration_cleanup_flags(self, block_migration, migrate_data):
         #                 block storage or instance path were shared
         is_shared_block_storage = not block_migration
         is_shared_instance_path = not block_migration
-        if migrate_data:
-            is_shared_block_storage = migrate_data.get(
-                    'is_shared_block_storage', is_shared_block_storage)
-            is_shared_instance_path = migrate_data.get(
-                    'is_shared_instance_path', is_shared_instance_path)
+        if isinstance(migrate_data, objects.LibvirtLiveMigrateData):
+            is_shared_block_storage = migrate_data.is_shared_block_storage
+            is_shared_instance_path = migrate_data.is_shared_instance_path
 
         # No instance booting at source host, but instance dir
         # must be deleted for preparing next block migration
@@ -5348,9 +5365,9 @@ def _post_live_migration(self, ctxt, instance,
                  instance=instance)
 
         self._clean_instance_console_tokens(ctxt, instance)
-        if migrate_data and migrate_data.get('migration'):
-            migrate_data['migration'].status = 'completed'
-            migrate_data['migration'].save()
+        if migrate_data and migrate_data.obj_attr_is_set('migration'):
+            migrate_data.migration.status = 'completed'
+            migrate_data.migration.save()
 
     def _consoles_enabled(self):
         """Returns whether a console is enable."""
@@ -5451,8 +5468,10 @@ def _rollback_live_migration(self, context, instance,
 
         # NOTE(danms): Pop out the migration object so we don't pass
         # it over RPC unintentionally below
-        if migrate_data:
+        if isinstance(migrate_data, dict):
             migration = migrate_data.pop('migration', None)
+        elif isinstance(migrate_data, migrate_data_obj.LiveMigrateData):
+            migration = migrate_data.migration
         else:
             migration = None
 
@@ -5473,6 +5492,8 @@ def _rollback_live_migration(self, context, instance,
                 block_migration, migrate_data)
 
         if do_cleanup:
+            if isinstance(migrate_data, migrate_data_obj.LiveMigrateData):
+                migrate_data = migrate_data.to_legacy_dict()
             self.compute_rpcapi.rollback_live_migration_at_destination(
                     context, instance, dest, destroy_disks=destroy_disks,
                     migrate_data=migrate_data)

nova/tests/unit/compute/test_compute.py

@@ -68,6 +68,7 @@
 from nova import objects
 from nova.objects import block_device as block_device_obj
 from nova.objects import instance as instance_obj
+from nova.objects import migrate_data as migrate_data_obj
 from nova import policy
 from nova import quota
 from nova.scheduler import client as scheduler_client
@@ -5625,11 +5626,13 @@ def test_live_migration_works_correctly(self, mock_save, event_mock):
 
         migration = objects.Migration()
 
-        ret = self.compute.live_migration(c, dest=dest,
-                                          instance=instance,
-                                          block_migration=False,
-                                          migration=migration,
-                                          migrate_data=migrate_data)
+        with mock.patch.object(self.compute, '_get_migrate_data_obj') as gmdo:
+            gmdo.return_value = migrate_data_obj.LiveMigrateData()
+            ret = self.compute.live_migration(c, dest=dest,
+                                              instance=instance,
+                                              block_migration=False,
+                                              migration=migration,
+                                              migrate_data=migrate_data)
 
         self.assertIsNone(ret)
         event_mock.assert_called_with(
@@ -5688,7 +5691,10 @@ def fakecleanup(*args, **kwargs):
 
         # start test
         self.mox.ReplayAll()
-        migrate_data = {'is_shared_instance_path': False}
+        migrate_data = objects.LibvirtLiveMigrateData(
+            is_shared_instance_path=False,
+            is_shared_block_storage=False,
+            block_migration=False)
         self.compute._post_live_migration(c, instance, dest,
                                           migrate_data=migrate_data)
         self.assertIn('cleanup', result)
@@ -5711,7 +5717,9 @@ def test_post_live_migration_working_correctly(self):
                         'power_state': power_state.PAUSED})
         instance.save()
 
-        migrate_data = {'migration': mock.MagicMock()}
+        migration_obj = objects.Migration()
+        migrate_data = migrate_data_obj.LiveMigrateData(
+            migration=migration_obj)
 
         # creating mocks
         with test.nested(
@@ -5727,12 +5735,13 @@ def test_post_live_migration_working_correctly(self):
                               'setup_networks_on_host'),
             mock.patch.object(self.compute.instance_events,
                               'clear_events_for_instance'),
-            mock.patch.object(self.compute, 'update_available_resource')
+            mock.patch.object(self.compute, 'update_available_resource'),
+            mock.patch.object(migration_obj, 'save'),
         ) as (
             post_live_migration, unfilter_instance,
             migrate_instance_start, post_live_migration_at_destination,
             post_live_migration_at_source, setup_networks_on_host,
-            clear_events, update_available_resource
+            clear_events, update_available_resource, mig_save
         ):
             self.compute._post_live_migration(c, instance, dest,
                                               migrate_data=migrate_data)
@@ -5753,8 +5762,8 @@ def test_post_live_migration_working_correctly(self):
                 [mock.call(c, instance, [])])
             clear_events.assert_called_once_with(instance)
             update_available_resource.assert_has_calls([mock.call(c)])
-            self.assertEqual('completed', migrate_data['migration'].status)
-            migrate_data['migration'].save.assert_called_once_with()
+            self.assertEqual('completed', migration_obj.status)
+            mig_save.assert_called_once_with()
 
     def test_post_live_migration_terminate_volume_connections(self):
         c = context.get_admin_context()

nova/tests/unit/objects/test_migrate_data.py

@@ -176,9 +176,10 @@ def test_from_legacy_with_pre_result(self):
             serial_listen_addr='127.0.0.1',
             bdms=[test_bdmi])
         obj2 = migrate_data.LibvirtLiveMigrateData()
-        obj2.from_legacy_dict(obj.to_legacy_dict())
+        obj2.from_legacy_dict(obj.to_legacy_dict(pre_migration_result=True))
         self.assertEqual(obj.to_legacy_dict(),
                          obj2.to_legacy_dict())
+        self.assertEqual(obj.bdms[0].serial, obj2.bdms[0].serial)
 
 
 class TestLibvirtLiveMigrateData(test_objects._LocalTest,