cluster/afr: Fixing the flaws in arbiter becoming source patch

Problem:
Setting the write_subvol value to read_subvol in case of metadata
transaction during pre-op (commit 19f9bcff4aada589d4321356c2670ed283f02c03)
might lead to the original problem of arbiter becoming source.

Scenario:
1) All bricks are up and good
2) 2 writes w1 and w2 are in progress in parallel
3) ctx->read_subvol is good for all the subvolumes
4) w1 succeeds on brick0 and fails on brick1, yet to do post-op on
   the disk
5) read/lookup comes on the same file and refreshes read_subvols back
   to all good
6) metadata transaction happens which makes ctx->write_subvol to be
   assigned with ctx->read_subvol which is all good
7) w2 succeeds on brick1 and fails on brick0 and this will update the
   brick in reverse order leading to arbiter becoming source

Fix:
Instead of setting the ctx->write_subvol to ctx->read_subvol in the
pre-op statge, if there is a metadata transaction, check in the
function __afr_set_in_flight_sb_status() if it is a data/metadata
transaction. Use the value of ctx->write_subvol if it is a data
transactions and ctx->read_subvol value for other transactions.

With this patch we assign the value of ctx->write_subvol in the
afr_transaction_perform_fop() with the on disk value, instead of
assigning it in the afr_changelog_pre_op() with the in memory value.

Change-Id: Id2025a7e965f0578af35b1abaac793b019c43cc4
BUG: 1482064
Signed-off-by: karthik-us <ksubrahm@redhat.com>

1 files changed, 25 insertions, 20 deletions

diff --git a/xlators/cluster/afr/src/afr-transaction.c b/xlators/cluster/afr/src/afr-transaction.c
index 440e8d1ed97..639f48687df 100644
--- a/xlators/cluster/afr/src/afr-transaction.c
+++ b/xlators/cluster/afr/src/afr-transaction.c
@@ -372,14 +372,27 @@ afr_txn_arbitrate_fop (call_frame_t *frame, xlator_t *this)
 int
 afr_transaction_perform_fop (call_frame_t *frame, xlator_t *this)
 {
-        afr_local_t     *local = NULL;
-        afr_private_t   *priv = NULL;
-        fd_t            *fd   = NULL;
+        afr_local_t   *local = NULL;
+        afr_private_t *priv  = NULL;
+        fd_t          *fd    = NULL;
+        int           i      = 0;
+        int           ret    = 0;
 
         local = frame->local;
         priv = this->private;
         fd    = local->fd;
 
+        if (local->transaction.type == AFR_DATA_TRANSACTION &&
+            !local->transaction.inherited) {
+                ret = afr_write_subvol_set (frame, this);
+                if (ret) {
+                        /*act as if operation failed on all subvols*/
+                        local->op_ret = -1;
+                        local->op_errno = -ret;
+                        for (i = 0; i < priv->child_count; i++)
+                                local->transaction.failed_subvols[i] = 1;
+                }
+        }
         /*  Perform fops with the lk-owner from top xlator.
          *  Eg: lk-owner of posix-lk and flush should be same,
          *  flush cant clear the  posix-lks without that lk-owner.
@@ -1116,32 +1129,28 @@ unlock:
 
 int
 afr_changelog_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
-		   int op_ret, int op_errno, dict_t *xattr, dict_t *xdata)
+                   int op_ret, int op_errno, dict_t *xattr, dict_t *xdata)
 {
         afr_local_t *local = NULL;
-        afr_private_t *priv = NULL;
         int call_count = -1;
         int child_index = -1;
 
         local = frame->local;
-        priv = this->private;
         child_index = (long) cookie;
 
-	if (op_ret == -1) {
+        if (op_ret == -1) {
                 local->op_errno = op_errno;
-		afr_transaction_fop_failed (frame, this, child_index);
+                afr_transaction_fop_failed (frame, this, child_index);
         }
 
-        if (priv->arbiter_count == 1 && !op_ret) {
-                if (xattr)
-                        local->transaction.pre_op_xdata[child_index] =
-                                                               dict_ref (xattr);
-        }
+        if (xattr)
+                local->transaction.pre_op_xdata[child_index] = dict_ref (xattr);
 
-	call_count = afr_frame_return (frame);
+        call_count = afr_frame_return (frame);
 
-        if (call_count == 0)
-		local->transaction.changelog_resume (frame, this);
+        if (call_count == 0) {
+                local->transaction.changelog_resume (frame, this);
+        }
 
         return 0;
 }
@@ -1748,10 +1757,6 @@ afr_changelog_pre_op (call_frame_t *frame, xlator_t *this)
 	if (pre_nop)
 		goto next;
 
-        ret = afr_write_subvol_set (frame, this);
-        if (ret)
-                goto err;
-
 	if (!local->pre_op_compat) {
 		dict_copy (xdata_req, local->xdata_req);
 		goto next;