From f16c121697b4f2efcb4f7074658c1f3412e61465 Mon Sep 17 00:00:00 2001 From: Eric Harney Date: Mon, 17 Nov 2014 19:52:15 +0530 Subject: [PATCH] GlusterFS: 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. Change-Id: Ib048c21f66926cb96ae02ced4bd16a752d52b301 --- cinder/volume/drivers/glusterfs.py | 81 ++++++++++++++++++++++++++---- 1 file changed, 70 insertions(+), 11 deletions(-) diff --git a/cinder/volume/drivers/glusterfs.py b/cinder/volume/drivers/glusterfs.py index ed6404e06..b4174ab19 100644 --- a/cinder/volume/drivers/glusterfs.py +++ b/cinder/volume/drivers/glusterfs.py @@ -55,6 +55,48 @@ 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(, 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(, 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 @@ -166,12 +208,12 @@ class GlusterfsDriver(remotefs_drv.RemoteFSSnapDriver): hashed) return path - @utils.synchronized('glusterfs', external=False) + @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) - @utils.synchronized('glusterfs', external=False) + @locked_volume_id_operation def create_volume(self, volume): """Creates a volume.""" @@ -185,7 +227,7 @@ class GlusterfsDriver(remotefs_drv.RemoteFSSnapDriver): return {'provider_location': volume['provider_location']} - @utils.synchronized('glusterfs', external=False) + @locked_volume_id_operation def create_volume_from_snapshot(self, volume, snapshot): self._create_volume_from_snapshot(volume, snapshot) @@ -229,7 +271,7 @@ class GlusterfsDriver(remotefs_drv.RemoteFSSnapDriver): self._set_rw_permissions_for_all(path_to_new_vol) - @utils.synchronized('glusterfs', external=False) + @locked_volume_id_operation def delete_volume(self, volume): """Deletes a logical volume.""" @@ -254,7 +296,7 @@ class GlusterfsDriver(remotefs_drv.RemoteFSSnapDriver): info_path = self._local_path_volume_info(volume) fileutils.delete_if_exists(info_path) - @utils.synchronized('glusterfs', external=False) + @locked_volume_id_snapshot_operation def create_snapshot(self, snapshot): """Apply locking to the create snapshot operation.""" @@ -264,7 +306,7 @@ class GlusterfsDriver(remotefs_drv.RemoteFSSnapDriver): return next(f for f in backing_chain if f.get('backing-filename', '') == snapshot_file) - @utils.synchronized('glusterfs', external=False) + @locked_volume_id_snapshot_operation def delete_snapshot(self, snapshot): """Apply locking to the delete snapshot operation.""" self._delete_snapshot(snapshot) @@ -380,7 +422,7 @@ class GlusterfsDriver(remotefs_drv.RemoteFSSnapDriver): def validate_connector(self, connector): pass - @utils.synchronized('glusterfs', external=False) + @locked_volume_id_operation def initialize_connection(self, volume, connector): """Allow connection to connector and return connection info.""" @@ -412,12 +454,29 @@ class GlusterfsDriver(remotefs_drv.RemoteFSSnapDriver): """Disallow connection from connector.""" pass - @utils.synchronized('glusterfs', 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) + """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) - @utils.synchronized('glusterfs', external=False) + @locked_volume_id_operation def extend_volume(self, volume, size_gb): volume_path = self.local_path(volume) volume_filename = os.path.basename(volume_path) -- 2.45.2