openstack/nova
Code Issues Proposed changes

libvirt: Improve _is_booted_from_volume implementation

Browse Source 
Currently this method cannot be used widely due to its parameters. It
requires device_info - the dict which can not be easily obtained in many
cases. Since it is often needed to figure out if an instance is booted
from volume, and the method name is "appropriate", this sometimes leads
to errors (when string result of get_instance_disk_info is passed as an
argument to _is_booted_from_volume; see also Id5901254).

This patch makes _is_booted_from_volume to use standard
block_device_info structure, which is accessible in almost any driver
method.

Closes-bug: 1596957
Closes-bug: 1587802
Change-Id: Ie424d172ac9d6aeb42d83e512f2a18713134be3b
This commit is contained in:
Feodor Tersin 2016-10-04 20:17:17 +03:00
parent 033d4d2689
commit ed994bb4d7
2 changed files with 90 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

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

@ -310,7 +310,8 @@ def fake_disk_info_byname(instance, type='qcow2'):
disk_info = OrderedDict()
# root disk
if instance.image_ref is not None:
if (instance.image_ref is not None and
instance.image_ref != uuids.fake_volume_backed_image_ref):
cache_name = imagecache.get_cache_fname(instance.image_ref)
disk_info['disk'] = {
'type': type,
@ -13635,7 +13636,6 @@ class LibvirtConnTestCase(test.NoDBTestCase):
self.assertEqual('/dev/nbd0', inst_sys_meta['rootfs_device_name'])
self.assertFalse(mock_instance.called)
mock_get_inst_path.assert_has_calls([mock.call(mock_instance)])
mock_is_booted_from_volume.assert_called_once_with(mock_instance, {})
mock_ensure_tree.assert_has_calls([mock.call('/tmp/rootfs')])
drvr.image_backend.by_name.assert_has_calls([mock.call(mock_instance,
'disk')])
@ -15306,22 +15306,52 @@ class LibvirtDriverTestCase(test.NoDBTestCase):
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver._disconnect_volume')
def test_migrate_disk_and_power_off_boot_from_volume(self,
disconnect_volume):
info = {'block_device_mapping': [{'boot_index': None,
'mount_device': '/dev/vdd',
'connection_info': None},
{'boot_index': 0,
'mount_device': '/dev/vda',
'connection_info': None}]}
info = {
'block_device_mapping': [
{'boot_index': None,
'mount_device': '/dev/vdd',
'connection_info': mock.sentinel.conn_info_vdd},
{'boot_index': 0,
'mount_device': '/dev/vda',
'connection_info': mock.sentinel.conn_info_vda}]}
flavor = {'root_gb': 1, 'ephemeral_gb': 0}
flavor_obj = objects.Flavor(**flavor)
# Note(Mike_D): The size of instance's ephemeral_gb is 0 gb.
self._test_migrate_disk_and_power_off(
flavor_obj, block_device_info=info,
params_for_instance={'image_ref': None,
'flavor': {'root_gb': 1,
'root_gb': 10,
'ephemeral_gb': 0,
'flavor': {'root_gb': 10,
'ephemeral_gb': 0}})
disconnect_volume.assert_called_with(
info['block_device_mapping'][1]['connection_info'], 'vda')
mock.sentinel.conn_info_vda, 'vda')
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver._disconnect_volume')
def test_migrate_disk_and_power_off_boot_from_volume_backed_snapshot(
self, disconnect_volume):
# Such instance has not empty image_ref, but must be considered as
# booted from volume.
info = {
'block_device_mapping': [
{'boot_index': None,
'mount_device': '/dev/vdd',
'connection_info': mock.sentinel.conn_info_vdd},
{'boot_index': 0,
'mount_device': '/dev/vda',
'connection_info': mock.sentinel.conn_info_vda}]}
flavor = {'root_gb': 1, 'ephemeral_gb': 0}
flavor_obj = objects.Flavor(**flavor)
self._test_migrate_disk_and_power_off(
flavor_obj, block_device_info=info,
params_for_instance={
'image_ref': uuids.fake_volume_backed_image_ref,
'root_gb': 10,
'ephemeral_gb': 0,
'flavor': {'root_gb': 10,
'ephemeral_gb': 0}})
disconnect_volume.assert_called_with(
mock.sentinel.conn_info_vda, 'vda')
@mock.patch('nova.utils.execute')
@mock.patch('nova.virt.libvirt.utils.copy_image')
@ -15492,6 +15522,26 @@ class LibvirtDriverTestCase(test.NoDBTestCase):
self.drvr.migrate_disk_and_power_off,
'ctx', instance, '10.0.0.1', flavor_obj, None)
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver'
'.get_instance_disk_info')
def test_migrate_disk_and_power_off_resize_error_rbd(self,
mock_get_disk_info):
# Check error on resize root disk down for rbd.
# The difference is that get_instance_disk_info always returns
# an emply list for rbd.
# Ephemeral size is not changed in this case (otherwise other check
# will raise the same error).
self.flags(images_type='rbd', group='libvirt')
instance = self._create_instance()
flavor = {'root_gb': 5, 'ephemeral_gb': 20}
flavor_obj = objects.Flavor(**flavor)
mock_get_disk_info.return_value = []
self.assertRaises(
exception.InstanceFaultRollback,
self.drvr.migrate_disk_and_power_off,
'ctx', instance, '10.0.0.1', flavor_obj, None)
@mock.patch('nova.virt.libvirt.driver.LibvirtDriver'
'.get_instance_disk_info')
def test_migrate_disk_and_power_off_resize_error_default_ephemeral(
@ -16114,14 +16164,22 @@ class LibvirtDriverTestCase(test.NoDBTestCase):
def test_is_booted_from_volume(self):
func = libvirt_driver.LibvirtDriver._is_booted_from_volume
instance, disk_mapping = {}, {}
bdm = []
bdi = {'block_device_mapping': bdm}
self.assertTrue(func(instance, disk_mapping))
disk_mapping['disk'] = 'map'
self.assertTrue(func(instance, disk_mapping))
self.assertFalse(func(bdi))
instance['image_ref'] = 'uuid'
self.assertFalse(func(instance, disk_mapping))
bdm.append({'boot_index': -1})
self.assertFalse(func(bdi))
bdm.append({'boot_index': None})
self.assertFalse(func(bdi))
bdm.append({'boot_index': 1})
self.assertFalse(func(bdi))
bdm.append({'boot_index': 0})
self.assertTrue(func(bdi))
@mock.patch('nova.virt.libvirt.driver.imagebackend')
@mock.patch(

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

@ -2929,14 +2929,15 @@ class LibvirtDriver(driver.ComputeDriver):
return 'rbd' if CONF.libvirt.images_type == 'rbd' else 'raw'
@staticmethod
def _is_booted_from_volume(instance, disk_mapping):
def _is_booted_from_volume(block_device_info):
"""Determines whether the VM is booting from volume
Determines whether the disk mapping indicates that the VM
Determines whether the block device info indicates that the VM
is booting from a volume.
"""
return ((not bool(instance.get('image_ref')))
or 'disk' not in disk_mapping)
block_device_mapping = driver.block_device_info_get_mapping(
block_device_info)
return bool(block_device.get_root_bdm(block_device_mapping))
def _inject_data(self, injection_image, instance, network_info,
admin_pass, files):
@ -3001,8 +3002,7 @@ class LibvirtDriver(driver.ComputeDriver):
admin_pass=None, inject_files=True,
fallback_from_host=None,
ignore_bdi_for_swap=False):
booted_from_volume = self._is_booted_from_volume(
instance, disk_mapping)
booted_from_volume = self._is_booted_from_volume(block_device_info)
def image(fname, image_type=CONF.libvirt.images_type):
return self.image_backend.by_name(instance,
@ -4755,13 +4755,10 @@ class LibvirtDriver(driver.ComputeDriver):
def _create_domain_setup_lxc(self, instance, image_meta,
block_device_info, disk_info):
inst_path = libvirt_utils.get_instance_path(instance)
disk_info = disk_info or {}
disk_mapping = disk_info.get('mapping', {})
if self._is_booted_from_volume(instance, disk_mapping):
block_device_mapping = driver.block_device_info_get_mapping(
block_device_info)
root_disk = block_device.get_root_bdm(block_device_mapping)
block_device_mapping = driver.block_device_info_get_mapping(
block_device_info)
root_disk = block_device.get_root_bdm(block_device_mapping)
if root_disk:
disk_info = blockinfo.get_info_from_bdm(
instance, CONF.libvirt.virt_type, image_meta, root_disk)
self._connect_volume(root_disk['connection_info'], disk_info)
@ -7094,17 +7091,13 @@ class LibvirtDriver(driver.ComputeDriver):
# Checks if the migration needs a disk resize down.
root_down = flavor.root_gb < instance.flavor.root_gb
ephemeral_down = flavor.ephemeral_gb < eph_size
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)
booted_from_volume = self._is_booted_from_volume(block_device_info)
if (root_down and not booted_from_volume) or ephemeral_down:
reason = _("Unable to resize disk down.")
raise exception.InstanceFaultRollback(
exception.ResizeError(reason=reason))
disk_info = jsonutils.loads(disk_info_text)
# NOTE(dgenin): Migration is not implemented for LVM backed instances.
if CONF.libvirt.images_type == 'lvm' and not booted_from_volume:
reason = _("Migration is not supported for LVM backed instances")
@ -7138,6 +7131,10 @@ class LibvirtDriver(driver.ComputeDriver):
disk_dev = vol['mount_device'].rpartition("/")[2]
self._disconnect_volume(connection_info, disk_dev)
disk_info_text = self.get_instance_disk_info(
instance, block_device_info=block_device_info)
disk_info = jsonutils.loads(disk_info_text)
try:
utils.execute('mv', inst_base, inst_base_resize)
# if we are migrating the instance with shared storage then