Fix copy configdrive during live-migration on HyperV
This bug was introduced due to an oversight when fixing https://launchpad.net/bugs/1322096 . In that fix, we use the "config_drive_cdrom" value from the conf to determine if a there is a iso configdrive attached to the instance. This approach is wrong due to the fact that that config value can be modified by the user. If that happens, instances that still have iso configdrives will be ommitted. This path fixes this problem by checking the instance itself if it has any iso attached to it. Closes-Bug: #1427467 Change-Id: Ia64389b4405109d7db6fbf9c000a0136e9862298
This commit is contained in:
committed by
Claudiu Belu
parent
54d237d03e
commit
eee893c7ee
@@ -18,7 +18,6 @@ Management class for live migration VM operations.
|
||||
"""
|
||||
import functools
|
||||
|
||||
from nova.virt import configdrive
|
||||
from oslo_config import cfg
|
||||
from oslo_log import log as logging
|
||||
from oslo_utils import excutils
|
||||
@@ -27,12 +26,12 @@ from hyperv.i18n import _
|
||||
from hyperv.nova import imagecache
|
||||
from hyperv.nova import utilsfactory
|
||||
from hyperv.nova import vmops
|
||||
from hyperv.nova import vmutilsv2
|
||||
from hyperv.nova import volumeops
|
||||
|
||||
LOG = logging.getLogger(__name__)
|
||||
CONF = cfg.CONF
|
||||
CONF.import_opt('use_cow_images', 'nova.virt.driver')
|
||||
CONF.import_opt('config_drive_cdrom', 'nova.virt.hyperv.vmops', 'hyperv')
|
||||
|
||||
|
||||
def check_os_version_requirement(function):
|
||||
@@ -58,6 +57,7 @@ class LiveMigrationOps(object):
|
||||
self._vmops = vmops.VMOps()
|
||||
self._volumeops = volumeops.VolumeOps()
|
||||
self._imagecache = imagecache.ImageCache()
|
||||
self._vmutils = vmutilsv2.VMUtilsV2()
|
||||
|
||||
@check_os_version_requirement
|
||||
def live_migration(self, context, instance_ref, dest, post_method,
|
||||
@@ -68,10 +68,7 @@ class LiveMigrationOps(object):
|
||||
|
||||
try:
|
||||
self._vmops.copy_vm_console_logs(instance_name, dest)
|
||||
if (configdrive.required_by(instance_ref) and
|
||||
CONF.hyperv.config_drive_cdrom):
|
||||
self._pathutils.copy_configdrive(instance_name, dest)
|
||||
|
||||
self._vmops.copy_vm_dvd_disks(instance_name, dest)
|
||||
self._livemigrutils.live_migrate_vm(instance_name,
|
||||
dest)
|
||||
except Exception:
|
||||
|
||||
@@ -242,11 +242,3 @@ class PathUtils(object):
|
||||
if force:
|
||||
raise vmutils.HyperVException(
|
||||
_("Could not unmount share: %s"), smbfs_share)
|
||||
|
||||
def copy_configdrive(self, instance_name, dest_host):
|
||||
local_configdrive_path = self.get_configdrive_path(
|
||||
instance_name, constants.DVD_FORMAT)
|
||||
remote_configdrive_path = self.get_configdrive_path(
|
||||
instance_name, constants.DVD_FORMAT, remote_server=dest_host)
|
||||
self.copyfile(local_configdrive_path,
|
||||
remote_configdrive_path)
|
||||
|
||||
@@ -688,3 +688,8 @@ class VMOps(object):
|
||||
if vm_serial_conn:
|
||||
instance_uuid = os.path.basename(vm_serial_conn)
|
||||
self.log_vm_serial_output(instance_name, instance_uuid)
|
||||
|
||||
def copy_vm_dvd_disks(self, vm_name, dest_host):
|
||||
dvd_disk_paths = self._vmutils.get_vm_dvd_disk_paths(vm_name)
|
||||
for path in dvd_disk_paths:
|
||||
self._pathutils.copyfile(path, dest_host)
|
||||
|
||||
@@ -298,3 +298,16 @@ class VMUtilsV2(vmutils.VMUtils):
|
||||
Subject=element.path_(),
|
||||
Definition=definition_path,
|
||||
MetricCollectionEnabled=self._METRIC_ENABLED)
|
||||
|
||||
def get_vm_dvd_disk_paths(self, vm_name):
|
||||
vm = self._lookup_vm_check(vm_name)
|
||||
|
||||
settings = vm.associators(
|
||||
wmi_result_class=self._VIRTUAL_SYSTEM_SETTING_DATA_CLASS)[0]
|
||||
sasds = settings.associators(
|
||||
wmi_result_class=self._STORAGE_ALLOC_SETTING_DATA_CLASS)
|
||||
|
||||
dvd_paths = [sasd.HostResource[0] for sasd in sasds
|
||||
if sasd.ResourceSubType == self._DVD_DISK_RES_SUB_TYPE]
|
||||
|
||||
return dvd_paths
|
||||
|
||||
@@ -34,10 +34,9 @@ class LiveMigrationOpsTestCase(test_base.HyperVBaseTestCase):
|
||||
self._livemigrops._livemigrutils = mock.MagicMock()
|
||||
|
||||
@mock.patch('hyperv.nova.vmops.VMOps.copy_vm_console_logs')
|
||||
@mock.patch('nova.virt.configdrive.required_by')
|
||||
@mock.patch('hyperv.nova.pathutils.PathUtils.copy_configdrive')
|
||||
def _test_live_migration(self, mock_copy_configdrive, mock_required_by,
|
||||
mock_copy_logs, side_effect, configdrive=False):
|
||||
@mock.patch('hyperv.nova.vmops.VMOps.copy_vm_dvd_disks')
|
||||
def _test_live_migration(self, mock_get_vm_dvd_paths,
|
||||
mock_copy_logs, side_effect):
|
||||
mock_instance = fake_instance.fake_instance_obj(self.context)
|
||||
mock_post = mock.MagicMock()
|
||||
mock_recover = mock.MagicMock()
|
||||
@@ -52,10 +51,6 @@ class LiveMigrationOpsTestCase(test_base.HyperVBaseTestCase):
|
||||
mock_recover.assert_called_once_with(self.context, mock_instance,
|
||||
fake_dest, False)
|
||||
else:
|
||||
if configdrive:
|
||||
mock_required_by.return_value = True
|
||||
self.flags(config_drive_cdrom=True, group='hyperv')
|
||||
|
||||
self._livemigrops.live_migration(context=self.context,
|
||||
instance_ref=mock_instance,
|
||||
dest=fake_dest,
|
||||
@@ -64,9 +59,6 @@ class LiveMigrationOpsTestCase(test_base.HyperVBaseTestCase):
|
||||
|
||||
mock_copy_logs.assert_called_once_with(mock_instance.name,
|
||||
fake_dest)
|
||||
if configdrive:
|
||||
mock_copy_configdrive.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)
|
||||
@@ -88,9 +80,6 @@ class LiveMigrationOpsTestCase(test_base.HyperVBaseTestCase):
|
||||
post_method=mock.DEFAULT,
|
||||
recover_method=mock.DEFAULT)
|
||||
|
||||
def test_live_migration_with_configdrive(self):
|
||||
self._test_live_migration(side_effect=None, configdrive=True)
|
||||
|
||||
@mock.patch('hyperv.nova.volumeops.VolumeOps'
|
||||
'.ebs_root_in_block_devices')
|
||||
@mock.patch('hyperv.nova.imagecache.ImageCache.get_cached_image')
|
||||
|
||||
@@ -134,20 +134,3 @@ class PathUtilsTestCase(test_base.HyperVBaseTestCase):
|
||||
self.assertRaises(vmutils.HyperVException,
|
||||
self._pathutils._get_instances_sub_dir,
|
||||
fake_dir_name)
|
||||
|
||||
@mock.patch.object(pathutils.PathUtils, 'get_configdrive_path')
|
||||
@mock.patch.object(pathutils.PathUtils, 'copyfile')
|
||||
def test_copy_configdrive(self, mock_copyfile, mock_get_configdrive_path):
|
||||
mock_get_configdrive_path.side_effect = [mock.sentinel.FAKE_LOCAL_PATH,
|
||||
mock.sentinel.FAKE_REMOTE_PATH
|
||||
]
|
||||
self._pathutils.copy_configdrive(self.fake_instance_name,
|
||||
mock.sentinel.DEST_HOST)
|
||||
|
||||
mock_get_configdrive_path.assert_has_calls(
|
||||
[mock.call(self.fake_instance_name, constants.DVD_FORMAT),
|
||||
mock.call(self.fake_instance_name, constants.DVD_FORMAT,
|
||||
remote_server=mock.sentinel.DEST_HOST)])
|
||||
|
||||
mock_copyfile.assert_called_once_with(mock.sentinel.FAKE_LOCAL_PATH,
|
||||
mock.sentinel.FAKE_REMOTE_PATH)
|
||||
|
||||
@@ -1010,3 +1010,19 @@ class VMOpsTestCase(test_base.HyperVBaseTestCase):
|
||||
mock_list_notes.assert_called_once_with()
|
||||
|
||||
self.assertEqual(response, [fake_uuid])
|
||||
|
||||
def test_copy_vm_dvd_disks(self):
|
||||
fake_paths = [mock.sentinel.FAKE_DVD_PATH1,
|
||||
mock.sentinel.FAKE_DVD_PATH2]
|
||||
mock_copy = self._vmops._pathutils.copyfile
|
||||
mock_get_dvd_disk_paths = self._vmops._vmutils.get_vm_dvd_disk_paths
|
||||
mock_get_dvd_disk_paths.return_value = fake_paths
|
||||
|
||||
self._vmops.copy_vm_dvd_disks(mock.sentinel.FAKE_VM_NAME,
|
||||
mock.sentinel.FAKE_DEST)
|
||||
|
||||
mock_get_dvd_disk_paths.assert_called_with(mock.sentinel.FAKE_VM_NAME)
|
||||
mock_copy.has_calls(mock.call(mock.sentinel.FAKE_DVD_PATH1,
|
||||
mock.sentinel.FAKE_DEST),
|
||||
mock.call(mock.sentinel.FAKE_DVD_PATH2,
|
||||
mock.sentinel.FAKE_DEST))
|
||||
|
||||
@@ -235,3 +235,15 @@ class VMUtilsV2TestCase(test_vmutils.VMUtilsTestCase):
|
||||
|
||||
self._vmutils._conn.query.assert_called_once_with(expected_query)
|
||||
self.assertEqual(expected_disks, ret_disks)
|
||||
|
||||
def test_get_vm_dvd_disk_paths(self):
|
||||
mock_vm = self._lookup_vm()
|
||||
mock_sasd1 = mock.MagicMock(
|
||||
ResourceSubType=self._vmutils._DVD_DISK_RES_SUB_TYPE,
|
||||
HostResource=[mock.sentinel.FAKE_DVD_PATH1])
|
||||
mock_settings = mock.MagicMock()
|
||||
mock_settings.associators.return_value = [mock_sasd1]
|
||||
mock_vm.associators.return_value = [mock_settings]
|
||||
|
||||
ret_val = self._vmutils.get_vm_dvd_disk_paths(self._FAKE_VM_NAME)
|
||||
self.assertEqual(mock.sentinel.FAKE_DVD_PATH1, ret_val[0])
|
||||
|
||||
Reference in New Issue
Block a user