Check instance on dest once during block migration

During live block migrations both pre_live_migration and
pre_block_migration check for the presence of the instance on
the destination and create the instance dir if it doesn't exist.

As a result the call to pre_block_migration always fails as
pre_live_migration has already created the instance dir on the
destination.

As it turns out the pre_block_migration call is completely
unnecessary. The libvirt driver is the only driver that uses the
call and the work it does can easily be done in the existing
pre_live_migration call. In order to streamline things we
completely remove the pre_block_migration call from all
drivers. Additionally we update the function definition for
pre_live_migration to pass needed disk info down to the
virt drivers.

Fixes bug: #1193359
Change-Id: Id9dfe482db3471d6a1b9b6c7d59a5ddc48775d7f

nova/compute/manager.py

@@ -3681,6 +3681,7 @@ class ComputeManager(manager.SchedulerDependentManager):
                                        instance,
                                        block_device_info,
                                        self._legacy_nw_info(network_info),
+                                       disk,
                                        migrate_data)
 
         # NOTE(tr3buchet): setup networks on destination host
@@ -3696,10 +3697,6 @@ class ComputeManager(manager.SchedulerDependentManager):
         self.driver.ensure_filtering_rules_for_instance(instance,
                                             self._legacy_nw_info(network_info))
 
-        # Preparation for block migration
-        if block_migration:
-            self.driver.pre_block_migration(context, instance, disk)
-
         self._notify_about_instance_usage(
                      context, instance, "live_migration.pre.end",
                      network_info=network_info)

nova/tests/compute/test_compute.py

@@ -3901,6 +3901,7 @@ class ComputeTestCase(BaseTestCase):
         self.compute.driver.pre_live_migration(mox.IsA(c), mox.IsA(instance),
                                                {'block_device_mapping': []},
                                                mox.IgnoreArg(),
+                                               mox.IgnoreArg(),
                                                mox.IgnoreArg())
         self.mox.StubOutWithMock(self.compute.driver,
                                  'ensure_filtering_rules_for_instance')

nova/tests/virt/hyperv/test_hypervapi.py

@@ -684,7 +684,7 @@ class HyperVAPITestCase(test.TestCase):
 
         self._mox.ReplayAll()
         self._conn.pre_live_migration(self._context, instance,
-                                      block_device_info, network_info)
+                                      block_device_info, None, network_info)
         self._mox.VerifyAll()
 
         if cow:

nova/tests/virt/libvirt/test_libvirt.py

@@ -19,7 +19,6 @@ import copy
 import errno
 import eventlet
 import fixtures
-import json
 import mox
 import os
 import re
@@ -2528,6 +2527,11 @@ class LibvirtConnTestCase(test.TestCase):
             def fixed_ips(self):
                 return ["test_ip_addr"]
 
+        def fake_none(*args, **kwargs):
+            return
+
+        self.stubs.Set(conn, '_create_images_and_backing', fake_none)
+
         inst_ref = {'id': 'foo'}
         c = context.get_admin_context()
         nw_info = FakeNetworkInfo()
@@ -2550,7 +2554,7 @@ class LibvirtConnTestCase(test.TestCase):
         conn.plug_vifs(mox.IsA(inst_ref), nw_info)
 
         self.mox.ReplayAll()
-        result = conn.pre_live_migration(c, inst_ref, vol, nw_info)
+        result = conn.pre_live_migration(c, inst_ref, vol, nw_info, None)
         self.assertEqual(result, None)
 
     def test_pre_live_migration_vol_backed_works_correctly_mocked(self):
@@ -2562,6 +2566,11 @@ class LibvirtConnTestCase(test.TestCase):
                   {'connection_info': 'dummy', 'mount_device': '/dev/sdb'}]}
             conn = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
 
+            def fake_none(*args, **kwargs):
+                return
+
+            self.stubs.Set(conn, '_create_images_and_backing', fake_none)
+
             class FakeNetworkInfo():
                 def fixed_ips(self):
                     return ["test_ip_addr"]
@@ -2587,50 +2596,13 @@ class LibvirtConnTestCase(test.TestCase):
                             'block_migration': False,
                             'instance_relative_path': inst_ref['name']
                             }
