openstack
/
nova
Code Issues Proposed changes

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
(cherry picked from commit 033d4d2689)
This commit is contained in:
Hiroyuki Eguchi 2016-07-04 08:14:25 +00:00 committed by Lee Yarwood
parent 842f1b6651
commit 5b2808c92c
2 changed files with 23 additions and 94 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

94
nova/tests/unit/virt/libvirt/test_driver.py
View File

@ -6988,10 +6988,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',
@ -7003,17 +7000,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)
@ -7031,21 +7021,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()
@ -7081,53 +7075,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']}
@ -7147,19 +7119,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,
@ -7182,20 +7147,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
@ -7232,20 +7190,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

23
nova/virt/libvirt/driver.py
View File

@ -2888,20 +2888,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
@ -5486,12 +5472,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())
@ -5543,8 +5523,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 "