openstack/cinder
Code Issues Proposed changes

Merge "qemu-img info --force-share for NFS driver"

Browse Source
This commit is contained in:
Zuul 2017-12-19 02:06:30 +00:00 committed by Gerrit Code Review
commit 3b86eb7a82
7 changed files with 76 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
cinder/image/image_utils.py
View File

@ -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_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): def validate_disk_format(disk_format):
return disk_format in VALID_DISK_FORMATS 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) 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.""" """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': if os.name == 'nt':
cmd = cmd[2:] cmd = cmd[2:]
@ -107,13 +118,24 @@ def qemu_img_info(path, run_as_root=True):
def get_qemu_img_version(): 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] info = utils.execute('qemu-img', '--version', check_exit_code=False)[0]
pattern = r"qemu-img version ([0-9\.]*)" pattern = r"qemu-img version ([0-9\.]*)"
version = re.match(pattern, info) version = re.match(pattern, info)
if not version: if not version:
LOG.warning("qemu-img is not installed.") LOG.warning("qemu-img is not installed.")
return None 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, 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}) 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 # 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 # 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 # 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. # whole function.
try: try:
# Use the empty tmp file to make sure qemu_img_info works. # 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 # There are a lot of cases that can cause a process execution
# error, but until we do more work to separate out the various # error, but until we do more work to separate out the various
# cases we'll keep the general catch here # cases we'll keep the general catch here

44
cinder/tests/unit/test_image_utils.py
View File

