SMBFS: Lock on a per-volume basis
This allows operations that do not conflict with each other (i.e. are on different volumes) to run concurrently. The prior locking scheme was too coarse and essentially made the driver single-threaded. This patch moves the implementation of the GlusterFS driver locking scheme to the RemoteFS base driver so that other similar volume drivers can use it. Closes-Bug: #1439352 Change-Id: I37be14e7525406e0ad568f6228c1303998c5235f
This commit is contained in:
Alessandro Pilotti
Lucian Petrut
parent
e7fdd10da6
commit
acf19befe5
|
|
@ -19,14 +19,16 @@ import mock
|
|||
|
||||
from cinder import exception
|
||||
from cinder import test
|
||||
from cinder import utils
|
||||
from cinder.volume.drivers import remotefs
|
||||
|
||||
|
||||
class RemoteFsSnapDriverTestCase(test.TestCase):
|
||||
|
||||
_FAKE_CONTEXT = 'fake_context'
|
||||
_FAKE_VOLUME_NAME = 'volume-4f711859-4928-4cb7-801a-a50c37ceaccc'
|
||||
_FAKE_VOLUME = {'id': '4f711859-4928-4cb7-801a-a50c37ceaccc',
|
||||
_FAKE_VOLUME_ID = '4f711859-4928-4cb7-801a-a50c37ceaccc'
|
||||
_FAKE_VOLUME_NAME = 'volume-%s' % _FAKE_VOLUME_ID
|
||||
_FAKE_VOLUME = {'id': _FAKE_VOLUME_ID,
|
||||
'size': 1,
|
||||
'provider_location': 'fake_share',
|
||||
'name': _FAKE_VOLUME_NAME,
|
||||
|
|
@ -295,3 +297,56 @@ class RemoteFsSnapDriverTestCase(test.TestCase):
|
|||
self.assertRaises(exception.InvalidVolume,
|
||||
self._driver._create_snapshot,
|
||||
fake_snapshot)
|
||||
|
||||
@mock.patch.object(utils, 'synchronized')
|
||||
def _locked_volume_operation_test_helper(self, mock_synchronized, func,
|
||||
expected_exception=False,
|
||||
*args, **kwargs):
|
||||
def mock_decorator(*args, **kwargs):
|
||||
def mock_inner(f):
|
||||
return f
|
||||
return mock_inner
|
||||
|
||||
mock_synchronized.side_effect = mock_decorator
|
||||
expected_lock = '%s-%s' % (self._driver.driver_prefix,
|
||||
self._FAKE_VOLUME_ID)
|
||||
|
||||
if expected_exception:
|
||||
self.assertRaises(expected_exception, func,
|
||||
self._driver,
|
||||
*args, **kwargs)
|
||||
else:
|
||||
ret_val = func(self._driver, *args, **kwargs)
|
||||
|
||||
mock_synchronized.assert_called_with(expected_lock,
|
||||
external=False)
|
||||
self.assertEqual(mock.sentinel.ret_val, ret_val)
|
||||
|
||||
def test_locked_volume_id_operation(self):
|
||||
mock_volume = {'id': self._FAKE_VOLUME_ID}
|
||||
|
||||
@remotefs.locked_volume_id_operation
|
||||
def synchronized_func(inst, volume):
|
||||
return mock.sentinel.ret_val
|
||||
|
||||
self._locked_volume_operation_test_helper(func=synchronized_func,
|
||||
volume=mock_volume)
|
||||
|
||||
def test_locked_volume_id_snapshot_operation(self):
|
||||
mock_snapshot = {'volume': {'id': self._FAKE_VOLUME_ID}}
|
||||
|
||||
@remotefs.locked_volume_id_operation
|
||||
def synchronized_func(inst, snapshot):
|
||||
return mock.sentinel.ret_val
|
||||
|
||||
self._locked_volume_operation_test_helper(func=synchronized_func,
|
||||
snapshot=mock_snapshot)
|
||||
|
||||
def test_locked_volume_id_operation_exception(self):
|
||||
@remotefs.locked_volume_id_operation
|
||||
def synchronized_func(inst):
|
||||
return mock.sentinel.ret_val
|
||||
|
||||
self._locked_volume_operation_test_helper(
|
||||
func=synchronized_func,
|
||||
expected_exception=exception.VolumeBackendAPIException)
|
||||
|
|
|
|||
|
|
@ -52,48 +52,6 @@ volume_opts = [
|
|||
CONF = cfg.CONF
|
||||
CONF.register_opts(volume_opts)
|
||||
|
||||
lock_tag = 'glusterfs'
|
||||
|
||||
|
||||
def locked_volume_id_operation(f, external=False):
|
||||
"""Lock decorator for volume operations.
|
||||
|
||||
Takes a named lock prior to executing the operation. The lock is named
|
||||
with the id of the volume. This lock can then be used
|
||||
by other operations to avoid operation conflicts on shared volumes.
|
||||
|
||||
May be applied to methods of signature:
|
||||
method(<self>, volume, *, **)
|
||||
"""
|
||||
|
||||
def lvo_inner1(inst, volume, *args, **kwargs):
|
||||
@utils.synchronized('%s-%s' % (lock_tag, volume['id']),
|
||||
external=external)
|
||||
def lvo_inner2(*_args, **_kwargs):
|
||||
return f(*_args, **_kwargs)
|
||||
return lvo_inner2(inst, volume, *args, **kwargs)
|
||||
return lvo_inner1
|
||||
|
||||
|
||||
def locked_volume_id_snapshot_operation(f, external=False):
|
||||
"""Lock decorator for volume operations that use snapshot objects.
|
||||
|
||||
Takes a named lock prior to executing the operation. The lock is named
|
||||
with the id of the volume. This lock can then be used
|
||||
by other operations to avoid operation conflicts on shared volumes.
|
||||
|
||||
May be applied to methods of signature:
|
||||
method(<self>, snapshot, *, **)
|
||||
"""
|
||||
|
||||
def lso_inner1(inst, snapshot, *args, **kwargs):
|
||||
@utils.synchronized('%s-%s' % (lock_tag, snapshot['volume']['id']),
|
||||
external=external)
|
||||
def lso_inner2(*_args, **_kwargs):
|
||||
return f(*_args, **_kwargs)
|
||||
return lso_inner2(inst, snapshot, *args, **kwargs)
|
||||
return lso_inner1
|
||||
|
||||
|
||||
class GlusterfsDriver(remotefs_drv.RemoteFSSnapDriver):
|
||||
"""Gluster based cinder driver. Creates file on Gluster share for using it
|
||||
|
|
@ -202,12 +160,7 @@ class GlusterfsDriver(remotefs_drv.RemoteFSSnapDriver):
|
|||
hashed)
|
||||
return path
|
||||
|
||||
@locked_volume_id_operation
|
||||
def create_cloned_volume(self, volume, src_vref):
|
||||
"""Creates a clone of the specified volume."""
|
||||
self._create_cloned_volume(volume, src_vref)
|
||||
|
||||
@locked_volume_id_operation
|
||||
@remotefs_drv.locked_volume_id_operation
|
||||
def create_volume(self, volume):
|
||||
"""Creates a volume."""
|
||||
|
||||
|
|
@ -221,10 +174,6 @@ class GlusterfsDriver(remotefs_drv.RemoteFSSnapDriver):
|
|||
|
||||
return {'provider_location': volume['provider_location']}
|
||||
|
||||
@locked_volume_id_operation
|
||||
def create_volume_from_snapshot(self, volume, snapshot):
|
||||
return self._create_volume_from_snapshot(volume, snapshot)
|
||||
|
||||
def _copy_volume_from_snapshot(self, snapshot, volume, volume_size):
|
||||
"""Copy data from snapshot to destination volume.
|
||||
|
||||
|
|
@ -265,7 +214,7 @@ class GlusterfsDriver(remotefs_drv.RemoteFSSnapDriver):
|
|||
|
||||
self._set_rw_permissions_for_all(path_to_new_vol)
|
||||
|
||||
@locked_volume_id_operation
|
||||
@remotefs_drv.locked_volume_id_operation
|
||||
def delete_volume(self, volume):
|
||||
"""Deletes a logical volume."""
|
||||
|
||||
|
|
@ -291,21 +240,10 @@ class GlusterfsDriver(remotefs_drv.RemoteFSSnapDriver):
|
|||
info_path = self._local_path_volume_info(volume)
|
||||
fileutils.delete_if_exists(info_path)
|
||||
|
||||
@locked_volume_id_snapshot_operation
|
||||
def create_snapshot(self, snapshot):
|
||||
"""Apply locking to the create snapshot operation."""
|
||||
|
||||
return self._create_snapshot(snapshot)
|
||||
|
||||
def _get_matching_backing_file(self, backing_chain, snapshot_file):
|
||||
return next(f for f in backing_chain
|
||||
if f.get('backing-filename', '') == snapshot_file)
|
||||
|
||||
@locked_volume_id_snapshot_operation
|
||||
def delete_snapshot(self, snapshot):
|
||||
"""Apply locking to the delete snapshot operation."""
|
||||
self._delete_snapshot(snapshot)
|
||||
|
||||
def ensure_export(self, ctx, volume):
|
||||
"""Synchronously recreates an export for a logical volume."""
|
||||
|
||||
|
|
@ -323,7 +261,7 @@ class GlusterfsDriver(remotefs_drv.RemoteFSSnapDriver):
|
|||
def validate_connector(self, connector):
|
||||
pass
|
||||
|
||||
@locked_volume_id_operation
|
||||
@remotefs_drv.locked_volume_id_operation
|
||||
def initialize_connection(self, volume, connector):
|
||||
"""Allow connection to connector and return connection info."""
|
||||
|
||||
|
|
@ -355,29 +293,7 @@ class GlusterfsDriver(remotefs_drv.RemoteFSSnapDriver):
|
|||
"""Disallow connection from connector."""
|
||||
pass
|
||||
|
||||
def copy_volume_to_image(self, context, volume, image_service, image_meta):
|
||||
"""Copy the volume to the specified image.
|
||||
|
||||
Warning: parameter order is non-standard to assist with locking
|
||||
decorators.
|
||||
"""
|
||||
|
||||
return self._copy_volume_to_image_with_lock(volume,
|
||||
context,
|
||||
image_service,
|
||||
image_meta)
|
||||
|
||||
@locked_volume_id_operation
|
||||
def _copy_volume_to_image_with_lock(self, volume, context,
|
||||
image_service, image_meta):
|
||||
"""Call private method for this, which handles per-volume locking."""
|
||||
|
||||
return self._copy_volume_to_image(context,
|
||||
volume,
|
||||
image_service,
|
||||
image_meta)
|
||||
|
||||
@locked_volume_id_operation
|
||||
@remotefs_drv.locked_volume_id_operation
|
||||
def extend_volume(self, volume, size_gb):
|
||||
volume_path = self.local_path(volume)
|
||||
volume_filename = os.path.basename(volume_path)
|
||||
|
|
|
|||
|
|
@ -15,6 +15,7 @@
|
|||
# under the License.
|
||||
|
||||
import hashlib
|
||||
import inspect
|
||||
import json
|
||||
import os
|
||||
import re
|
||||
|
|
@ -29,6 +30,7 @@ from oslo_utils import units
|
|||
from cinder import compute
|
||||
from cinder import db
|
||||
from cinder import exception
|
||||
from cinder import utils
|
||||
from cinder.i18n import _, _LE, _LI, _LW
|
||||
from cinder.image import image_utils
|
||||
from cinder.volume import driver
|
||||
|
|
@ -87,11 +89,43 @@ CONF = cfg.CONF
|
|||
CONF.register_opts(nas_opts)
|
||||
|
||||
|
||||
def locked_volume_id_operation(f, external=False):
|
||||
"""Lock decorator for volume operations.
|
||||
|
||||
Takes a named lock prior to executing the operation. The lock is named
|
||||
with the id of the volume. This lock can then be used
|
||||
by other operations to avoid operation conflicts on shared volumes.
|
||||
|
||||
May be applied to methods of signature:
|
||||
method(<self>, volume, *, **)
|
||||
"""
|
||||
|
||||
def lvo_inner1(inst, *args, **kwargs):
|
||||
lock_tag = inst.driver_prefix
|
||||
call_args = inspect.getcallargs(f, inst, *args, **kwargs)
|
||||
|
||||
if call_args.get('volume'):
|
||||
volume_id = call_args['volume']['id']
|
||||
elif call_args.get('snapshot'):
|
||||
volume_id = call_args['snapshot']['volume']['id']
|
||||
else:
|
||||
err_msg = _('The decorated method must accept either a volume or '
|
||||
'a snapshot object')
|
||||
raise exception.VolumeBackendAPIException(data=err_msg)
|
||||
|
||||
@utils.synchronized('%s-%s' % (lock_tag, volume_id),
|
||||
external=external)
|
||||
def lvo_inner2():
|
||||
return f(inst, *args, **kwargs)
|
||||
return lvo_inner2()
|
||||
return lvo_inner1
|
||||
|
||||
|
||||
class RemoteFSDriver(driver.VolumeDriver):
|
||||
"""Common base for drivers that work like NFS."""
|
||||
|
||||
driver_volume_type = None
|
||||
driver_prefix = None
|
||||
driver_prefix = 'remotefs'
|
||||
volume_backend_name = None
|
||||
SHARE_FORMAT_REGEX = r'.+:/.+'
|
||||
|
||||
|
|
@ -1368,3 +1402,33 @@ class RemoteFSSnapDriver(RemoteFSDriver):
|
|||
path_to_delete = os.path.join(
|
||||
self._local_volume_dir(snapshot['volume']), file_to_delete)
|
||||
self._execute('rm', '-f', path_to_delete, run_as_root=True)
|
||||
|
||||
@locked_volume_id_operation
|
||||
def create_snapshot(self, snapshot):
|
||||
"""Apply locking to the create snapshot operation."""
|
||||
|
||||
return self._create_snapshot(snapshot)
|
||||
|
||||
@locked_volume_id_operation
|
||||
def delete_snapshot(self, snapshot):
|
||||
"""Apply locking to the delete snapshot operation."""
|
||||
|
||||
return self._delete_snapshot(snapshot)
|
||||
|
||||
@locked_volume_id_operation
|
||||
def create_volume_from_snapshot(self, volume, snapshot):
|
||||
return self._create_volume_from_snapshot(volume, snapshot)
|
||||
|
||||
@locked_volume_id_operation
|
||||
def create_cloned_volume(self, volume, src_vref):
|
||||
"""Creates a clone of the specified volume."""
|
||||
return self._create_cloned_volume(volume, src_vref)
|
||||
|
||||
@locked_volume_id_operation
|
||||
def copy_volume_to_image(self, context, volume, image_service, image_meta):
|
||||
"""Copy the volume to the specified image."""
|
||||
|
||||
return self._copy_volume_to_image(context,
|
||||
volume,
|
||||
image_service,
|
||||
image_meta)
|
||||
|
|
|
|||
|
|
@ -105,6 +105,7 @@ class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver):
|
|||
return super(SmbfsDriver, self)._qemu_img_info_base(
|
||||
path, volume_name, self.configuration.smbfs_mount_point_base)
|
||||
|
||||
@remotefs_drv.locked_volume_id_operation
|
||||
def initialize_connection(self, volume, connector):
|
||||
"""Allow connection to connector and return connection info.
|
||||
|
||||
|
|
@ -220,7 +221,7 @@ class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver):
|
|||
|
||||
return volume_format
|
||||
|
||||
@utils.synchronized('smbfs', external=False)
|
||||
@remotefs_drv.locked_volume_id_operation
|
||||
def delete_volume(self, volume):
|
||||
"""Deletes a logical volume."""
|
||||
if not volume['provider_location']:
|
||||
|
|
@ -387,12 +388,6 @@ class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver):
|
|||
return False
|
||||
return True
|
||||
|
||||
@utils.synchronized('smbfs', external=False)
|
||||
def create_snapshot(self, snapshot):
|
||||
"""Apply locking to the create snapshot operation."""
|
||||
|
||||
return self._create_snapshot(snapshot)
|
||||
|
||||
def _create_snapshot_online(self, snapshot, backing_filename,
|
||||
new_snap_path):
|
||||
msg = _("This driver does not support snapshotting in-use volumes.")
|
||||
|
|
@ -415,13 +410,7 @@ class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver):
|
|||
"format: %s") % volume_format
|
||||
raise exception.InvalidVolume(err_msg)
|
||||
|
||||
@utils.synchronized('smbfs', external=False)
|
||||
def delete_snapshot(self, snapshot):
|
||||
"""Apply locking to the delete snapshot operation."""
|
||||
|
||||
return self._delete_snapshot(snapshot)
|
||||
|
||||
@utils.synchronized('smbfs', external=False)
|
||||
@remotefs_drv.locked_volume_id_operation
|
||||
def extend_volume(self, volume, size_gb):
|
||||
LOG.info(_LI('Extending volume %s.'), volume['id'])
|
||||
self._extend_volume(volume, size_gb)
|
||||
|
|
@ -473,14 +462,6 @@ class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver):
|
|||
'extend volume %s to %sG.'
|
||||
% (volume['id'], size_gb))
|
||||
|
||||
@utils.synchronized('smbfs', external=False)
|
||||
def copy_volume_to_image(self, context, volume, image_service, image_meta):
|
||||
self._copy_volume_to_image(context, volume, image_service, image_meta)
|
||||
|
||||
@utils.synchronized('smbfs', external=False)
|
||||
def create_volume_from_snapshot(self, volume, snapshot):
|
||||
return self._create_volume_from_snapshot(volume, snapshot)
|
||||
|
||||
def _copy_volume_from_snapshot(self, snapshot, volume, volume_size):
|
||||
"""Copy data from snapshot to destination volume.
|
||||
|
||||
|
|
@ -548,11 +529,6 @@ class SmbfsDriver(remotefs_drv.RemoteFSSnapDriver):
|
|||
reason=(_("Expected volume size was %d") % volume['size'])
|
||||
+ (_(" but size is now %d.") % virt_size))
|
||||
|
||||
@utils.synchronized('smbfs', external=False)
|
||||
def create_cloned_volume(self, volume, src_vref):
|
||||
"""Creates a clone of the specified volume."""
|
||||
return self._create_cloned_volume(volume, src_vref)
|
||||
|
||||
def _ensure_share_mounted(self, smbfs_share):
|
||||
mnt_flags = []
|
||||
if self.shares.get(smbfs_share) is not None:
|
||||
|
|
|
|||
Loading…
Reference in New Issue