diff --git a/cinder/image/image_utils.py b/cinder/image/image_utils.py index e1abbb575c2..dcc256cf49a 100644 --- a/cinder/image/image_utils.py +++ b/cinder/image/image_utils.py @@ -71,6 +71,9 @@ QEMU_IMG_FORMAT_MAP = { } QEMU_IMG_FORMAT_MAP_INV = {v: k for k, v in QEMU_IMG_FORMAT_MAP.items()} +QEMU_IMG_VERSION = None +QEMU_IMG_MIN_FORCE_SHARE_VERSION = [2, 10, 0] + def validate_disk_format(disk_format): return disk_format in VALID_DISK_FORMATS @@ -88,9 +91,17 @@ def from_qemu_img_disk_format(disk_format): return QEMU_IMG_FORMAT_MAP_INV.get(disk_format, disk_format) -def qemu_img_info(path, run_as_root=True): +def qemu_img_info(path, run_as_root=True, force_share=False): """Return an object containing the parsed output from qemu-img info.""" - cmd = ['env', 'LC_ALL=C', 'qemu-img', 'info', path] + cmd = ['env', 'LC_ALL=C', 'qemu-img', 'info'] + if force_share: + if qemu_img_supports_force_share(): + cmd.append('--force-share') + else: + msg = _("qemu-img --force-share requested, but " + "qemu-img does not support this parameter") + LOG.warning(msg) + cmd.append(path) if os.name == 'nt': cmd = cmd[2:] @@ -107,13 +118,24 @@ def qemu_img_info(path, run_as_root=True): def get_qemu_img_version(): + """The qemu-img version will be cached until the process is restarted.""" + + global QEMU_IMG_VERSION + if QEMU_IMG_VERSION is not None: + return QEMU_IMG_VERSION + info = utils.execute('qemu-img', '--version', check_exit_code=False)[0] pattern = r"qemu-img version ([0-9\.]*)" version = re.match(pattern, info) if not version: LOG.warning("qemu-img is not installed.") return None - return _get_version_from_string(version.groups()[0]) + QEMU_IMG_VERSION = _get_version_from_string(version.groups()[0]) + return QEMU_IMG_VERSION + + +def qemu_img_supports_force_share(): + return get_qemu_img_version() > [2, 10, 0] def _get_qemu_convert_cmd(src, dest, out_format, src_format=None, @@ -288,7 +310,8 @@ 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): +def get_qemu_data(image_id, has_meta, disk_format_raw, dest, run_as_root, + force_share=False): # 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 @@ -297,7 +320,9 @@ def get_qemu_data(image_id, has_meta, disk_format_raw, dest, run_as_root): # 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) + data = qemu_img_info(dest, + run_as_root=run_as_root, + force_share=force_share) # 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 diff --git a/cinder/tests/unit/test_image_utils.py b/cinder/tests/unit/test_image_utils.py index 156b27fae0b..94f0a8f34e3 100644 --- a/cinder/tests/unit/test_image_utils.py +++ b/cinder/tests/unit/test_image_utils.py @@ -54,7 +54,9 @@ class TestQemuImgInfo(test.TestCase): test_path = mock.sentinel.path mock_exec.return_value = (mock_out, mock_err) - output = image_utils.qemu_img_info(test_path, run_as_root=False) + output = image_utils.qemu_img_info(test_path, + force_share=False, + run_as_root=False) mock_exec.assert_called_once_with('env', 'LC_ALL=C', 'qemu-img', 'info', test_path, run_as_root=False, prlimit=image_utils.QEMU_IMG_LIMITS) @@ -343,7 +345,9 @@ class TestVerifyImage(test.TestCase): self.assertIsNone(output) mock_fetch.assert_called_once_with(ctxt, image_service, image_id, dest, None, None) - mock_info.assert_called_once_with(dest, run_as_root=True) + mock_info.assert_called_once_with(dest, + run_as_root=True, + force_share=False) mock_fileutils.remove_path_on_error.assert_called_once_with(dest) (mock_fileutils.remove_path_on_error.return_value.__enter__ .assert_called_once_with()) @@ -731,7 +735,7 @@ class TestFetchToVolumeFormat(test.TestCase): self.assertIsNone(output) mock_temp.assert_called_once_with() mock_info.assert_has_calls([ - mock.call(tmp, run_as_root=True), + mock.call(tmp, force_share=False, run_as_root=True), mock.call(tmp, run_as_root=True)]) mock_fetch.assert_called_once_with(ctxt, image_service, image_id, tmp, None, None) @@ -782,7 +786,7 @@ class TestFetchToVolumeFormat(test.TestCase): self.assertIsNone(output) mock_temp.assert_called_once_with() mock_info.assert_has_calls([ - mock.call(tmp, run_as_root=run_as_root), + mock.call(tmp, force_share=False, run_as_root=run_as_root), mock.call(tmp, run_as_root=run_as_root)]) mock_fetch.assert_called_once_with(ctxt, image_service, image_id, tmp, user_id, project_id) @@ -834,7 +838,7 @@ class TestFetchToVolumeFormat(test.TestCase): self.assertIsNone(output) mock_temp.assert_called_once_with() mock_info.assert_has_calls([ - mock.call(tmp, run_as_root=run_as_root), + mock.call(tmp, force_share=False, run_as_root=run_as_root), mock.call(tmp, run_as_root=run_as_root)]) mock_fetch.assert_called_once_with(ctxt, image_service, image_id, tmp, user_id, project_id) @@ -884,7 +888,7 @@ class TestFetchToVolumeFormat(test.TestCase): self.assertIsNone(output) mock_temp.assert_called_once_with() mock_info.assert_has_calls([ - mock.call(tmp, run_as_root=run_as_root), + mock.call(tmp, force_share=False, run_as_root=run_as_root), mock.call(tmp, run_as_root=run_as_root)]) mock_fetch.assert_called_once_with(ctxt, image_service, image_id, tmp, user_id, project_id) @@ -937,8 +941,8 @@ class TestFetchToVolumeFormat(test.TestCase): self.assertIsNone(output) self.assertEqual(2, mock_temp.call_count) mock_info.assert_has_calls([ - mock.call(tmp, run_as_root=True), - mock.call(dummy, run_as_root=True), + mock.call(tmp, force_share=False, run_as_root=True), + mock.call(dummy, force_share=False, run_as_root=True), mock.call(tmp, run_as_root=True)]) mock_fetch.assert_called_once_with(ctxt, image_service, image_id, tmp, None, None) @@ -987,7 +991,9 @@ class TestFetchToVolumeFormat(test.TestCase): self.assertIsNone(output) image_service.show.assert_called_once_with(ctxt, image_id) mock_temp.assert_called_once_with() - mock_info.assert_called_once_with(tmp, run_as_root=run_as_root) + mock_info.assert_called_once_with(tmp, + force_share=False, + run_as_root=run_as_root) mock_fetch.assert_called_once_with(ctxt, image_service, image_id, tmp, user_id, project_id) self.assertFalse(mock_repl_xen.called) @@ -1032,7 +1038,9 @@ class TestFetchToVolumeFormat(test.TestCase): image_service.show.assert_called_once_with(ctxt, image_id) mock_temp.assert_called_once_with() - mock_info.assert_called_once_with(tmp, run_as_root=run_as_root) + mock_info.assert_called_once_with(tmp, + force_share=False, + run_as_root=run_as_root) self.assertFalse(mock_fetch.called) self.assertFalse(mock_repl_xen.called) self.assertFalse(mock_copy.called) @@ -1075,7 +1083,9 @@ class TestFetchToVolumeFormat(test.TestCase): image_service.show.assert_called_once_with(ctxt, image_id) mock_temp.assert_called_once_with() - mock_info.assert_called_once_with(tmp, run_as_root=run_as_root) + mock_info.assert_called_once_with(tmp, + force_share=False, + run_as_root=run_as_root) self.assertFalse(mock_fetch.called) self.assertFalse(mock_repl_xen.called) self.assertFalse(mock_copy.called) @@ -1120,7 +1130,7 @@ class TestFetchToVolumeFormat(test.TestCase): image_service.show.assert_called_once_with(ctxt, image_id) mock_temp.assert_called_once_with() mock_info.assert_has_calls([ - mock.call(tmp, run_as_root=run_as_root), + mock.call(tmp, force_share=False, run_as_root=run_as_root), mock.call(tmp, run_as_root=run_as_root)]) mock_fetch.assert_called_once_with(ctxt, image_service, image_id, tmp, user_id, project_id) @@ -1213,7 +1223,7 @@ class TestFetchToVolumeFormat(test.TestCase): image_service.show.assert_called_once_with(ctxt, image_id) mock_temp.assert_called_once_with() mock_info.assert_has_calls([ - mock.call(tmp, run_as_root=run_as_root), + mock.call(tmp, force_share=False, run_as_root=run_as_root), mock.call(tmp, run_as_root=run_as_root)]) mock_fetch.assert_called_once_with(ctxt, image_service, image_id, tmp, user_id, project_id) @@ -1261,7 +1271,7 @@ class TestFetchToVolumeFormat(test.TestCase): image_service.show.assert_called_once_with(ctxt, image_id) mock_temp.assert_called_once_with() mock_info.assert_has_calls([ - mock.call(tmp, run_as_root=run_as_root), + mock.call(tmp, force_share=False, run_as_root=run_as_root), mock.call(tmp, run_as_root=run_as_root)]) mock_fetch.assert_called_once_with(ctxt, image_service, image_id, tmp, user_id, project_id) @@ -1308,7 +1318,7 @@ class TestFetchToVolumeFormat(test.TestCase): self.assertIsNone(output) mock_temp.assert_called_once_with() mock_info.assert_has_calls([ - mock.call(tmp, run_as_root=run_as_root), + mock.call(tmp, force_share=False, run_as_root=run_as_root), mock.call(tmp, run_as_root=run_as_root)]) mock_fetch.assert_called_once_with(ctxt, image_service, image_id, tmp, user_id, project_id) @@ -1345,7 +1355,9 @@ class TestFetchToVolumeFormat(test.TestCase): 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_info.assert_called_once_with(dest, + force_share=False, + run_as_root=run_as_root) mock_fetch.assert_called_once_with(ctxt, image_service, image_id, dest, None, None) diff --git a/cinder/tests/unit/volume/drivers/test_nfs.py b/cinder/tests/unit/volume/drivers/test_nfs.py index 4a060cce92c..c4f0247791b 100644 --- a/cinder/tests/unit/volume/drivers/test_nfs.py +++ b/cinder/tests/unit/volume/drivers/test_nfs.py @@ -1195,7 +1195,9 @@ class NfsDriverTestCase(test.TestCase): drv._copy_volume_from_snapshot(fake_snap, dest_volume, size) mock_read_info_file.assert_called_once_with(info_path) - mock_img_info.assert_called_once_with(snap_path, run_as_root=True) + mock_img_info.assert_called_once_with(snap_path, + force_share=True, + run_as_root=True) used_qcow = nfs_conf['nfs_qcow2_volumes'] mock_convert_image.assert_called_once_with( src_vol_path, dest_vol_path, 'qcow2' if used_qcow else 'raw', @@ -1308,7 +1310,9 @@ class NfsDriverTestCase(test.TestCase): conn_info = drv.initialize_connection(volume, None) - mock_img_utils.assert_called_once_with(vol_path, run_as_root=True) + mock_img_utils.assert_called_once_with(vol_path, + force_share=True, + run_as_root=True) self.assertEqual('nfs', conn_info['driver_volume_type']) self.assertEqual(volume.name, conn_info['data']['name']) self.assertEqual(self.TEST_MNT_POINT_BASE, diff --git a/cinder/tests/unit/volume/drivers/test_quobyte.py b/cinder/tests/unit/volume/drivers/test_quobyte.py index d26aa8cbd50..ecec8eb687e 100644 --- a/cinder/tests/unit/volume/drivers/test_quobyte.py +++ b/cinder/tests/unit/volume/drivers/test_quobyte.py @@ -625,6 +625,7 @@ class QuobyteDriverTestCase(test.TestCase): drv.extend_volume(volume, 3) image_utils.qemu_img_info.assert_called_once_with(volume_path, + force_share=False, run_as_root=False) image_utils.resize_image.assert_called_once_with(volume_path, 3) @@ -679,6 +680,7 @@ class QuobyteDriverTestCase(test.TestCase): drv._read_info_file.assert_called_once_with(info_path) image_utils.qemu_img_info.assert_called_once_with(snap_path, + force_share=False, run_as_root=False) (image_utils.convert_image. assert_called_once_with(src_vol_path, @@ -760,6 +762,7 @@ class QuobyteDriverTestCase(test.TestCase): drv.get_active_image_from_info.assert_called_once_with(volume) image_utils.qemu_img_info.assert_called_once_with(vol_path, + force_share=False, run_as_root=False) self.assertEqual('raw', conn_info['data']['format']) @@ -806,6 +809,7 @@ class QuobyteDriverTestCase(test.TestCase): mock_get_active_image_from_info.assert_called_once_with(volume) mock_local_volume_dir.assert_called_once_with(volume) mock_qemu_img_info.assert_called_once_with(volume_path, + force_share=False, run_as_root=False) mock_upload_volume.assert_called_once_with( mock.ANY, mock.ANY, mock.ANY, upload_path, run_as_root=False) @@ -852,6 +856,7 @@ class QuobyteDriverTestCase(test.TestCase): mock_get_active_image_from_info.assert_called_once_with(volume) mock_local_volume_dir.assert_called_with(volume) mock_qemu_img_info.assert_called_once_with(volume_path, + force_share=False, run_as_root=False) mock_convert_image.assert_called_once_with( volume_path, upload_path, 'raw', run_as_root=False) @@ -902,6 +907,7 @@ class QuobyteDriverTestCase(test.TestCase): mock_get_active_image_from_info.assert_called_once_with(volume) mock_local_volume_dir.assert_called_with(volume) mock_qemu_img_info.assert_called_once_with(volume_path, + force_share=False, run_as_root=False) mock_convert_image.assert_called_once_with( volume_path, upload_path, 'raw', run_as_root=False) diff --git a/cinder/tests/unit/volume/drivers/test_remotefs.py b/cinder/tests/unit/volume/drivers/test_remotefs.py index cf1d67d2cfe..2f8045369fe 100644 --- a/cinder/tests/unit/volume/drivers/test_remotefs.py +++ b/cinder/tests/unit/volume/drivers/test_remotefs.py @@ -433,6 +433,7 @@ class RemoteFsSnapDriverTestCase(test.TestCase): fake_vol_name, basedir) mock_qemu_img_info.assert_called_with(mock.sentinel.image_path, + force_share=False, run_as_root=True) @ddt.data([None, '/fake_basedir'], diff --git a/cinder/volume/drivers/nfs.py b/cinder/volume/drivers/nfs.py index 298771c9847..cc7b0fbe883 100644 --- a/cinder/volume/drivers/nfs.py +++ b/cinder/volume/drivers/nfs.py @@ -126,7 +126,8 @@ class NfsDriver(remotefs.RemoteFSSnapDriverDistributed): active_vol = self.get_active_image_from_info(volume) volume_dir = self._local_volume_dir(volume) path_to_vol = os.path.join(volume_dir, active_vol) - info = self._qemu_img_info(path_to_vol, volume['name']) + info = self._qemu_img_info(path_to_vol, + volume['name']) data = {'export': volume.provider_location, 'name': active_vol} @@ -533,6 +534,7 @@ class NfsDriver(remotefs.RemoteFSSnapDriverDistributed): path, volume_name, self.configuration.nfs_mount_point_base, + force_share=True, run_as_root=True) def _check_snapshot_support(self, setup_checking=False): diff --git a/cinder/volume/drivers/remotefs.py b/cinder/volume/drivers/remotefs.py index 3cdc5c66810..5df218bce29 100644 --- a/cinder/volume/drivers/remotefs.py +++ b/cinder/volume/drivers/remotefs.py @@ -737,6 +737,7 @@ class RemoteFSSnapDriverBase(RemoteFSDriver): json.dump(snap_info, f, indent=1, sort_keys=True) def _qemu_img_info_base(self, path, volume_name, basedir, + force_share=False, run_as_root=False): """Sanitize image_utils' qemu_img_info. @@ -746,6 +747,7 @@ class RemoteFSSnapDriverBase(RemoteFSDriver): run_as_root = run_as_root or self._execute_as_root info = image_utils.qemu_img_info(path, + force_share=force_share, run_as_root=run_as_root) if info.image: info.image = os.path.basename(info.image)