cluster/dht: pass a destination subvol to fop2 variants to avoid races.

The destination subvol used in the fop2 variants is either stored in
inode-ctx1 or local->cached_subvol. However, it is not guaranteed that
a value stored in these locations before invocation of fop2 is still
present after the invocation as these locations are shared among
different concurrent operations. So, to preserve the atomicity of
"check dst-subvol and invoke fop2 variant if dst-subvol found", we
pass down the dst-subvol to fop2 variant.

This patch also fixes error handling in some fop2 variants.

Change-Id: Icc226228a246d3f223e3463519736c4495b364d2
BUG: 1225809
Signed-off-by: Raghavendra G <rgowdapp@redhat.com>
Reviewed-on: http://review.gluster.org/10966
Tested-by: Gluster Build System <jenkins@build.gluster.com>

1 files changed, 52 insertions, 44 deletions

diff --git a/xlators/cluster/dht/src/dht-inode-read.c b/xlators/cluster/dht/src/dht-inode-read.c
index 78e3ef4233b..46df5b6ea99 100644
--- a/xlators/cluster/dht/src/dht-inode-read.c
+++ b/xlators/cluster/dht/src/dht-inode-read.c
@@ -15,13 +15,13 @@
 
 #include "dht-common.h"
 
-int dht_access2 (xlator_t *this, call_frame_t *frame, int ret);
-int dht_readv2 (xlator_t *this, call_frame_t *frame, int ret);
-int dht_attr2 (xlator_t *this, call_frame_t *frame, int ret);
-int dht_open2 (xlator_t *this, call_frame_t *frame, int ret);
-int dht_flush2 (xlator_t *this, call_frame_t *frame, int ret);
-int dht_lk2 (xlator_t *this, call_frame_t *frame, int ret);
-int dht_fsync2 (xlator_t *this, call_frame_t *frame, int ret);
+int dht_access2 (xlator_t *this, xlator_t *dst_node, call_frame_t *frame);
+int dht_readv2 (xlator_t *this, xlator_t *dst_node, call_frame_t *frame);
+int dht_attr2 (xlator_t *this, xlator_t *dst_node, call_frame_t *frame);
+int dht_open2 (xlator_t *this, xlator_t *dst_node, call_frame_t *frame);
+int dht_flush2 (xlator_t *this, xlator_t *dst_node, call_frame_t *frame);
+int dht_lk2 (xlator_t *this, xlator_t *dst_node, call_frame_t *frame);
+int dht_fsync2 (xlator_t *this, xlator_t *dst_node, call_frame_t *frame);
 
 int
 dht_open_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
@@ -58,22 +58,21 @@ out:
 }
 
 int
-dht_open2 (xlator_t *this, call_frame_t *frame, int op_ret)
+dht_open2 (xlator_t *this, xlator_t *subvol, call_frame_t *frame)
 {
-        dht_local_t *local  = NULL;
-        xlator_t    *subvol = NULL;
+        dht_local_t *local    = NULL;
         int          op_errno = EINVAL;
 
-        local = frame->local;
-        if (!local)
+        if (!frame || !frame->local)
                 goto out;
 
+        local = frame->local;
         op_errno = ENOENT;
-        if (op_ret)
+
+        if (subvol == NULL)
                 goto out;
 
         local->call_cnt = 2;
-        subvol = local->cached_subvol;
 
         STACK_WIND (frame, dht_open_cbk, subvol, subvol->fops->open,
                     &local->loc, local->rebalance.flags, local->fd,
@@ -169,7 +168,7 @@ dht_file_attr_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
                 } else {
                         /* value is already set in fd_ctx, that means no need
                            to check for whether its complete or not. */
-                        dht_attr2 (this, frame, 0);
+                        dht_attr2 (this, subvol, frame);
                         return 0;
                 }
         }
@@ -182,10 +181,9 @@ err:
 }
 
 int
-dht_attr2 (xlator_t *this, call_frame_t *frame, int op_ret)
+dht_attr2 (xlator_t *this, xlator_t *subvol, call_frame_t *frame)
 {
-        dht_local_t *local  = NULL;
-        xlator_t    *subvol = NULL;
+        dht_local_t *local    = NULL;
         int          op_errno = EINVAL;
 
         local = frame->local;
@@ -193,10 +191,9 @@ dht_attr2 (xlator_t *this, call_frame_t *frame, int op_ret)
                 goto out;
 
         op_errno = local->op_errno;
-        if (op_ret == -1)
+        if (subvol == NULL)
                 goto out;
 
-        subvol = local->cached_subvol;
         local->call_cnt = 2;
 
         if (local->fop == GF_FOP_FSTAT) {
@@ -206,6 +203,7 @@ dht_attr2 (xlator_t *this, call_frame_t *frame, int op_ret)
                 STACK_WIND (frame, dht_file_attr_cbk, subvol,
                             subvol->fops->stat, &local->loc, NULL);
         }
+
         return 0;
 
 out:
@@ -413,7 +411,7 @@ dht_readv_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
                 } else {
                         /* value is already set in fd_ctx, that means no need
                            to check for whether its complete or not. */
-                        dht_readv2 (this, frame, 0);
+                        dht_readv2 (this, subvol, frame);
                         return 0;
                 }
         }
