From 3e936ac8096cfc3f9936f438e9fa6f6b81e15768 Mon Sep 17 00:00:00 2001 From: Evgeny Antyshev Date: Thu, 28 Sep 2017 11:06:49 +0000 Subject: [PATCH] Add ploop to parallels naming conversion It worked silently until src_format was started to be passed to convert_image in I4071927c491870626fe174b75ecaf8ef6da39cf5 Now we should also have ploop-to-parallels mapping as in upload_volume. Also, place all conversions to a single structure. Change-Id: I8ad66a625f12cc48e1daec51e9f8abe6ae8932d6 --- cinder/image/image_utils.py | 19 ++++---- .../unit/api/contrib/test_volume_actions.py | 6 +-- .../unit/volume/drivers/test_vzstorage.py | 44 ++++++++++--------- cinder/volume/drivers/vzstorage.py | 35 +++------------ 4 files changed, 42 insertions(+), 62 deletions(-) diff --git a/cinder/image/image_utils.py b/cinder/image/image_utils.py index 25f241734eb..854759815ce 100644 --- a/cinder/image/image_utils.py +++ b/cinder/image/image_utils.py @@ -61,13 +61,15 @@ QEMU_IMG_LIMITS = processutils.ProcessLimits( address_space=1 * units.Gi) VALID_DISK_FORMATS = ('raw', 'vmdk', 'vdi', 'qcow2', - 'vhd', 'vhdx', 'parallels') + 'vhd', 'vhdx', 'ploop') QEMU_IMG_FORMAT_MAP = { # Convert formats of Glance images to how they are processed with qemu-img. 'iso': 'raw', 'vhd': 'vpc', + 'ploop': 'parallels', } +QEMU_IMG_FORMAT_MAP_INV = {v: k for k, v in QEMU_IMG_FORMAT_MAP.items()} def validate_disk_format(disk_format): @@ -80,6 +82,12 @@ def fixup_disk_format(disk_format): return QEMU_IMG_FORMAT_MAP.get(disk_format, disk_format) +def from_qemu_img_disk_format(disk_format): + """Return the conventional format derived from qemu-img format.""" + + return QEMU_IMG_FORMAT_MAP_INV.get(disk_format, disk_format) + + def qemu_img_info(path, run_as_root=True): """Return an object containing the parsed output from qemu-img info.""" cmd = ['env', 'LC_ALL=C', 'qemu-img', 'info', path] @@ -467,14 +475,7 @@ def upload_volume(context, image_service, image_meta, volume_path, reason=_("fmt=%(fmt)s backed by:%(backing_file)s") % {'fmt': fmt, 'backing_file': backing_file}) - out_format = image_meta['disk_format'] - # qemu-img accepts 'vpc' as argument for 'vhd 'format and 'parallels' - # as argument for 'ploop'. - if out_format == 'vhd': - out_format = 'vpc' - if out_format == 'ploop': - out_format = 'parallels' - + out_format = fixup_disk_format(image_meta['disk_format']) convert_image(volume_path, tmp, out_format, run_as_root=run_as_root) diff --git a/cinder/tests/unit/api/contrib/test_volume_actions.py b/cinder/tests/unit/api/contrib/test_volume_actions.py index 0ceedf6bcbd..98f54045d39 100644 --- a/cinder/tests/unit/api/contrib/test_volume_actions.py +++ b/cinder/tests/unit/api/contrib/test_volume_actions.py @@ -924,12 +924,12 @@ class VolumeImageActionsTest(test.TestCase): body) @mock.patch.object(volume_api.API, "copy_volume_to_image") - def test_copy_volume_to_image_disk_format_parallels(self, - mock_copy_to_image): + def test_copy_volume_to_image_disk_format_ploop(self, + mock_copy_to_image): volume = utils.create_volume(self.context, metadata={'test': 'test'}) img = {"container_format": 'bare', - "disk_format": 'parallels', + "disk_format": 'ploop', "image_name": 'image_name'} body = {"os-volume_upload_image": img} req = fakes.HTTPRequest.blank('/v3/%s/volumes/%s/action' % diff --git a/cinder/tests/unit/volume/drivers/test_vzstorage.py b/cinder/tests/unit/volume/drivers/test_vzstorage.py index 116d6ea12a7..464e8e0e561 100644 --- a/cinder/tests/unit/volume/drivers/test_vzstorage.py +++ b/cinder/tests/unit/volume/drivers/test_vzstorage.py @@ -14,6 +14,7 @@ import collections import copy +import ddt import errno import os @@ -34,6 +35,7 @@ from cinder.volume.drivers import vzstorage _orig_path_exists = os.path.exists +@ddt.ddt class VZStorageTestCase(test.TestCase): _FAKE_SHARE = "10.0.0.1,10.0.0.2:/cluster123:123123" @@ -45,20 +47,19 @@ class VZStorageTestCase(test.TestCase): _FAKE_SNAPSHOT_PATH = ( _FAKE_VOLUME_PATH + '-snapshot' + _FAKE_SNAPSHOT_ID) - _FAKE_VZ_CONFIG = mock.MagicMock() - _FAKE_VZ_CONFIG.vzstorage_shares_config = '/fake/config/path' - _FAKE_VZ_CONFIG.vzstorage_sparsed_volumes = False - _FAKE_VZ_CONFIG.vzstorage_used_ratio = 0.7 - _FAKE_VZ_CONFIG.vzstorage_mount_point_base = _FAKE_MNT_BASE - _FAKE_VZ_CONFIG.vzstorage_default_volume_format = 'raw' - _FAKE_VZ_CONFIG.nas_secure_file_operations = 'auto' - _FAKE_VZ_CONFIG.nas_secure_file_permissions = 'auto' - def setUp(self): super(VZStorageTestCase, self).setUp() - cfg = copy.copy(self._FAKE_VZ_CONFIG) - self._vz_driver = vzstorage.VZStorageDriver(configuration=cfg) + self._cfg = mock.MagicMock() + self._cfg.vzstorage_shares_config = '/fake/config/path' + self._cfg.vzstorage_sparsed_volumes = False + self._cfg.vzstorage_used_ratio = 0.7 + self._cfg.vzstorage_mount_point_base = self._FAKE_MNT_BASE + self._cfg.vzstorage_default_volume_format = 'raw' + self._cfg.nas_secure_file_operations = 'auto' + self._cfg.nas_secure_file_permissions = 'auto' + + self._vz_driver = vzstorage.VZStorageDriver(configuration=self._cfg) self._vz_driver._local_volume_dir = mock.Mock( return_value=self._FAKE_MNT_POINT) self._vz_driver._execute = mock.Mock() @@ -85,7 +86,7 @@ class VZStorageTestCase(test.TestCase): self.snap.volume = self.vol def _path_exists(self, path): - if path.startswith(self._FAKE_VZ_CONFIG.vzstorage_shares_config): + if path.startswith(self._cfg.vzstorage_shares_config): return True return _orig_path_exists(path) @@ -125,9 +126,8 @@ class VZStorageTestCase(test.TestCase): @mock.patch('os.path.exists') def test_setup_invalid_mount_point_base(self, mock_exists): mock_exists.side_effect = self._path_exists - conf = copy.copy(self._FAKE_VZ_CONFIG) - conf.vzstorage_mount_point_base = './tmp' - vz_driver = vzstorage.VZStorageDriver(configuration=conf) + self._cfg.vzstorage_mount_point_base = './tmp' + vz_driver = vzstorage.VZStorageDriver(configuration=self._cfg) self.assertRaises(exception.VzStorageException, vz_driver.do_setup, mock.sentinel.context) @@ -142,13 +142,15 @@ class VZStorageTestCase(test.TestCase): self._vz_driver.do_setup, mock.sentinel.context) - def test_initialize_connection(self): + @ddt.data({'qemu_fmt': 'parallels', 'glance_fmt': 'ploop'}, + {'qemu_fmt': 'qcow2', 'glance_fmt': 'qcow2'}) + @ddt.unpack + def test_initialize_connection(self, qemu_fmt, glance_fmt): drv = self._vz_driver - file_format = 'raw' info = mock.Mock() - info.file_format = file_format - snap_info = """{"volume_format": "raw", - "active": "%s"}""" % self.vol.id + info.file_format = qemu_fmt + snap_info = """{"volume_format": "%s", + "active": "%s"}""" % (qemu_fmt, self.vol.id) with mock.patch.object(drv, '_qemu_img_info', return_value=info): with mock.patch.object(drv, '_read_file', return_value=snap_info): @@ -156,7 +158,7 @@ class VZStorageTestCase(test.TestCase): name = drv.get_active_image_from_info(self.vol) expected = {'driver_volume_type': 'vzstorage', 'data': {'export': self._FAKE_SHARE, - 'format': file_format, + 'format': glance_fmt, 'name': name}, 'mount_point_base': self._FAKE_MNT_BASE} self.assertEqual(expected, ret) diff --git a/cinder/volume/drivers/vzstorage.py b/cinder/volume/drivers/vzstorage.py index f8d8c7e4a53..75724f35ae5 100644 --- a/cinder/volume/drivers/vzstorage.py +++ b/cinder/volume/drivers/vzstorage.py @@ -72,30 +72,6 @@ DISK_FORMAT_RAW = 'raw' DISK_FORMAT_QCOW2 = 'qcow2' DISK_FORMAT_PLOOP = 'ploop' -# Due to the inconsistency in qemu-img format convention -# it calls ploop disk format "parallels". -# Convert it here to properly name it in Cinder -# and, hence, in Nova and Libvirt -FROM_QEMU_FORMAT_MAP = {k: k for k in image_utils.VALID_DISK_FORMATS} -FROM_QEMU_FORMAT_MAP['parallels'] = DISK_FORMAT_PLOOP -TO_QEMU_FORMAT_MAP = {v: k for k, v in FROM_QEMU_FORMAT_MAP.items()} - - -def _to_qemu_format(fmt): - """Convert from Qemu format name - - param fmt: Qemu format name - """ - return TO_QEMU_FORMAT_MAP[fmt] - - -def _from_qemu_format(fmt): - """Convert to Qemu format name - - param fmt: conventional format name - """ - return FROM_QEMU_FORMAT_MAP[fmt] - class PloopDevice(object): """Setup a ploop device for ploop image @@ -214,7 +190,6 @@ class VZStorageDriver(remotefs_drv.RemoteFSSnapDriver): ret = super(VZStorageDriver, self)._qemu_img_info_base( path, volume_name, self.configuration.vzstorage_mount_point_base) - ret.file_format = _from_qemu_format(ret.file_format) # We need only backing_file and file_format d = {'file_format': ret.file_format, 'backing_file': ret.backing_file} @@ -382,7 +357,7 @@ class VZStorageDriver(remotefs_drv.RemoteFSSnapDriver): active_file_path = os.path.join(self._local_volume_dir(volume), active_file) img_info = self._qemu_img_info(active_file_path, volume.name) - return img_info.file_format + return image_utils.from_qemu_img_disk_format(img_info.file_format) def _create_ploop(self, volume_path, volume_size): os.mkdir(volume_path) @@ -487,13 +462,14 @@ class VZStorageDriver(remotefs_drv.RemoteFSSnapDriver): def copy_image_to_volume(self, context, volume, image_service, image_id): """Fetch the image from image_service and write it to the volume.""" volume_format = self.get_volume_format(volume) + qemu_volume_format = image_utils.fixup_disk_format(volume_format) image_path = self.local_path(volume) if volume_format == DISK_FORMAT_PLOOP: image_path = os.path.join(image_path, PLOOP_BASE_DELTA_NAME) image_utils.fetch_to_volume_format( context, image_service, image_id, - image_path, _to_qemu_format(volume_format), + image_path, qemu_volume_format, self.configuration.volume_dd_blocksize) if volume_format == DISK_FORMAT_PLOOP: @@ -516,6 +492,7 @@ class VZStorageDriver(remotefs_drv.RemoteFSSnapDriver): snap_info = self._read_info_file(info_path) vol_dir = self._local_volume_dir(snapshot.volume) out_format = self.choose_volume_format(volume) + qemu_out_format = image_utils.fixup_disk_format(out_format) volume_format = self.get_volume_format(snapshot.volume) volume_path = self.local_path(volume) @@ -534,7 +511,7 @@ class VZStorageDriver(remotefs_drv.RemoteFSSnapDriver): image_utils.convert_image(path_to_snap_img, volume_path, - _to_qemu_format(out_format)) + qemu_out_format) elif volume_format == DISK_FORMAT_PLOOP: with PloopDevice(self.local_path(snapshot.volume), snapshot.id, @@ -542,7 +519,7 @@ class VZStorageDriver(remotefs_drv.RemoteFSSnapDriver): base_file = os.path.join(volume_path, 'root.hds') image_utils.convert_image(dev, base_file, - _to_qemu_format(out_format)) + qemu_out_format) else: msg = _("Unsupported volume format %s") % volume_format raise exception.InvalidVolume(msg)