From 4cfc201b725f4e376a84d017f9ddc5e72f907fa3 Mon Sep 17 00:00:00 2001 From: AlexMuresan Date: Mon, 21 Aug 2017 15:19:06 +0300 Subject: [PATCH] imageutils: allow passing subformat when converting For some image formats (e.g. vhd, vhdx), qemu-img allows specifying image subformat options. This can be useful when requesting fixed size images. This change allows the imageutils convert function to accept an image subformat. This will be used by the SMBFS volume driver. Change-Id: I8706584e3abfb936072896d8c4902d82db32ab5f --- cinder/image/image_utils.py | 68 ++++++++++++++++++++------- cinder/tests/unit/test_image_utils.py | 34 ++++++++++---- 2 files changed, 76 insertions(+), 26 deletions(-) diff --git a/cinder/image/image_utils.py b/cinder/image/image_utils.py index 854759815ce..3fc9ba249a2 100644 --- a/cinder/image/image_utils.py +++ b/cinder/image/image_utils.py @@ -116,6 +116,38 @@ def get_qemu_img_version(): return _get_version_from_string(version.groups()[0]) +def _get_qemu_convert_cmd(src, dest, out_format, src_format=None, + out_subformat=None, cache_mode=None, + prefix=None): + + if out_format == 'vhd': + # qemu-img still uses the legacy vpc name + out_format == 'vpc' + + cmd = ['qemu-img', 'convert', '-O', out_format] + + if prefix: + cmd = list(prefix) + cmd + + if cache_mode: + cmd += ('-t', cache_mode) + + if out_subformat: + cmd += ('-o', 'subformat=%s' % out_subformat) + + # AMI images can be raw or qcow2 but qemu-img doesn't accept "ami" as + # an image format, so we use automatic detection. + # TODO(geguileo): This fixes unencrypted AMI image case, but we need to + # fix the encrypted case. + + if (src_format or '').lower() not in ('', 'ami'): + cmd += ('-f', src_format) # prevent detection of format + + cmd += [src, dest] + + return cmd + + def _get_version_from_string(version_string): return [int(x) for x in version_string.split('.')] @@ -138,12 +170,10 @@ def check_qemu_img_version(minimum_version): def _convert_image(prefix, source, dest, out_format, - src_format=None, run_as_root=True): + out_subformat=None, src_format=None, + run_as_root=True): """Convert image to other format.""" - cmd = prefix + ('qemu-img', 'convert', - '-O', out_format, source, dest) - # Check whether O_DIRECT is supported and set '-t none' if it is # This is needed to ensure that all data hit the device before # it gets unmapped remotely from the host for some backends @@ -157,17 +187,17 @@ def _convert_image(prefix, source, dest, out_format, volume_utils.check_for_odirect_support(source, dest, 'oflag=direct')): - cmd = prefix + ('qemu-img', 'convert', - '-t', 'none') + cache_mode = 'none' + else: + # use default + cache_mode = None - # AMI images can be raw or qcow2 but qemu-img doesn't accept "ami" as - # an image format, so we use automatic detection. - # TODO(geguileo): This fixes unencrypted AMI image case, but we need to - # fix the encrypted case. - if (src_format or '').lower() not in ('', 'ami'): - cmd += ('-f', src_format) # prevent detection of format - - cmd += ('-O', out_format, source, dest) + cmd = _get_qemu_convert_cmd(source, dest, + out_format=out_format, + src_format=src_format, + out_subformat=out_subformat, + cache_mode=cache_mode, + prefix=prefix) start_time = timeutils.utcnow() utils.execute(*cmd, run_as_root=run_as_root) @@ -201,14 +231,15 @@ def _convert_image(prefix, source, dest, out_format, LOG.info(msg, {"sz": fsz_mb, "mbps": mbps}) -def convert_image(source, dest, out_format, src_format=None, - run_as_root=True, throttle=None): +def convert_image(source, dest, out_format, out_subformat=None, + src_format=None, run_as_root=True, throttle=None): if not throttle: throttle = throttling.Throttle.get_default() with throttle.subcommand(source, dest) as throttle_cmd: _convert_image(tuple(throttle_cmd['prefix']), source, dest, out_format, + out_subformat=out_subformat, src_format=src_format, run_as_root=run_as_root) @@ -349,8 +380,8 @@ def fetch_to_raw(context, image_service, def fetch_to_volume_format(context, image_service, image_id, dest, volume_format, blocksize, - user_id=None, project_id=None, size=None, - run_as_root=True): + volume_subformat=None, user_id=None, + project_id=None, size=None, run_as_root=True): qemu_img = True image_meta = image_service.show(context, image_id) @@ -430,6 +461,7 @@ def fetch_to_volume_format(context, image_service, disk_format = fixup_disk_format(image_meta['disk_format']) convert_image(tmp, dest, volume_format, + out_subformat=volume_subformat, src_format=disk_format, run_as_root=run_as_root) diff --git a/cinder/tests/unit/test_image_utils.py b/cinder/tests/unit/test_image_utils.py index d4a9c50ce0a..156b27fae0b 100644 --- a/cinder/tests/unit/test_image_utils.py +++ b/cinder/tests/unit/test_image_utils.py @@ -1,4 +1,3 @@ - # Copyright (c) 2013 eNovance , Inc. # All Rights Reserved. # @@ -140,7 +139,7 @@ class TestConvertImage(test.TestCase): self.assertIsNone(output) mock_exec.assert_called_once_with('cgcmd', 'qemu-img', 'convert', - '-t', 'none', '-O', out_format, + '-O', out_format, '-t', 'none', source, dest, run_as_root=True) mock_exec.reset_mock() @@ -174,7 +173,7 @@ class TestConvertImage(test.TestCase): mock_info.assert_called_once_with(source, run_as_root=True) self.assertIsNone(output) mock_exec.assert_called_once_with('cgcmd', 'qemu-img', 'convert', - '-t', 'none', '-O', out_format, + '-O', out_format, '-t', 'none', source, dest, run_as_root=True) mock_exec.reset_mock() @@ -200,13 +199,17 @@ class TestConvertImage(test.TestCase): source = mock.sentinel.source dest = mock.sentinel.dest out_format = mock.sentinel.out_format + out_subformat = 'fake_subformat' mock_info.return_value.virtual_size = 1048576 - output = image_utils.convert_image(source, dest, out_format) + output = image_utils.convert_image(source, dest, out_format, + out_subformat=out_subformat) self.assertIsNone(output) mock_exec.assert_called_once_with('qemu-img', 'convert', '-O', - out_format, source, dest, + out_format, '-o', + 'subformat=%s' % out_subformat, + source, dest, run_as_root=True) @mock.patch('cinder.volume.utils.check_for_odirect_support', @@ -222,13 +225,17 @@ class TestConvertImage(test.TestCase): source = mock.sentinel.source dest = mock.sentinel.dest out_format = mock.sentinel.out_format + out_subformat = 'fake_subformat' mock_info.side_effect = ValueError - output = image_utils.convert_image(source, dest, out_format) + output = image_utils.convert_image(source, dest, out_format, + out_subformat=out_subformat) self.assertIsNone(output) mock_exec.assert_called_once_with('qemu-img', 'convert', '-O', - out_format, source, dest, + out_format, '-o', + 'subformat=%s' % out_subformat, + source, dest, run_as_root=True) @mock.patch('cinder.image.image_utils.qemu_img_info') @@ -248,7 +255,7 @@ class TestConvertImage(test.TestCase): self.assertIsNone(output) mock_exec.assert_called_once_with('qemu-img', 'convert', - '-t', 'none', '-O', out_format, + '-O', out_format, '-t', 'none', source, dest, run_as_root=True) @@ -708,6 +715,7 @@ class TestFetchToVolumeFormat(test.TestCase): image_id = mock.sentinel.image_id dest = mock.sentinel.dest volume_format = mock.sentinel.volume_format + out_subformat = None blocksize = mock.sentinel.blocksize data = mock_info.return_value @@ -730,6 +738,7 @@ class TestFetchToVolumeFormat(test.TestCase): self.assertFalse(mock_repl_xen.called) self.assertFalse(mock_copy.called) mock_convert.assert_called_once_with(tmp, dest, volume_format, + out_subformat=out_subformat, run_as_root=True, src_format='raw') @@ -752,6 +761,7 @@ class TestFetchToVolumeFormat(test.TestCase): image_id = mock.sentinel.image_id dest = mock.sentinel.dest volume_format = mock.sentinel.volume_format + out_subformat = None blocksize = mock.sentinel.blocksize ctxt.user_id = user_id = mock.sentinel.user_id project_id = mock.sentinel.project_id @@ -779,6 +789,7 @@ class TestFetchToVolumeFormat(test.TestCase): self.assertFalse(mock_repl_xen.called) self.assertFalse(mock_copy.called) mock_convert.assert_called_once_with(tmp, dest, volume_format, + out_subformat=out_subformat, run_as_root=run_as_root, src_format='raw') @@ -800,6 +811,7 @@ class TestFetchToVolumeFormat(test.TestCase): image_id = mock.sentinel.image_id dest = mock.sentinel.dest volume_format = mock.sentinel.volume_format + out_subformat = None blocksize = mock.sentinel.blocksize ctxt.user_id = user_id = mock.sentinel.user_id project_id = mock.sentinel.project_id @@ -829,6 +841,7 @@ class TestFetchToVolumeFormat(test.TestCase): mock_repl_xen.assert_called_once_with(tmp) self.assertFalse(mock_copy.called) mock_convert.assert_called_once_with(tmp, dest, volume_format, + out_subformat=out_subformat, run_as_root=run_as_root, src_format=expect_format) @@ -848,6 +861,7 @@ class TestFetchToVolumeFormat(test.TestCase): image_id = mock.sentinel.image_id dest = mock.sentinel.dest volume_format = mock.sentinel.volume_format + out_subformat = None blocksize = mock.sentinel.blocksize ctxt.user_id = user_id = mock.sentinel.user_id project_id = mock.sentinel.project_id @@ -876,6 +890,7 @@ class TestFetchToVolumeFormat(test.TestCase): tmp, user_id, project_id) self.assertFalse(mock_copy.called) mock_convert.assert_called_once_with(tmp, dest, volume_format, + out_subformat=out_subformat, run_as_root=run_as_root, src_format=expect_format) @@ -900,6 +915,7 @@ class TestFetchToVolumeFormat(test.TestCase): image_id = mock.sentinel.image_id dest = mock.sentinel.dest volume_format = mock.sentinel.volume_format + out_subformat = None blocksize = mock.sentinel.blocksize data = mock_info.return_value @@ -929,6 +945,7 @@ class TestFetchToVolumeFormat(test.TestCase): self.assertFalse(mock_repl_xen.called) self.assertFalse(mock_copy.called) mock_convert.assert_called_once_with(tmp, dest, volume_format, + out_subformat=out_subformat, run_as_root=True, src_format='raw') @@ -1298,6 +1315,7 @@ class TestFetchToVolumeFormat(test.TestCase): mock_repl_xen.assert_called_once_with(tmp) self.assertFalse(mock_copy.called) mock_convert.assert_called_once_with(tmp, dest, volume_format, + out_subformat=None, run_as_root=run_as_root, src_format='raw')