From 70a0cc921f5fbbc7a6b59b4e3c87d3f24eb15c63 Mon Sep 17 00:00:00 2001 From: ShunliZhou Date: Thu, 23 Mar 2017 12:34:11 +0800 Subject: [PATCH] Separate out routine for getting qemu_img_info Without try...catch like fetch_to_volume_format, fetch_verify_image will prevent a raw image from being used when qemu-img is not installed. So we add a try...catch for qemu_img_info for fetch_verify_image and separate this into its own routine. This fixes the failure if you create a volume with a raw, nonqemu image and don't have the qemu packages on your system. Change-Id: I3aaf43a453e7096161780d9bfc2515c66a3a9f2c Closes-Bug: #1674771 --- cinder/image/image_utils.py | 113 ++++++++++++++++---------- cinder/tests/unit/test_image_utils.py | 82 ++++++++++++++++++- 2 files changed, 151 insertions(+), 44 deletions(-) diff --git a/cinder/image/image_utils.py b/cinder/image/image_utils.py index b97356bb6c4..806ed23289b 100644 --- a/cinder/image/image_utils.py +++ b/cinder/image/image_utils.py @@ -221,35 +221,78 @@ def fetch(context, image_service, image_id, path, _user_id, _project_id): LOG.info(msg, {"sz": fsz_mb, "mbps": mbps}) +def get_qemu_data(image_id, has_meta, disk_format_raw, dest, run_as_root): + # We may be on a system that doesn't have qemu-img installed. That + # is ok if we are working with a RAW image. This logic checks to see + # if qemu-img is installed. If not we make sure the image is RAW and + # throw an exception if not. Otherwise we stop before needing + # qemu-img. Systems with qemu-img will always progress through the + # whole function. + try: + # Use the empty tmp file to make sure qemu_img_info works. + data = qemu_img_info(dest, run_as_root=run_as_root) + # There are a lot of cases that can cause a process execution + # error, but until we do more work to separate out the various + # cases we'll keep the general catch here + except processutils.ProcessExecutionError: + data = None + if has_meta: + if not disk_format_raw: + raise exception.ImageUnacceptable( + reason=_("qemu-img is not installed and image is of " + "type %s. Only RAW images can be used if " + "qemu-img is not installed.") % + disk_format_raw, + image_id=image_id) + else: + raise exception.ImageUnacceptable( + reason=_("qemu-img is not installed and the disk " + "format is not specified. Only RAW images " + "can be used if qemu-img is not installed."), + image_id=image_id) + return data + + def fetch_verify_image(context, image_service, image_id, dest, user_id=None, project_id=None, size=None, run_as_root=True): fetch(context, image_service, image_id, dest, None, None) + image_meta = image_service.show(context, image_id) with fileutils.remove_path_on_error(dest): - data = qemu_img_info(dest, run_as_root=run_as_root) - fmt = data.file_format - if fmt is None: - raise exception.ImageUnacceptable( - reason=_("'qemu-img info' parsing failed."), - image_id=image_id) + has_meta = False if not image_meta else True + try: + format_raw = True if image_meta['disk_format'] == 'raw' else False + except TypeError: + format_raw = False + data = get_qemu_data(image_id, has_meta, format_raw, + dest, run_as_root) + # We can only really do verification of the image if we have + # qemu data to use + if data is not None: + fmt = data.file_format + if fmt is None: + raise exception.ImageUnacceptable( + reason=_("'qemu-img info' parsing failed."), + image_id=image_id) - backing_file = data.backing_file - if backing_file is not None: - raise exception.ImageUnacceptable( - image_id=image_id, - reason=(_("fmt=%(fmt)s backed by: %(backing_file)s") % - {'fmt': fmt, 'backing_file': backing_file})) + backing_file = data.backing_file + if backing_file is not None: + raise exception.ImageUnacceptable( + image_id=image_id, + reason=(_("fmt=%(fmt)s backed by: %(backing_file)s") % + {'fmt': fmt, 'backing_file': backing_file})) - # NOTE(xqueralt): If the image virtual size doesn't fit in the - # requested volume there is no point on resizing it because it will - # generate an unusable image. - if size is not None and data.virtual_size > size: - params = {'image_size': data.virtual_size, 'volume_size': size} - reason = _("Size is %(image_size)dGB and doesn't fit in a " - "volume of size %(volume_size)dGB.") % params - raise exception.ImageUnacceptable(image_id=image_id, reason=reason) + # NOTE(xqueralt): If the image virtual size doesn't fit in the + # requested volume there is no point on resizing it because it will + # generate an unusable image. + if size is not None and data.virtual_size > size: + params = {'image_size': data.virtual_size, 'volume_size': size} + reason = _("Size is %(image_size)dGB and doesn't fit in a " + "volume of size %(volume_size)dGB.") % params + raise exception.ImageUnacceptable(image_id=image_id, + reason=reason) def fetch_to_vhd(context, image_service, @@ -280,31 +323,15 @@ def fetch_to_volume_format(context, image_service, # Unfortunately it seems that you can't pipe to 'qemu-img convert' because # it seeks. Maybe we can think of something for a future version. with temporary_file() as tmp: - # We may be on a system that doesn't have qemu-img installed. That - # is ok if we are working with a RAW image. This logic checks to see - # if qemu-img is installed. If not we make sure the image is RAW and - # throw an exception if not. Otherwise we stop before needing - # qemu-img. Systems with qemu-img will always progress through the - # whole function. + has_meta = False if not image_meta else True try: - # Use the empty tmp file to make sure qemu_img_info works. - qemu_img_info(tmp, run_as_root=run_as_root) - except processutils.ProcessExecutionError: + format_raw = True if image_meta['disk_format'] == 'raw' else False + except TypeError: + format_raw = False + data = get_qemu_data(image_id, has_meta, format_raw, + tmp, run_as_root) + if data is None: qemu_img = False - if image_meta: - if image_meta['disk_format'] != 'raw': - raise exception.ImageUnacceptable( - reason=_("qemu-img is not installed and image is of " - "type %s. Only RAW images can be used if " - "qemu-img is not installed.") % - image_meta['disk_format'], - image_id=image_id) - else: - raise exception.ImageUnacceptable( - reason=_("qemu-img is not installed and the disk " - "format is not specified. Only RAW images " - "can be used if qemu-img is not installed."), - image_id=image_id) tmp_images = TemporaryImages.for_image_service(image_service) tmp_image = tmp_images.get(context, image_id) diff --git a/cinder/tests/unit/test_image_utils.py b/cinder/tests/unit/test_image_utils.py index aad892a5631..8a31b190363 100644 --- a/cinder/tests/unit/test_image_utils.py +++ b/cinder/tests/unit/test_image_utils.py @@ -792,7 +792,7 @@ class TestFetchToVolumeFormat(test.TestCase): blocksize) self.assertIsNone(output) - image_service.show.assert_called_once_with(ctxt, image_id) + self.assertEqual(2, image_service.show.call_count) self.assertEqual(2, mock_temp.call_count) mock_info.assert_has_calls([ mock.call(tmp, run_as_root=True), @@ -1243,6 +1243,86 @@ class TestFetchToVolumeFormat(test.TestCase): mock_convert.assert_called_once_with(tmp, dest, volume_format, run_as_root=run_as_root) + @mock.patch('cinder.image.image_utils.fetch') + @mock.patch('cinder.image.image_utils.qemu_img_info', + side_effect=processutils.ProcessExecutionError) + @mock.patch('cinder.image.image_utils.temporary_file') + @mock.patch('cinder.image.image_utils.CONF') + def test_no_qemu_img_fetch_verify_image(self, mock_conf, + mock_temp, mock_info, + mock_fetch): + ctxt = mock.sentinel.context + image_service = mock.Mock(temp_images=None) + image_id = mock.sentinel.image_id + dest = mock.sentinel.dest + ctxt.user_id = user_id = mock.sentinel.user_id + project_id = mock.sentinel.project_id + size = 4321 + run_as_root = mock.sentinel.run_as_root + + image_service.show.return_value = {'disk_format': 'raw', + 'size': 41126400} + + image_utils.fetch_verify_image( + ctxt, image_service, image_id, dest, + user_id=user_id, project_id=project_id, size=size, + run_as_root=run_as_root) + + image_service.show.assert_called_once_with(ctxt, image_id) + mock_info.assert_called_once_with(dest, run_as_root=run_as_root) + mock_fetch.assert_called_once_with(ctxt, image_service, image_id, + dest, None, None) + + @mock.patch('cinder.image.image_utils.qemu_img_info', + side_effect=processutils.ProcessExecutionError) + @mock.patch('cinder.image.image_utils.temporary_file') + @mock.patch('cinder.image.image_utils.CONF') + def test_get_qemu_data_returns_none(self, mock_conf, mock_temp, mock_info): + image_id = mock.sentinel.image_id + dest = mock.sentinel.dest + run_as_root = mock.sentinel.run_as_root + disk_format_raw = True + has_meta = True + + output = image_utils.get_qemu_data(image_id, has_meta, + disk_format_raw, dest, + run_as_root=run_as_root) + + self.assertIsNone(output) + + @mock.patch('cinder.image.image_utils.qemu_img_info', + side_effect=processutils.ProcessExecutionError) + @mock.patch('cinder.image.image_utils.temporary_file') + @mock.patch('cinder.image.image_utils.CONF') + def test_get_qemu_data_with_image_meta_exception(self, mock_conf, + mock_temp, mock_info): + image_id = mock.sentinel.image_id + dest = mock.sentinel.dest + run_as_root = mock.sentinel.run_as_root + disk_format_raw = False + has_meta = True + self.assertRaises( + exception.ImageUnacceptable, + image_utils.get_qemu_data, image_id, has_meta, disk_format_raw, + dest, run_as_root=run_as_root) + + @mock.patch('cinder.image.image_utils.qemu_img_info', + side_effect=processutils.ProcessExecutionError) + @mock.patch('cinder.image.image_utils.temporary_file') + @mock.patch('cinder.image.image_utils.CONF') + def test_get_qemu_data_without_image_meta_except(self, mock_conf, + mock_temp, mock_info): + image_id = mock.sentinel.image_id + dest = mock.sentinel.dest + run_as_root = mock.sentinel.run_as_root + + disk_format_raw = False + has_meta = False + self.assertRaises( + exception.ImageUnacceptable, + image_utils.get_qemu_data, image_id, has_meta, disk_format_raw, + dest, run_as_root=run_as_root) + class TestXenserverUtils(test.TestCase): def test_is_xenserver_format(self):