Hyper-V: properly handle shared storage during migrations

In case of migrations, we attempt to move the instance files without
checking whether shared storage is used. Note that the block_migration
flag is ignored.

After a live migration is performed, we always try to delete the
instance files. Also, if the migration fails, the destination node is
not cleaned up, as the HyperVLiveMigrateData object is not used at the
moment.

This change addresses those issues.

Partial-Bug: #1565895

Change-Id: I0ac0a2d2e7a8771024a486dd5931bd05b1ecd074
This commit is contained in:
Lucian Petrut 2016-06-16 11:41:20 +03:00
parent b48ed5217a
commit 5ee7f3b30c
10 changed files with 273 additions and 71 deletions

View File

@ -5345,8 +5345,8 @@ class ComputeManager(manager.Manager):
"""
# NOTE(pkoniszewski): block migration specific params are set inside
# migrate_data objects for drivers that expose block live migration
# information (i.e. Libvirt and Xenapi). For other drivers cleanup is
# not needed.
# information (i.e. Libvirt, Xenapi and HyperV). For other drivers
# cleanup is not needed.
is_shared_block_storage = True
is_shared_instance_path = True
if isinstance(migrate_data, migrate_data_obj.LibvirtLiveMigrateData):
@ -5355,6 +5355,9 @@ class ComputeManager(manager.Manager):
elif isinstance(migrate_data, migrate_data_obj.XenapiLiveMigrateData):
is_shared_block_storage = not migrate_data.block_migration
is_shared_instance_path = not migrate_data.block_migration
elif isinstance(migrate_data, migrate_data_obj.HyperVLiveMigrateData):
is_shared_instance_path = migrate_data.is_shared_instance_path
is_shared_block_storage = migrate_data.is_shared_instance_path
# No instance booting at source host, but instance dir
# must be deleted for preparing next block migration

View File

@ -5030,6 +5030,22 @@ class ComputeManagerMigrationTestCase(test.NoDBTestCase):
self.assertFalse(do_cleanup)
self.assertFalse(destroy_disks)
def test_live_migration_cleanup_flags_block_migrate_hyperv(self):
migrate_data = objects.HyperVLiveMigrateData(
is_shared_instance_path=False)
do_cleanup, destroy_disks = self.compute._live_migration_cleanup_flags(
migrate_data)
self.assertTrue(do_cleanup)
self.assertTrue(destroy_disks)
def test_live_migration_cleanup_flags_shared_hyperv(self):
migrate_data = objects.HyperVLiveMigrateData(
is_shared_instance_path=True)
do_cleanup, destroy_disks = self.compute._live_migration_cleanup_flags(
migrate_data)
self.assertFalse(do_cleanup)
self.assertFalse(destroy_disks)
class ComputeManagerInstanceUsageAuditTestCase(test.TestCase):
def setUp(self):

View File

@ -292,14 +292,16 @@ class HyperVDriverTestCase(test_base.HyperVBaseTestCase):
mock_destroy.assert_called_once_with(
mock.sentinel.context, mock.sentinel.instance,
mock.sentinel.network_info, mock.sentinel.block_device_info)
mock.sentinel.network_info, mock.sentinel.block_device_info,
destroy_disks=mock.sentinel.destroy_disks)
def test_pre_live_migration(self):
self.driver.pre_live_migration(
migrate_data = self.driver.pre_live_migration(
mock.sentinel.context, mock.sentinel.instance,
mock.sentinel.block_device_info, mock.sentinel.network_info,
mock.sentinel.disk_info, mock.sentinel.migrate_data)
self.assertEqual(mock.sentinel.migrate_data, migrate_data)
pre_live_migration = self.driver._livemigrationops.pre_live_migration
pre_live_migration.assert_called_once_with(
mock.sentinel.context, mock.sentinel.instance,
@ -313,7 +315,8 @@ class HyperVDriverTestCase(test_base.HyperVBaseTestCase):
post_live_migration = self.driver._livemigrationops.post_live_migration
post_live_migration.assert_called_once_with(
mock.sentinel.context, mock.sentinel.instance,
mock.sentinel.block_device_info)
mock.sentinel.block_device_info,
mock.sentinel.migrate_data)
def test_post_live_migration_at_destination(self):
self.driver.post_live_migration_at_destination(

View File

@ -17,6 +17,7 @@ import mock
from os_win import exceptions as os_win_exc
from oslo_config import cfg
from nova.objects import migrate_data as migrate_data_obj
from nova.tests.unit import fake_instance
from nova.tests.unit.virt.hyperv import test_base
from nova.virt.hyperv import livemigrationops
@ -35,49 +36,103 @@ class LiveMigrationOpsTestCase(test_base.HyperVBaseTestCase):
self._livemigrops._livemigrutils = mock.MagicMock()
self._livemigrops._pathutils = mock.MagicMock()
self._livemigrops._block_dev_man = mock.MagicMock()
self._pathutils = self._livemigrops._pathutils
@mock.patch.object(serialconsoleops.SerialConsoleOps,
'stop_console_handler')
@mock.patch('nova.virt.hyperv.vmops.VMOps.copy_vm_dvd_disks')
def _test_live_migration(self, mock_get_vm_dvd_paths,
mock_stop_console_handler, side_effect):
def _test_live_migration(self, mock_copy_dvd_disk,
mock_stop_console_handler,
side_effect=None,
shared_storage=False,
migrate_data_received=True,
migrate_data_version='1.1'):
mock_instance = fake_instance.fake_instance_obj(self.context)
mock_post = mock.MagicMock()
mock_recover = mock.MagicMock()
mock_copy_logs = self._livemigrops._pathutils.copy_vm_console_logs
fake_dest = mock.sentinel.DESTINATION
mock_check_shared_inst_dir = (
self._pathutils.check_remote_instances_dir_shared)
mock_check_shared_inst_dir.return_value = shared_storage
self._livemigrops._livemigrutils.live_migrate_vm.side_effect = [
side_effect]
if migrate_data_received:
migrate_data = migrate_data_obj.HyperVLiveMigrateData()
if migrate_data_version != '1.0':
migrate_data.is_shared_instance_path = shared_storage
else:
migrate_data = None
if side_effect is os_win_exc.HyperVException:
self.assertRaises(os_win_exc.HyperVException,
self._livemigrops.live_migration,
self.context, mock_instance, fake_dest,
mock_post, mock_recover, False, None)
mock_post, mock_recover,
mock.sentinel.block_migr,
migrate_data)
mock_recover.assert_called_once_with(self.context, mock_instance,
fake_dest, False)
fake_dest,
mock.sentinel.block_migr,
migrate_data)
else:
self._livemigrops.live_migration(context=self.context,
instance_ref=mock_instance,
dest=fake_dest,
post_method=mock_post,
recover_method=mock_recover)
recover_method=mock_recover,
block_migration=(
mock.sentinel.block_migr),
migrate_data=migrate_data)
post_call_args = mock_post.call_args_list
self.assertEqual(1, len(post_call_args))
mock_stop_console_handler.assert_called_once_with(
mock_instance.name)
post_call_args_list = post_call_args[0][0]
self.assertEqual((self.context, mock_instance,
fake_dest, mock.sentinel.block_migr),
post_call_args_list[:-1])
# The last argument, the migrate_data object, should be created
# by the callee if not received.
migrate_data_arg = post_call_args_list[-1]
self.assertIsInstance(
migrate_data_arg,
migrate_data_obj.HyperVLiveMigrateData)
self.assertEqual(shared_storage,
migrate_data_arg.is_shared_instance_path)
if not migrate_data_received or migrate_data_version == '1.0':
mock_check_shared_inst_dir.assert_called_once_with(fake_dest)
else:
self.assertFalse(mock_check_shared_inst_dir.called)
mock_stop_console_handler.assert_called_once_with(mock_instance.name)
if not shared_storage:
mock_copy_logs.assert_called_once_with(mock_instance.name,
fake_dest)
mock_live_migr = self._livemigrops._livemigrutils.live_migrate_vm
mock_live_migr.assert_called_once_with(mock_instance.name,
fake_dest)
mock_post.assert_called_once_with(self.context, mock_instance,
fake_dest, False)
mock_copy_dvd_disk.assert_called_once_with(mock_instance.name,
fake_dest)
else:
self.assertFalse(mock_copy_logs.called)
self.assertFalse(mock_copy_dvd_disk.called)
mock_live_migr = self._livemigrops._livemigrutils.live_migrate_vm
mock_live_migr.assert_called_once_with(mock_instance.name,
fake_dest)
def test_live_migration(self):
self._test_live_migration(side_effect=None)
self._test_live_migration(migrate_data_received=False)
def test_live_migration_old_migrate_data_version(self):
self._test_live_migration(migrate_data_version='1.0')
def test_live_migration_exception(self):
self._test_live_migration(side_effect=os_win_exc.HyperVException)
def test_live_migration_shared_storage(self):
self._test_live_migration(shared_storage=True)
@mock.patch('nova.virt.hyperv.volumeops.VolumeOps.get_disk_path_mapping')
@mock.patch('nova.virt.hyperv.imagecache.ImageCache.get_cached_image')
@mock.patch('nova.virt.hyperv.volumeops.VolumeOps'
@ -125,11 +180,43 @@ class LiveMigrationOpsTestCase(test_base.HyperVBaseTestCase):
self._test_pre_live_migration(phys_disks_attached=False)
@mock.patch('nova.virt.hyperv.volumeops.VolumeOps.disconnect_volumes')
def test_post_live_migration(self, mock_disconnect_volumes):
def _test_post_live_migration(self, mock_disconnect_volumes,
shared_storage=False):
migrate_data = migrate_data_obj.HyperVLiveMigrateData(
is_shared_instance_path=shared_storage)
self._livemigrops.post_live_migration(
self.context, mock.sentinel.instance,
mock.sentinel.block_device_info)
mock.sentinel.block_device_info,
migrate_data)
mock_disconnect_volumes.assert_called_once_with(
mock.sentinel.block_device_info)
self._livemigrops._pathutils.get_instance_dir.assert_called_once_with(
mock.sentinel.instance.name, create_dir=False, remove_dir=True)
mock_get_inst_dir = self._pathutils.get_instance_dir
if not shared_storage:
mock_get_inst_dir.assert_called_once_with(
mock.sentinel.instance.name,
create_dir=False, remove_dir=True)
else:
self.assertFalse(mock_get_inst_dir.called)
def test_post_block_migration(self):
self._test_post_live_migration()
def test_post_live_migration_shared_storage(self):
self._test_post_live_migration(shared_storage=True)
@mock.patch.object(migrate_data_obj, 'HyperVLiveMigrateData')
def test_check_can_live_migrate_destination(self, mock_migr_data_cls):
mock_instance = fake_instance.fake_instance_obj(self.context)
migr_data = self._livemigrops.check_can_live_migrate_destination(
mock.sentinel.context, mock_instance, mock.sentinel.src_comp_info,
mock.sentinel.dest_comp_info)
mock_check_shared_inst_dir = (
self._pathutils.check_remote_instances_dir_shared)
mock_check_shared_inst_dir.assert_called_once_with(mock_instance.host)
self.assertEqual(mock_migr_data_cls.return_value, migr_data)
self.assertEqual(mock_check_shared_inst_dir.return_value,
migr_data.is_shared_instance_path)

View File

@ -49,16 +49,21 @@ class MigrationOpsTestCase(test_base.HyperVBaseTestCase):
self._migrationops._imagecache = mock.MagicMock()
self._migrationops._block_dev_man = mock.MagicMock()
def _check_migrate_disk_files(self, host):
def _check_migrate_disk_files(self, shared_storage=False):
instance_path = 'fake/instance/path'
self._migrationops._pathutils.get_instance_dir.return_value = (
instance_path)
dest_instance_path = 'remote/instance/path'
self._migrationops._pathutils.get_instance_dir.side_effect = (
instance_path, dest_instance_path)
get_revert_dir = (
self._migrationops._pathutils.get_instance_migr_revert_dir)
self._migrationops._hostutils.get_local_ips.return_value = [host]
check_shared_storage = (
self._migrationops._pathutils.check_dirs_shared_storage)
check_shared_storage.return_value = shared_storage
self._migrationops._pathutils.exists.return_value = True
expected_get_dir = [mock.call(mock.sentinel.instance_name)]
expected_get_dir = [mock.call(mock.sentinel.instance_name),
mock.call(mock.sentinel.instance_name,
mock.sentinel.dest_path)]
expected_move_calls = [mock.call(instance_path,
get_revert_dir.return_value)]
@ -67,24 +72,26 @@ class MigrationOpsTestCase(test_base.HyperVBaseTestCase):
disk_files=[self._FAKE_DISK],
dest=mock.sentinel.dest_path)
self._migrationops._hostutils.get_local_ips.assert_called_once_with()
self._migrationops._pathutils.exists.assert_called_once_with(
dest_instance_path)
check_shared_storage.assert_called_once_with(
instance_path, dest_instance_path)
get_revert_dir.assert_called_with(mock.sentinel.instance_name,
remove_dir=True, create_dir=True)
if host == mock.sentinel.dest_path:
if shared_storage:
fake_dest_path = '%s_tmp' % instance_path
self._migrationops._pathutils.exists.assert_called_once_with(
fake_dest_path)
self._migrationops._pathutils.rmtree.assert_called_once_with(
fake_dest_path)
self._migrationops._pathutils.makedirs.assert_called_once_with(
fake_dest_path)
expected_move_calls.append(mock.call(fake_dest_path,
instance_path))
self._migrationops._pathutils.rmtree.assert_called_once_with(
fake_dest_path)
else:
fake_dest_path = instance_path
expected_get_dir.append(mock.call(mock.sentinel.instance_name,
mock.sentinel.dest_path,
remove_dir=True))
fake_dest_path = dest_instance_path
self._migrationops._pathutils.makedirs.assert_called_once_with(
fake_dest_path)
check_remove_dir = self._migrationops._pathutils.check_remove_dir
check_remove_dir.assert_called_once_with(fake_dest_path)
self._migrationops._pathutils.get_instance_dir.assert_has_calls(
expected_get_dir)
self._migrationops._pathutils.copy.assert_called_once_with(
@ -93,10 +100,10 @@ class MigrationOpsTestCase(test_base.HyperVBaseTestCase):
expected_move_calls)
def test_migrate_disk_files(self):
self._check_migrate_disk_files(host=mock.sentinel.other_dest_path)
self._check_migrate_disk_files()
def test_migrate_disk_files_same_host(self):
self._check_migrate_disk_files(host=mock.sentinel.dest_path)
self._check_migrate_disk_files(shared_storage=True)
@mock.patch.object(migrationops.MigrationOps,
'_cleanup_failed_disk_migration')

View File

@ -176,3 +176,39 @@ class PathUtilsTestCase(test_base.HyperVBaseTestCase):
self.assertEqual(42, actual_age)
mock_time.time.assert_called_once_with()
mock_getmtime.assert_called_once_with(mock.sentinel.filename)
@mock.patch('os.path.exists')
@mock.patch('tempfile.NamedTemporaryFile')
def test_check_dirs_shared_storage(self, mock_named_tempfile,
mock_exists):
fake_src_dir = 'fake_src_dir'
fake_dest_dir = 'fake_dest_dir'
mock_exists.return_value = True
mock_tmpfile = mock_named_tempfile.return_value.__enter__.return_value
mock_tmpfile.name = 'fake_tmp_fname'
expected_src_tmp_path = os.path.join(fake_src_dir,
mock_tmpfile.name)
self._pathutils.check_dirs_shared_storage(
fake_src_dir, fake_dest_dir)
mock_named_tempfile.assert_called_once_with(dir=fake_dest_dir)
mock_exists.assert_called_once_with(expected_src_tmp_path)
@mock.patch.object(pathutils.PathUtils, 'check_dirs_shared_storage')
@mock.patch.object(pathutils.PathUtils, 'get_instances_dir')
def test_check_remote_instances_shared(self, mock_get_instances_dir,
mock_check_dirs_shared_storage):
mock_get_instances_dir.side_effect = [mock.sentinel.local_inst_dir,
mock.sentinel.remote_inst_dir]
shared_storage = self._pathutils.check_remote_instances_dir_shared(
mock.sentinel.dest)
self.assertEqual(mock_check_dirs_shared_storage.return_value,
shared_storage)
mock_get_instances_dir.assert_has_calls(
[mock.call(), mock.call(mock.sentinel.dest)])
mock_check_dirs_shared_storage.assert_called_once_with(
mock.sentinel.local_inst_dir, mock.sentinel.remote_inst_dir)

View File

@ -233,18 +233,21 @@ class HyperVDriver(driver.ComputeDriver):
block_device_info,
destroy_disks=True,
migrate_data=None):
self.destroy(context, instance, network_info, block_device_info)
self.destroy(context, instance, network_info, block_device_info,
destroy_disks=destroy_disks)
def pre_live_migration(self, context, instance, block_device_info,
network_info, disk_info, migrate_data=None):
self._livemigrationops.pre_live_migration(context, instance,
block_device_info,
network_info)
return migrate_data
def post_live_migration(self, context, instance, block_device_info,
migrate_data=None):
self._livemigrationops.post_live_migration(context, instance,
block_device_info)
block_device_info,
migrate_data)
def post_live_migration_at_destination(self, context, instance,
network_info,

View File

@ -51,25 +51,39 @@ class LiveMigrationOps(object):
LOG.debug("live_migration called", instance=instance_ref)
instance_name = instance_ref["name"]
try:
self._vmops.copy_vm_dvd_disks(instance_name, dest)
if migrate_data and 'is_shared_instance_path' in migrate_data:
shared_storage = migrate_data.is_shared_instance_path
else:
shared_storage = (
self._pathutils.check_remote_instances_dir_shared(dest))
if migrate_data:
migrate_data.is_shared_instance_path = shared_storage
else:
migrate_data = migrate_data_obj.HyperVLiveMigrateData(
is_shared_instance_path=shared_storage)
try:
# We must make sure that the console log workers are stopped,
# otherwise we won't be able to delete / move VM log files.
self._serial_console_ops.stop_console_handler(instance_name)
self._pathutils.copy_vm_console_logs(instance_name, dest)
if not shared_storage:
self._pathutils.copy_vm_console_logs(instance_name, dest)
self._vmops.copy_vm_dvd_disks(instance_name, dest)
self._livemigrutils.live_migrate_vm(instance_name,
dest)
except Exception:
with excutils.save_and_reraise_exception():
LOG.debug("Calling live migration recover_method "
"for instance: %s", instance_name)
recover_method(context, instance_ref, dest, block_migration)
recover_method(context, instance_ref, dest, block_migration,
migrate_data)
LOG.debug("Calling live migration post_method for instance: %s",
instance_name)
post_method(context, instance_ref, dest, block_migration)
post_method(context, instance_ref, dest,
block_migration, migrate_data)
def pre_live_migration(self, context, instance, block_device_info,
network_info):
@ -93,11 +107,14 @@ class LiveMigrationOps(object):
instance.host,
disk_path_mapping)
def post_live_migration(self, context, instance, block_device_info):
def post_live_migration(self, context, instance, block_device_info,
migrate_data):
self._volumeops.disconnect_volumes(block_device_info)
self._pathutils.get_instance_dir(instance.name,
create_dir=False,
remove_dir=True)
if not migrate_data.is_shared_instance_path:
self._pathutils.get_instance_dir(instance.name,
create_dir=False,
remove_dir=True)
def post_live_migration_at_destination(self, ctxt, instance_ref,
network_info, block_migration):
@ -108,8 +125,14 @@ class LiveMigrationOps(object):
src_compute_info, dst_compute_info,
block_migration=False,
disk_over_commit=False):
LOG.debug("check_can_live_migrate_destination called", instance_ref)
return migrate_data_obj.HyperVLiveMigrateData()
LOG.debug("check_can_live_migrate_destination called",
instance=instance_ref)
migrate_data = migrate_data_obj.HyperVLiveMigrateData()
migrate_data.is_shared_instance_path = (
self._pathutils.check_remote_instances_dir_shared(
instance_ref.host))
return migrate_data
def cleanup_live_migration_destination_check(self, ctxt, dest_check_data):
LOG.debug("cleanup_live_migration_destination_check called")

View File

@ -39,7 +39,6 @@ LOG = logging.getLogger(__name__)
class MigrationOps(object):
def __init__(self):
self._hostutils = utilsfactory.get_hostutils()
self._vmutils = utilsfactory.get_vmutils()
self._vhdutils = utilsfactory.get_vhdutils()
self._pathutils = pathutils.PathUtils()
@ -51,29 +50,27 @@ class MigrationOps(object):
def _migrate_disk_files(self, instance_name, disk_files, dest):
# TODO(mikal): it would be nice if this method took a full instance,
# because it could then be passed to the log messages below.
same_host = False
if dest in self._hostutils.get_local_ips():
same_host = True
LOG.debug("Migration target is the source host")
else:
LOG.debug("Migration target host: %s", dest)
instance_path = self._pathutils.get_instance_dir(instance_name)
dest_path = self._pathutils.get_instance_dir(instance_name, dest)
revert_path = self._pathutils.get_instance_migr_revert_dir(
instance_name, remove_dir=True, create_dir=True)
dest_path = None
shared_storage = (self._pathutils.exists(dest_path) and
self._pathutils.check_dirs_shared_storage(
instance_path, dest_path))
try:
if same_host:
if shared_storage:
# Since source and target are the same, we copy the files to
# a temporary location before moving them into place
# a temporary location before moving them into place.
# This applies when the migration target is the source host or
# when shared storage is used for the instance files.
dest_path = '%s_tmp' % instance_path
if self._pathutils.exists(dest_path):
self._pathutils.rmtree(dest_path)
self._pathutils.makedirs(dest_path)
else:
dest_path = self._pathutils.get_instance_dir(
instance_name, dest, remove_dir=True)
self._pathutils.check_remove_dir(dest_path)
self._pathutils.makedirs(dest_path)
for disk_file in disk_files:
# Skip the config drive as the instance is already configured
if os.path.basename(disk_file).lower() != 'configdrive.vhd':
@ -84,8 +81,9 @@ class MigrationOps(object):
self._pathutils.move_folder_files(instance_path, revert_path)
if same_host:
if shared_storage:
self._pathutils.move_folder_files(dest_path, instance_path)
self._pathutils.rmtree(dest_path)
except Exception:
with excutils.save_and_reraise_exception():
self._cleanup_failed_disk_migration(instance_path, revert_path,

View File

@ -14,15 +14,19 @@
# under the License.
import os
import tempfile
import time
from os_win.utils import pathutils
from oslo_log import log as logging
import nova.conf
from nova import exception
from nova.i18n import _
from nova.virt.hyperv import constants
LOG = logging.getLogger(__name__)
CONF = nova.conf.CONF
ERROR_INVALID_NAME = 123
@ -169,3 +173,25 @@ class PathUtils(pathutils.PathUtils):
def get_age_of_file(self, file_name):
return time.time() - os.path.getmtime(file_name)
def check_dirs_shared_storage(self, src_dir, dest_dir):
# Check if shared storage is being used by creating a temporary
# file at the destination path and checking if it exists at the
# source path.
LOG.debug("Checking if %(src_dir)s and %(dest_dir)s point "
"to the same location.",
dict(src_dir=src_dir, dest_dir=dest_dir))
with tempfile.NamedTemporaryFile(dir=dest_dir) as tmp_file:
src_path = os.path.join(src_dir,
os.path.basename(tmp_file.name))
shared_storage = os.path.exists(src_path)
return shared_storage
def check_remote_instances_dir_shared(self, dest):
# Checks if the instances dir from a remote host points
# to the same storage location as the local instances dir.
local_inst_dir = self.get_instances_dir()
remote_inst_dir = self.get_instances_dir(dest)
return self.check_dirs_shared_storage(local_inst_dir,
remote_inst_dir)