diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 02403cad7439..3bcff330073d 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -15079,7 +15079,7 @@ class LibvirtVolumeSnapshotTestCase(test.NoDBTestCase): self.mox.VerifyAll() -def _fake_convert_image(source, dest, out_format, +def _fake_convert_image(source, dest, in_format, out_format, run_as_root=True): libvirt_driver.libvirt_utils.files[dest] = '' @@ -15221,7 +15221,8 @@ class LVMSnapshotTests(_BaseSnapshotTests): mock_volume_info.assert_has_calls([mock.call('/dev/nova-vg/lv')]) mock_convert_image.assert_called_once_with( - '/dev/nova-vg/lv', mock.ANY, disk_format, run_as_root=True) + '/dev/nova-vg/lv', mock.ANY, 'raw', disk_format, + run_as_root=True) def test_raw(self): self._test_lvm_snapshot('raw') diff --git a/nova/tests/unit/virt/libvirt/test_utils.py b/nova/tests/unit/virt/libvirt/test_utils.py index 6773beacdf2e..6f75a928f7bd 100644 --- a/nova/tests/unit/virt/libvirt/test_utils.py +++ b/nova/tests/unit/virt/libvirt/test_utils.py @@ -594,7 +594,8 @@ disk size: 4.4M target = 't.qcow2' self.executes = [] expected_commands = [('qemu-img', 'convert', '-O', 'raw', - 't.qcow2.part', 't.qcow2.converted'), + 't.qcow2.part', 't.qcow2.converted', + '-f', 'qcow2'), ('rm', 't.qcow2.part'), ('mv', 't.qcow2.converted', 't.qcow2')] images.fetch_to_raw(context, image_id, target, user_id, project_id, diff --git a/nova/virt/images.py b/nova/virt/images.py index 5b9374b682d1..e2b5b913d816 100644 --- a/nova/virt/images.py +++ b/nova/virt/images.py @@ -66,9 +66,31 @@ def qemu_img_info(path): return imageutils.QemuImgInfo(out) -def convert_image(source, dest, out_format, run_as_root=False): +def convert_image(source, dest, in_format, out_format, run_as_root=False): """Convert image to other format.""" + if in_format is None: + raise RuntimeError("convert_image without input format is a security" + "risk") + _convert_image(source, dest, in_format, out_format, run_as_root) + + +def convert_image_unsafe(source, dest, out_format, run_as_root=False): + """Convert image to other format, doing unsafe automatic input format + detection. Do not call this function. + """ + + # NOTE: there is only 1 caller of this function: + # imagebackend.Lvm.create_image. It is not easy to fix that without a + # larger refactor, so for the moment it has been manually audited and + # allowed to continue. Remove this function when Lvm.create_image has + # been fixed. + _convert_image(source, dest, None, out_format, run_as_root) + + +def _convert_image(source, dest, in_format, out_format, run_as_root): cmd = ('qemu-img', 'convert', '-O', out_format, source, dest) + if in_format is not None: + cmd = cmd + ('-f', in_format) utils.execute(*cmd, run_as_root=run_as_root) @@ -123,7 +145,7 @@ def fetch_to_raw(context, image_href, path, user_id, project_id, max_size=0): staged = "%s.converted" % path LOG.debug("%s was %s, converting to raw" % (image_href, fmt)) with fileutils.remove_path_on_error(staged): - convert_image(path_tmp, staged, 'raw') + convert_image(path_tmp, staged, fmt, 'raw') os.unlink(path_tmp) data = qemu_img_info(staged) diff --git a/nova/virt/libvirt/imagebackend.py b/nova/virt/libvirt/imagebackend.py index 5e14f61691f4..151ebc4457f6 100644 --- a/nova/virt/libvirt/imagebackend.py +++ b/nova/virt/libvirt/imagebackend.py @@ -477,7 +477,7 @@ class Raw(Image): self.correct_format() def snapshot_extract(self, target, out_format): - images.convert_image(self.path, target, out_format) + images.convert_image(self.path, target, self.driver_format, out_format) @staticmethod def is_file_in_instance_path(): @@ -631,7 +631,16 @@ class Lvm(Image): size, sparse=self.sparse) if self.ephemeral_key_uuid is not None: encrypt_lvm_image() - images.convert_image(base, self.path, 'raw', run_as_root=True) + # NOTE: by calling convert_image_unsafe here we're + # telling qemu-img convert to do format detection on the input, + # because we don't know what the format is. For example, + # we might have downloaded a qcow2 image, or created an + # ephemeral filesystem locally, we just don't know here. Having + # audited this, all current sources have been sanity checked, + # either because they're locally generated, or because they have + # come from images.fetch_to_raw. However, this is major code smell. + images.convert_image_unsafe(base, self.path, self.driver_format, + run_as_root=True) if resize: disk.resize2fs(self.path, run_as_root=True) @@ -678,8 +687,8 @@ class Lvm(Image): lvm.remove_volumes([self.lv_path]) def snapshot_extract(self, target, out_format): - images.convert_image(self.path, target, out_format, - run_as_root=True) + images.convert_image(self.path, target, self.driver_format, + out_format, run_as_root=True) def get_model(self, connection): return imgmodel.LocalBlockImage(self.path) @@ -786,7 +795,7 @@ class Rbd(Image): self.driver.resize(self.rbd_name, size) def snapshot_extract(self, target, out_format): - images.convert_image(self.path, target, out_format) + images.convert_image(self.path, target, 'raw', out_format) @staticmethod def is_shared_block_storage():