diff --git a/nova/tests/fixtures/libvirt_imagebackend.py b/nova/tests/fixtures/libvirt_imagebackend.py index a414fe62aa58..a17d83c12c51 100644 --- a/nova/tests/fixtures/libvirt_imagebackend.py +++ b/nova/tests/fixtures/libvirt_imagebackend.py @@ -215,7 +215,9 @@ class LibvirtImageBackendFixture(fixtures.Fixture): return image_init - def _fake_cache(self, fetch_func, filename, size=None, *args, **kwargs): + def _fake_cache( + self, fetch_func, filename, size=None, safe=False, *args, + **kwargs): # Execute the template function so we can test the arguments it was # called with. fetch_func(target=filename, *args, **kwargs) diff --git a/nova/tests/functional/regressions/test_bug_2079850.py b/nova/tests/functional/regressions/test_bug_2079850.py index 4e9f1fcf65f7..515a51fbe3e0 100644 --- a/nova/tests/functional/regressions/test_bug_2079850.py +++ b/nova/tests/functional/regressions/test_bug_2079850.py @@ -3,6 +3,8 @@ import functools import os import shutil +from unittest import mock + import fixtures from oslo_utils.fixture import uuidsentinel as uuids @@ -11,7 +13,6 @@ from oslo_utils import units import nova.conf -from nova import exception from nova import objects from nova import test from nova.virt.libvirt import driver @@ -53,17 +54,16 @@ class TestBugBackingFilePartitionTables(test.NoDBTestCase): file_path = os.path.join(self.base_dir.path, 'test_file') libvirt_utils.create_image(file_path, 'raw', '64M') self.assertTrue(os.path.exists(file_path)) - # nova should ensure that any file we create has a partition table - # inspector = format_inspector.GPTInspector.from_file(file_path) - # self.assertIsNotNone(inspector) - # inspector.safety_check() - - # however the libvirt_utils.create_image method does not create a - # partition table so we should expect this to fail + # FIXME(sean-k-mooney): oslo currently detect vfat as an mbr partition self.assertRaises( format_inspector.ImageFormatError, format_inspector.GPTInspector.from_file, file_path) + # nova files should pass the RawFileInspector safety check + inspector = format_inspector.RawFileInspector.from_file(file_path) + self.assertIsNotNone(inspector) + inspector.safety_check() + def test_cache_file(self): """Test the qcow2 cache interaction for ephemeral disks @@ -85,8 +85,11 @@ class TestBugBackingFilePartitionTables(test.NoDBTestCase): os_type=None, is_block_dev=False) # this need to be multiples of 1G size = 1 * units.Gi - fname = "ephemeral_%s_%s" % (size, ".qcow") - e = self.assertRaises(exception.InvalidDiskInfo, - image.cache, fetch_func=fn, context=None, filename=fname, - size=size, ephemeral_size=1) - self.assertIn("Base image failed safety check", str(e)) + fname = "ephemeral_%s_%s" % (size, ".img") + with mock.patch.object( + imagebackend, '_update_utime_ignore_eacces') as m: + + image.cache( + fetch_func=fn, context=None, filename=fname, + size=size, ephemeral_size=1, safe=True) + m.assert_called_once() diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index edea0177ac17..f83416e24011 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -14903,6 +14903,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, 'qcow2', virt_disk_size, backing_file=backfile_path, + safe=False ) @mock.patch('nova.virt.libvirt.imagebackend.Image.exists', @@ -15054,12 +15055,14 @@ class LibvirtConnTestCase(test.NoDBTestCase, 'qcow2', disk_info_byname['disk']['virt_disk_size'], backing_file=root_backing, + safe=False ), mock.call( CONF.instances_path + '/disk.local', 'qcow2', disk_info_byname['disk.local']['virt_disk_size'], backing_file=ephemeral_backing, + safe=True ), ]) @@ -16626,7 +16629,8 @@ class LibvirtConnTestCase(test.NoDBTestCase, context=self.context) backend.disks['disk.swap'].cache.assert_called_once_with( fetch_func=mock.ANY, filename='swap_%i' % expected, - size=expected * units.Mi, context=self.context, swap_mb=expected) + size=expected * units.Mi, context=self.context, swap_mb=expected, + safe=True) @mock.patch.object(nova.virt.libvirt.imagebackend.Image, 'cache') def test_create_vz_container_with_swap(self, mock_cache): @@ -16711,7 +16715,7 @@ class LibvirtConnTestCase(test.NoDBTestCase, backend.disks['disk.eph0'].cache.assert_called_once_with( fetch_func=mock.ANY, context=self.context, filename=filename, size=100 * units.Gi, ephemeral_size=mock.ANY, - specified_fs=None) + specified_fs=None, safe=True) def test_create_image_resize_snap_backend(self): drvr = libvirt_driver.LibvirtDriver(fake.FakeVirtAPI(), False) diff --git a/nova/tests/unit/virt/libvirt/test_imagebackend.py b/nova/tests/unit/virt/libvirt/test_imagebackend.py index 329b4701adaf..9d35f66fa47d 100644 --- a/nova/tests/unit/virt/libvirt/test_imagebackend.py +++ b/nova/tests/unit/virt/libvirt/test_imagebackend.py @@ -514,7 +514,7 @@ class Qcow2TestCase(_ImageTestCase, test.NoDBTestCase): image.cache(fn, self.TEMPLATE) - fn.assert_called_once_with(target=self.TEMPLATE_PATH) + fn.assert_called_once_with(target=self.TEMPLATE_PATH, safe=False) mock_exists.assert_has_calls(exist_calls) @mock.patch.object(os.path, 'exists') @@ -545,7 +545,7 @@ class Qcow2TestCase(_ImageTestCase, test.NoDBTestCase): image.cache(fn, self.TEMPLATE) - fn.assert_called_once_with(target=self.TEMPLATE_PATH) + fn.assert_called_once_with(target=self.TEMPLATE_PATH, safe=False) mock_exists.assert_has_calls(exist_calls) @mock.patch.object(os.path, 'exists') @@ -587,7 +587,8 @@ class Qcow2TestCase(_ImageTestCase, test.NoDBTestCase): mock_verify.assert_called_once_with(self.TEMPLATE_PATH, self.SIZE) mock_create.assert_called_once_with( - self.PATH, 'qcow2', self.SIZE, backing_file=self.TEMPLATE_PATH) + self.PATH, 'qcow2', self.SIZE, backing_file=self.TEMPLATE_PATH, + safe=False) fn.assert_called_once_with(target=self.TEMPLATE_PATH) mock_exist.assert_has_calls(exist_calls) self.assertTrue(mock_sync.called) @@ -828,7 +829,7 @@ class LvmTestCase(_ImageTestCase, test.NoDBTestCase): image.cache(fn, self.TEMPLATE) - fn.assert_called_once_with(target=self.TEMPLATE_PATH) + fn.assert_called_once_with(target=self.TEMPLATE_PATH, safe=False) mock_ensure.assert_called_once_with(self.TEMPLATE_DIR) mock_exists.assert_has_calls(exist_calls) @@ -1369,7 +1370,7 @@ class RbdTestCase(_ImageTestCase, test.NoDBTestCase): image.cache(fn, self.TEMPLATE) mock_ensure.assert_called_once_with(self.TEMPLATE_DIR) - fn.assert_called_once_with(target=self.TEMPLATE_PATH) + fn.assert_called_once_with(target=self.TEMPLATE_PATH, safe=False) mock_img_exist.assert_called_with() mock_os_exist.assert_has_calls([ mock.call(self.TEMPLATE_DIR), mock.call(self.TEMPLATE_PATH) @@ -1392,7 +1393,7 @@ class RbdTestCase(_ImageTestCase, test.NoDBTestCase): mock_os_exist.assert_has_calls([ mock.call(self.TEMPLATE_DIR), mock.call(self.TEMPLATE_PATH) ]) - fn.assert_called_once_with(target=self.TEMPLATE_PATH) + fn.assert_called_once_with(target=self.TEMPLATE_PATH, safe=False) @mock.patch.object(os.path, 'exists', return_value=True) @mock.patch.object(imagebackend.Rbd, 'exists', return_value=True) @@ -2078,7 +2079,7 @@ class PloopTestCase(_ImageTestCase, test.NoDBTestCase): mock_ensure.assert_called_once_with(self.TEMPLATE_DIR) mock_exists.assert_has_calls(exist_calls) - fn.assert_called_once_with(target=self.TEMPLATE_PATH) + fn.assert_called_once_with(target=self.TEMPLATE_PATH, safe=False) @mock.patch.object(imagebackend.Ploop, 'get_disk_size', return_value=2048) diff --git a/nova/virt/libvirt/driver.py b/nova/virt/libvirt/driver.py index 16834846e2d4..3dcb9222b27f 100644 --- a/nova/virt/libvirt/driver.py +++ b/nova/virt/libvirt/driver.py @@ -4926,7 +4926,8 @@ class LibvirtDriver(driver.ComputeDriver): '%dG' % ephemeral_size, specified_fs) return - libvirt_utils.create_image(target, 'raw', f'{ephemeral_size}G') + libvirt_utils.create_image( + target, 'raw', f'{ephemeral_size}G', safe=True) # Run as root only for block devices. disk_api.mkfs(os_type, fs_label, target, run_as_root=is_block_dev, @@ -5169,11 +5170,9 @@ class LibvirtDriver(driver.ComputeDriver): vm_mode=vm_mode) fname = "ephemeral_%s_%s" % (ephemeral_gb, file_extension) size = ephemeral_gb * units.Gi - disk_image.cache(fetch_func=fn, - context=context, - filename=fname, - size=size, - ephemeral_size=ephemeral_gb) + disk_image.cache( + fetch_func=fn, context=context, filename=fname, size=size, + ephemeral_size=ephemeral_gb, safe=True) for idx, eph in enumerate(driver.block_device_info_get_ephemerals( block_device_info)): @@ -5195,12 +5194,10 @@ class LibvirtDriver(driver.ComputeDriver): vm_mode=vm_mode) size = eph['size'] * units.Gi fname = "ephemeral_%s_%s" % (eph['size'], file_extension) - disk_image.cache(fetch_func=fn, - context=context, - filename=fname, - size=size, - ephemeral_size=eph['size'], - specified_fs=specified_fs) + disk_image.cache( + fetch_func=fn, context=context, filename=fname, size=size, + ephemeral_size=eph['size'], specified_fs=specified_fs, + safe=True) if swap_mb > 0: size = swap_mb * units.Mi @@ -5208,9 +5205,10 @@ class LibvirtDriver(driver.ComputeDriver): swap = image('disk.swap', disk_info_mapping=disk_info_mapping) # Short circuit the exists() tests if we already created a disk created_disks = created_disks or not swap.exists() - swap.cache(fetch_func=self._create_swap, context=context, - filename="swap_%s" % swap_mb, - size=size, swap_mb=swap_mb) + swap.cache( + fetch_func=self._create_swap, context=context, + filename="swap_%s" % swap_mb, size=size, swap_mb=swap_mb, + safe=True) if created_disks: LOG.debug('Created local disks', instance=instance) @@ -11650,14 +11648,16 @@ class LibvirtDriver(driver.ComputeDriver): os_type=instance.os_type, filename=cache_name, size=info['virt_disk_size'], - ephemeral_size=info['virt_disk_size'] / units.Gi) + ephemeral_size=info['virt_disk_size'] / units.Gi, + safe=True) elif cache_name.startswith('swap'): flavor = instance.get_flavor() swap_mb = flavor.swap disk.cache(fetch_func=self._create_swap, filename="swap_%s" % swap_mb, size=swap_mb * units.Mi, - swap_mb=swap_mb) + swap_mb=swap_mb, + safe=True) else: self._try_fetch_image_cache(disk, libvirt_utils.fetch_image, diff --git a/nova/virt/libvirt/imagebackend.py b/nova/virt/libvirt/imagebackend.py index 65077e63a198..a86322cf97d4 100644 --- a/nova/virt/libvirt/imagebackend.py +++ b/nova/virt/libvirt/imagebackend.py @@ -135,7 +135,8 @@ class Image(metaclass=abc.ABCMeta): return False @abc.abstractmethod - def create_image(self, prepare_template, base, size, *args, **kwargs): + def create_image( + self, prepare_template, base, size, safe=False, *args, **kwargs): """Create image from template. Contains specific behavior for each image type. @@ -144,6 +145,7 @@ class Image(metaclass=abc.ABCMeta): Should accept `target` argument. :base: Template name :size: Size of created image in bytes + :safe: True if image contains a safe filesystem """ pass @@ -263,7 +265,8 @@ class Image(metaclass=abc.ABCMeta): def exists(self): return os.path.exists(self.path) - def cache(self, fetch_func, filename, size=None, *args, **kwargs): + def cache(self, fetch_func, filename, size=None, safe=False, *args, + **kwargs): """Creates image from template. Ensures that template and image not already exists. @@ -298,8 +301,9 @@ class Image(metaclass=abc.ABCMeta): fetch_func(target=target, *args, **kwargs) if not self.exists() or not os.path.exists(base): - self.create_image(fetch_func_sync, base, size, - *args, **kwargs) + self.create_image( + fetch_func_sync, base, size, safe=safe, *args, + **kwargs) if size: # create_image() only creates the base image if needed, so @@ -593,7 +597,8 @@ class Flat(Image): if os.path.exists(self.path): self.driver_format = self.resolve_driver_format() - def create_image(self, prepare_template, base, size, *args, **kwargs): + def create_image( + self, prepare_template, base, size, safe=False, *args, **kwargs): filename = self._get_lock_name(base) @utils.synchronized(filename, external=True, lock_path=self.lock_path) @@ -663,13 +668,14 @@ class Qcow2(Image): self.disk_info_path = os.path.join(os.path.dirname(path), 'disk.info') self.resolve_driver_format() - def create_image(self, prepare_template, base, size, *args, **kwargs): + def create_image( + self, prepare_template, base, size, safe=False, *args, **kwargs): filename = self._get_lock_name(base) @utils.synchronized(filename, external=True, lock_path=self.lock_path) - def create_qcow2_image(base, target, size): + def create_qcow2_image(base, target, size, safe=False): libvirt_utils.create_image( - target, 'qcow2', size, backing_file=base) + target, 'qcow2', size, backing_file=base, safe=safe) # Download the unmodified base image unless we already have a copy. if not os.path.exists(base): @@ -679,7 +685,9 @@ class Qcow2(Image): # before we inspect it for other attributes. We do this each time # because additional safety checks could have been added since we # downloaded the image. - if not CONF.workarounds.disable_deep_image_inspection: + # NOTE(sean-k-mooney) If the image was created by nova as a swap + # or ephemeral disk it is safe to skip the deep inspection. + if not CONF.workarounds.disable_deep_image_inspection and not safe: inspector = format_inspector.detect_file_format(base) try: inspector.safety_check() @@ -724,7 +732,7 @@ class Qcow2(Image): if not os.path.exists(self.path): with fileutils.remove_path_on_error(self.path): - create_qcow2_image(base, self.path, size) + create_qcow2_image(base, self.path, size, safe=safe) def resize_image(self, size): image = imgmodel.LocalFileImage(self.path, imgmodel.FORMAT_QCOW2) @@ -800,7 +808,8 @@ class Lvm(Image): def _can_fallocate(self): return False - def create_image(self, prepare_template, base, size, *args, **kwargs): + def create_image( + self, prepare_template, base, size, safe=False, *args, **kwargs): def encrypt_lvm_image(): dmcrypt.create_volume(self.path.rpartition('/')[2], self.lv_path, @@ -997,7 +1006,8 @@ class Rbd(Image): LOG.warning("Ignoring failure to remove %(path)s: " "%(error)s", {'path': base, 'error': e}) - def create_image(self, prepare_template, base, size, *args, **kwargs): + def create_image( + self, prepare_template, base, size, safe=False, *args, **kwargs): if not self.exists(): self._remove_non_raw_cache_image(base) @@ -1272,7 +1282,8 @@ class Ploop(Image): # Create new ploop disk (in case of epehemeral) or # copy ploop disk from glance image - def create_image(self, prepare_template, base, size, *args, **kwargs): + def create_image( + self, prepare_template, base, size, safe=False, *args, **kwargs): filename = os.path.basename(base) # Copy main file of ploop disk, restore DiskDescriptor.xml for it diff --git a/nova/virt/libvirt/utils.py b/nova/virt/libvirt/utils.py index bc3ea9e76f5a..202d7187a4a0 100644 --- a/nova/virt/libvirt/utils.py +++ b/nova/virt/libvirt/utils.py @@ -133,7 +133,8 @@ def create_image( disk_format: str, disk_size: ty.Optional[ty.Union[str, int]], backing_file: ty.Optional[str] = None, - encryption: ty.Optional[EncryptionOptions] = None + encryption: ty.Optional[EncryptionOptions] = None, + safe: bool = False, ) -> None: """Disk image creation with qemu-img :param path: Desired location of the disk image @@ -147,6 +148,7 @@ def create_image( :param backing_file: (Optional) Backing file to use. :param encryption: (Optional) Dict detailing various encryption attributes such as the format and passphrase. + :param safe: If True, the image is know to be safe. """ cmd = [ 'env', 'LC_ALL=C', 'LANG=C', 'qemu-img', 'create', '-f', disk_format @@ -157,7 +159,10 @@ def create_image( # before we inspect it for other attributes. We do this each time # because additional safety checks could have been added since we # downloaded the image. - if not CONF.workarounds.disable_deep_image_inspection: + # Note(sean-k-mooney): We only need to perform the safety check for + # the backing file if the image is not created by nova for swap or + # ephemeral disks. + if not CONF.workarounds.disable_deep_image_inspection and not safe: inspector = format_inspector.detect_file_format(backing_file) try: inspector.safety_check()