From 659ec75eaf052742a6269e0aae258f1c874749f7 Mon Sep 17 00:00:00 2001 From: Chet Burgess Date: Wed, 3 Jul 2013 17:00:37 +0000 Subject: [PATCH] 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 | 5 +-- nova/tests/compute/test_compute.py | 1 + nova/tests/virt/hyperv/test_hypervapi.py | 2 +- nova/tests/virt/libvirt/test_libvirt.py | 52 ++++++------------------ nova/tests/virt/xenapi/test_xenapi.py | 2 +- nova/virt/driver.py | 16 ++------ nova/virt/fake.py | 2 +- nova/virt/hyperv/driver.py | 2 +- nova/virt/libvirt/driver.py | 27 ++++++------ nova/virt/xenapi/driver.py | 9 +--- 10 files changed, 35 insertions(+), 83 deletions(-) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index ad493e1fc902..cb0724ab8b0d 100755 --- a/nova/compute/manager.py +++ b/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) diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py index abbe9399616a..980d977258e6 100644 --- a/nova/tests/compute/test_compute.py +++ b/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') diff --git a/nova/tests/virt/hyperv/test_hypervapi.py b/nova/tests/virt/hyperv/test_hypervapi.py index fbf6eb3e81b1..34844a58ba30 100644 --- a/nova/tests/virt/hyperv/test_hypervapi.py +++ b/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: diff --git a/nova/tests/virt/libvirt/test_libvirt.py b/nova/tests/virt/libvirt/test_libvirt.py index cdc24e3f6176..57eff239967e 100644 --- a/nova/tests/virt/libvirt/test_libvirt.py +++ b/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) diff --git a/nova/tests/virt/xenapi/test_xenapi.py b/nova/tests/virt/xenapi/test_xenapi.py index 7da2378c5190..42b665a53032 100644 --- a/nova/tests/virt/xenapi/test_xenapi.py +++ b/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 diff --git a/nova/virt/driver.py b/nova/virt/driver.py index 3dff484dc821..400428e65ce3 100755 --- a/nova/virt/driver.py +++ b/nova/virt/driver.py @@ -474,29 +474,19 @@ class ComputeDriver(object): """ raise NotImplementedError() - def pre_live_migration(self, ctxt, instance_ref, - block_device_info, network_info, - migrate_data=None): + def pre_live_migration(self, ctxt, instance_ref, block_device_info, + network_info, disk_info, 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): diff --git a/nova/virt/fake.py b/nova/virt/fake.py index b91fb4fc4772..8ef83dc8c3bf 100755 --- a/nova/virt/fake.py +++ b/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): diff --git a/nova/virt/hyperv/driver.py b/nova/virt/hyperv/driver.py index e0f533db6de2..345b26932a4c 100755 --- a/nova/virt/hyperv/driver.py +++ b/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) diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 47d1f79e829f..3517802a9885 100755 --- a/nova/virt/libvirt/driver.py +++ b/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): - """Preparation for block migration.""" - # 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): + def _create_images_and_backing(self, context, instance, instance_dir, + 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']) diff --git a/nova/virt/xenapi/driver.py b/nova/virt/xenapi/driver.py index 1e94ab9ab71a..385a05b6357c 100755 --- a/nova/virt/xenapi/driver.py +++ b/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: