From e00240f5d7e2d8e385b8c0d6ea18ed9eddd6e5d1 Mon Sep 17 00:00:00 2001 From: Claudiu Belu Date: Wed, 13 Sep 2017 11:31:12 +0300 Subject: [PATCH] Fixes cold migration path trimming issue During cold migration, the following steps happen: 1. The source node will stop the VM, copy the VM files to a _revert path, destroy the VM, and return the _revert path as disk_info. 2. The destination node will get that disk_info, and if it was configured not to move the storage (multiple CSVs cluster scenario), it will just copy the files to a path without the ending _revert. The issue is that rstrip can strip more characters than desired. It will remove all characters contained by "_revert" from the given path's ending. For example: "instance_000000be_revert".rstrip("_revert") => "instance_000000b" Change-Id: I784835800a961be4ba755a4d51edbc931eccdc57 Closes-Bug: #1716862 --- compute_hyperv/nova/migrationops.py | 6 ++++-- compute_hyperv/tests/unit/test_migrationops.py | 8 ++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/compute_hyperv/nova/migrationops.py b/compute_hyperv/nova/migrationops.py index 071d4a59..9d1d8300 100644 --- a/compute_hyperv/nova/migrationops.py +++ b/compute_hyperv/nova/migrationops.py @@ -235,7 +235,9 @@ class MigrationOps(object): # make sure that the source is not a remote local path. # e.g.: \\win-srv\\C$\OpenStack\Instances\.. # CSVs, local paths, and shares are fine. - inst_dir = source_inst_dir.rstrip('_revert') + # NOTE(claudiub): get rid of the final _revert part of the path. + # rstrip can remove more than _revert, which is not desired. + inst_dir = source_inst_dir.rsplit('_revert', 1)[0] LOG.warning( 'Host is configured not to copy disks on cold migration, but ' 'the instance will not be able to start with the remote path: ' @@ -246,7 +248,7 @@ class MigrationOps(object): else: # make a copy on the source node's configured location. # strip the _revert from the source backup dir. - inst_dir = source_inst_dir.rstrip('_revert') + inst_dir = source_inst_dir.rsplit('_revert', 1)[0] self._pathutils.check_dir(inst_dir, create_dir=True) export_path = self._pathutils.get_export_dir( diff --git a/compute_hyperv/tests/unit/test_migrationops.py b/compute_hyperv/tests/unit/test_migrationops.py index ebc8351e..a028dd05 100644 --- a/compute_hyperv/tests/unit/test_migrationops.py +++ b/compute_hyperv/tests/unit/test_migrationops.py @@ -276,9 +276,9 @@ class MigrationOpsTestCase(test_base.HyperVBaseTestCase): recon_parent_vhd.assert_called_once_with( mock.sentinel.diff_vhd_path, fake_base_vhd) - @ddt.data((False, '\\\\fake-srv\\C$\\inst_dir_revert', True), - (False, '\\\\fake-srv\\share_path\\inst_dir_revert'), - (True, 'C:\\fake_inst_dir_revert')) + @ddt.data((False, '\\\\fake-srv\\C$\\inst_dir_0000000e_revert', True), + (False, '\\\\fake-srv\\share_path\\inst_dir_0000000e_revert'), + (True, 'C:\\fake_inst_dir_0000000e_revert')) @ddt.unpack def test_migrate_disks_from_source(self, move_disks_on_migration, source_inst_dir, is_remote_path=False): @@ -305,7 +305,7 @@ class MigrationOpsTestCase(test_base.HyperVBaseTestCase): mock_instance.name, create_dir=True, remove_dir=True) expected_inst_dir = mock_get_inst_dir.return_value else: - expected_inst_dir = source_inst_dir.rstrip('_revert') + expected_inst_dir = source_inst_dir[0: - len('_revert')] self._migrationops._pathutils.check_dir.assert_called_once_with( expected_inst_dir, create_dir=True)