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

@@ -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',
+        cache_mode = 'none'
-                        '-t', 'none')
+    else:
+        # use default
+        cache_mode = None
 
-        # AMI images can be raw or qcow2 but qemu-img doesn't accept "ami" as
+    cmd = _get_qemu_convert_cmd(source, dest,
-        # an image format, so we use automatic detection.
+                                out_format=out_format,
-        # TODO(geguileo): This fixes unencrypted AMI image case, but we need to
+                                src_format=src_format,
-        # fix the encrypted case.
+                                out_subformat=out_subformat,
-        if (src_format or '').lower() not in ('', 'ami'):
+                                cache_mode=cache_mode,
-            cmd += ('-f', src_format)  # prevent detection of format
+                                prefix=prefix)
-
-        cmd += ('-O', out_format, source, dest)
 
     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,
+def convert_image(source, dest, out_format, out_subformat=None,
-                  run_as_root=True, throttle=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,
+                           volume_subformat=None, user_id=None,
-                           run_as_root=True):
+                           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)
 

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')