@@ -427,10 +425,9 @@ out:
 }
 
 int
-dht_readv2 (xlator_t *this, call_frame_t *frame, int op_ret)
+dht_readv2 (xlator_t *this, xlator_t *subvol, call_frame_t *frame)
 {
         dht_local_t *local  = NULL;
-        xlator_t    *subvol = NULL;
         int          op_errno = EINVAL;
 
         local = frame->local;
@@ -438,11 +435,10 @@ dht_readv2 (xlator_t *this, call_frame_t *frame, int op_ret)
                 goto out;
 
         op_errno = local->op_errno;
-        if (op_ret == -1)
+        if (subvol == NULL)
                 goto out;
 
         local->call_cnt = 2;
-        subvol = local->cached_subvol;
 
         STACK_WIND (frame, dht_readv_cbk, subvol, subvol->fops->readv,
                     local->fd, local->rebalance.size, local->rebalance.offset,
@@ -547,10 +543,9 @@ out:
 }
 
 int
-dht_access2 (xlator_t *this, call_frame_t *frame, int op_ret)
+dht_access2 (xlator_t *this, xlator_t *subvol, call_frame_t *frame)
 {
-        dht_local_t *local  = NULL;
-        xlator_t    *subvol = NULL;
+        dht_local_t *local    = NULL;
         int          op_errno = EINVAL;
 
         local = frame->local;
@@ -558,11 +553,10 @@ dht_access2 (xlator_t *this, call_frame_t *frame, int op_ret)
                 goto out;
 
         op_errno = local->op_errno;
-        if (op_ret == -1)
+        if (subvol == NULL)
                 goto out;
 
         local->call_cnt = 2;
-        subvol = local->cached_subvol;
 
         STACK_WIND (frame, dht_access_cbk, subvol, subvol->fops->access,
                     &local->loc, local->rebalance.flags, NULL);
@@ -636,7 +630,7 @@ dht_flush_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
         /* If context is set, then send flush() it to the destination */
         dht_inode_ctx_get1 (this, inode, &subvol);
         if (subvol) {
-                dht_flush2 (this, frame, 0);
+                dht_flush2 (this, subvol, frame);
                 return 0;
         }
 
@@ -647,17 +641,20 @@ out:
 }
 
 int
-dht_flush2 (xlator_t *this, call_frame_t *frame, int op_ret)
+dht_flush2 (xlator_t *this, xlator_t *subvol, call_frame_t *frame)
 {
-        dht_local_t  *local  = NULL;
-        xlator_t     *subvol = NULL;
+        dht_local_t *local    = NULL;
+        int32_t      op_errno = EINVAL;
+
+        if ((frame == NULL) || (frame->local == NULL))
+                goto out;
 
         local = frame->local;
 
-        dht_inode_ctx_get1 (this, local->fd->inode, &subvol);
+        op_errno = local->op_errno;
 
-        if (!subvol)
-                subvol = local->cached_subvol;
+        if (subvol == NULL)
+                goto out;
 
         local->call_cnt = 2; /* This is the second attempt */
 
@@ -665,6 +662,10 @@ dht_flush2 (xlator_t *this, call_frame_t *frame, int op_ret)
                     subvol, subvol->fops->flush, local->fd, NULL);
 
         return 0;
+
+out:
+        DHT_STACK_UNWIND (flush, frame, -1, op_errno, NULL);
+        return 0;
 }
 
 
@@ -758,7 +759,7 @@ dht_fsync_cbk (call_frame_t *frame, void *cookie, xlator_t *this, int op_ret,
                 if (!ret)
                         return 0;
         } else {
-                dht_fsync2 (this, frame, 0);
+                dht_fsync2 (this, subvol, frame);
                 return 0;
         }
 
@@ -772,16 +773,19 @@ out:
 }
 
 int
-dht_fsync2 (xlator_t *this, call_frame_t *frame, int op_ret)
+dht_fsync2 (xlator_t *this, xlator_t *subvol, call_frame_t *frame)
 {
-        dht_local_t  *local  = NULL;
-        xlator_t     *subvol = NULL;
+        dht_local_t *local    = NULL;
+        int32_t      op_errno = EINVAL;
+
+        if ((frame == NULL) || (frame->local == NULL))
+                goto out;
 
         local = frame->local;
+        op_errno = local->op_errno;
 
-        dht_inode_ctx_get1 (this, local->fd->inode, &subvol);
-        if (!subvol)
-                subvol = local->cached_subvol;
+        if (subvol == NULL)
+                goto out;
 
         local->call_cnt = 2; /* This is the second attempt */
 
@@ -789,6 +793,10 @@ dht_fsync2 (xlator_t *this, call_frame_t *frame, int op_ret)
                     local->fd, local->rebalance.flags, NULL);
 
         return 0;
+
+out:
+        DHT_STACK_UNWIND (fsync, frame, -1, op_errno, NULL, NULL, NULL);
+        return 0;
 }
 
 int