Merge "imagebackend: Add disk_info_mapping as an optional attribute of Image"

This commit is contained in:
Zuul 2022-08-26 14:26:40 +00:00 committed by Gerrit Code Review
commit 86dd1d3be3
5 changed files with 149 additions and 73 deletions

View File

@ -154,7 +154,9 @@ class LibvirtImageBackendFixture(fixtures.Fixture):
# their construction. Tests can use this to assert that disks were
# created of the expected type.
def image_init(instance=None, disk_name=None, path=None):
def image_init(
instance=None, disk_name=None, path=None, disk_info_mapping=None
):
# There's nothing special about this path except that it's
# predictable and unique for (instance, disk).
if path is None:
@ -169,6 +171,7 @@ class LibvirtImageBackendFixture(fixtures.Fixture):
# the real constructor.
setattr(disk, 'path', path)
setattr(disk, 'is_block_dev', mock.sentinel.is_block_dev)
setattr(disk, 'disk_info_mapping', disk_info_mapping)
# Used by tests. Note that image_init is a closure over image_type.
setattr(disk, 'image_type', image_type)
@ -217,16 +220,16 @@ class LibvirtImageBackendFixture(fixtures.Fixture):
self.imported_files.append((local_filename, remote_filename))
def _fake_libvirt_info(
self, mock_disk, disk_info, cache_mode, extra_specs, disk_unit=None,
self, mock_disk, cache_mode, extra_specs, disk_unit=None,
boot_order=None,
):
# For tests in test_virt_drivers which expect libvirt_info to be
# functional
info = config.LibvirtConfigGuestDisk()
info.source_type = 'file'
info.source_device = disk_info['type']
info.target_bus = disk_info['bus']
info.target_dev = disk_info['dev']
info.source_device = mock_disk.disk_info_mapping['type']
info.target_bus = mock_disk.disk_info_mapping['bus']
info.target_dev = mock_disk.disk_info_mapping['dev']
info.driver_cache = cache_mode
info.driver_format = 'raw'
info.source_path = mock_disk.path

View File

