a3470f
From 3d79f49f2c7752f8f43a35563f7a1253c901db60 Mon Sep 17 00:00:00 2001
a3470f
From: Ravishankar N <ravishankar@redhat.com>
a3470f
Date: Tue, 27 Mar 2018 20:54:25 +0530
a3470f
Subject: [PATCH 227/236] afr: add quorum checks in pre-op
a3470f
a3470f
Upstream patch: https://review.gluster.org/#/c/19781/
a3470f
a3470f
Problem:
a3470f
We seem to be winding the FOP if pre-op did not succeed on quorum bricks
a3470f
and then failing the FOP with EROFS since the fop did not meet quorum.
a3470f
This essentially masks the actual error due to which pre-op failed. (See
a3470f
BZ).
a3470f
a3470f
Fix:
a3470f
Skip FOP phase if pre-op quorum is not met and go to post-op.
a3470f
a3470f
Change-Id: Ie58a41e8fa1ad79aa06093706e96db8eef61b6d9
a3470f
BUG: 1554291
a3470f
Signed-off-by: Ravishankar N <ravishankar@redhat.com>
a3470f
Reviewed-on: https://code.engineering.redhat.com/gerrit/136227
a3470f
Tested-by: RHGS Build Bot <nigelb@redhat.com>
a3470f
Reviewed-by: Pranith Kumar Karampuri <pkarampu@redhat.com>
a3470f
---
a3470f
 xlators/cluster/afr/src/afr-transaction.c | 64 +++++++++++++++----------------
a3470f
 1 file changed, 31 insertions(+), 33 deletions(-)
a3470f
a3470f
diff --git a/xlators/cluster/afr/src/afr-transaction.c b/xlators/cluster/afr/src/afr-transaction.c
a3470f
index 993029d..88dc821 100644
a3470f
--- a/xlators/cluster/afr/src/afr-transaction.c
a3470f
+++ b/xlators/cluster/afr/src/afr-transaction.c
a3470f
@@ -144,6 +144,29 @@ afr_needs_changelog_update (afr_local_t *local)
a3470f
         return _gf_false;
a3470f
 }
a3470f
 
a3470f
+gf_boolean_t
a3470f
+afr_changelog_has_quorum (afr_local_t *local, xlator_t *this)
a3470f
+{
a3470f
+        afr_private_t *priv = NULL;
a3470f
+        int i = 0;
a3470f
+        unsigned char *success_children = NULL;
a3470f
+
a3470f
+        priv = this->private;
a3470f
+        success_children = alloca0 (priv->child_count);
a3470f
+
a3470f
+        for (i = 0; i < priv->child_count; i++) {
a3470f
+                if (!local->transaction.failed_subvols[i]) {
a3470f
+                        success_children[i] = 1;
a3470f
+                }
a3470f
+        }
a3470f
+
a3470f
+        if (afr_has_quorum (success_children, this)) {
a3470f
+                return _gf_true;
a3470f
+        }
a3470f
+
a3470f
+        return _gf_false;
a3470f
+}
a3470f
+
a3470f
 int
a3470f
 afr_transaction_fop (call_frame_t *frame, xlator_t *this)
a3470f
 {
a3470f
@@ -157,17 +180,16 @@ afr_transaction_fop (call_frame_t *frame, xlator_t *this)
a3470f
         priv = this->private;
a3470f
 
a3470f
         failed_subvols = local->transaction.failed_subvols;
a3470f
-
a3470f
         call_count = priv->child_count - AFR_COUNT (failed_subvols,
a3470f
                                                     priv->child_count);
a3470f
-
a3470f
-        if (call_count == 0) {
a3470f
+        /* Fail if pre-op did not succeed on quorum no. of bricks. */
a3470f
+        if (!afr_changelog_has_quorum (local, this) || !call_count) {
a3470f
+                local->op_ret = -1;
a3470f
+                /* local->op_errno is already captured in changelog cbk. */
a3470f
                 afr_transaction_resume (frame, this);
a3470f
                 return 0;
a3470f
         }
a3470f
-
a3470f
         local->call_count = call_count;
a3470f
-
a3470f
         for (i = 0; i < priv->child_count; i++) {
a3470f
                 if (local->transaction.pre_op[i] && !failed_subvols[i]) {
a3470f
 			local->transaction.wind (frame, this, i);
a3470f
@@ -531,33 +553,6 @@ afr_set_pending_dict (afr_private_t *priv, dict_t *xattr, int **pending)
a3470f
 
a3470f
 /* {{{ pending */
a3470f
 
a3470f
-
a3470f
-void
a3470f
-afr_handle_post_op_quorum (afr_local_t *local, xlator_t *this)
a3470f
-{
a3470f
-        afr_private_t *priv = NULL;
a3470f
-        int i = 0;
a3470f
-        unsigned char *post_op_children = NULL;
a3470f
-
a3470f
-        priv = this->private;
a3470f
-        post_op_children = alloca0 (priv->child_count);
a3470f
-
a3470f
-        for (i = 0; i < priv->child_count; i++) {
a3470f
-                if (!local->transaction.failed_subvols[i]) {
a3470f
-                        post_op_children[i] = 1;
a3470f
-                }
a3470f
-        }
a3470f
-
a3470f
-        if (afr_has_quorum (post_op_children, this)) {
a3470f
-                return;
a3470f
-        }
a3470f
-
a3470f
-        local->op_ret = -1;
a3470f
-        /*local->op_errno is already captured in post-op callback.*/
a3470f
-
a3470f
-        return;
a3470f
-}
a3470f
-
a3470f
 int
a3470f
 afr_changelog_post_op_done (call_frame_t *frame, xlator_t *this)
a3470f
 {
a3470f
@@ -568,7 +563,10 @@ afr_changelog_post_op_done (call_frame_t *frame, xlator_t *this)
a3470f
         int_lock = &local->internal_lock;
a3470f
 
a3470f
         /* Fail the FOP if post-op did not succeed on quorum no. of bricks. */
a3470f
-        afr_handle_post_op_quorum (local, this);
a3470f
+        if (!afr_changelog_has_quorum (local, this)) {
a3470f
+                local->op_ret = -1;
a3470f
+                /*local->op_errno is already captured in changelog cbk*/
a3470f
+        }
a3470f
 
a3470f
 	if (local->transaction.resume_stub) {
a3470f
 		call_resume (local->transaction.resume_stub);
a3470f
-- 
a3470f
1.8.3.1
a3470f