@ -54,7 +54,9 @@ class TestQemuImgInfo(test.TestCase):
test_path = mock.sentinel.path test_path = mock.sentinel.path
mock_exec.return_value = (mock_out, mock_err) 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', mock_exec.assert_called_once_with('env', 'LC_ALL=C', 'qemu-img',
'info', test_path, run_as_root=False, 'info', test_path, run_as_root=False,
prlimit=image_utils.QEMU_IMG_LIMITS) prlimit=image_utils.QEMU_IMG_LIMITS)
@ -343,7 +345,9 @@ class TestVerifyImage(test.TestCase):
self.assertIsNone(output) self.assertIsNone(output)
mock_fetch.assert_called_once_with(ctxt, image_service, image_id, mock_fetch.assert_called_once_with(ctxt, image_service, image_id,
dest, None, None) 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.assert_called_once_with(dest)
(mock_fileutils.remove_path_on_error.return_value.__enter__ (mock_fileutils.remove_path_on_error.return_value.__enter__
.assert_called_once_with()) .assert_called_once_with())
@ -731,7 +735,7 @@ class TestFetchToVolumeFormat(test.TestCase):
self.assertIsNone(output) self.assertIsNone(output)
mock_temp.assert_called_once_with() mock_temp.assert_called_once_with()
mock_info.assert_has_calls([ 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.call(tmp, run_as_root=True)])
mock_fetch.assert_called_once_with(ctxt, image_service, image_id, mock_fetch.assert_called_once_with(ctxt, image_service, image_id,
tmp, None, None) tmp, None, None)
@ -782,7 +786,7 @@ class TestFetchToVolumeFormat(test.TestCase):
self.assertIsNone(output) self.assertIsNone(output)
mock_temp.assert_called_once_with() mock_temp.assert_called_once_with()
mock_info.assert_has_calls([ 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.call(tmp, run_as_root=run_as_root)])
mock_fetch.assert_called_once_with(ctxt, image_service, image_id, mock_fetch.assert_called_once_with(ctxt, image_service, image_id,
tmp, user_id, project_id) tmp, user_id, project_id)
@ -834,7 +838,7 @@ class TestFetchToVolumeFormat(test.TestCase):
self.assertIsNone(output) self.assertIsNone(output)
mock_temp.assert_called_once_with() mock_temp.assert_called_once_with()
mock_info.assert_has_calls([ 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.call(tmp, run_as_root=run_as_root)])
mock_fetch.assert_called_once_with(ctxt, image_service, image_id, mock_fetch.assert_called_once_with(ctxt, image_service, image_id,
tmp, user_id, project_id) tmp, user_id, project_id)
@ -884,7 +888,7 @@ class TestFetchToVolumeFormat(test.TestCase):
self.assertIsNone(output) self.assertIsNone(output)
mock_temp.assert_called_once_with() mock_temp.assert_called_once_with()
mock_info.assert_has_calls([ 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.call(tmp, run_as_root=run_as_root)])
mock_fetch.assert_called_once_with(ctxt, image_service, image_id, mock_fetch.assert_called_once_with(ctxt, image_service, image_id,
tmp, user_id, project_id) tmp, user_id, project_id)
@ -937,8 +941,8 @@ class TestFetchToVolumeFormat(test.TestCase):
self.assertIsNone(output) self.assertIsNone(output)
self.assertEqual(2, mock_temp.call_count) self.assertEqual(2, mock_temp.call_count)
mock_info.assert_has_calls([ mock_info.assert_has_calls([
mock.call(tmp, run_as_root=True), mock.call(tmp, force_share=False, run_as_root=True),
mock.call(dummy, run_as_root=True), mock.call(dummy, force_share=False, run_as_root=True),
mock.call(tmp, run_as_root=True)]) mock.call(tmp, run_as_root=True)])
mock_fetch.assert_called_once_with(ctxt, image_service, image_id, mock_fetch.assert_called_once_with(ctxt, image_service, image_id,
tmp, None, None) tmp, None, None)
@ -987,7 +991,9 @@ class TestFetchToVolumeFormat(test.TestCase):
self.assertIsNone(output) self.assertIsNone(output)
image_service.show.assert_called_once_with(ctxt, image_id) image_service.show.assert_called_once_with(ctxt, image_id)
mock_temp.assert_called_once_with() 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, mock_fetch.assert_called_once_with(ctxt, image_service, image_id,
tmp, user_id, project_id) tmp, user_id, project_id)
self.assertFalse(mock_repl_xen.called) self.assertFalse(mock_repl_xen.called)
@ -1032,7 +1038,9 @@ class TestFetchToVolumeFormat(test.TestCase):
image_service.show.assert_called_once_with(ctxt, image_id) image_service.show.assert_called_once_with(ctxt, image_id)
mock_temp.assert_called_once_with() 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_fetch.called)
self.assertFalse(mock_repl_xen.called) self.assertFalse(mock_repl_xen.called)
self.assertFalse(mock_copy.called) self.assertFalse(mock_copy.called)
@ -1075,7 +1083,9 @@ class TestFetchToVolumeFormat(test.TestCase):
image_service.show.assert_called_once_with(ctxt, image_id) image_service.show.assert_called_once_with(ctxt, image_id)
mock_temp.assert_called_once_with() 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_fetch.called)
self.assertFalse(mock_repl_xen.called) self.assertFalse(mock_repl_xen.called)
self.assertFalse(mock_copy.called) self.assertFalse(mock_copy.called)
@ -1120,7 +1130,7 @@ class TestFetchToVolumeFormat(test.TestCase):
image_service.show.assert_called_once_with(ctxt, image_id) image_service.show.assert_called_once_with(ctxt, image_id)
mock_temp.assert_called_once_with() mock_temp.assert_called_once_with()
mock_info.assert_has_calls([ 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.call(tmp, run_as_root=run_as_root)])
mock_fetch.assert_called_once_with(ctxt, image_service, image_id, mock_fetch.assert_called_once_with(ctxt, image_service, image_id,
tmp, user_id, project_id) tmp, user_id, project_id)
@ -1213,7 +1223,7 @@ class TestFetchToVolumeFormat(test.TestCase):
image_service.show.assert_called_once_with(ctxt, image_id) image_service.show.assert_called_once_with(ctxt, image_id)
mock_temp.assert_called_once_with() mock_temp.assert_called_once_with()
mock_info.assert_has_calls([ 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.call(tmp, run_as_root=run_as_root)])
mock_fetch.assert_called_once_with(ctxt, image_service, image_id, mock_fetch.assert_called_once_with(ctxt, image_service, image_id,
tmp, user_id, project_id) tmp, user_id, project_id)
@ -1261,7 +1271,7 @@ class TestFetchToVolumeFormat(test.TestCase):
image_service.show.assert_called_once_with(ctxt, image_id) image_service.show.assert_called_once_with(ctxt, image_id)
mock_temp.assert_called_once_with() mock_temp.assert_called_once_with()
mock_info.assert_has_calls([ 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.call(tmp, run_as_root=run_as_root)])
mock_fetch.assert_called_once_with(ctxt, image_service, image_id, mock_fetch.assert_called_once_with(ctxt, image_service, image_id,
tmp, user_id, project_id) tmp, user_id, project_id)
@ -1308,7 +1318,7 @@ class TestFetchToVolumeFormat(test.TestCase):
self.assertIsNone(output) self.assertIsNone(output)
mock_temp.assert_called_once_with() mock_temp.assert_called_once_with()
mock_info.assert_has_calls([ 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.call(tmp, run_as_root=run_as_root)])
mock_fetch.assert_called_once_with(ctxt, image_service, image_id, mock_fetch.assert_called_once_with(ctxt, image_service, image_id,
tmp, user_id, project_id) tmp, user_id, project_id)
@ -1345,7 +1355,9 @@ class TestFetchToVolumeFormat(test.TestCase):
run_as_root=run_as_root) run_as_root=run_as_root)
image_service.show.assert_called_once_with(ctxt, image_id) 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, mock_fetch.assert_called_once_with(ctxt, image_service, image_id,
dest, None, None) dest, None, None)