@ -15659,10 +15659,9 @@ class LibvirtConnTestCase(test.NoDBTestCase,
instance = objects.Instance(**instance_ref)
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
image_meta = objects.ImageMeta.from_dict({'disk_format': 'raw'})
disk_info = blockinfo.get_disk_info(CONF.libvirt.virt_type,
instance,
image_meta)
disk_info['mapping'].pop('disk.local')
disk_info = blockinfo.get_disk_info(
CONF.libvirt.virt_type, instance, image_meta,
block_device_info=block_device_info)
with test.nested(
mock.patch('oslo_concurrency.processutils.execute'),
@ -20542,8 +20541,10 @@ class LibvirtConnTestCase(test.NoDBTestCase,
drvr._get_disk_config_image_type())
self.assertEqual(2, drvr.image_backend.by_name.call_count)
call1 = mock.call(instance, 'disk.config', 'rbd')
call2 = mock.call(instance, 'disk.config', 'flat')
call1 = mock.call(instance, 'disk.config', 'rbd',
disk_info_mapping=disk_mapping['disk.config'])
call2 = mock.call(instance, 'disk.config', 'flat',
disk_info_mapping=disk_mapping['disk.config'])
drvr.image_backend.by_name.assert_has_calls([call1, call2])
self.assertEqual(mock.sentinel.diskconfig, diskconfig)
@ -23160,6 +23161,9 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin):
}
instance = self._create_instance(params=inst_params)
image_meta = objects.ImageMeta.from_dict({})
disk_info = blockinfo.get_disk_info(
CONF.libvirt.virt_type, instance, image_meta)
disk_images = {'image_id': instance.image_ref}
instance_dir = libvirt_utils.get_instance_path(instance)
disk_path = os.path.join(instance_dir, 'disk')
@ -23179,7 +23183,8 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin):
]
drvr._create_and_inject_local_root(
self.context, instance, False, '', disk_images, None, None)
self.context, instance, disk_info['mapping'], False, '',
disk_images, None, None)
mock_fetch_calls = [
mock.call(test.MatchType(nova.virt.libvirt.imagebackend.Qcow2),
@ -23262,9 +23267,13 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin):
# config_drive is True by default, configdrive.required_by()
# returns True
instance_ref = self._create_instance()
image_meta = objects.ImageMeta.from_dict({})
disk_info = blockinfo.get_disk_info(
CONF.libvirt.virt_type, instance_ref, image_meta)
disk_images = {'image_id': None}
drvr._create_and_inject_local_root(self.context, instance_ref, False,
drvr._create_and_inject_local_root(self.context, instance_ref,
disk_info['mapping'], False,
'', disk_images, get_injection_info(),
None)
self.assertFalse(mock_inject.called)
@ -23284,6 +23293,9 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin):
mock_image.get.return_value = {'locations': [], 'disk_format': 'raw'}
instance = self._create_instance()
image_meta = objects.ImageMeta.from_dict({})
disk_info = blockinfo.get_disk_info(
CONF.libvirt.virt_type, instance, image_meta)
disk_images = {'image_id': 'foo'}
self.flags(images_type='rbd', group='libvirt')
drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False)
@ -23294,6 +23306,7 @@ class LibvirtDriverTestCase(test.NoDBTestCase, TraitsComparisonMixin):
mock_fetch.reset_mock()
drvr._create_and_inject_local_root(self.context,
instance,
disk_info['mapping'],
False,
'',
disk_images,

View File

@ -163,7 +163,13 @@ class _ImageTestCase(object):
self.assertEqual(fs.source_file, image.path)
def test_libvirt_info(self):
image = self.image_class(self.INSTANCE, self.NAME)
disk_info = {
'bus': 'virtio',
'dev': '/dev/vda',
'type': 'cdrom',
}
image = self.image_class(
self.INSTANCE, self.NAME, disk_info_mapping=disk_info)
extra_specs = {
'quota:disk_read_bytes_sec': 10 * units.Mi,
'quota:disk_read_iops_sec': 1 * units.Ki,
@ -172,15 +178,9 @@ class _ImageTestCase(object):
'quota:disk_total_bytes_sec': 30 * units.Mi,
'quota:disk_total_iops_sec': 3 * units.Ki,
}
disk_info = {
'bus': 'virtio',
'dev': '/dev/vda',
'type': 'cdrom',
}
disk = image.libvirt_info(
disk_info, cache_mode="none", extra_specs=extra_specs,
boot_order="1")
cache_mode="none", extra_specs=extra_specs, boot_order="1")
self.assertIsInstance(disk, vconfig.LibvirtConfigGuestDisk)
self.assertEqual("/dev/vda", disk.target_dev)
@ -205,16 +205,18 @@ class _ImageTestCase(object):
get_disk_size.assert_called_once_with(image.path)
def _test_libvirt_info_scsi_with_unit(self, disk_unit):
# The address should be set if bus is scsi and unit is set.
# Otherwise, it should not be set at all.
image = self.image_class(self.INSTANCE, self.NAME)
disk_info = {
'bus': 'scsi',
'dev': '/dev/sda',
'type': 'disk',
}
# The address should be set if bus is scsi and unit is set.
# Otherwise, it should not be set at all.
image = self.image_class(
self.INSTANCE, self.NAME, disk_info_mapping=disk_info)
disk = image.libvirt_info(
disk_info, cache_mode='none', extra_specs={}, disk_unit=disk_unit)
cache_mode='none', extra_specs={}, disk_unit=disk_unit)
if disk_unit:
self.assertEqual(0, disk.device_addr.controller)
self.assertEqual(disk_unit, disk.device_addr.unit)

View File

