libvirt: Delete duplicate check when live-migrating
A year ago a useless check was added: I7989128d The above patch was aimed to enable live-migration when instance is booted from volume and has not local disk by adding a new check. However, the same check has been already checked in _is_shared_block_storage method. The last part of the _is_shared_block_storage method does the same that above patch does: - check whether the instance is booted from volume - check whether the instance has not a local disk Also this check calls _is_booted_from_volume incorrectly. Parameter disk_mapping of _is_booted_from_volume must be a dict, but this check specifies a string instead. And finally introduced _has_local_disk method is wrong, because it does not take into accont disk.ephN names. This change reverts I7989128d, improves and simplifies related tests. Closes-Bug: 1598661 Related-Bug: 1469006 Co-Authored-By: Feodor Tersin <ftersin@hotmail.com> Change-Id: Id59012547c3318d94b65ab9f7c37c33c3a08b0e0
This commit is contained in:
parent
5d1b4c8ce6
commit
033d4d2689
|
@ -7094,10 +7094,7 @@ class LibvirtConnTestCase(test.NoDBTestCase):
|
||||||
def _mock_can_live_migrate_source(self, block_migration=False,
|
def _mock_can_live_migrate_source(self, block_migration=False,
|
||||||
is_shared_block_storage=False,
|
is_shared_block_storage=False,
|
||||||
is_shared_instance_path=False,
|
is_shared_instance_path=False,
|
||||||
is_booted_from_volume=False,
|
disk_available_mb=1024):
|
||||||
disk_available_mb=1024,
|
|
||||||
block_device_info=None,
|
|
||||||
block_device_text=None):
|
|
||||||
instance = objects.Instance(**self.test_instance)
|
instance = objects.Instance(**self.test_instance)
|
||||||
dest_check_data = objects.LibvirtLiveMigrateData(
|
dest_check_data = objects.LibvirtLiveMigrateData(
|
||||||
filename='file',
|
filename='file',
|
||||||
|
@ -7109,17 +7106,10 @@ class LibvirtConnTestCase(test.NoDBTestCase):
|
||||||
|
|
||||||
self.mox.StubOutWithMock(drvr, '_is_shared_block_storage')
|
self.mox.StubOutWithMock(drvr, '_is_shared_block_storage')
|
||||||
drvr._is_shared_block_storage(instance, dest_check_data,
|
drvr._is_shared_block_storage(instance, dest_check_data,
|
||||||
block_device_info).AndReturn(is_shared_block_storage)
|
None).AndReturn(is_shared_block_storage)
|
||||||
self.mox.StubOutWithMock(drvr, '_check_shared_storage_test_file')
|
self.mox.StubOutWithMock(drvr, '_check_shared_storage_test_file')
|
||||||
drvr._check_shared_storage_test_file('file', instance).AndReturn(
|
drvr._check_shared_storage_test_file('file', instance).AndReturn(
|
||||||
is_shared_instance_path)
|
is_shared_instance_path)
|
||||||
self.mox.StubOutWithMock(drvr, "get_instance_disk_info")
|
|
||||||
drvr.get_instance_disk_info(instance,
|
|
||||||
block_device_info=block_device_info).\
|
|
||||||
AndReturn(block_device_text)
|
|
||||||
self.mox.StubOutWithMock(drvr, '_is_booted_from_volume')
|
|
||||||
drvr._is_booted_from_volume(instance, block_device_text).AndReturn(
|
|
||||||
is_booted_from_volume)
|
|
||||||
|
|
||||||
return (instance, dest_check_data, drvr)
|
return (instance, dest_check_data, drvr)
|
||||||
|
|
||||||
|
@ -7137,21 +7127,25 @@ class LibvirtConnTestCase(test.NoDBTestCase):
|
||||||
dest_check_data)
|
dest_check_data)
|
||||||
self.assertIsInstance(ret, objects.LibvirtLiveMigrateData)
|
self.assertIsInstance(ret, objects.LibvirtLiveMigrateData)
|
||||||
self.assertIn('is_shared_block_storage', ret)
|
self.assertIn('is_shared_block_storage', ret)
|
||||||
|
self.assertFalse(ret.is_shared_block_storage)
|
||||||
self.assertIn('is_shared_instance_path', ret)
|
self.assertIn('is_shared_instance_path', ret)
|
||||||
|
self.assertFalse(ret.is_shared_instance_path)
|
||||||
|
|
||||||
def test_check_can_live_migrate_source_shared_block_storage(self):
|
def test_check_can_live_migrate_source_shared_block_storage(self):
|
||||||
instance, dest_check_data, drvr = self._mock_can_live_migrate_source(
|
instance, dest_check_data, drvr = self._mock_can_live_migrate_source(
|
||||||
is_shared_block_storage=True)
|
is_shared_block_storage=True)
|
||||||
self.mox.ReplayAll()
|
self.mox.ReplayAll()
|
||||||
drvr.check_can_live_migrate_source(self.context, instance,
|
ret = drvr.check_can_live_migrate_source(self.context, instance,
|
||||||
dest_check_data)
|
dest_check_data)
|
||||||
|
self.assertTrue(ret.is_shared_block_storage)
|
||||||
|
|
||||||
def test_check_can_live_migrate_source_shared_instance_path(self):
|
def test_check_can_live_migrate_source_shared_instance_path(self):
|
||||||
instance, dest_check_data, drvr = self._mock_can_live_migrate_source(
|
instance, dest_check_data, drvr = self._mock_can_live_migrate_source(
|
||||||
is_shared_instance_path=True)
|
is_shared_instance_path=True)
|
||||||
self.mox.ReplayAll()
|
self.mox.ReplayAll()
|
||||||
drvr.check_can_live_migrate_source(self.context, instance,
|
ret = drvr.check_can_live_migrate_source(self.context, instance,
|
||||||
dest_check_data)
|
dest_check_data)
|
||||||
|
self.assertTrue(ret.is_shared_instance_path)
|
||||||
|
|
||||||
def test_check_can_live_migrate_source_non_shared_fails(self):
|
def test_check_can_live_migrate_source_non_shared_fails(self):
|
||||||
instance, dest_check_data, drvr = self._mock_can_live_migrate_source()
|
instance, dest_check_data, drvr = self._mock_can_live_migrate_source()
|
||||||
|
@ -7187,53 +7181,31 @@ class LibvirtConnTestCase(test.NoDBTestCase):
|
||||||
drvr.check_can_live_migrate_source,
|
drvr.check_can_live_migrate_source,
|
||||||
self.context, instance, dest_check_data)
|
self.context, instance, dest_check_data)
|
||||||
|
|
||||||
def test_check_can_live_migrate_source_with_dest_not_enough_disk(self):
|
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
||||||
|
'get_instance_disk_info')
|
||||||
|
def test_check_can_live_migrate_source_with_dest_not_enough_disk(
|
||||||
|
self, mock_get_bdi):
|
||||||
|
mock_get_bdi.return_value = '[{"virt_disk_size":2}]'
|
||||||
|
|
||||||
instance, dest_check_data, drvr = self._mock_can_live_migrate_source(
|
instance, dest_check_data, drvr = self._mock_can_live_migrate_source(
|
||||||
block_migration=True,
|
block_migration=True,
|
||||||
disk_available_mb=0)
|
disk_available_mb=0)
|
||||||
|
|
||||||
drvr.get_instance_disk_info(instance,
|
|
||||||
block_device_info=None).AndReturn(
|
|
||||||
'[{"virt_disk_size":2}]')
|
|
||||||
|
|
||||||
self.mox.ReplayAll()
|
self.mox.ReplayAll()
|
||||||
|
|
||||||
self.assertRaises(exception.MigrationError,
|
self.assertRaises(exception.MigrationError,
|
||||||
drvr.check_can_live_migrate_source,
|
drvr.check_can_live_migrate_source,
|
||||||
self.context, instance, dest_check_data)
|
self.context, instance, dest_check_data)
|
||||||
|
mock_get_bdi.assert_called_once_with(instance, block_device_info=None)
|
||||||
def test_check_can_live_migrate_source_booted_from_volume(self):
|
|
||||||
instance, dest_check_data, drvr = self._mock_can_live_migrate_source(
|
|
||||||
is_booted_from_volume=True,
|
|
||||||
block_device_text='[]')
|
|
||||||
self.mox.ReplayAll()
|
|
||||||
drvr.check_can_live_migrate_source(self.context, instance,
|
|
||||||
dest_check_data)
|
|
||||||
|
|
||||||
def test_check_can_live_migrate_source_booted_from_volume_with_swap(self):
|
|
||||||
instance, dest_check_data, drvr = self._mock_can_live_migrate_source(
|
|
||||||
is_booted_from_volume=True,
|
|
||||||
block_device_text='[{"path":"disk.swap"}]')
|
|
||||||
self.mox.ReplayAll()
|
|
||||||
self.assertRaises(exception.InvalidSharedStorage,
|
|
||||||
drvr.check_can_live_migrate_source,
|
|
||||||
self.context, instance, dest_check_data)
|
|
||||||
|
|
||||||
@mock.patch.object(host.Host, 'has_min_version', return_value=False)
|
@mock.patch.object(host.Host, 'has_min_version', return_value=False)
|
||||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
||||||
'_assert_dest_node_has_enough_disk')
|
'_assert_dest_node_has_enough_disk')
|
||||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
|
||||||
'_has_local_disk')
|
|
||||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
|
||||||
'_is_booted_from_volume')
|
|
||||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
|
||||||
'get_instance_disk_info')
|
|
||||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
||||||
'_is_shared_block_storage', return_value=False)
|
'_is_shared_block_storage', return_value=False)
|
||||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
||||||
'_check_shared_storage_test_file', return_value=False)
|
'_check_shared_storage_test_file', return_value=False)
|
||||||
def test_check_can_live_migrate_source_block_migration_with_bdm_error(
|
def test_check_can_live_migrate_source_block_migration_with_bdm_error(
|
||||||
self, mock_check, mock_shared_block, mock_get_bdi,
|
self, mock_check, mock_shared_block, mock_enough,
|
||||||
mock_booted_from_volume, mock_has_local, mock_enough,
|
|
||||||
mock_min_version):
|
mock_min_version):
|
||||||
|
|
||||||
bdi = {'block_device_mapping': ['bdm']}
|
bdi = {'block_device_mapping': ['bdm']}
|
||||||
|
@ -7253,19 +7225,12 @@ class LibvirtConnTestCase(test.NoDBTestCase):
|
||||||
@mock.patch.object(host.Host, 'has_min_version', return_value=True)
|
@mock.patch.object(host.Host, 'has_min_version', return_value=True)
|
||||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
||||||
'_assert_dest_node_has_enough_disk')
|
'_assert_dest_node_has_enough_disk')
|
||||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
|
||||||
'_has_local_disk')
|
|
||||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
|
||||||
'_is_booted_from_volume')
|
|
||||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
|
||||||
'get_instance_disk_info')
|
|
||||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
||||||
'_is_shared_block_storage', return_value=False)
|
'_is_shared_block_storage', return_value=False)
|
||||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
||||||
'_check_shared_storage_test_file', return_value=False)
|
'_check_shared_storage_test_file', return_value=False)
|
||||||
def test_check_can_live_migrate_source_bm_with_bdm_tunnelled_error(
|
def test_check_can_live_migrate_source_bm_with_bdm_tunnelled_error(
|
||||||
self, mock_check, mock_shared_block, mock_get_bdi,
|
self, mock_check, mock_shared_block, mock_enough,
|
||||||
mock_booted_from_volume, mock_has_local, mock_enough,
|
|
||||||
mock_min_version):
|
mock_min_version):
|
||||||
|
|
||||||
self.flags(live_migration_tunnelled=True,
|
self.flags(live_migration_tunnelled=True,
|
||||||
|
@ -7288,20 +7253,13 @@ class LibvirtConnTestCase(test.NoDBTestCase):
|
||||||
@mock.patch.object(host.Host, 'has_min_version', return_value=True)
|
@mock.patch.object(host.Host, 'has_min_version', return_value=True)
|
||||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
||||||
'_assert_dest_node_has_enough_disk')
|
'_assert_dest_node_has_enough_disk')
|
||||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
|
||||||
'_has_local_disk')
|
|
||||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
|
||||||
'_is_booted_from_volume')
|
|
||||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
|
||||||
'get_instance_disk_info')
|
|
||||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
||||||
'_is_shared_block_storage')
|
'_is_shared_block_storage')
|
||||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
||||||
'_check_shared_storage_test_file')
|
'_check_shared_storage_test_file')
|
||||||
def _test_check_can_live_migrate_source_block_migration_none(
|
def _test_check_can_live_migrate_source_block_migration_none(
|
||||||
self, block_migrate, is_shared_instance_path, is_share_block,
|
self, block_migrate, is_shared_instance_path, is_share_block,
|
||||||
mock_check, mock_shared_block, mock_get_bdi,
|
mock_check, mock_shared_block, mock_enough, mock_verson):
|
||||||
mock_booted_from_volume, mock_has_local, mock_enough, mock_verson):
|
|
||||||
|
|
||||||
mock_check.return_value = is_shared_instance_path
|
mock_check.return_value = is_shared_instance_path
|
||||||
mock_shared_block.return_value = is_share_block
|
mock_shared_block.return_value = is_share_block
|
||||||
|
@ -7338,20 +7296,12 @@ class LibvirtConnTestCase(test.NoDBTestCase):
|
||||||
'_assert_dest_node_has_enough_disk')
|
'_assert_dest_node_has_enough_disk')
|
||||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
||||||
'_assert_dest_node_has_enough_disk')
|
'_assert_dest_node_has_enough_disk')
|
||||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
|
||||||
'_has_local_disk')
|
|
||||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
|
||||||
'_is_booted_from_volume')
|
|
||||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
|
||||||
'get_instance_disk_info')
|
|
||||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
||||||
'_is_shared_block_storage')
|
'_is_shared_block_storage')
|
||||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
||||||
'_check_shared_storage_test_file')
|
'_check_shared_storage_test_file')
|
||||||
def test_check_can_live_migration_source_disk_over_commit_none(self,
|
def test_check_can_live_migration_source_disk_over_commit_none(self,
|
||||||
mock_check, mock_shared_block, mock_get_bdi,
|
mock_check, mock_shared_block, mock_enough, mock_disk_check):
|
||||||
mock_booted_from_volume, mock_has_local,
|
|
||||||
mock_enough, mock_disk_check):
|
|
||||||
|
|
||||||
mock_check.return_value = False
|
mock_check.return_value = False
|
||||||
mock_shared_block.return_value = False
|
mock_shared_block.return_value = False
|
||||||
|
|
|
@ -2938,20 +2938,6 @@ class LibvirtDriver(driver.ComputeDriver):
|
||||||
return ((not bool(instance.get('image_ref')))
|
return ((not bool(instance.get('image_ref')))
|
||||||
or 'disk' not in disk_mapping)
|
or 'disk' not in disk_mapping)
|
||||||
|
|
||||||
@staticmethod
|
|
||||||
def _has_local_disk(instance, disk_mapping):
|
|
||||||
"""Determines whether the VM has a local disk
|
|
||||||
|
|
||||||
Determines whether the disk mapping indicates that the VM
|
|
||||||
has a local disk (e.g. ephemeral, swap disk and config-drive).
|
|
||||||
"""
|
|
||||||
if disk_mapping:
|
|
||||||
if ('disk.local' in disk_mapping or
|
|
||||||
'disk.swap' in disk_mapping or
|
|
||||||
'disk.config' in disk_mapping):
|
|
||||||
return True
|
|
||||||
return False
|
|
||||||
|
|
||||||
def _inject_data(self, injection_image, instance, network_info,
|
def _inject_data(self, injection_image, instance, network_info,
|
||||||
admin_pass, files):
|
admin_pass, files):
|
||||||
"""Injects data in a disk image
|
"""Injects data in a disk image
|
||||||
|
@ -5564,12 +5550,6 @@ class LibvirtDriver(driver.ComputeDriver):
|
||||||
self._is_shared_block_storage(instance, dest_check_data,
|
self._is_shared_block_storage(instance, dest_check_data,
|
||||||
block_device_info))
|
block_device_info))
|
||||||
|
|
||||||
disk_info_text = self.get_instance_disk_info(
|
|
||||||
instance, block_device_info=block_device_info)
|
|
||||||
booted_from_volume = self._is_booted_from_volume(instance,
|
|
||||||
disk_info_text)
|
|
||||||
has_local_disk = self._has_local_disk(instance, disk_info_text)
|
|
||||||
|
|
||||||
if 'block_migration' not in dest_check_data:
|
if 'block_migration' not in dest_check_data:
|
||||||
dest_check_data.block_migration = (
|
dest_check_data.block_migration = (
|
||||||
not dest_check_data.is_on_shared_storage())
|
not dest_check_data.is_on_shared_storage())
|
||||||
|
@ -5621,8 +5601,7 @@ class LibvirtDriver(driver.ComputeDriver):
|
||||||
LOG.error(msg, instance=instance)
|
LOG.error(msg, instance=instance)
|
||||||
raise exception.MigrationPreCheckError(reason=msg)
|
raise exception.MigrationPreCheckError(reason=msg)
|
||||||
elif not (dest_check_data.is_shared_block_storage or
|
elif not (dest_check_data.is_shared_block_storage or
|
||||||
dest_check_data.is_shared_instance_path or
|
dest_check_data.is_shared_instance_path):
|
||||||
(booted_from_volume and not has_local_disk)):
|
|
||||||
reason = _("Live migration can not be used "
|
reason = _("Live migration can not be used "
|
||||||
"without shared storage except "
|
"without shared storage except "
|
||||||
"a booted from volume VM which "
|
"a booted from volume VM which "
|
||||||
|
|
Loading…
Reference in New Issue