diff options
author | Sakshi <sabansal@redhat.com> | 2015-07-16 14:31:03 +0530 |
---|---|---|
committer | Raghavendra G <rgowdapp@redhat.com> | 2015-08-27 06:25:34 -0700 |
commit | 9e51aa646fdc5840b6fa9b12b35c5cc2af274c3c (patch) | |
tree | a7ff968e51340b9e590805bc721fd8ae43c4b3ad /xlators/cluster/dht/src/dht-common.c | |
parent | cf3d6f14ae031ba2f5269cea6dbf80e60d00cce5 (diff) |
dht : lock on subvols to prevent lookup vs rmdir race
There is a possibility that while an rmdir is completed on
some non-hashed subvol and proceeding to others. A lookup
selfheal can recreate the same directory on those subvols
for which the rmdir had succeeded. The fix is to take a
blocking inodelk on the subvols before starting rmdir.
Since selfheal requires lock on all subvols, if an rmdir
is in progess acquiring locks will fail and vice versa.
Change-Id: I841a44758c3b88f5e04d1cb73ad36e0cac9fdabb
BUG: 1245065
Signed-off-by: Sakshi <sabansal@redhat.com>
Reviewed-on: http://review.gluster.org/11725
Tested-by: NetBSD Build System <jenkins@build.gluster.org>
Reviewed-by: Raghavendra G <rgowdapp@redhat.com>
Diffstat (limited to 'xlators/cluster/dht/src/dht-common.c')
-rw-r--r-- | xlators/cluster/dht/src/dht-common.c | 180 |
1 files changed, 157 insertions, 23 deletions
diff --git a/xlators/cluster/dht/src/dht-common.c b/xlators/cluster/dht/src/dht-common.c index 1f67d660d15..9de1550f53b 100644 --- a/xlators/cluster/dht/src/dht-common.c +++ b/xlators/cluster/dht/src/dht-common.c @@ -35,6 +35,10 @@ int dht_setxattr2 (xlator_t *this, xlator_t *subvol, call_frame_t *frame); int +dht_rmdir_unlock (call_frame_t *frame, xlator_t *this); + + +int dht_aggregate_quota_xattr (dict_t *dst, char *key, data_t *value) { int ret = -1; @@ -4524,7 +4528,6 @@ dht_readdirp_cbk (call_frame_t *frame, void *cookie, xlator_t *this, int op_ret, * corresponding hashed subvolume will take care of the * directory entry. */ - if (readdir_optimize) { if (prev->this == local->first_up_subvol) goto list; @@ -5009,7 +5012,7 @@ out: if (local && local->lock.locks) { /* store op_errno for failure case*/ local->op_errno = op_errno; - local->refresh_layout_unlock (frame, this, op_ret); + local->refresh_layout_unlock (frame, this, op_ret, 0); if (op_ret == 0) { DHT_STACK_UNWIND (mknod, frame, op_ret, op_errno, @@ -5054,7 +5057,7 @@ dht_mknod_linkfile_create_cbk (call_frame_t *frame, void *cookie, return 0; err: if (local->lock.locks) - local->refresh_layout_unlock (frame, this, -1); + local->refresh_layout_unlock (frame, this, -1, 0); return 0; } @@ -5159,7 +5162,7 @@ dht_mknod_do (call_frame_t *frame) local->umask, local->params); return 0; err: - local->refresh_layout_unlock (frame, this, -1); + local->refresh_layout_unlock (frame, this, -1, 0); return 0; } @@ -5174,7 +5177,7 @@ dht_mknod_unlock_cbk (call_frame_t *frame, void *cookie, xlator_t *this, } int32_t -dht_mknod_finish (call_frame_t *frame, xlator_t *this, int op_ret) +dht_mknod_finish (call_frame_t *frame, xlator_t *this, int op_ret, int invoke_cbk) { dht_local_t *local = NULL, *lock_local = NULL; call_frame_t *lock_frame = NULL; @@ -5249,7 +5252,7 @@ dht_mknod_lock_cbk (call_frame_t *frame, void *cookie, xlator_t *this, return 0; err: - dht_mknod_finish (frame, this, -1); + dht_mknod_finish (frame, this, -1, 0); return 0; } @@ -5280,7 +5283,7 @@ dht_mknod_lock (call_frame_t *frame, xlator_t *subvol) local->lock.lk_count = count; ret = dht_blocking_inodelk (frame, lk_array, count, - dht_mknod_lock_cbk); + IGNORE_ENOENT_ESTALE, dht_mknod_lock_cbk); if (ret < 0) { local->lock.locks = NULL; @@ -5808,7 +5811,7 @@ out: if (local && local->lock.locks) { /* store op_errno for failure case*/ local->op_errno = op_errno; - local->refresh_layout_unlock (frame, this, op_ret); + local->refresh_layout_unlock (frame, this, op_ret, 0); if (op_ret == 0) { DHT_STACK_UNWIND (create, frame, op_ret, op_errno, fd, @@ -5849,7 +5852,7 @@ dht_create_linkfile_create_cbk (call_frame_t *frame, void *cookie, return 0; err: if (local->lock.locks) - local->refresh_layout_unlock (frame, this, -1); + local->refresh_layout_unlock (frame, this, -1, 0); return 0; } @@ -6013,7 +6016,7 @@ dht_create_do (call_frame_t *frame) local->umask, local->fd, local->params); return 0; err: - local->refresh_layout_unlock (frame, this, -1); + local->refresh_layout_unlock (frame, this, -1, 0); return 0; } @@ -6027,7 +6030,7 @@ dht_create_unlock_cbk (call_frame_t *frame, void *cookie, xlator_t *this, } int32_t -dht_create_finish (call_frame_t *frame, xlator_t *this, int op_ret) +dht_create_finish (call_frame_t *frame, xlator_t *this, int op_ret, int invoke_cbk) { dht_local_t *local = NULL, *lock_local = NULL; call_frame_t *lock_frame = NULL; @@ -6102,7 +6105,7 @@ dht_create_lock_cbk (call_frame_t *frame, void *cookie, xlator_t *this, return 0; err: - dht_create_finish (frame, this, -1); + dht_create_finish (frame, this, -1, 0); return 0; } @@ -6133,7 +6136,7 @@ dht_create_lock (call_frame_t *frame, xlator_t *subvol) local->lock.lk_count = count; ret = dht_blocking_inodelk (frame, lk_array, count, - dht_create_lock_cbk); + IGNORE_ENOENT_ESTALE, dht_create_lock_cbk); if (ret < 0) { local->lock.locks = NULL; @@ -6593,6 +6596,7 @@ unlock: this_call_cnt = dht_frame_return (frame); if (is_last_call (this_call_cnt)) { if (local->need_selfheal) { + dht_rmdir_unlock (frame, this); local->layout = dht_layout_get (this, local->loc.inode); @@ -6616,6 +6620,7 @@ unlock: 1); } + dht_rmdir_unlock (frame, this); DHT_STACK_UNWIND (rmdir, frame, local->op_ret, local->op_errno, &local->preparent, &local->postparent, NULL); @@ -6684,6 +6689,7 @@ unlock: if (done) { if (local->need_selfheal && local->fop_succeeded) { + dht_rmdir_unlock (frame, this); local->layout = dht_layout_get (this, local->loc.inode); @@ -6718,6 +6724,7 @@ unlock: } + dht_rmdir_unlock (frame, this); DHT_STACK_UNWIND (rmdir, frame, local->op_ret, local->op_errno, &local->preparent, &local->postparent, NULL); @@ -6729,11 +6736,110 @@ unlock: int +dht_rmdir_unlock_cbk (call_frame_t *frame, void *cookie, xlator_t *this, + int32_t op_ret, int32_t op_errno, dict_t *xdata) +{ + DHT_STACK_DESTROY (frame); + return 0; +} + + +int +dht_rmdir_unlock (call_frame_t *frame, xlator_t *this) +{ + dht_local_t *local = NULL, *lock_local = NULL; + call_frame_t *lock_frame = NULL; + int lock_count = 0; + + local = frame->local; + lock_count = dht_lock_count (local->lock.locks, local->lock.lk_count); + + if (lock_count == 0) + goto done; + + lock_frame = copy_frame (frame); + if (lock_frame == NULL) + goto done; + + lock_local = dht_local_init (lock_frame, &local->loc, NULL, + lock_frame->root->op); + if (lock_local == NULL) + goto done; + + lock_local->lock.locks = local->lock.locks; + lock_local->lock.lk_count = local->lock.lk_count; + + local->lock.locks = NULL; + local->lock.lk_count = 0; + dht_unlock_inodelk (lock_frame, lock_local->lock.locks, + lock_local->lock.lk_count, + dht_rmdir_unlock_cbk); + lock_frame = NULL; + +done: + if (lock_frame != NULL) { + DHT_STACK_DESTROY (lock_frame); + } + + return 0; +} + + +int +dht_rmdir_lock_cbk (call_frame_t *frame, void *cookie, xlator_t *this, + int32_t op_ret, int32_t op_errno, dict_t *xdata) +{ + dht_local_t *local = NULL; + dht_conf_t *conf = NULL; + int i = 0; + + VALIDATE_OR_GOTO (this->private, err); + + conf = this->private; + local = frame->local; + + if (op_ret < 0) { + gf_msg (this->name, GF_LOG_WARNING, op_errno, + DHT_MSG_INODE_LK_ERROR, + "acquiring inodelk failed rmdir for %s)", + local->loc.path); + + local->op_ret = -1; + local->op_errno = (op_errno == EAGAIN) ? EBUSY : op_errno; + goto err; + } + + for (i = 0; i < conf->subvolume_cnt; i++) { + if (local->hashed_subvol && + (local->hashed_subvol == conf->subvolumes[i])) + continue; + + STACK_WIND (frame, dht_rmdir_cbk, + conf->subvolumes[i], + conf->subvolumes[i]->fops->rmdir, + &local->loc, local->flags, NULL); + } + + return 0; + +err: + /* No harm in calling an extra rmdir unlock */ + dht_rmdir_unlock (frame, this); + DHT_STACK_UNWIND (rmdir, frame, local->op_ret, local->op_errno, + &local->preparent, &local->postparent, NULL); + + return 0; +} + + +int dht_rmdir_do (call_frame_t *frame, xlator_t *this) { dht_local_t *local = NULL; dht_conf_t *conf = NULL; - int i = 0; + dht_lock_t **lk_array = NULL; + int i = 0, ret = -1; + int count = 1; xlator_t *hashed_subvol = NULL; char gfid[GF_UUID_BUF_SIZE] ={0}; @@ -6747,7 +6853,6 @@ dht_rmdir_do (call_frame_t *frame, xlator_t *this) local->call_cnt = conf->subvolume_cnt; - /* first remove from non-hashed_subvol */ hashed_subvol = dht_subvol_get_hashed (this, &local->loc); @@ -6771,20 +6876,49 @@ dht_rmdir_do (call_frame_t *frame, xlator_t *this) return 0; } - for (i = 0; i < conf->subvolume_cnt; i++) { - if (hashed_subvol && - (hashed_subvol == conf->subvolumes[i])) - continue; + count = conf->subvolume_cnt; - STACK_WIND (frame, dht_rmdir_cbk, - conf->subvolumes[i], - conf->subvolumes[i]->fops->rmdir, - &local->loc, local->flags, NULL); + lk_array = GF_CALLOC (count, sizeof (*lk_array), gf_common_mt_char); + if (lk_array == NULL) { + local->op_ret = -1; + local->op_errno = ENOMEM; + goto err; + } + + for (i = 0; i < count; i++) { + lk_array[i] = dht_lock_new (frame->this, + conf->subvolumes[i], + &local->loc, F_WRLCK, + DHT_LAYOUT_HEAL_DOMAIN); + if (lk_array[i] == NULL) { + local->op_ret = -1; + local->op_errno = EINVAL; + goto err; + } + } + + local->lock.locks = lk_array; + local->lock.lk_count = count; + + ret = dht_blocking_inodelk (frame, lk_array, count, + IGNORE_ENOENT_ESTALE, + dht_rmdir_lock_cbk); + if (ret < 0) { + local->lock.locks = NULL; + local->lock.lk_count = 0; + local->op_ret = -1; + local->op_errno = errno ? errno : EINVAL; + goto err; } return 0; err: + if (lk_array != NULL) { + dht_lock_array_free (lk_array, count); + GF_FREE (lk_array); + } + DHT_STACK_UNWIND (rmdir, frame, local->op_ret, local->op_errno, &local->preparent, &local->postparent, NULL); return 0; |