From 12b361f0b44f3d34899982b643482228044d3d86 Mon Sep 17 00:00:00 2001
From: Lucian Petrut <lpetrut@cloudbasesolutions.com>
Date: Mon, 26 Aug 2019 10:49:22 +0300
Subject: [PATCH] Avoid error state for recovered instances after failed
 migrations

Most users expect that if a live migration fails but the instance
is fully recovered, it shouldn't enter 'error' state. Setting
the migration status to 'error' should be enough. This simplifies
debugging, making it clear that the instance dosn't have to be
manually recovered.

This patch changed this behavior, indirectly affecting the Hyper-V
driver: Idfdce9e7dd8106af01db0358ada15737cb846395

We'll stop propagating exceptions when managing to recover the
instance, which matches the reference driver (libvirt) behavior.

Change-Id: I16c200ebf0a996865ddce5688434e9c9ad69e331
Closes-Bug: 1841411
---
 .../unit/virt/hyperv/test_livemigrationops.py | 20 ++++++++-----------
 nova/virt/hyperv/livemigrationops.py          |  9 ++++-----
 2 files changed, 12 insertions(+), 17 deletions(-)

diff --git a/nova/tests/unit/virt/hyperv/test_livemigrationops.py b/nova/tests/unit/virt/hyperv/test_livemigrationops.py
index de2431d31387..8a3df843b9ee 100644
--- a/nova/tests/unit/virt/hyperv/test_livemigrationops.py
+++ b/nova/tests/unit/virt/hyperv/test_livemigrationops.py
@@ -66,18 +66,7 @@ class LiveMigrationOpsTestCase(test_base.HyperVBaseTestCase):
         else:
             migrate_data = None
 
-        if side_effect is os_win_exc.HyperVException:
-            self.assertRaises(os_win_exc.HyperVException,
-                              self._livemigrops.live_migration,
-                              self.context, mock_instance, fake_dest,
-                              mock_post, mock_recover,
-                              mock.sentinel.block_migr,
-                              migrate_data)
-            mock_recover.assert_called_once_with(self.context, mock_instance,
-                                                 fake_dest,
-                                                 migrate_data)
-        else:
-            self._livemigrops.live_migration(context=self.context,
+        self._livemigrops.live_migration(context=self.context,
                                              instance_ref=mock_instance,
                                              dest=fake_dest,
                                              post_method=mock_post,
@@ -85,6 +74,13 @@ class LiveMigrationOpsTestCase(test_base.HyperVBaseTestCase):
                                              block_migration=(
                                                 mock.sentinel.block_migr),
                                              migrate_data=migrate_data)
+
+        if side_effect is os_win_exc.HyperVException:
+            mock_recover.assert_called_once_with(self.context, mock_instance,
+                                                 fake_dest,
+                                                 migrate_data)
+            mock_post.assert_not_called()
+        else:
             post_call_args = mock_post.call_args_list
             self.assertEqual(1, len(post_call_args))
 
diff --git a/nova/virt/hyperv/livemigrationops.py b/nova/virt/hyperv/livemigrationops.py
index 0cb76e94cbb3..1b0f81eae041 100644
--- a/nova/virt/hyperv/livemigrationops.py
+++ b/nova/virt/hyperv/livemigrationops.py
@@ -19,7 +19,6 @@ Management class for live migration VM operations.
 
 from os_win import utilsfactory
 from oslo_log import log as logging
-from oslo_utils import excutils
 
 import nova.conf
 from nova import exception
@@ -78,10 +77,10 @@ class LiveMigrationOps(object):
                 dest,
                 migrate_disks=not shared_storage)
         except Exception:
-            with excutils.save_and_reraise_exception():
-                LOG.debug("Calling live migration recover_method "
-                          "for instance: %s", instance_name)
-                recover_method(context, instance_ref, dest, migrate_data)
+            LOG.exception("Live migration failed. Attempting rollback.",
+                          instance=instance_ref)
+            recover_method(context, instance_ref, dest, migrate_data)
+            return
 
         LOG.debug("Calling live migration post_method for instance: %s",
                   instance_name)