From 40a37269c52977a718e91012cfd4580b2e31ec65 Mon Sep 17 00:00:00 2001
From: He Jie Xu <soulxu@gmail.com>
Date: Fri, 7 Nov 2014 23:29:18 +0800
Subject: [PATCH] Set vm state error when raising unexpected exception in live
 migrate

The instance stuck at migrating task_state when unexpected exception.
This is confuse for user, this patch set the vm_state to error.

Change-Id: Ib1b97452bc5e777c66c4d368f71156dbe1e116b7
Partial-Bug: 1397153
---
 nova/conductor/manager.py                   | 25 +++++++++++-------
 nova/tests/unit/conductor/test_conductor.py | 28 +++++++++++----------
 2 files changed, 31 insertions(+), 22 deletions(-)

diff --git a/nova/conductor/manager.py b/nova/conductor/manager.py
index 5311ca661edc..f87470505c14 100644
--- a/nova/conductor/manager.py
+++ b/nova/conductor/manager.py
@@ -564,6 +564,19 @@ class ComputeTaskManager(base.Base):
     def _live_migrate(self, context, instance, scheduler_hint,
                       block_migration, disk_over_commit):
         destination = scheduler_hint.get("host")
+
+        def _set_vm_state(context, instance, ex, vm_state=None,
+                          task_state=None):
+            request_spec = {'instance_properties': {
+                'uuid': instance['uuid'], },
+            }
+            scheduler_utils.set_vm_state_and_notify(context,
+                'compute_task', 'migrate_server',
+                dict(vm_state=vm_state,
+                     task_state=task_state,
+                     expected_task_state=task_states.MIGRATING,),
+                ex, request_spec, self.db)
+
         try:
             live_migrate.execute(context, instance, destination,
                              block_migration, disk_over_commit)
@@ -581,20 +594,14 @@ class ComputeTaskManager(base.Base):
                 exception.LiveMigrationWithOldNovaNotSafe) as ex:
             with excutils.save_and_reraise_exception():
                 # TODO(johngarbutt) - eventually need instance actions here
-                request_spec = {'instance_properties': {
-                    'uuid': instance['uuid'], },
-                }
-                scheduler_utils.set_vm_state_and_notify(context,
-                        'compute_task', 'migrate_server',
-                        dict(vm_state=instance['vm_state'],
-                             task_state=None,
-                             expected_task_state=task_states.MIGRATING,),
-                        ex, request_spec, self.db)
+                _set_vm_state(context, instance, ex, instance['vm_state'])
         except Exception as ex:
             LOG.error(_LE('Migration of instance %(instance_id)s to host'
                           ' %(dest)s unexpectedly failed.'),
                       {'instance_id': instance['uuid'], 'dest': destination},
                       exc_info=True)
+            _set_vm_state(context, instance, ex, vm_states.ERROR,
+                          instance['task_state'])
             raise exception.MigrationError(reason=six.text_type(ex))
 
     def build_instances(self, context, instances, image, filter_properties,
diff --git a/nova/tests/unit/conductor/test_conductor.py b/nova/tests/unit/conductor/test_conductor.py
index 53357c4cefa4..f40883d93c60 100644
--- a/nova/tests/unit/conductor/test_conductor.py
+++ b/nova/tests/unit/conductor/test_conductor.py
@@ -1727,26 +1727,28 @@ class ConductorTaskTestCase(_BaseTaskTestCase, test_compute.BaseTestCase):
         ex = exc.LiveMigrationWithOldNovaNotSafe(server='dummy')
         self._test_migrate_server_deals_with_expected_exceptions(ex)
 
-    def test_migrate_server_deals_with_unexpected_exceptions(self):
+    @mock.patch.object(scheduler_utils, 'set_vm_state_and_notify')
+    @mock.patch.object(live_migrate, 'execute')
+    def test_migrate_server_deals_with_unexpected_exceptions(self,
+            mock_live_migrate, mock_set_state):
+        expected_ex = IOError('fake error')
+        mock_live_migrate.side_effect = expected_ex
         instance = fake_instance.fake_db_instance()
         inst_obj = objects.Instance._from_db_object(
             self.context, objects.Instance(), instance, [])
-        self.mox.StubOutWithMock(live_migrate, 'execute')
-        self.mox.StubOutWithMock(scheduler_utils,
-                'set_vm_state_and_notify')
-
-        expected_ex = IOError('fake error')
-        live_migrate.execute(self.context, mox.IsA(objects.Instance),
-                             'destination', 'block_migration',
-                             'disk_over_commit').AndRaise(expected_ex)
-        self.mox.ReplayAll()
-
-        self.conductor = utils.ExceptionHelper(self.conductor)
-
         ex = self.assertRaises(exc.MigrationError,
             self.conductor.migrate_server, self.context, inst_obj,
             {'host': 'destination'}, True, False, None, 'block_migration',
             'disk_over_commit')
+        request_spec = {'instance_properties': {
+                'uuid': instance['uuid'], },
+        }
+        mock_set_state.assert_called_once_with(self.context,
+                        'compute_task', 'migrate_server',
+                        dict(vm_state=vm_states.ERROR,
+                             task_state=inst_obj.task_state,
+                             expected_task_state=task_states.MIGRATING,),
+                        expected_ex, request_spec, self.conductor.db)
         self.assertEqual(ex.kwargs['reason'], six.text_type(expected_ex))
 
     def test_set_vm_state_and_notify(self):