Properly handle shared storage during live migration
After a live migration is performed, we always try to delete the instance files. If the migration fails, the destination node is not cleaned up as the HyperVLiveMigrateData object is not used at the moment and the nova compute manager won't know whether a cleanup is needed or not. Also, in order to support using CSVs for storing instance files, we need to explicitly specify whether image files will be migrated or not when requesting os-win to migrate the VM. This change addresses those issues. Depends-On: I48dc6da29534bf0c477c742311ac15438d8b63ad Change-Id: Icf2f258ef8900b5970ff153a7b981ee53e58dd2d Partial-Bug: #1565895
This commit is contained in:
Lucian Petrut
parent
94558ef556
commit
e2d304810f
|
|
@ -67,11 +67,13 @@ class ClusterLiveMigrationOps(livemigrationops.LiveMigrationOps):
|
|||
with excutils.save_and_reraise_exception():
|
||||
LOG.debug("Calling live migration recover_method "
|
||||
"for instance.", instance=instance_ref)
|
||||
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.",
|
||||
instance=instance_ref)
|
||||
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):
|
||||
|
|
@ -81,7 +83,8 @@ class ClusterLiveMigrationOps(livemigrationops.LiveMigrationOps):
|
|||
super(ClusterLiveMigrationOps, self).pre_live_migration(
|
||||
context, instance, block_device_info, network_info)
|
||||
|
||||
def post_live_migration(self, context, instance, block_device_info):
|
||||
def post_live_migration(self, context, instance, block_device_info,
|
||||
migrate_data):
|
||||
if not self._is_instance_clustered(instance.name):
|
||||
super(ClusterLiveMigrationOps, self).post_live_migration(
|
||||
context, instance, block_device_info)
|
||||
context, instance, block_device_info, migrate_data)
|
||||
|
|
|
|||
|
|
@ -240,18 +240,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):
|
||||
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_source(self, context, instance, network_info):
|
||||
"""Unplug VIFs from networks at source."""
|
||||
|
|
|
|||
|
|
@ -50,28 +50,41 @@ class LiveMigrationOps(object):
|
|||
LOG.debug("live_migration called", instance=instance_ref)
|
||||
instance_name = instance_ref["name"]
|
||||
|
||||
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)
|
||||
|
||||
shared_storage = (
|
||||
self._pathutils.check_remote_instances_dir_shared(dest))
|
||||
if not shared_storage:
|
||||
self._vmops.copy_vm_dvd_disks(instance_name, dest)
|
||||
self._pathutils.copy_vm_console_logs(instance_name, dest)
|
||||
|
||||
self._livemigrutils.live_migrate_vm(instance_name,
|
||||
dest)
|
||||
self._livemigrutils.live_migrate_vm(
|
||||
instance_name,
|
||||
dest,
|
||||
migrate_disks=not shared_storage)
|
||||
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):
|
||||
|
|
@ -98,11 +111,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):
|
||||
|
|
@ -114,8 +130,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")
|
||||
|
|
|
|||
|
|
@ -50,14 +50,16 @@ class ClusterLiveMigrationOpsTestCase(test_base.HyperVBaseTestCase):
|
|||
|
||||
self.livemigrops.live_migration(
|
||||
self._fake_context, mock_instance, dest, post_method,
|
||||
mock.sentinel.recover_method, block_migration=False,
|
||||
migrate_data=None)
|
||||
mock.sentinel.recover_method,
|
||||
block_migration=mock.sentinel.block_migration,
|
||||
migrate_data=mock.sentinel.migrate_data)
|
||||
|
||||
clustutils = self.livemigrops._clustutils
|
||||
clustutils.live_migrate_vm.assert_called_once_with(
|
||||
mock_instance.name, dest)
|
||||
post_method.assert_called_once_with(
|
||||
self._fake_context, mock_instance, dest, False)
|
||||
self._fake_context, mock_instance, dest,
|
||||
mock.sentinel.block_migration, mock.sentinel.migrate_data)
|
||||
|
||||
def test_live_migration_in_cluster_exception(self):
|
||||
mock_instance = fake_instance.fake_instance_obj(self._fake_context)
|
||||
|
|
@ -75,10 +77,14 @@ class ClusterLiveMigrationOpsTestCase(test_base.HyperVBaseTestCase):
|
|||
os_win_exc.HyperVVMNotFoundException,
|
||||
self.livemigrops.live_migration,
|
||||
self._fake_context, mock_instance, dest, mock.sentinel.post_method,
|
||||
recover_method, block_migration=False, migrate_data=None)
|
||||
recover_method,
|
||||
block_migration=mock.sentinel.block_migration,
|
||||
migrate_data=mock.sentinel.migrate_data)
|
||||
|
||||
recover_method.assert_called_once_with(
|
||||
self._fake_context, mock_instance, dest, False)
|
||||
self._fake_context, mock_instance, dest,
|
||||
mock.sentinel.block_migration,
|
||||
mock.sentinel.migrate_data)
|
||||
|
||||
@mock.patch.object(base_livemigrationops.LiveMigrationOps,
|
||||
'live_migration')
|
||||
|
|
@ -126,7 +132,8 @@ class ClusterLiveMigrationOpsTestCase(test_base.HyperVBaseTestCase):
|
|||
def test_post_live_migration_clustered(self, mock_post_live_migration):
|
||||
self.livemigrops.post_live_migration(self._fake_context,
|
||||
mock.sentinel.fake_instance,
|
||||
mock.sentinel.bdi)
|
||||
mock.sentinel.bdi,
|
||||
mock.sentinel.migrate_data)
|
||||
|
||||
self.assertFalse(mock_post_live_migration.called)
|
||||
|
||||
|
|
@ -136,8 +143,10 @@ class ClusterLiveMigrationOpsTestCase(test_base.HyperVBaseTestCase):
|
|||
self.livemigrops._clustutils.vm_exists.return_value = False
|
||||
self.livemigrops.post_live_migration(self._fake_context,
|
||||
mock.sentinel.fake_instance,
|
||||
mock.sentinel.bdi)
|
||||
mock.sentinel.bdi,
|
||||
mock.sentinel.migrate_data)
|
||||
|
||||
mock_post_live_migration.assert_called_once_with(
|
||||
self._fake_context, mock.sentinel.fake_instance,
|
||||
mock.sentinel.bdi)
|
||||
mock.sentinel.bdi,
|
||||
mock.sentinel.migrate_data)
|
||||
|
|
|
|||
|
|
@ -307,14 +307,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,
|
||||
|
|
@ -328,7 +330,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_source(self):
|
||||
self.driver.post_live_migration_at_source(
|
||||
|
|
|
|||
|
|
@ -13,7 +13,9 @@
|
|||
# License for the specific language governing permissions and limitations
|
||||
# under the License.
|
||||
|
||||
import ddt
|
||||
import mock
|
||||
from nova.objects import migrate_data as migrate_data_obj
|
||||
from os_win import exceptions as os_win_exc
|
||||
from oslo_config import cfg
|
||||
|
||||
|
|
@ -25,6 +27,7 @@ from hyperv.tests.unit import test_base
|
|||
CONF = cfg.CONF
|
||||
|
||||
|
||||
@ddt.ddt
|
||||
class LiveMigrationOpsTestCase(test_base.HyperVBaseTestCase):
|
||||
"""Unit tests for the Hyper-V LiveMigrationOps class."""
|
||||
|
||||
|
|
@ -34,13 +37,21 @@ class LiveMigrationOpsTestCase(test_base.HyperVBaseTestCase):
|
|||
self._livemigrops = livemigrationops.LiveMigrationOps()
|
||||
self._livemigrops._block_dev_man = mock.MagicMock()
|
||||
self._livemigrops._pathutils = mock.MagicMock()
|
||||
self._pathutils = self._livemigrops._pathutils
|
||||
|
||||
@ddt.data({'migrate_data_received': False},
|
||||
{'migrate_data_version': '1.0'},
|
||||
{'shared_storage': True},
|
||||
{'side_effect': os_win_exc.HyperVException})
|
||||
@ddt.unpack
|
||||
@mock.patch.object(serialconsoleops.SerialConsoleOps,
|
||||
'stop_console_handler')
|
||||
@mock.patch('hyperv.nova.vmops.VMOps.copy_vm_dvd_disks')
|
||||
def _test_live_migration(self, mock_copy_dvd_disks,
|
||||
mock_stop_console_handler, side_effect=None,
|
||||
shared_storage=False):
|
||||
def test_live_migration(self, mock_copy_dvd_disks,
|
||||
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()
|
||||
|
|
@ -52,23 +63,56 @@ class LiveMigrationOpsTestCase(test_base.HyperVBaseTestCase):
|
|||
self._livemigrops._pathutils.check_remote_instances_dir_shared)
|
||||
mock_check_shared_storage.return_value = shared_storage
|
||||
|
||||
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)
|
||||
|
||||
mock_post.assert_called_once_with(self.context,
|
||||
mock_instance,
|
||||
fake_dest,
|
||||
mock.sentinel.block_migr,
|
||||
mock.ANY)
|
||||
# The last argument, the migrate_data object, should be created
|
||||
# by the callee if not received.
|
||||
post_call_args_list = mock_post.call_args_list[0][0]
|
||||
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_storage.assert_called_once_with(fake_dest)
|
||||
else:
|
||||
self.assertFalse(mock_check_shared_storage.called)
|
||||
|
||||
mock_stop_console_handler.assert_called_once_with(
|
||||
mock_instance.name)
|
||||
mock_check_shared_storage.assert_called_once_with(fake_dest)
|
||||
|
||||
if not shared_storage:
|
||||
mock_copy_logs.assert_called_once_with(mock_instance.name,
|
||||
|
|
@ -81,18 +125,9 @@ class LiveMigrationOpsTestCase(test_base.HyperVBaseTestCase):
|
|||
|
||||
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)
|
||||
|
||||
def test_live_migration(self):
|
||||
self._test_live_migration()
|
||||
|
||||
def test_live_migration_shared_storage(self):
|
||||
self._test_live_migration(shared_storage=True)
|
||||
|
||||
def test_live_migration_exception(self):
|
||||
self._test_live_migration(side_effect=os_win_exc.HyperVException)
|
||||
fake_dest,
|
||||
migrate_disks=(
|
||||
not shared_storage))
|
||||
|
||||
@mock.patch('hyperv.nova.volumeops.VolumeOps.get_disk_path_mapping')
|
||||
@mock.patch('hyperv.nova.imagecache.ImageCache.get_cached_image')
|
||||
|
|
@ -140,15 +175,29 @@ class LiveMigrationOpsTestCase(test_base.HyperVBaseTestCase):
|
|||
def test_pre_live_migration_without_phys_disks_attached(self):
|
||||
self._test_pre_live_migration(phys_disks_attached=False)
|
||||
|
||||
@ddt.data({},
|
||||
{'shared_storage': True})
|
||||
@ddt.unpack
|
||||
@mock.patch('hyperv.nova.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)
|
||||
|
||||
@mock.patch.object(livemigrationops.vmops.VMOps, 'post_start_vifs')
|
||||
def test_post_live_migration_at_destination(self, mock_post_start_vifs):
|
||||
|
|
@ -158,3 +207,18 @@ class LiveMigrationOpsTestCase(test_base.HyperVBaseTestCase):
|
|||
|
||||
mock_post_start_vifs.assert_called_once_with(
|
||||
mock.sentinel.instance, mock.sentinel.network_info)
|
||||
|
||||
@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)
|
||||
|
|
|
|||
Loading…
Reference in New Issue