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,
|
||||
is_shared_block_storage=False,
|
||||
is_shared_instance_path=False,
|
||||
is_booted_from_volume=False,
|
||||
disk_available_mb=1024,
|
||||
block_device_info=None,
|
||||
block_device_text=None):
|
||||
disk_available_mb=1024):
|
||||
instance = objects.Instance(**self.test_instance)
|
||||
dest_check_data = objects.LibvirtLiveMigrateData(
|
||||
filename='file',
|
||||
|
@ -7109,17 +7106,10 @@ class LibvirtConnTestCase(test.NoDBTestCase):
|
|||
|
||||
self.mox.StubOutWithMock(drvr, '_is_shared_block_storage')
|
||||
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')
|
||||
drvr._check_shared_storage_test_file('file', instance).AndReturn(
|
||||
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)
|
||||
|
||||
|
@ -7137,21 +7127,25 @@ class LibvirtConnTestCase(test.NoDBTestCase):
|
|||
dest_check_data)
|
||||
self.assertIsInstance(ret, objects.LibvirtLiveMigrateData)
|
||||
self.assertIn('is_shared_block_storage', ret)
|
||||
self.assertFalse(ret.is_shared_block_storage)
|
||||
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):
|
||||
instance, dest_check_data, drvr = self._mock_can_live_migrate_source(
|
||||
is_shared_block_storage=True)
|
||||
self.mox.ReplayAll()
|
||||
drvr.check_can_live_migrate_source(self.context, instance,
|
||||
dest_check_data)
|
||||
ret = drvr.check_can_live_migrate_source(self.context, instance,
|
||||
dest_check_data)
|
||||
self.assertTrue(ret.is_shared_block_storage)
|
||||
|
||||
def test_check_can_live_migrate_source_shared_instance_path(self):
|
||||
instance, dest_check_data, drvr = self._mock_can_live_migrate_source(
|
||||
is_shared_instance_path=True)
|
||||
self.mox.ReplayAll()
|
||||
drvr.check_can_live_migrate_source(self.context, instance,
|
||||
dest_check_data)
|
||||
ret = drvr.check_can_live_migrate_source(self.context, instance,
|
||||
dest_check_data)
|
||||
self.assertTrue(ret.is_shared_instance_path)
|
||||
|
||||
def test_check_can_live_migrate_source_non_shared_fails(self):
|
||||
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,
|
||||
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(
|
||||
block_migration=True,
|
||||
disk_available_mb=0)
|
||||
|
||||
drvr.get_instance_disk_info(instance,
|
||||
block_device_info=None).AndReturn(
|
||||
'[{"virt_disk_size":2}]')
|
||||
|
||||
self.mox.ReplayAll()
|
||||
|
||||
self.assertRaises(exception.MigrationError,
|
||||
drvr.check_can_live_migrate_source,
|
||||
self.context, instance, dest_check_data)
|
||||
|
||||
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_get_bdi.assert_called_once_with(instance, block_device_info=None)
|
||||
|
||||
@mock.patch.object(host.Host, 'has_min_version', return_value=False)
|
||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
||||
'_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.'
|
||||
'_is_shared_block_storage', return_value=False)
|
||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
||||
'_check_shared_storage_test_file', return_value=False)
|
||||
def test_check_can_live_migrate_source_block_migration_with_bdm_error(
|
||||
self, mock_check, mock_shared_block, mock_get_bdi,
|
||||
mock_booted_from_volume, mock_has_local, mock_enough,
|
||||
self, mock_check, mock_shared_block, mock_enough,
|
||||
mock_min_version):
|
||||
|
||||
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('nova.virt.libvirt.driver.LibvirtDriver.'
|
||||
'_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.'
|
||||
'_is_shared_block_storage', return_value=False)
|
||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
||||
'_check_shared_storage_test_file', return_value=False)
|
||||
def test_check_can_live_migrate_source_bm_with_bdm_tunnelled_error(
|
||||
self, mock_check, mock_shared_block, mock_get_bdi,
|
||||
mock_booted_from_volume, mock_has_local, mock_enough,
|
||||
self, mock_check, mock_shared_block, mock_enough,
|
||||
mock_min_version):
|
||||
|
||||
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('nova.virt.libvirt.driver.LibvirtDriver.'
|
||||
'_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.'
|
||||
'_is_shared_block_storage')
|
||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
||||
'_check_shared_storage_test_file')
|
||||
def _test_check_can_live_migrate_source_block_migration_none(
|
||||
self, block_migrate, is_shared_instance_path, is_share_block,
|
||||
mock_check, mock_shared_block, mock_get_bdi,
|
||||
mock_booted_from_volume, mock_has_local, mock_enough, mock_verson):
|
||||
mock_check, mock_shared_block, mock_enough, mock_verson):
|
||||
|
||||
mock_check.return_value = is_shared_instance_path
|
||||
mock_shared_block.return_value = is_share_block
|
||||
|
@ -7338,20 +7296,12 @@ class LibvirtConnTestCase(test.NoDBTestCase):
|
|||
'_assert_dest_node_has_enough_disk')
|
||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
||||
'_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.'
|
||||
'_is_shared_block_storage')
|
||||
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver.'
|
||||
'_check_shared_storage_test_file')
|
||||
def test_check_can_live_migration_source_disk_over_commit_none(self,
|
||||
mock_check, mock_shared_block, mock_get_bdi,
|
||||
mock_booted_from_volume, mock_has_local,
|
||||
mock_enough, mock_disk_check):
|
||||
mock_check, mock_shared_block, mock_enough, mock_disk_check):
|
||||
|
||||
mock_check.return_value = False
|
||||
mock_shared_block.return_value = False
|
||||
|
|
|
@ -2938,20 +2938,6 @@ class LibvirtDriver(driver.ComputeDriver):
|
|||
return ((not bool(instance.get('image_ref')))
|
||||
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,
|
||||
admin_pass, files):
|
||||
"""Injects data in a disk image
|
||||
|
@ -5564,12 +5550,6 @@ class LibvirtDriver(driver.ComputeDriver):
|
|||
self._is_shared_block_storage(instance, dest_check_data,
|
||||
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:
|
||||
dest_check_data.block_migration = (
|
||||
not dest_check_data.is_on_shared_storage())
|
||||
|
@ -5621,8 +5601,7 @@ class LibvirtDriver(driver.ComputeDriver):
|
|||
LOG.error(msg, instance=instance)
|
||||
raise exception.MigrationPreCheckError(reason=msg)
|
||||
elif not (dest_check_data.is_shared_block_storage or
|
||||
dest_check_data.is_shared_instance_path or
|
||||
(booted_from_volume and not has_local_disk)):
|
||||
dest_check_data.is_shared_instance_path):
|
||||
reason = _("Live migration can not be used "
|
||||
"without shared storage except "
|
||||
"a booted from volume VM which "
|
||||
|
|
Loading…
Reference in New Issue