From bf379e85fb74cb3ac8db8c926967813697328559 Mon Sep 17 00:00:00 2001 From: Pranith Kumar K Date: Fri, 22 Sep 2017 12:50:43 +0530 Subject: [PATCH 620/620] features/locks: Maintain separation of lock->client_pid, flock->l_pid Backport of https://review.gluster.org/17826 Problem: grant_blocked_locks() constructs flock from lock. Locks xlator uses frame->root->pid interchangeably flock->l_pid. With gNFS frame->root->pid (which translates to lock->client_pid) is not same as flock->l_pid, this leads to lk's cbk returning flock with l_pid from lock->client_pid instead of input flock->l_pid. This triggers EC's error code path leading to failure of lk call, because the response' flock->l_pid is different from request's flock->l_pid. Fix: Maintain separation of lock->client_pid, flock->l_pid. Always unwind with flock with correct pid. BUG: 1411338 Change-Id: Ifab35c458662cf0082b902f37782f8c5321d823d Signed-off-by: Pranith Kumar K Reviewed-on: https://code.engineering.redhat.com/gerrit/119040 --- xlators/features/locks/src/common.c | 46 ++++++++++++------------------------- xlators/features/locks/src/posix.c | 3 --- 2 files changed, 15 insertions(+), 34 deletions(-) diff --git a/xlators/features/locks/src/common.c b/xlators/features/locks/src/common.c index 68904f6..015464c 100644 --- a/xlators/features/locks/src/common.c +++ b/xlators/features/locks/src/common.c @@ -485,6 +485,7 @@ new_posix_lock (struct gf_flock *flock, client_t *client, pid_t client_pid, lock->lk_flags = lk_flags; lock->blocking = blocking; + memcpy (&lock->user_flock, flock, sizeof (lock->user_flock)); INIT_LIST_HEAD (&lock->list); @@ -522,6 +523,7 @@ __copy_lock(posix_lock_t *src) GF_FREE(dst); dst = NULL; } + INIT_LIST_HEAD (&dst->list); } return dst; @@ -531,7 +533,7 @@ __copy_lock(posix_lock_t *src) void posix_lock_to_flock (posix_lock_t *lock, struct gf_flock *flock) { - flock->l_pid = lock->client_pid; + flock->l_pid = lock->user_flock.l_pid; flock->l_type = lock->fl_type; flock->l_start = lock->fl_start; flock->l_owner = lock->owner; @@ -601,18 +603,19 @@ __delete_unlck_locks (pl_inode_t *pl_inode) /* Add two locks */ static posix_lock_t * -add_locks (posix_lock_t *l1, posix_lock_t *l2) +add_locks (posix_lock_t *l1, posix_lock_t *l2, posix_lock_t *dst) { posix_lock_t *sum = NULL; - sum = GF_CALLOC (1, sizeof (posix_lock_t), - gf_locks_mt_posix_lock_t); + sum = __copy_lock (dst); if (!sum) return NULL; sum->fl_start = min (l1->fl_start, l2->fl_start); sum->fl_end = max (l1->fl_end, l2->fl_end); + posix_lock_to_flock (sum, &sum->user_flock); + return sum; } @@ -637,6 +640,7 @@ subtract_locks (posix_lock_t *big, posix_lock_t *small) } v.locks[0]->fl_type = small->fl_type; + v.locks[0]->user_flock.l_type = small->fl_type; goto done; } @@ -653,6 +657,8 @@ subtract_locks (posix_lock_t *big, posix_lock_t *small) v.locks[0]->fl_end = small->fl_start - 1; v.locks[2]->fl_start = small->fl_end + 1; + posix_lock_to_flock (v.locks[0], &v.locks[0]->user_flock); + posix_lock_to_flock (v.locks[2], &v.locks[2]->user_flock); goto done; } @@ -666,6 +672,7 @@ subtract_locks (posix_lock_t *big, posix_lock_t *small) } v.locks[0]->fl_start = small->fl_end + 1; + posix_lock_to_flock (v.locks[0], &v.locks[0]->user_flock); goto done; } @@ -677,6 +684,7 @@ subtract_locks (posix_lock_t *big, posix_lock_t *small) } v.locks[0]->fl_end = small->fl_start - 1; + posix_lock_to_flock (v.locks[0], &v.locks[0]->user_flock); goto done; } @@ -787,7 +795,6 @@ __insert_and_merge (pl_inode_t *pl_inode, posix_lock_t *lock) posix_lock_t *sum = NULL; int i = 0; struct _values v = { .locks = {0, 0, 0} }; - client_t *client = NULL; list_for_each_entry_safe (conf, t, &pl_inode->ext_list, list) { if (conf->blocked) @@ -798,17 +805,7 @@ __insert_and_merge (pl_inode_t *pl_inode, posix_lock_t *lock) if (same_owner (conf, lock)) { if (conf->fl_type == lock->fl_type && conf->lk_flags == lock->lk_flags) { - sum = add_locks (lock, conf); - - sum->fl_type = lock->fl_type; - sum->client = lock->client; - client = sum->client; - sum->client_uid = - gf_strdup (client->client_uid); - sum->fd_num = lock->fd_num; - sum->client_pid = lock->client_pid; - sum->owner = lock->owner; - sum->lk_flags = lock->lk_flags; + sum = add_locks (lock, conf, lock); __delete_lock (conf); __destroy_lock (conf); @@ -820,18 +817,7 @@ __insert_and_merge (pl_inode_t *pl_inode, posix_lock_t *lock) return; } else { - sum = add_locks (lock, conf); - - sum->fl_type = conf->fl_type; - sum->client = conf->client; - client = sum->client; - sum->client_uid = - gf_strdup (client->client_uid); - - sum->fd_num = conf->fd_num; - sum->client_pid = conf->client_pid; - sum->owner = conf->owner; - sum->lk_flags = conf->lk_flags; + sum = add_locks (lock, conf, conf); v = subtract_locks (sum, lock); @@ -847,9 +833,6 @@ __insert_and_merge (pl_inode_t *pl_inode, posix_lock_t *lock) if (!v.locks[i]) continue; - INIT_LIST_HEAD (&v.locks[i]->list); - posix_lock_to_flock (v.locks[i], - &v.locks[i]->user_flock); __insert_and_merge (pl_inode, v.locks[i]); } @@ -984,6 +967,7 @@ pl_send_prelock_unlock (xlator_t *this, pl_inode_t *pl_inode, flock.l_whence = old_lock->user_flock.l_whence; flock.l_start = old_lock->user_flock.l_start; flock.l_len = old_lock->user_flock.l_len; + flock.l_pid = old_lock->user_flock.l_pid; unlock_lock = new_posix_lock (&flock, old_lock->client, diff --git a/xlators/features/locks/src/posix.c b/xlators/features/locks/src/posix.c index c9b3f82..feca2f1 100644 --- a/xlators/features/locks/src/posix.c +++ b/xlators/features/locks/src/posix.c @@ -2304,7 +2304,6 @@ pl_lk (call_frame_t *frame, xlator_t *this, /* fall through */ case F_RESLK_LCK: - memcpy (&reqlock->user_flock, flock, sizeof (struct gf_flock)); reqlock->frame = frame; reqlock->this = this; @@ -2388,8 +2387,6 @@ pl_lk (call_frame_t *frame, xlator_t *this, reqlock->frame = frame; reqlock->this = this; - memcpy (&reqlock->user_flock, flock, sizeof (struct gf_flock)); - pthread_mutex_lock (&pl_inode->mutex); { if (pl_inode->migrated) { -- 1.8.3.1