8
cinder/tests/unit/volume/drivers/test_nfs.py
View File

@ -1195,7 +1195,9 @@ class NfsDriverTestCase(test.TestCase):
drv._copy_volume_from_snapshot(fake_snap, dest_volume, size) drv._copy_volume_from_snapshot(fake_snap, dest_volume, size)
mock_read_info_file.assert_called_once_with(info_path) 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'] used_qcow = nfs_conf['nfs_qcow2_volumes']
mock_convert_image.assert_called_once_with( mock_convert_image.assert_called_once_with(
src_vol_path, dest_vol_path, 'qcow2' if used_qcow else 'raw', 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) 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('nfs', conn_info['driver_volume_type'])
self.assertEqual(volume.name, conn_info['data']['name']) self.assertEqual(volume.name, conn_info['data']['name'])
self.assertEqual(self.TEST_MNT_POINT_BASE, self.assertEqual(self.TEST_MNT_POINT_BASE,

6
cinder/tests/unit/volume/drivers/test_quobyte.py
View File

@ -625,6 +625,7 @@ class QuobyteDriverTestCase(test.TestCase):
drv.extend_volume(volume, 3) drv.extend_volume(volume, 3)
image_utils.qemu_img_info.assert_called_once_with(volume_path, image_utils.qemu_img_info.assert_called_once_with(volume_path,
force_share=False,
run_as_root=False) run_as_root=False)
image_utils.resize_image.assert_called_once_with(volume_path, 3) 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) drv._read_info_file.assert_called_once_with(info_path)
image_utils.qemu_img_info.assert_called_once_with(snap_path, image_utils.qemu_img_info.assert_called_once_with(snap_path,
force_share=False,
run_as_root=False) run_as_root=False)
(image_utils.convert_image. (image_utils.convert_image.
assert_called_once_with(src_vol_path, 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) drv.get_active_image_from_info.assert_called_once_with(volume)
image_utils.qemu_img_info.assert_called_once_with(vol_path, image_utils.qemu_img_info.assert_called_once_with(vol_path,
force_share=False,
run_as_root=False) run_as_root=False)
self.assertEqual('raw', conn_info['data']['format']) 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_get_active_image_from_info.assert_called_once_with(volume)
mock_local_volume_dir.assert_called_once_with(volume) mock_local_volume_dir.assert_called_once_with(volume)
mock_qemu_img_info.assert_called_once_with(volume_path, mock_qemu_img_info.assert_called_once_with(volume_path,
force_share=False,
run_as_root=False) run_as_root=False)
mock_upload_volume.assert_called_once_with( mock_upload_volume.assert_called_once_with(
mock.ANY, mock.ANY, mock.ANY, upload_path, run_as_root=False) 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_get_active_image_from_info.assert_called_once_with(volume)
mock_local_volume_dir.assert_called_with(volume) mock_local_volume_dir.assert_called_with(volume)
mock_qemu_img_info.assert_called_once_with(volume_path, mock_qemu_img_info.assert_called_once_with(volume_path,
force_share=False,
run_as_root=False) run_as_root=False)
mock_convert_image.assert_called_once_with( mock_convert_image.assert_called_once_with(
volume_path, upload_path, 'raw', run_as_root=False) 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_get_active_image_from_info.assert_called_once_with(volume)
mock_local_volume_dir.assert_called_with(volume) mock_local_volume_dir.assert_called_with(volume)
mock_qemu_img_info.assert_called_once_with(volume_path, mock_qemu_img_info.assert_called_once_with(volume_path,
force_share=False,
run_as_root=False) run_as_root=False)
mock_convert_image.assert_called_once_with( mock_convert_image.assert_called_once_with(
volume_path, upload_path, 'raw', run_as_root=False) volume_path, upload_path, 'raw', run_as_root=False)

1
cinder/tests/unit/volume/drivers/test_remotefs.py
View File

@ -433,6 +433,7 @@ class RemoteFsSnapDriverTestCase(test.TestCase):
fake_vol_name, basedir) fake_vol_name, basedir)
mock_qemu_img_info.assert_called_with(mock.sentinel.image_path, mock_qemu_img_info.assert_called_with(mock.sentinel.image_path,
force_share=False,
run_as_root=True) run_as_root=True)
@ddt.data([None, '/fake_basedir'], @ddt.data([None, '/fake_basedir'],

4
cinder/volume/drivers/nfs.py
View File

@ -126,7 +126,8 @@ class NfsDriver(remotefs.RemoteFSSnapDriverDistributed):
active_vol = self.get_active_image_from_info(volume) active_vol = self.get_active_image_from_info(volume)
volume_dir = self._local_volume_dir(volume) volume_dir = self._local_volume_dir(volume)
path_to_vol = os.path.join(volume_dir, active_vol) 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, data = {'export': volume.provider_location,
'name': active_vol} 'name': active_vol}
@ -533,6 +534,7 @@ class NfsDriver(remotefs.RemoteFSSnapDriverDistributed):
path, path,
volume_name, volume_name,
self.configuration.nfs_mount_point_base, self.configuration.nfs_mount_point_base,
force_share=True,
run_as_root=True) run_as_root=True)
def _check_snapshot_support(self, setup_checking=False): def _check_snapshot_support(self, setup_checking=False):

2
cinder/volume/drivers/remotefs.py
View File

@ -737,6 +737,7 @@ class RemoteFSSnapDriverBase(RemoteFSDriver):
json.dump(snap_info, f, indent=1, sort_keys=True) json.dump(snap_info, f, indent=1, sort_keys=True)
def _qemu_img_info_base(self, path, volume_name, basedir, def _qemu_img_info_base(self, path, volume_name, basedir,
force_share=False,
run_as_root=False): run_as_root=False):
"""Sanitize image_utils' qemu_img_info. """Sanitize image_utils' qemu_img_info.
@ -746,6 +747,7 @@ class RemoteFSSnapDriverBase(RemoteFSDriver):
run_as_root = run_as_root or self._execute_as_root run_as_root = run_as_root or self._execute_as_root
info = image_utils.qemu_img_info(path, info = image_utils.qemu_img_info(path,
force_share=force_share,
run_as_root=run_as_root) run_as_root=run_as_root)
if info.image: if info.image:
info.image = os.path.basename(info.image) info.image = os.path.basename(info.image)