Handle instance directories correctly for migrates.

In Grizzly we changed how we name instance directories. Unfortunately
the decision as to what directory name to use is based on whether
there is a pre-existing directory on the compute node with the old
name.

In the case of live migration, that doesn't work because we have
libvirt copy the directory across for us. So, we need to pass
around the directory name in the migration request. This is better
anyway, as it means that ops types don't need to handle an instance
directory magically renaming itself at some point.

Resolves bug 1175286.

Change-Id: Ib102e644a8742726fa2049303354d466b8109e0b

4 files changed, 40 insertions, 16 deletions

diff --git a/nova/tests/virt/libvirt/fake_libvirt_utils.py b/nova/tests/virt/libvirt/fake_libvirt_utils.py
index ba00a7091..23b758e03 100644
--- a/nova/tests/virt/libvirt/fake_libvirt_utils.py
+++ b/nova/tests/virt/libvirt/fake_libvirt_utils.py
@@ -197,8 +197,9 @@ def fetch_image(context, target, image_id, user_id, project_id):
     pass
 
 
-def get_instance_path(instance, forceold=False):
-    return libvirt_utils.get_instance_path(instance, forceold=forceold)
+def get_instance_path(instance, forceold=False, relative=False):
+    return libvirt_utils.get_instance_path(instance, forceold=forceold,
+                                           relative=relative)
 
 
 def pick_disk_driver_name(is_block_dev=False):
diff --git a/nova/tests/virt/libvirt/test_libvirt.py b/nova/tests/virt/libvirt/test_libvirt.py
index 6b70fe1c6..484162d54 100644
--- a/nova/tests/virt/libvirt/test_libvirt.py
+++ b/nova/tests/virt/libvirt/test_libvirt.py
@@ -2264,9 +2264,9 @@ class LibvirtConnTestCase(test.TestCase):
         conn._check_shared_storage_test_file("file").AndReturn(False)
 
         self.mox.StubOutWithMock(conn, "_assert_dest_node_has_enough_disk")
-        conn._assert_dest_node_has_enough_disk(self.context, instance_ref,
-                                        dest_check_data['disk_available_mb'],
-                                               False)
+        conn._assert_dest_node_has_enough_disk(
+            self.context, instance_ref, dest_check_data['disk_available_mb'],
+            False)
 
         self.mox.ReplayAll()
         conn.check_can_live_migrate_source(self.context, instance_ref,
@@ -2467,13 +2467,14 @@ class LibvirtConnTestCase(test.TestCase):
             self.mox.ReplayAll()
             migrate_data = {'is_shared_storage': False,
                             'is_volume_backed': True,
-                            'block_migration': False
+                            'block_migration': False,
+                            'instance_relative_path': inst_ref['name']
                             }
             ret = conn.pre_live_migration(c, inst_ref, vol, nw_info,
                                           migrate_data)
             self.assertEqual(ret, None)
             self.assertTrue(os.path.exists('%s/%s/' % (tmpdir,
-                                                       inst_ref['uuid'])))
+                                                       inst_ref['name'])))
         db.instance_destroy(self.context, inst_ref['uuid'])
 
     def test_pre_block_migration_works_correctly(self):
diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py
index becc132de..36ded5f29 100755
--- a/nova/virt/libvirt/driver.py
+++ b/nova/virt/libvirt/driver.py
@@ -2975,15 +2975,14 @@ class LibvirtDriver(driver.ComputeDriver):
         filename = dest_check_data["filename"]
         self._cleanup_shared_storage_test_file(filename)
 
-    def check_can_live_migrate_source(self, ctxt, instance_ref,
-                                      dest_check_data):
+    def check_can_live_migrate_source(self, ctxt, instance, dest_check_data):
         """Check if it is possible to execute live migration.
 
         This checks if the live migration can succeed, based on the
         results from check_can_live_migrate_destination.
 
         :param context: security context
-        :param instance_ref: nova.db.sqlalchemy.models.Instance
+        :param instance: nova.db.sqlalchemy.models.Instance
         :param dest_check_data: result of check_can_live_migrate_destination
         :returns: a dict containing migration info
         """
