openstack
/
compute-hyperv
Code Issues Proposed changes

refactors cold migration / resize 

Simplifies the cold resize / migration process by making the
destination node pull the VM files from the source instead of
vice-versa. Doing so, the logic is simplified greatly, the
destination node only has to copy the files, regardless if it
is on shared storage or not.

Solves the migration issue when the compute nodes have different
instances_path config option set, as the destination node will
copy the files to its configured instances_path folder.

Co-Authored-By: Claudiu Belu <cbelu@cloudbasesolutions.com>

Change-Id: Ia44850675e8cb8eb21017e1c4321802574dfe6a0
This commit is contained in:
Adelina Tuvenie 2016-11-04 09:34:30 +02:00 committed by Claudiu Belu
parent d57d92c970
commit d3d59920b9
6 changed files with 144 additions and 194 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
hyperv/nova/cluster/driver.py
View File

@ -48,7 +48,7 @@ class HyperVClusterDriver(driver.HyperVDriver):
block_device_info=None,
timeout=0, retry_interval=0):
self._clops.remove_from_cluster(instance)
super(HyperVClusterDriver, self).migrate_disk_and_power_off(
return super(HyperVClusterDriver, self).migrate_disk_and_power_off(
context, instance, dest, flavor, network_info,
block_device_info, timeout, retry_interval)

83
hyperv/nova/migrationops.py
View File

@ -25,7 +25,7 @@ from oslo_log import log as logging
from oslo_utils import excutils
from oslo_utils import units
from hyperv.i18n import _, _LW, _LE
from hyperv.i18n import _, _LW
from hyperv.nova import block_device_manager
from hyperv.nova import constants
from hyperv.nova import imagecache
@ -46,58 +46,17 @@ class MigrationOps(object):
self._imagecache = imagecache.ImageCache()
self._block_dev_man = block_device_manager.BlockDeviceInfoManager()
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.
instance_path = self._pathutils.get_instance_dir(instance_name)
dest_path = self._pathutils.get_instance_dir(instance_name, dest)
def _move_vm_files(self, instance):
instance_path = self._pathutils.get_instance_dir(instance.name)
revert_path = self._pathutils.get_instance_migr_revert_dir(
instance_name, remove_dir=True, create_dir=True)
instance.name, remove_dir=True, create_dir=True)
shared_storage = (self._pathutils.exists(dest_path) and
self._pathutils.check_dirs_shared_storage(
instance_path, dest_path))
# copy the given instance's files to a _revert folder, as backup.
LOG.debug("Moving instance files to a revert path: %s",
revert_path, instance=instance)
self._pathutils.move_folder_files(instance_path, revert_path)
try:
if shared_storage:
# Since source and target are the same, we copy the files to
# 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
self._pathutils.check_remove_dir(dest_path)
self._pathutils.makedirs(dest_path)
for disk_file in disk_files:
LOG.debug('Copying disk "%(disk_file)s" to '
'"%(dest_path)s"',
{'disk_file': disk_file, 'dest_path': dest_path})
self._pathutils.copy(disk_file, dest_path)
self._pathutils.move_folder_files(instance_path, revert_path)
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,
dest_path)
def _cleanup_failed_disk_migration(self, instance_path,
revert_path, dest_path):
try:
if dest_path and self._pathutils.exists(dest_path):
self._pathutils.rmtree(dest_path)
if self._pathutils.exists(revert_path):
self._pathutils.move_folder_files(revert_path, instance_path)
self._pathutils.rmtree(revert_path)
except Exception as ex:
# Log and ignore this exception
LOG.exception(ex)
LOG.error(_LE("Cannot cleanup migration files"))
return revert_path
def _check_target_flavor(self, instance, flavor):
new_root_gb = flavor.root_gb
@ -121,17 +80,12 @@ class MigrationOps(object):
self._check_target_flavor(instance, flavor)
self._vmops.power_off(instance, timeout, retry_interval)
instance_path = self._move_vm_files(instance)
(disk_files,
volume_drives) = self._vmutils.get_vm_storage_paths(instance.name)
self._vmops.destroy(instance, destroy_disks=True)
if disk_files:
self._migrate_disk_files(instance.name, disk_files, dest)
self._vmops.destroy(instance, destroy_disks=False)
# disk_info is not used
return ""
# return the instance's path location.
return instance_path
def confirm_migration(self, context, migration, instance, network_info):
LOG.debug("confirm_migration called", instance=instance)
@ -245,10 +199,21 @@ class MigrationOps(object):
self._vhdutils.reconnect_parent_vhd(diff_vhd_path,
base_vhd_path)
def _migrate_disks_from_source(self, migration, instance,
source_inst_dir):
source_inst_dir = self._pathutils.get_remote_path(
migration.source_compute, source_inst_dir)
inst_dir = self._pathutils.get_instance_dir(
instance.name, create_dir=True, remove_dir=True)
self._pathutils.copy_folder_files(source_inst_dir, inst_dir)
def finish_migration(self, context, migration, instance, disk_info,
network_info, image_meta, resize_instance=False,
block_device_info=None, power_on=True):
LOG.debug("finish_migration called", instance=instance)
self._migrate_disks_from_source(migration, instance, disk_info)
vm_gen = self._vmops.get_image_vm_generation(instance.uuid, image_meta)
self._check_and_update_disks(context, instance, vm_gen, image_meta,
block_device_info,

25
hyperv/nova/pathutils.py
View File

@ -45,6 +45,23 @@ class PathUtils(pathutils.PathUtils):
super(PathUtils, self).__init__()
self._vmutils = utilsfactory.get_vmutils()
def copy_folder_files(self, src_dir, dest_dir):
"""Copies the files of the given src_dir to dest_dir.
It will ignore any nested folders.
:param src_dir: Given folder from which to copy files.
:param dest_dir: Folder to which to copy files.
"""
# NOTE(claudiub): this will have to be moved to os-win.
for fname in os.listdir(src_dir):
src = os.path.join(src_dir, fname)
# ignore subdirs.
if os.path.isfile(src):
self.copy(src, os.path.join(dest_dir, fname))
def get_instances_dir(self, remote_server=None):
local_instance_path = os.path.normpath(CONF.instances_path)
@ -55,11 +72,11 @@ class PathUtils(pathutils.PathUtils):
# In this case, we expect the instance dir to have the same
# location on the remote server.
path = local_instance_path
return self._get_remote_unc_path(remote_server, path)
return self.get_remote_path(remote_server, path)
else:
return local_instance_path
def _get_remote_unc_path(self, remote_server, remote_path):
def get_remote_path(self, remote_server, remote_path):
if remote_path.startswith('\\\\'):
remote_unc_path = remote_path
else:
@ -114,8 +131,8 @@ class PathUtils(pathutils.PathUtils):
instance_dir = vmutils.get_vm_config_root_dir(
instance_name)
if remote_server:
instance_dir = self._get_remote_unc_path(remote_server,
instance_dir)
instance_dir = self.get_remote_path(remote_server,
instance_dir)
LOG.info(_LI("Found instance dir at non-default location: %s"),
instance_dir)
except os_win_exc.HyperVVMNotFoundException:

19
hyperv/tests/unit/cluster/test_driver.py
View File

@ -67,14 +67,17 @@ class HyperVClusterTestCase(test_base.HyperVBaseTestCase):
@mock.patch.object(base_driver.HyperVDriver, 'migrate_disk_and_power_off')
def test_migrate_disk_and_power_off(self, mock_superclass_migrate):
self.driver.migrate_disk_and_power_off(self.context,
mock.sentinel.fake_instance,
mock.sentinel.destination,
mock.sentinel.flavor,
mock.sentinel.network_info,
mock.sentinel.block_dev_info,
mock.sentinel.timeout,
mock.sentinel.retry_interval)
disk_info = self.driver.migrate_disk_and_power_off(
self.context,
mock.sentinel.fake_instance,
mock.sentinel.destination,
mock.sentinel.flavor,
mock.sentinel.network_info,
mock.sentinel.block_dev_info,
mock.sentinel.timeout,
mock.sentinel.retry_interval)
self.assertEqual(mock_superclass_migrate.return_value, disk_info)
self.driver._clops.remove_from_cluster.assert_called_once_with(
mock.sentinel.fake_instance)
mock_superclass_migrate.assert_called_once_with(

164
hyperv/tests/unit/test_migrationops.py
View File

@ -57,109 +57,22 @@ class MigrationOpsTestCase(test_base.HyperVBaseTestCase):
self._imagecache = self._migrationops._imagecache
self._block_dev_man = self._migrationops._block_dev_man
def _check_migrate_disk_files(self, shared_storage=False):
instance_path = 'fake/instance/path'
dest_instance_path = 'remote/instance/path'
self._migrationops._pathutils.get_instance_dir.side_effect = (
instance_path, dest_instance_path)
get_revert_dir = (
def test_move_vm_files(self):
mock_instance = fake_instance.fake_instance_obj(self.context)
vm_files_path = self._migrationops._move_vm_files(mock_instance)
mock_get_inst_dir = self._migrationops._pathutils.get_instance_dir
mock_get_inst_dir.assert_called_once_with(mock_instance.name)
mock_get_revert_dir = (
self._migrationops._pathutils.get_instance_migr_revert_dir)
check_shared_storage = (
self._migrationops._pathutils.check_dirs_shared_storage)
check_shared_storage.return_value = shared_storage
self._migrationops._pathutils.exists.return_value = True
mock_get_revert_dir.assert_called_once_with(
mock_instance.name, remove_dir=True, create_dir=True)
fake_disk_files = [os.path.join(instance_path, disk_name)
for disk_name in
['root.vhd', 'configdrive.vhd', 'configdrive.iso',
'eph0.vhd', 'eph1.vhdx']]
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)]
self._migrationops._migrate_disk_files(
instance_name=mock.sentinel.instance_name,
disk_files=fake_disk_files,
dest=mock.sentinel.dest_path)
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 shared_storage:
fake_dest_path = '%s_tmp' % instance_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 = 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_has_calls(
mock.call(fake_disk_file, fake_dest_path)
for fake_disk_file in fake_disk_files)
self.assertEqual(len(fake_disk_files),
self._migrationops._pathutils.copy.call_count)
self._migrationops._pathutils.move_folder_files.assert_has_calls(
expected_move_calls)
def test_migrate_disk_files(self):
self._check_migrate_disk_files()
def test_migrate_disk_files_same_host(self):
self._check_migrate_disk_files(shared_storage=True)
@mock.patch.object(migrationops.MigrationOps,
'_cleanup_failed_disk_migration')
def test_migrate_disk_files_exception(self, mock_cleanup):
instance_path = 'fake/instance/path'
fake_dest_path = '%s_tmp' % instance_path
self._migrationops._pathutils.get_instance_dir.return_value = (
instance_path)
get_revert_dir = (
self._migrationops._pathutils.get_instance_migr_revert_dir)
self._migrationops._hostutils.get_local_ips.return_value = [
mock.sentinel.dest_path]
self._migrationops._pathutils.copy.side_effect = IOError(
"Expected exception.")
self.assertRaises(IOError, self._migrationops._migrate_disk_files,
instance_name=mock.sentinel.instance_name,
disk_files=[self._FAKE_DISK],
dest=mock.sentinel.dest_path)
mock_cleanup.assert_called_once_with(instance_path,
get_revert_dir.return_value,
fake_dest_path)
def test_cleanup_failed_disk_migration(self):
self._migrationops._pathutils.exists.return_value = True
self._migrationops._cleanup_failed_disk_migration(
instance_path=mock.sentinel.instance_path,
revert_path=mock.sentinel.revert_path,
dest_path=mock.sentinel.dest_path)
expected = [mock.call(mock.sentinel.dest_path),
mock.call(mock.sentinel.revert_path)]
self._migrationops._pathutils.exists.assert_has_calls(expected)
move_folder_files = self._migrationops._pathutils.move_folder_files
move_folder_files.assert_called_once_with(
mock.sentinel.revert_path, mock.sentinel.instance_path)
self._migrationops._pathutils.rmtree.assert_has_calls([
mock.call(mock.sentinel.dest_path),
mock.call(mock.sentinel.revert_path)])
mock_move = self._migrationops._pathutils.move_folder_files
mock_move.assert_called_once_with(mock_get_inst_dir.return_value,
mock_get_revert_dir.return_value)
self.assertEqual(mock_get_revert_dir.return_value, vm_files_path)
def test_check_target_flavor(self):
mock_instance = fake_instance.fake_instance_obj(self.context)
@ -193,32 +106,25 @@ class MigrationOpsTestCase(test_base.HyperVBaseTestCase):
instance,
mock.sentinel.FAKE_VM_GEN)
@mock.patch.object(migrationops.MigrationOps, '_migrate_disk_files')
@mock.patch.object(migrationops.MigrationOps, '_move_vm_files')
@mock.patch.object(migrationops.MigrationOps, '_check_target_flavor')
def test_migrate_disk_and_power_off(self, mock_check_flavor,
mock_migrate_disk_files):
mock_move_vm_files):
instance = fake_instance.fake_instance_obj(self.context)
flavor = mock.MagicMock()
network_info = mock.MagicMock()
disk_files = [mock.MagicMock()]
volume_drives = [mock.MagicMock()]
mock_get_vm_st_path = self._migrationops._vmutils.get_vm_storage_paths
mock_get_vm_st_path.return_value = (disk_files, volume_drives)
self._migrationops.migrate_disk_and_power_off(
disk_info = self._migrationops.migrate_disk_and_power_off(
self.context, instance, mock.sentinel.FAKE_DEST, flavor,
network_info, None, self._FAKE_TIMEOUT, self._FAKE_RETRY_INTERVAL)
self.assertEqual(mock_move_vm_files.return_value, disk_info)
mock_check_flavor.assert_called_once_with(instance, flavor)
self._migrationops._vmops.power_off.assert_called_once_with(
instance, self._FAKE_TIMEOUT, self._FAKE_RETRY_INTERVAL)
mock_get_vm_st_path.assert_called_once_with(instance.name)
mock_migrate_disk_files.assert_called_once_with(
instance.name, disk_files, mock.sentinel.FAKE_DEST)
mock_move_vm_files.assert_called_once_with(instance)
self._migrationops._vmops.destroy.assert_called_once_with(
instance, destroy_disks=False)
instance, destroy_disks=True)
def test_confirm_migration(self):
mock_instance = fake_instance.fake_instance_obj(self.context)
@ -226,10 +132,11 @@ class MigrationOpsTestCase(test_base.HyperVBaseTestCase):
context=self.context,
migration=mock.sentinel.migration, instance=mock_instance,
network_info=mock.sentinel.network_info)
get_instance_migr_revert_dir = (
self._migrationops._pathutils.get_instance_migr_revert_dir)
get_instance_migr_revert_dir.assert_called_with(mock_instance.name,
remove_dir=True)
get_instance_migr_revert_dir.assert_called_with(
mock_instance.name, remove_dir=True)
def test_revert_migration_files(self):
instance_path = (
@ -366,11 +273,30 @@ class MigrationOpsTestCase(test_base.HyperVBaseTestCase):
recon_parent_vhd.assert_called_once_with(
mock.sentinel.diff_vhd_path, fake_base_vhd)
def test_migrate_disks_from_source(self):
mock_migration = mock.MagicMock()
mock_instance = fake_instance.fake_instance_obj(self.context)
self._migrationops._migrate_disks_from_source(
mock_migration, mock_instance, mock.sentinel.source_dir)
mock_get_remote_path = self._migrationops._pathutils.get_remote_path
mock_get_remote_path.assert_called_once_with(
mock_migration.source_compute, mock.sentinel.source_dir)
mock_get_inst_dir = self._migrationops._pathutils.get_instance_dir
mock_get_inst_dir.assert_called_once_with(
mock_instance.name, create_dir=True, remove_dir=True)
mock_copy = self._migrationops._pathutils.copy_folder_files
mock_copy.assert_called_once_with(mock_get_remote_path.return_value,
mock_get_inst_dir.return_value)
@mock.patch.object(migrationops.MigrationOps,
'_check_and_attach_config_drive')
@mock.patch.object(migrationops.MigrationOps, '_check_and_update_disks')
def test_finish_migration(self, mock_check_and_update_disks,
mock_check_attach_config_drive):
@mock.patch.object(migrationops.MigrationOps, '_migrate_disks_from_source')
def test_finish_migration(self, mock_migrate_disks_from_source,
mock_check_and_update_disks,
mock_check_attach_config_drive):
mock_instance = fake_instance.fake_instance_obj(self.context)
get_image_vm_gen = self._vmops.get_image_vm_generation
@ -381,6 +307,8 @@ class MigrationOpsTestCase(test_base.HyperVBaseTestCase):
image_meta=mock.sentinel.image_meta, resize_instance=True,
block_device_info=mock.sentinel.block_device_info)
mock_migrate_disks_from_source.assert_called_once_with(
mock.sentinel.migration, mock_instance, mock.sentinel.disk_info)
get_image_vm_gen.assert_called_once_with(mock_instance.uuid,
mock.sentinel.image_meta)
mock_check_and_update_disks.assert_called_once_with(

45
hyperv/tests/unit/test_pathutils.py
View File

@ -37,6 +37,24 @@ class PathUtilsTestCase(test_base.HyperVBaseTestCase):
self._pathutils = pathutils.PathUtils()
@mock.patch.object(pathutils.PathUtils, 'copy')
@mock.patch.object(os.path, 'isfile')
@mock.patch.object(os, 'listdir')
def test_copy_folder_files(self, mock_listdir, mock_isfile, mock_copy):
src_dir = 'src'
dest_dir = 'dest'
fname = 'tmp_file.txt'
subdir = 'tmp_folder'
src_fname = os.path.join(src_dir, fname)
dest_fname = os.path.join(dest_dir, fname)
# making sure src_subdir is not copied.
mock_listdir.return_value = [fname, subdir]
mock_isfile.side_effect = [True, False]
self._pathutils.copy_folder_files(src_dir, dest_dir)
mock_copy.assert_called_once_with(src_fname, dest_fname)
@ddt.data({'conf_instances_path': r'c:\inst_dir',
'expected_dir': r'c:\inst_dir'},
{'conf_instances_path': r'c:\inst_dir',
@ -60,6 +78,25 @@ class PathUtilsTestCase(test_base.HyperVBaseTestCase):
self.assertEqual(expected_dir, instances_dir)
def test_get_remote_path_share(self):
fake_remote_path = '\\\\fake_path'
actual_path = self._pathutils.get_remote_path(mock.sentinel.server,
fake_remote_path)
self.assertEqual(fake_remote_path, actual_path)
def test_get_remote_path_normal(self):
fake_server = 'fake_server'
fake_remote_path = 'C:\\fake_path'
actual_path = self._pathutils.get_remote_path(fake_server,
fake_remote_path)
expected_path = ('\\\\%(remote_server)s\\%(path)s' %
dict(remote_server=fake_server,
path=fake_remote_path.replace(':', '$')))
self.assertEqual(expected_path, actual_path)
@mock.patch.object(pathutils.PathUtils, 'get_instances_dir')
@mock.patch.object(pathutils.PathUtils, '_check_dir')
def test_get_instances_sub_dir(self, mock_check_dir,
@ -128,14 +165,14 @@ class PathUtilsTestCase(test_base.HyperVBaseTestCase):
'remote_server': mock.sentinel.remote_server})
@ddt.unpack
@mock.patch.object(pathutils.PathUtils, '_get_instances_sub_dir')
@mock.patch.object(pathutils.PathUtils, '_get_remote_unc_path')
@mock.patch.object(pathutils.PathUtils, 'get_remote_path')
@mock.patch.object(pathutils.PathUtils, '_check_dir')
@mock.patch.object(pathutils.os.path, 'exists')
@mock.patch('os_win.utilsfactory.get_vmutils')
def test_get_instance_dir(self, mock_get_vmutils,
mock_exists,
mock_check_dir,
mock_get_remote_unc_path,
mock_get_remote_path,
mock_get_instances_sub_dir,
configured_dir_exists=False,
remote_server=None, vm_exists=False):
@ -152,7 +189,7 @@ class PathUtilsTestCase(test_base.HyperVBaseTestCase):
else os_win_exc.HyperVVMNotFoundException(
vm_name=mock.sentinel.instance_name))
mock_get_remote_unc_path.return_value = mock.sentinel.remote_root_dir
mock_get_remote_path.return_value = mock.sentinel.remote_root_dir
instance_dir = self._pathutils.get_instance_dir(
mock.sentinel.instance_name,
@ -170,7 +207,7 @@ class PathUtilsTestCase(test_base.HyperVBaseTestCase):
if remote_server:
expected_instance_dir = mock.sentinel.remote_root_dir
mock_get_remote_unc_path.assert_called_once_with(
mock_get_remote_path.assert_called_once_with(
mock.sentinel.remote_server,
mock.sentinel.config_root_dir)
else: