Make resize/migrated shared storage aware

Fixes bug 1177247 (for stable/grizzly)

Added some logic to check for whether or not we are on a shared
filesystem and set shared_storage accordingly. We perform similar
checks in other areas of the code, typically through RPC calls.
However, all the resize/migrate code is slated to be refactored for
Hava, so the idea was to keep this patch as minimally intrusive as
possible.

When shared_storage is true, we pass that on to the cleanup call
so that it no longer executes an rm via SSH, which was ultimately
destroying the original instance directory.

Change-Id: Ie9decedd373c000211c171df64e1e96fe78e5081
Cherry-Pick: 9290bdd

nova/tests/test_libvirt.py

@@ -3715,6 +3715,38 @@ def test_set_cache_mode_invalid_object(self):
         conn.set_cache_mode(fake_conf)
         self.assertEqual(fake_conf.driver_cache, 'fake')
 
+    def _test_shared_storage_detection(self, is_same):
+        conn = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
+        self.mox.StubOutWithMock(conn, 'get_host_ip_addr')
+        self.mox.StubOutWithMock(utils, 'execute')
+        self.mox.StubOutWithMock(os.path, 'exists')
+        self.mox.StubOutWithMock(os, 'unlink')
+        conn.get_host_ip_addr().AndReturn('bar')
+        utils.execute('ssh', 'foo', 'touch', mox.IgnoreArg())
+        os.path.exists(mox.IgnoreArg()).AndReturn(is_same)
+        if is_same:
+            os.unlink(mox.IgnoreArg())
+        else:
+            utils.execute('ssh', 'foo', 'rm', mox.IgnoreArg())
+        self.mox.ReplayAll()
+        return conn._is_storage_shared_with('foo', '/path')
+
+    def test_shared_storage_detection_same_host(self):
+        self.assertTrue(self._test_shared_storage_detection(True))
+
+    def test_shared_storage_detection_different_host(self):
+        self.assertFalse(self._test_shared_storage_detection(False))
+
+    def test_shared_storage_detection_easy(self):
+        conn = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), True)
+        self.mox.StubOutWithMock(conn, 'get_host_ip_addr')
+        self.mox.StubOutWithMock(utils, 'execute')
+        self.mox.StubOutWithMock(os.path, 'exists')
+        self.mox.StubOutWithMock(os, 'unlink')
+        conn.get_host_ip_addr().AndReturn('foo')
+        self.mox.ReplayAll()
+        self.assertTrue(conn._is_storage_shared_with('foo', '/path'))
+
 
 class HostStateTestCase(test.TestCase):
 
@@ -4734,6 +4766,7 @@ def test_migrate_disk_and_power_off_exception(self):
         .migrate_disk_and_power_off. """
 
         self.counter = 0
+        self.checked_shared_storage = False
 
         def fake_get_instance_disk_info(instance, xml=None):
             return '[]'
@@ -4752,11 +4785,17 @@ def fake_execute(*args, **kwargs):
         def fake_os_path_exists(path):
             return True
 
+        def fake_is_storage_shared(dest, inst_base):
+            self.checked_shared_storage = True
+            return False
+
         self.stubs.Set(self.libvirtconnection, 'get_instance_disk_info',
                        fake_get_instance_disk_info)
         self.stubs.Set(self.libvirtconnection, '_destroy', fake_destroy)
         self.stubs.Set(self.libvirtconnection, 'get_host_ip_addr',
                        fake_get_host_ip_addr)
+        self.stubs.Set(self.libvirtconnection, '_is_storage_shared_with',
+                       fake_is_storage_shared)
         self.stubs.Set(utils, 'execute', fake_execute)
         self.stubs.Set(os.path, 'exists', fake_os_path_exists)
 

nova/virt/libvirt/driver.py

@@ -3413,16 +3413,39 @@ def manage_image_cache(self, context, all_instances):
         """Manage the local cache of images."""
         self.image_cache_manager.verify_base_images(context, all_instances)
 
-    def _cleanup_remote_migration(self, dest, inst_base, inst_base_resize):
+    def _cleanup_remote_migration(self, dest, inst_base, inst_base_resize,
+                                  shared_storage=False):
         """Used only for cleanup in case migrate_disk_and_power_off fails."""
         try:
             if os.path.exists(inst_base_resize):
                 utils.execute('rm', '-rf', inst_base)
                 utils.execute('mv', inst_base_resize, inst_base)
-                utils.execute('ssh', dest, 'rm', '-rf', inst_base)
+                if not shared_storage:
+                    utils.execute('ssh', dest, 'rm', '-rf', inst_base)
         except Exception:
             pass
 
+    def _is_storage_shared_with(self, dest, inst_base):
+        # NOTE (rmk): There are two methods of determining whether we are
+        #             on the same filesystem: the source and dest IP are the
+        #             same, or we create a file on the dest system via SSH
+        #             and check whether the source system can also see it.
+        shared_storage = (dest == self.get_host_ip_addr())
+        if not shared_storage:
+            tmp_file = uuid.uuid4().hex + '.tmp'
+            tmp_path = os.path.join(inst_base, tmp_file)
+
+            try:
+                utils.execute('ssh', dest, 'touch', tmp_path)
+                if os.path.exists(tmp_path):
+                    shared_storage = True
+                    os.unlink(tmp_path)
+                else:
+                    utils.execute('ssh', dest, 'rm', tmp_path)
+            except Exception:
+                pass
+        return shared_storage
+
     def migrate_disk_and_power_off(self, context, instance, dest,
                                    instance_type, network_info,
                                    block_device_info=None):
@@ -3445,12 +3468,13 @@ def migrate_disk_and_power_off(self, context, instance, dest,
         # copy disks to destination
         # rename instance dir to +_resize at first for using
         # shared storage for instance dir (eg. NFS).
-        same_host = (dest == self.get_host_ip_addr())
         inst_base = libvirt_utils.get_instance_path(instance)
         inst_base_resize = inst_base + "_resize"
+
+        shared_storage = self._is_storage_shared_with(dest, inst_base)
         try:
             utils.execute('mv', inst_base, inst_base_resize)
-            if same_host:
+            if shared_storage:
                 dest = None
                 utils.execute('mkdir', '-p', inst_base)
             else:
@@ -3466,7 +3490,7 @@ def migrate_disk_and_power_off(self, context, instance, dest,
                     utils.execute('qemu-img', 'convert', '-f', 'qcow2',
                                   '-O', 'qcow2', from_path, tmp_path)
 
-                    if same_host:
+                    if shared_storage:
                         utils.execute('mv', tmp_path, img_path)
                     else:
                         libvirt_utils.copy_image(tmp_path, img_path, host=dest)
@@ -3477,7 +3501,8 @@ def migrate_disk_and_power_off(self, context, instance, dest,
         except Exception:
             with excutils.save_and_reraise_exception():
                 self._cleanup_remote_migration(dest, inst_base,
-                                               inst_base_resize)
+                                               inst_base_resize,
+                                               shared_storage)
 
         return disk_info_text