openstack
/
cinder
Code Issues Proposed changes

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

NFS initialize_connection fails due to new locking
code in qemu-img info.  Bug 1718133 tracked a related issue.

Fix the NFS driver to work with the new version of
qemu-img info by disabling locking for qemu-img info
queries during initialize_connection and create_snapshot.

Closes-Bug: #1731992
Change-Id: Ia3623af2048d6cbda65deebf4404e6b5fefe1bfc
This commit is contained in:
Eric Harney 2017-11-09 14:15:39 -05:00
parent d5147cb8e5
commit 524a74c6ba
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_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

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

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

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

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

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

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

@ -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'],

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

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