@@ -3001,7 +3000,7 @@ class LibvirtDriver(driver.ComputeDriver):
                 reason = _("Block migration can not be used "
                            "with shared storage.")
                 raise exception.InvalidLocalStorage(reason=reason, path=source)
-            self._assert_dest_node_has_enough_disk(ctxt, instance_ref,
+            self._assert_dest_node_has_enough_disk(ctxt, instance,
                                     dest_check_data['disk_available_mb'],
                                     dest_check_data['disk_over_commit'])
 
@@ -3010,6 +3009,14 @@ class LibvirtDriver(driver.ComputeDriver):
                        "without shared storage.")
             raise exception.InvalidSharedStorage(reason=reason, path=source)
         dest_check_data.update({"is_shared_storage": shared})
+
+        # NOTE(mikal): include the instance directory name here because it
+        # doesn't yet exist on the destination but we want to force that
+        # same name to be used
+        instance_path = libvirt_utils.get_instance_path(instance,
+                                                        relative=True)
+        dest_check_data['instance_relative_path'] = instance_path
+
         return dest_check_data
 
     def _assert_dest_node_has_enough_disk(self, context, instance_ref,
@@ -3260,19 +3267,28 @@ class LibvirtDriver(driver.ComputeDriver):
         is_shared_storage = True
         is_volume_backed = False
         is_block_migration = True
+        instance_relative_path = None
         if migrate_data:
             is_shared_storage = migrate_data.get('is_shared_storage', True)
             is_volume_backed = migrate_data.get('is_volume_backed', False)
             is_block_migration = migrate_data.get('block_migration', True)
+            instance_relative_path = migrate_data.get('instance_relative_path')
+
+        if not is_shared_storage:
+            # NOTE(mikal): this doesn't use libvirt_utils.get_instance_path
+            # because we are ensuring that the same instance directory name
+            # is used as was at the source
+            if instance_relative_path:
+                instance_dir = os.path.join(CONF.instances_path,
+                                            instance_relative_path)
+            else:
+                instance_dir = libvirt_utils.get_instance_path(instance)
 
-        if is_volume_backed and not (is_block_migration or is_shared_storage):
-
-            # Create the instance directory on destination compute node.
-            instance_dir = libvirt_utils.get_instance_path(instance)
             if os.path.exists(instance_dir):
                 raise exception.DestinationDiskExists(path=instance_dir)
             os.mkdir(instance_dir)
 
+        if is_volume_backed and not (is_block_migration or is_shared_storage):
             # Touch the console.log file, required by libvirt.
             console_file = self._get_console_log_path(instance)
             libvirt_utils.file_open(console_file, 'a').close()
diff --git a/nova/virt/libvirt/utils.py b/nova/virt/libvirt/utils.py
index 6853ed1e8..c444dcfb2 100755
--- a/nova/virt/libvirt/utils.py
+++ b/nova/virt/libvirt/utils.py
@@ -601,7 +601,7 @@ def fetch_image(context, target, image_id, user_id, project_id):
     images.fetch_to_raw(context, image_id, target, user_id, project_id)
 
 
-def get_instance_path(instance, forceold=False):
+def get_instance_path(instance, forceold=False, relative=False):
     """Determine the correct path for instance storage.
 
     This method determines the directory name for instance storage, while
@@ -610,10 +610,16 @@ def get_instance_path(instance, forceold=False):
 
     :param instance: the instance we want a path for
     :param forceold: force the use of the pre-grizzly format
+    :param relative: if True, just the relative path is returned
 
     :returns: a path to store information about that instance
     """
     pre_grizzly_name = os.path.join(CONF.instances_path, instance['name'])
     if forceold or os.path.exists(pre_grizzly_name):
+        if relative:
+            return instance['name']
         return pre_grizzly_name
+
+    if relative:
+        return instance['uuid']
     return os.path.join(CONF.instances_path, instance['uuid'])