Merge "Fix pre_live_migration rollback"

3 files changed, 17 insertions, 13 deletions

diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index a560c036cd..f885d95c4d 100644
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -8412,7 +8412,8 @@ class ComputeManager(manager.Manager):
         migrate_data.migration = migration
         self._rollback_live_migration(context, instance, dest,
                                       migrate_data=migrate_data,
-                                      source_bdms=source_bdms)
+                                      source_bdms=source_bdms,
+                                      pre_live_migration=True)
 
     def _do_pre_live_migration_from_source(self, context, dest, instance,
                                            block_migration, migration,
@@ -9167,7 +9168,8 @@ class ComputeManager(manager.Manager):
     def _rollback_live_migration(self, context, instance,
                                  dest, migrate_data=None,
                                  migration_status='failed',
-                                 source_bdms=None):
+                                 source_bdms=None,
+                                 pre_live_migration=False):
         """Recovers Instance/volume state from migrating -> running.
 
         :param context: security context
@@ -9217,8 +9219,14 @@ class ComputeManager(manager.Manager):
         #                  for nova-network)
         # NOTE(mriedem): This is a no-op for neutron.
         self.network_api.setup_networks_on_host(context, instance, self.host)
-        self.driver.rollback_live_migration_at_source(context, instance,
-                                                      migrate_data)
+
+        # NOTE(erlon): We should make sure that rollback_live_migration_at_src
+        # is not called in the pre_live_migration rollback as that will trigger
+        # the src host to re-attach interfaces which were not detached
+        # previously.
+        if not pre_live_migration:
+            self.driver.rollback_live_migration_at_source(context, instance,
+                                                          migrate_data)
 
         # NOTE(lyarwood): Fetch the current list of BDMs, disconnect any
         # connected volumes from the dest and delete any volume attachments
diff --git a/nova/tests/functional/regressions/test_bug_1944619.py b/nova/tests/functional/regressions/test_bug_1944619.py
index 3274ff5a15..82b7475dca 100644
--- a/nova/tests/functional/regressions/test_bug_1944619.py
+++ b/nova/tests/functional/regressions/test_bug_1944619.py
@@ -72,11 +72,5 @@ class TestRollbackWithHWOffloadedOVS(
                 self._live_migrate(self.server,
                                    migration_expected_state='failed',
                                    server_expected_state='MIGRATING')
-        # FIXME(erlon): In the current behavior,
-        # rollback_live_migration_at_source is called if an error happens
-        # during the  pre_live_migration phase on the destination and therefore
-        # triggers the observed bug. rollback_live_migration_at_source should
-        # *not* be called for when errors happen during pre_live_migration
-        # phase.
-        mlpr.assert_called_once()
+        mlpr.assert_not_called()
         mlpp.assert_called_once()
diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py
index cd1a9369c4..760ea79e87 100644
--- a/nova/tests/unit/compute/test_compute_mgr.py
+++ b/nova/tests/unit/compute/test_compute_mgr.py
@@ -9539,7 +9539,8 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase,
         self.assertEqual('error', self.migration.status)
         mock_rollback_live_mig.assert_called_once_with(
             self.context, self.instance, 'dest-host',
-            migrate_data=migrate_data, source_bdms=source_bdms)
+            migrate_data=migrate_data, source_bdms=source_bdms,
+            pre_live_migration=True)
 
     @mock.patch('nova.compute.rpcapi.ComputeAPI.pre_live_migration')
     @mock.patch('nova.compute.manager.ComputeManager._rollback_live_migration')
@@ -9574,7 +9575,8 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase,
         self.assertEqual('error', self.migration.status)
         mock_rollback_live_mig.assert_called_once_with(
             self.context, self.instance, 'dest-host',
-            migrate_data=migrate_data, source_bdms=source_bdms)
+            migrate_data=migrate_data, source_bdms=source_bdms,
+            pre_live_migration=True)
 
     @mock.patch('nova.compute.rpcapi.ComputeAPI.pre_live_migration')
     @mock.patch('nova.compute.manager.ComputeManager._rollback_live_migration')