diff --git a/nova/tests/unit/virt/libvirt/test_driver.py b/nova/tests/unit/virt/libvirt/test_driver.py index 3494b8f12423..5ab612b3621e 100644 --- a/nova/tests/unit/virt/libvirt/test_driver.py +++ b/nova/tests/unit/virt/libvirt/test_driver.py @@ -13434,7 +13434,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] = '' @@ -13573,7 +13573,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 37424a096fad..f028fc5c5301 100644 --- a/nova/tests/unit/virt/libvirt/test_utils.py +++ b/nova/tests/unit/virt/libvirt/test_utils.py @@ -635,7 +635,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 57d27aad93fc..ed869387034d 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) @@ -122,7 +144,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 8655b48f52ef..f5e464ea98f8 100644 --- a/nova/virt/libvirt/imagebackend.py +++ b/nova/virt/libvirt/imagebackend.py @@ -443,7 +443,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(): @@ -586,7 +586,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) @@ -633,8 +642,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) class Rbd(Image): @@ -736,7 +745,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():