@ -4638,12 +4638,16 @@ class LibvirtDriver(driver.ComputeDriver):
ignore_bdi_for_swap=False):
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,
fname + suffix, image_type)
def image(
fname, image_type=CONF.libvirt.images_type, disk_info_mapping=None
):
return self.image_backend.by_name(
instance, fname + suffix, image_type,
disk_info_mapping=disk_info_mapping)
def raw(fname):
return image(fname, image_type='raw')
def raw(fname, disk_info_mapping=None):
return image(
fname, image_type='raw', disk_info_mapping=disk_info_mapping)
created_instance_dir = True
@ -4662,8 +4666,6 @@ class LibvirtDriver(driver.ComputeDriver):
flavor = instance.get_flavor()
swap_mb = 0
if 'disk.swap' in disk_mapping:
mapping = disk_mapping['disk.swap']
if ignore_bdi_for_swap:
# This is a workaround to support legacy swap resizing,
# which does not touch swap size specified in bdm,
@ -4677,12 +4679,17 @@ class LibvirtDriver(driver.ComputeDriver):
# leaving the work with bdm only.
swap_mb = flavor['swap']
else:
disk_info_mapping = disk_mapping['disk.swap']
disk_device = disk_info_mapping['dev']
swap = driver.block_device_info_get_swap(block_device_info)
if driver.swap_is_usable(swap):
swap_mb = swap['swap_size']
elif (flavor['swap'] > 0 and
not block_device.volume_in_mapping(
mapping['dev'], block_device_info)):
elif (
flavor['swap'] > 0 and
not block_device.volume_in_mapping(
disk_device, block_device_info,
)
):
swap_mb = flavor['swap']
if swap_mb > 0:
@ -4715,8 +4722,8 @@ class LibvirtDriver(driver.ComputeDriver):
image_id=disk_images['ramdisk_id'])
created_disks = self._create_and_inject_local_root(
context, instance, booted_from_volume, suffix, disk_images,
injection_info, fallback_from_host)
context, instance, disk_mapping, booted_from_volume, suffix,
disk_images, injection_info, fallback_from_host)
# Lookup the filesystem type if required
os_type_with_default = nova.privsep.fs.get_fs_type_for_os_type(
@ -4729,7 +4736,9 @@ class LibvirtDriver(driver.ComputeDriver):
vm_mode = fields.VMMode.get_from_instance(instance)
ephemeral_gb = instance.flavor.ephemeral_gb
if 'disk.local' in disk_mapping:
disk_image = image('disk.local')
disk_info_mapping = disk_mapping['disk.local']
disk_image = image(
'disk.local', disk_info_mapping=disk_info_mapping)
# Short circuit the exists() tests if we already created a disk
created_disks = created_disks or not disk_image.exists()
@ -4748,7 +4757,9 @@ class LibvirtDriver(driver.ComputeDriver):
for idx, eph in enumerate(driver.block_device_info_get_ephemerals(
block_device_info)):
disk_image = image(blockinfo.get_eph_disk(idx))
disk_name = blockinfo.get_eph_disk(idx)
disk_info_mapping = disk_mapping[disk_name]
disk_image = image(disk_name, disk_info_mapping=disk_info_mapping)
# Short circuit the exists() tests if we already created a disk
created_disks = created_disks or not disk_image.exists()
@ -4787,7 +4798,7 @@ class LibvirtDriver(driver.ComputeDriver):
return (created_instance_dir, created_disks)
def _create_and_inject_local_root(self, context, instance,
def _create_and_inject_local_root(self, context, instance, disk_mapping,
booted_from_volume, suffix, disk_images,
injection_info, fallback_from_host):
created_disks = False
@ -4804,7 +4815,10 @@ class LibvirtDriver(driver.ComputeDriver):
if size == 0 or suffix == '.rescue':
size = None
backend = self.image_backend.by_name(instance, 'disk' + suffix)
disk_name = 'disk' + suffix
disk_info_mapping = disk_mapping[disk_name]
backend = self.image_backend.by_name(
instance, disk_name, disk_info_mapping=disk_info_mapping)
created_disks = not backend.exists()
if instance.task_state == task_states.RESIZE_FINISH:
@ -5407,7 +5421,9 @@ class LibvirtDriver(driver.ComputeDriver):
if image_type is None:
image_type = CONF.libvirt.images_type
disk_unit = None
disk = self.image_backend.by_name(instance, name, image_type)
disk_info_mapping = disk_mapping[name]
disk = self.image_backend.by_name(
instance, name, image_type, disk_info_mapping=disk_info_mapping)
if (name == 'disk.config' and image_type == 'rbd' and
not disk.exists()):
# This is likely an older config drive that has not been migrated
@ -5416,18 +5432,21 @@ class LibvirtDriver(driver.ComputeDriver):
# remove this fall back once we know all config drives are in rbd.
# NOTE(vladikr): make sure that the flat image exist, otherwise
# the image will be created after the domain definition.
flat_disk = self.image_backend.by_name(instance, name, 'flat')
flat_disk = self.image_backend.by_name(
instance, name, 'flat', disk_info_mapping=disk_info_mapping)
if flat_disk.exists():
disk = flat_disk
LOG.debug('Config drive not found in RBD, falling back to the '
'instance directory', instance=instance)
disk_info = disk_mapping[name]
if 'unit' in disk_mapping and disk_info['bus'] == 'scsi':
# The 'unit' key is global to the disk_mapping (rather than for an
# individual disk) because it is used solely to track the incrementing
# unit number.
if 'unit' in disk_mapping and disk_info_mapping['bus'] == 'scsi':
disk_unit = disk_mapping['unit']
disk_mapping['unit'] += 1 # Increments for the next disk added
disk_mapping['unit'] += 1 # Increments for the next disk
conf = disk.libvirt_info(
disk_info, self.disk_cachemode, flavor['extra_specs'],
disk_unit=disk_unit, boot_order=boot_order)
self.disk_cachemode, flavor['extra_specs'], disk_unit=disk_unit,
boot_order=boot_order)
return conf
def _get_guest_fs_config(

View File

@ -82,13 +82,22 @@ class Image(metaclass=abc.ABCMeta):
SUPPORTS_CLONE = False
def __init__(self, path, source_type, driver_format, is_block_dev=False):
def __init__(
self,
path,
source_type,
driver_format,
is_block_dev=False,
disk_info_mapping=None
):
"""Image initialization.
:param path: libvirt's representation of the path of this disk.
:param source_type: block or file
:param driver_format: raw or qcow2
:param is_block_dev:
:param disk_info_mapping: disk_info['mapping'][device] metadata
specific to this image generated by nova.virt.libvirt.blockinfo.
"""
if (CONF.ephemeral_storage_encryption.enabled and
not self._supports_encryption()):
@ -105,6 +114,8 @@ class Image(metaclass=abc.ABCMeta):
self.is_block_dev = is_block_dev
self.preallocate = False
self.disk_info_mapping = disk_info_mapping
# NOTE(dripton): We store lines of json (path, disk_format) in this
# file, for some image types, to prevent attacks based on changing the
# disk_format.
@ -145,22 +156,23 @@ class Image(metaclass=abc.ABCMeta):
pass
def libvirt_info(
self, disk_info, cache_mode, extra_specs, boot_order=None,
disk_unit=None,
self, cache_mode, extra_specs, boot_order=None, disk_unit=None,
):
"""Get `LibvirtConfigGuestDisk` filled for this image.
:disk_info: Metadata generated by libvirt.blockinfo.get_disk_mapping
:cache_mode: Caching mode for this image
:extra_specs: Instance type extra specs dict.
:boot_order: Disk device boot order
"""
disk_bus = disk_info['bus']
if self.disk_info_mapping is None:
raise AttributeError(
'Image must have disk_info_mapping to call libvirt_info()')
disk_bus = self.disk_info_mapping['bus']
info = vconfig.LibvirtConfigGuestDisk()
info.source_type = self.source_type
info.source_device = disk_info['type']
info.source_device = self.disk_info_mapping['type']
info.target_bus = disk_bus
info.target_dev = disk_info['dev']
info.target_dev = self.disk_info_mapping['dev']
info.driver_cache = cache_mode
info.driver_discard = self.discard_mode
info.driver_io = self.driver_io
@ -522,11 +534,16 @@ class Flat(Image):
when creating a disk from a qcow2 if force_raw_images is not set in config.
"""
def __init__(self, instance=None, disk_name=None, path=None):
def __init__(
self, instance=None, disk_name=None, path=None, disk_info_mapping=None
):
self.disk_name = disk_name
path = (path or os.path.join(libvirt_utils.get_instance_path(instance),
disk_name))
super(Flat, self).__init__(path, "file", "raw", is_block_dev=False)
super().__init__(
path, "file", "raw", is_block_dev=False,
disk_info_mapping=disk_info_mapping
)
self.preallocate = (
strutils.to_slug(CONF.preallocate_images) == 'space')
@ -614,10 +631,15 @@ class Flat(Image):
class Qcow2(Image):
def __init__(self, instance=None, disk_name=None, path=None):
def __init__(
self, instance=None, disk_name=None, path=None, disk_info_mapping=None
):
path = (path or os.path.join(libvirt_utils.get_instance_path(instance),
disk_name))
super(Qcow2, self).__init__(path, "file", "qcow2", is_block_dev=False)
super().__init__(
path, "file", "qcow2", is_block_dev=False,
disk_info_mapping=disk_info_mapping
)
self.preallocate = (
strutils.to_slug(CONF.preallocate_images) == 'space')
@ -695,7 +717,10 @@ class Lvm(Image):
def escape(filename):
return filename.replace('_', '__')
def __init__(self, instance=None, disk_name=None, path=None):
def __init__(
self, instance=None, disk_name=None, path=None,
disk_info_mapping=None
):
self.ephemeral_key_uuid = instance.get('ephemeral_key_uuid')
if self.ephemeral_key_uuid is not None:
@ -724,7 +749,10 @@ class Lvm(Image):
self.lv_path = os.path.join('/dev', self.vg, self.lv)
path = '/dev/mapper/' + dmcrypt.volume_name(self.lv)
super(Lvm, self).__init__(path, "block", "raw", is_block_dev=True)
super(Lvm, self).__init__(
path, "block", "raw", is_block_dev=True,
disk_info_mapping=disk_info_mapping
)
# TODO(sbauza): Remove the config option usage and default the
# LVM logical volume creation to preallocate the full size only.
@ -832,7 +860,9 @@ class Rbd(Image):
SUPPORTS_CLONE = True
def __init__(self, instance=None, disk_name=None, path=None):
def __init__(
self, instance=None, disk_name=None, path=None, disk_info_mapping=None
):
if not CONF.libvirt.images_rbd_pool:
raise RuntimeError(_('You should specify'
' images_rbd_pool'
@ -854,31 +884,32 @@ class Rbd(Image):
if self.driver.ceph_conf:
path += ':conf=' + self.driver.ceph_conf
super(Rbd, self).__init__(path, "block", "rbd", is_block_dev=False)
super().__init__(
path, "block", "rbd", is_block_dev=False,
disk_info_mapping=disk_info_mapping
)
self.discard_mode = CONF.libvirt.hw_disk_discard
def libvirt_info(
self, disk_info, cache_mode, extra_specs, boot_order=None,
disk_unit=None,
self, cache_mode, extra_specs, boot_order=None, disk_unit=None
):
"""Get `LibvirtConfigGuestDisk` filled for this image.
:disk_info: Metadata generated by libvirt.blockinfo.get_disk_mapping
:cache_mode: Caching mode for this image
:extra_specs: Instance type extra specs dict.
:boot_order: Disk device boot order
"""
info = vconfig.LibvirtConfigGuestDisk()
disk_bus = disk_info['bus']
disk_bus = self.disk_info_mapping['bus']
hosts, ports = self.driver.get_mon_addrs()
info.source_device = disk_info['type']
info.source_device = self.disk_info_mapping['type']
info.driver_format = 'raw'
info.driver_cache = cache_mode
info.driver_discard = self.discard_mode
info.target_bus = disk_bus
info.target_dev = disk_info['dev']
info.target_dev = self.disk_info_mapping['dev']
info.source_type = 'network'
info.source_protocol = 'rbd'
info.source_name = '%s/%s' % (self.driver.pool, self.rbd_name)
@ -1195,10 +1226,15 @@ class Rbd(Image):
class Ploop(Image):
def __init__(self, instance=None, disk_name=None, path=None):
def __init__(
self, instance=None, disk_name=None, path=None, disk_info_mapping=None
):
path = (path or os.path.join(libvirt_utils.get_instance_path(instance),
disk_name))
super(Ploop, self).__init__(path, "file", "ploop", is_block_dev=False)
super().__init__(
path, "file", "ploop", is_block_dev=False,
disk_info_mapping=disk_info_mapping
)
self.resolve_driver_format()
@ -1301,13 +1337,14 @@ class Backend(object):
raise RuntimeError(_('Unknown image_type=%s') % image_type)
return image
def by_name(self, instance, name, image_type=None):
def by_name(self, instance, name, image_type=None, disk_info_mapping=None):
"""Return an Image object for a disk with the given name.
:param instance: the instance which owns this disk
:param name: The name of the disk
:param image_type: (Optional) Image type.
Default is CONF.libvirt.images_type.
:param disk_info_mapping: (Optional) Disk info mapping dict
:return: An Image object for the disk with given name and instance.
:rtype: Image
"""
@ -1316,7 +1353,9 @@ class Backend(object):
# default inline in the method, and not in the kwarg declaration.
image_type = image_type or CONF.libvirt.images_type
backend = self.backend(image_type)
return backend(instance=instance, disk_name=name)
return backend(
instance=instance, disk_name=name,
disk_info_mapping=disk_info_mapping)
def by_libvirt_path(self, instance, path, image_type=None):
"""Return an Image object for a disk with the given libvirt path.