-            ret = conn.pre_live_migration(c, inst_ref, vol, nw_info,
+            ret = conn.pre_live_migration(c, inst_ref, vol, nw_info, None,
                                           migrate_data)
             self.assertEqual(ret, None)
             self.assertTrue(os.path.exists('%s/%s/' % (tmpdir,
                                                        inst_ref['name'])))
         db.instance_destroy(self.context, inst_ref['uuid'])
 
-    def test_pre_block_migration_works_correctly(self):
-        # Replace instances_path since this testcase creates tmpfile
-        with utils.tempdir() as tmpdir:
-            self.flags(instances_path=tmpdir)
-
-            # Test data
-            instance_ref = db.instance_create(self.context, self.test_instance)
-            dummy_info = [{'path': '%s/disk' % tmpdir,
-                           'disk_size': 10737418240,
-                           'type': 'raw',
-                           'backing_file': ''},
-                          {'backing_file': 'otherdisk_1234567',
-                           'path': '%s/otherdisk' % tmpdir,
-                           'virt_disk_size': 10737418240}]
-            dummyjson = json.dumps(dummy_info)
-
-            # qemu-img should be mockd since test environment might not have
-            # large disk space.
-            self.mox.StubOutWithMock(imagebackend.Image, 'cache')
-            imagebackend.Image.cache(context=mox.IgnoreArg(),
-                                     fetch_func=mox.IgnoreArg(),
-                                     filename='otherdisk_1234567',
-                                     image_id=self.test_instance['image_ref'],
-                                     project_id='fake',
-                                     size=10737418240L,
-                                     user_id=None).AndReturn(None)
-            self.mox.ReplayAll()
-
-            conn = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
-            conn.pre_block_migration(self.context, instance_ref,
-                                     dummyjson)
-
-            self.assertTrue(os.path.exists('%s/%s/' %
-                                           (tmpdir, instance_ref['uuid'])))
-
-        db.instance_destroy(self.context, instance_ref['uuid'])
-
     def test_get_instance_disk_info_works_correctly(self):
         # Test data
         instance_ref = db.instance_create(self.context, self.test_instance)

nova/tests/virt/xenapi/test_xenapi.py

@@ -3092,7 +3092,7 @@ class XenAPILiveMigrateTestCase(stubs.XenAPITestBase):
         # ensure method is present
         stubs.stubout_session(self.stubs, stubs.FakeSessionForVMTests)
         self.conn = xenapi_conn.XenAPIDriver(fake.FakeVirtAPI(), False)
-        self.conn.pre_live_migration(None, None, None, None)
+        self.conn.pre_live_migration(None, None, None, None, None)
 
     def test_post_live_migration_at_destination(self):
         # ensure method is present

nova/virt/driver.py

@@ -474,29 +474,19 @@ class ComputeDriver(object):
         """
         raise NotImplementedError()
 
-    def pre_live_migration(self, ctxt, instance_ref,
+    def pre_live_migration(self, ctxt, instance_ref, block_device_info,
-                           block_device_info, network_info,
+                           network_info, disk_info, migrate_data=None):
-                           migrate_data=None):
         """Prepare an instance for live migration
 
         :param ctxt: security context
         :param instance_ref: instance object that will be migrated
         :param block_device_info: instance block device information
         :param network_info: instance network information
+        :param disk_info: instance disk information
         :param migrate_data: implementation specific data dict.
         """
         raise NotImplementedError()
 
-    def pre_block_migration(self, ctxt, instance_ref, disk_info):
-        """Prepare a block device for migration
-
-        :param ctxt: security context
-        :param instance_ref: instance object that will have its disk migrated
-        :param disk_info: information about disk to be migrated (as returned
-                          from get_instance_disk_info())
-        """
-        raise NotImplementedError()
-
     def live_migration(self, ctxt, instance_ref, dest,
                        post_method, recover_method, block_migration=False,
                        migrate_data=None):

nova/virt/fake.py

@@ -382,7 +382,7 @@ class FakeDriver(driver.ComputeDriver):
         return
 
     def pre_live_migration(self, context, instance_ref, block_device_info,
-                           network_info, migrate_data=None):
+                           network_info, disk, migrate_data=None):
         return
 
     def unfilter_instance(self, instance_ref, network_info):

nova/virt/hyperv/driver.py

@@ -115,7 +115,7 @@ class HyperVDriver(driver.ComputeDriver):
                                               block_migration, migrate_data)
 
     def pre_live_migration(self, context, instance, block_device_info,
-                           network_info, migrate_data=None):
+                           network_info, disk, migrate_data=None):
         self._livemigrationops.pre_live_migration(context, instance,
                                                   block_device_info,
                                                   network_info)

nova/virt/libvirt/driver.py

@@ -1438,7 +1438,9 @@ class LibvirtDriver(driver.ComputeDriver):
         # NOTE (rmk): Re-populate any missing backing files.
         disk_info_json = self.get_instance_disk_info(instance['name'], xml,
                                                      block_device_info)
-        self._create_images_and_backing(context, instance, disk_info_json)
+        instance_dir = libvirt_utils.get_instance_path(instance)
+        self._create_images_and_backing(context, instance, instance_dir,
+                                        disk_info_json)
 
         # Initialize all the necessary networking, block devices and
         # start the instance.
@@ -3395,7 +3397,7 @@ class LibvirtDriver(driver.ComputeDriver):
                                           instance['project_id'])
 
     def pre_live_migration(self, context, instance, block_device_info,
-                           network_info, migrate_data=None):
+                           network_info, disk_info, migrate_data=None):
         """Preparation live migration."""
         # Steps for volume backed instance live migration w/o shared storage.
         is_shared_storage = True
@@ -3422,6 +3424,10 @@ class LibvirtDriver(driver.ComputeDriver):
                 raise exception.DestinationDiskExists(path=instance_dir)
             os.mkdir(instance_dir)
 
+            # Ensure images and backing files are present.
+            self._create_images_and_backing(context, instance, instance_dir,
+                                            disk_info)
+
         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)
@@ -3467,28 +3473,21 @@ class LibvirtDriver(driver.ComputeDriver):
                              instance=instance)
                     greenthread.sleep(1)
 
-    def pre_block_migration(self, context, instance, disk_info_json):
+    def _create_images_and_backing(self, context, instance, instance_dir,
-        """Preparation for block migration."""
+                                   disk_info_json):
-        # NOTE (rmk): When preparing for a block migration, the instance dir
-        #             should not exist on the destination hypervisor.
-        instance_dir = libvirt_utils.get_instance_path(instance)
-        if os.path.exists(instance_dir):
-            raise exception.DestinationDiskExists(path=instance_dir)
-        os.mkdir(instance_dir)
-        self._create_images_and_backing(context, instance, disk_info_json)
-
-    def _create_images_and_backing(self, context, instance, disk_info_json):
         """
         :params context: security context
         :params instance:
             nova.db.sqlalchemy.models.Instance object
             instance object that is migrated.
+        :params instance_dir:
+            instance path to use, calculated externally to handle block
+            migrating an instance with an old style instance path
         :params disk_info_json:
             json strings specified in get_instance_disk_info
 
         """
         disk_info = jsonutils.loads(disk_info_json)
-        instance_dir = libvirt_utils.get_instance_path(instance)
 
         for info in disk_info:
             base = os.path.basename(info['path'])

nova/virt/xenapi/driver.py

@@ -465,13 +465,6 @@ class XenAPIDriver(driver.ComputeDriver):
         """
         pass
 
-    def pre_block_migration(self, ctxt, instance_ref, disk_info_json):
-        """Used by libvirt for live migration. We rely on xenapi
-        checks to do this for us. May be used in the future to
-        populate the vdi/vif maps.
-        """
-        pass
-
     def live_migration(self, ctxt, instance_ref, dest,
                        post_method, recover_method, block_migration=False,
                        migrate_data=None):
@@ -495,7 +488,7 @@ class XenAPIDriver(driver.ComputeDriver):
                                  recover_method, block_migration, migrate_data)
 
     def pre_live_migration(self, context, instance_ref, block_device_info,
-                           network_info, migrate_data=None):
+                           network_info, data, migrate_data=None):
         """Preparation live migration.
 
         :params block_device_info: