openstack
/
nova
Code Issues Proposed changes

libvirt: clean instance's directory when block migration fails 

In method pre_live_migration, we create instance's directory at destination
host during block live migration. If some exceptions happen before creating
domain succesfully like failure of connecting volume to destination host,
then we can't destroy instance in method rollback_live_migration_at_destination.
In this case we don't remove the instance's directory, this will lead an
DestinationDiskExists when live migration same instance to same destination.
This commit ensure the instance's directory is always cleaned at destination
host. This commit also extracts instance path calculation into a utility function.

Closes-Bug: #1392947
Change-Id: If86e73b07f17d28078ee48983e9e92aafb30d913
This commit is contained in:
ChangBo Guo(gcb) 2014-11-14 13:42:41 +08:00
parent ce5d9172ab
commit be22375255
5 changed files with 112 additions and 18 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
nova/tests/unit/virt/libvirt/fake_libvirt_utils.py
View File

@ -195,6 +195,11 @@ def get_instance_path(instance, forceold=False, relative=False):
relative=relative)
def get_instance_path_at_destination(instance, migrate_data=None):
return libvirt_utils.get_instance_path_at_destination(instance,
migrate_data)
def pick_disk_driver_name(hypervisor_version, is_block_dev=False):
return "qemu"

48
nova/tests/unit/virt/libvirt/test_driver.py
View File

@ -6456,13 +6456,49 @@ class LibvirtConnTestCase(test.NoDBTestCase):
recover_method=fake_recover_method,
migrate_data=migrate_data)
def test_rollback_live_migration_at_destination(self):
@mock.patch('shutil.rmtree')
@mock.patch('os.path.exists', return_value=True)
@mock.patch('nova.virt.libvirt.utils.get_instance_path_at_destination')
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.destroy')
def test_rollback_live_migration_at_dest_not_shared(self, mock_destroy,
mock_get_instance_path,
mock_exist,
mock_shutil
):
# destroy method may raise InstanceTerminationFailure or
# InstancePowerOffFailure, here use their base class Invalid.
mock_destroy.side_effect = exception.Invalid(reason='just test')
fake_instance_path = os.path.join(cfg.CONF.instances_path,
'/fake_instance_uuid')
mock_get_instance_path.return_value = fake_instance_path
conn = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
with mock.patch.object(conn, "destroy") as mock_destroy:
conn.rollback_live_migration_at_destination("context",
"instance", [], None, True, None)
mock_destroy.assert_called_once_with("context",
"instance", [], None, True, None)
migrate_data = {'is_shared_instance_path': False}
self.assertRaises(exception.Invalid,
conn.rollback_live_migration_at_destination,
"context", "instance", [], None, True, migrate_data)
mock_exist.assert_called_once_with(fake_instance_path)
mock_shutil.assert_called_once_with(fake_instance_path)
@mock.patch('shutil.rmtree')
@mock.patch('os.path.exists')
@mock.patch('nova.virt.libvirt.utils.get_instance_path_at_destination')
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.destroy')
def test_rollback_live_migration_at_dest_shared(self, mock_destroy,
mock_get_instance_path,
mock_exist,
mock_shutil
):
conn = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
migrate_data = {'is_shared_instance_path': True}
conn.rollback_live_migration_at_destination("context", "instance", [],
None, True, migrate_data)
mock_destroy.assert_called_once_with("context", "instance", [],
None, True, migrate_data)
self.assertFalse(mock_get_instance_path.called)
self.assertFalse(mock_exist.called)
self.assertFalse(mock_shutil.called)
def _do_test_create_images_and_backing(self, disk_type):
conn = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)

21
nova/tests/unit/virt/libvirt/test_utils.py
View File

@ -655,3 +655,24 @@ disk size: 4.4M
with_actual_path = True
out = libvirt_utils.get_disk_backing_file('')
self.assertEqual(out, 'c')
def test_get_instance_path_at_destination(self):
instance = dict(name='fake_inst', uuid='fake_uuid')
migrate_data = None
inst_path_at_dest = libvirt_utils.get_instance_path_at_destination(
instance, migrate_data)
expected_path = os.path.join(CONF.instances_path, instance['uuid'])
self.assertEqual(expected_path, inst_path_at_dest)
migrate_data = {}
inst_path_at_dest = libvirt_utils.get_instance_path_at_destination(
instance, migrate_data)
expected_path = os.path.join(CONF.instances_path, instance['uuid'])
self.assertEqual(expected_path, inst_path_at_dest)
migrate_data = dict(instance_relative_path='fake_relative_path')
inst_path_at_dest = libvirt_utils.get_instance_path_at_destination(
instance, migrate_data)
expected_path = os.path.join(CONF.instances_path, 'fake_relative_path')
self.assertEqual(expected_path, inst_path_at_dest)

29
nova/virt/libvirt/driver.py
View File

@ -5411,8 +5411,21 @@ class LibvirtDriver(driver.ComputeDriver):
destroy_disks=True,
migrate_data=None):
"""Clean up destination node after a failed live migration."""
self.destroy(context, instance, network_info, block_device_info,
destroy_disks, migrate_data)
try:
self.destroy(context, instance, network_info, block_device_info,
destroy_disks, migrate_data)
finally:
# NOTE(gcb): Failed block live migration may leave instance
# directory at destination node, ensure it is always deleted.
is_shared_instance_path = True
if migrate_data:
is_shared_instance_path = migrate_data.get(
'is_shared_instance_path', True)
if not is_shared_instance_path:
instance_dir = libvirt_utils.get_instance_path_at_destination(
instance, migrate_data)
if os.path.exists(instance_dir):
shutil.rmtree(instance_dir)
def pre_live_migration(self, context, instance, block_device_info,
network_info, disk_info, migrate_data=None):
@ -5421,14 +5434,12 @@ class LibvirtDriver(driver.ComputeDriver):
is_shared_block_storage = True
is_shared_instance_path = True
is_block_migration = True
instance_relative_path = None
if migrate_data:
is_shared_block_storage = migrate_data.get(
'is_shared_block_storage', True)
is_shared_instance_path = migrate_data.get(
'is_shared_instance_path', True)
is_block_migration = migrate_data.get('block_migration', True)
instance_relative_path = migrate_data.get('instance_relative_path')
if not (is_shared_instance_path and is_shared_block_storage):
# NOTE(mikal): live migration of instances using config drive is
@ -5438,14 +5449,8 @@ class LibvirtDriver(driver.ComputeDriver):
raise exception.NoLiveMigrationForConfigDriveInLibVirt()
if not is_shared_instance_path:
# 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)
instance_dir = libvirt_utils.get_instance_path_at_destination(
instance, migrate_data)
if os.path.exists(instance_dir):
raise exception.DestinationDiskExists(path=instance_dir)

27
nova/virt/libvirt/utils.py
View File

@ -488,6 +488,33 @@ def get_instance_path(instance, forceold=False, relative=False):
return os.path.join(CONF.instances_path, instance['uuid'])
def get_instance_path_at_destination(instance, migrate_data=None):
"""Get the the instance path on destination node while live migration.
This method determines the directory name for instance storage on
destination node, while live migration.
:param instance: the instance we want a path for
:param migrate_data: if not None, it is a dict which holds data
required for live migration without shared
storage.
:returns: a path to store information about that instance
"""
instance_relative_path = None
if migrate_data:
instance_relative_path = migrate_data.get('instance_relative_path')
# 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 = get_instance_path(instance)
return instance_dir
def get_arch(image_meta):
"""Determine the architecture of the guest (or host).