Merge "xapi: cleanup volume sr on live migration rollback"

nova/tests/unit/virt/xenapi/test_vmops.py

@@ -1286,6 +1286,31 @@ class LiveMigrateHelperTestCase(VMOpsTestBase):
                                                  "sr_uuid")
 
 
+class RollbackLiveMigrateDestinationTestCase(VMOpsTestBase):
+    @mock.patch.object(volume_utils, 'find_sr_by_uuid', return_value='sr_ref')
+    @mock.patch.object(volume_utils, 'forget_sr')
+    def test_rollback_dest_calls_sr_forget(self, forget_sr, sr_ref):
+        block_device_info = {'block_device_mapping': [{'connection_info':
+                                {'data': {'volume_id': 'fake-uuid',
+                                          'target_iqn': 'fake-iqn',
+                                          'target_portal': 'fake-portal'}}}]}
+        self.vmops.rollback_live_migration_at_destination('instance',
+                                                          block_device_info)
+        forget_sr.assert_called_once_with(self.vmops._session, 'sr_ref')
+
+    @mock.patch.object(volume_utils, 'forget_sr')
+    @mock.patch.object(volume_utils, 'find_sr_by_uuid',
+                       side_effect=test.TestingException)
+    def test_rollback_dest_handles_exception(self, find_sr_ref, forget_sr):
+        block_device_info = {'block_device_mapping': [{'connection_info':
+                                {'data': {'volume_id': 'fake-uuid',
+                                          'target_iqn': 'fake-iqn',
+                                          'target_portal': 'fake-portal'}}}]}
+        self.vmops.rollback_live_migration_at_destination('instance',
+                                                          block_device_info)
+        self.assertFalse(forget_sr.called)
+
+
 @mock.patch.object(vmops.VMOps, '_resize_ensure_vm_is_shutdown')
 @mock.patch.object(vmops.VMOps, '_apply_orig_vm_name_label')
 @mock.patch.object(vmops.VMOps, '_update_instance_progress')

nova/tests/unit/virt/xenapi/test_xenapi.py

@@ -3814,7 +3814,7 @@ class XenAPILiveMigrateTestCase(stubs.XenAPITestBaseNoDB):
 
         with mock.patch.object(conn, "destroy") as mock_destroy:
             conn.rollback_live_migration_at_destination("context",
-                    "instance", [], None)
+                    "instance", [], {'block_device_mapping': []})
             self.assertFalse(mock_destroy.called)
 
 

nova/virt/xenapi/driver.py

@@ -550,8 +550,11 @@ class XenAPIDriver(driver.ComputeDriver):
         # NOTE(johngarbutt) Destroying the VM is not appropriate here
         # and in the cases where it might make sense,
         # XenServer has already done it.
-        # TODO(johngarbutt) investigate if any cleanup is required here
-        pass
+        # NOTE(sulo): The only cleanup we do explicitly is to forget
+        # any volume that was attached to the destination during
+        # live migration. XAPI should take care of all other cleanup.
+        self._vmops.rollback_live_migration_at_destination(instance,
+                                                           block_device_info)
 
     def pre_live_migration(self, context, instance, block_device_info,
                            network_info, disk_info, migrate_data=None):

nova/virt/xenapi/vmops.py

@@ -2370,6 +2370,23 @@ class VMOps(object):
         vm_ref = self._get_vm_opaque_ref(instance)
         vm_utils.strip_base_mirror_from_vdis(self._session, vm_ref)
 
+    def rollback_live_migration_at_destination(self, instance,
+                                               block_device_info):
+        bdms = block_device_info['block_device_mapping'] or []
+
+        for bdm in bdms:
+            conn_data = bdm['connection_info']['data']
+            uuid, label, params = volume_utils.parse_sr_info(conn_data)
+            try:
+                sr_ref = volume_utils.find_sr_by_uuid(self._session,
+                                                      uuid)
+
+                if sr_ref:
+                    volume_utils.forget_sr(self._session, sr_ref)
+            except Exception:
+                LOG.exception(_LE('Failed to forget the SR for volume %s'),
+                              params['id'], instance=instance)
+
     def get_per_instance_usage(self):
         """Get usage info about each active instance."""
         usage = {}