openstack
/
compute-hyperv
Code Issues Proposed changes

Properly handle shared storage in case of cold migrations 

At the moment, if the destination host is other than the source
host, we attempt to move the instance files without checking if
shared storage is being used.

This change fixes this issue. In case of shared storage, we'll have
the same workflow as when the migration source is the migration
destination as well.

Closes-Bug: #1565895

Change-Id: I711976b42a5de5a28de22e010b48c10d96b68ba5
This commit is contained in:
Lucian Petrut 2016-04-04 17:22:29 +03:00 committed by Petrut Lucian
parent d8ed718c58
commit 60c06bdd6f
4 changed files with 68 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
hyperv/nova/migrationops.py
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,25 @@ 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,7 +79,7 @@ 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)
except Exception:
with excutils.save_and_reraise_exception():

12
hyperv/nova/pathutils.py
View File

@ -14,6 +14,7 @@
# under the License.
import os
import tempfile
import time
import nova.conf
@ -167,3 +168,14 @@ 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.
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

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

@ -47,16 +47,21 @@ class MigrationOpsTestCase(test_base.HyperVBaseTestCase):
self._migrationops._imagecache = mock.MagicMock()
self._migrationops._block_dev_manager = 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)]
@ -65,24 +70,24 @@ 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))
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(
@ -91,10 +96,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')

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

@ -154,3 +154,22 @@ class PathUtilsTestCase(test_base.HyperVBaseTestCase):
self.assertEqual(5, ret)
mock_getmtime.assert_called_once_with(mock.sentinel.file_name)
@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)