From d77a9d1b2ba6aa582685d14e4301e4ea888f34bd Mon Sep 17 00:00:00 2001

From: Anuradha Talur <atalur@redhat.com>

Date: Fri, 26 Jun 2015 11:40:00 +0530

Subject: [PATCH 146/190] cluster/afr : set pending xattrs for replaced brick

       Backport of: http://review.gluster.org/10448/

Patch for bug 1140649 was already merged downstream,

commit-id : a4f226f50a2ef943b5db095877bab5a3eebf7283 .

There were a few amendments to be made.

This patch contains delta changes b/w :

1) http://review.gluster.org/10448/ &

2) https://code.engineering.redhat.com/gerrit/#/c/51021/

Change-Id: Iad0c89578706b0b7e424b271977908e830c5236b

BUG: 1140649

Signed-off-by: Anuradha Talur <atalur@redhat.com>

Reviewed-on: https://code.engineering.redhat.com/gerrit/51660

Reviewed-by: Ravishankar Narayanankutty <ravishankar@redhat.com>

Tested-by: Ravishankar Narayanankutty <ravishankar@redhat.com>

---

 tests/basic/afr/replace-brick-self-heal.t     |   33 +++++----

 xlators/cluster/afr/src/afr-inode-write.c     |  102 ++++++++-----------------

 xlators/cluster/afr/src/afr-messages.h        |   12 +++-

 xlators/cluster/afr/src/afr-self-heal-entry.c |    7 ++

 4 files changed, 69 insertions(+), 85 deletions(-)

diff --git a/tests/basic/afr/replace-brick-self-heal.t b/tests/basic/afr/replace-brick-self-heal.t

index 1901466..8ced7df 100644

--- a/tests/basic/afr/replace-brick-self-heal.t

+++ b/tests/basic/afr/replace-brick-self-heal.t

@@ -3,16 +3,6 @@

 . $(dirname $0)/../../volume.rc

 cleanup;

-function match_dirs {

-        diff <(ls $1 | sort) <(ls $2 | sort)

-        if [ $? -eq 0 ];

-        then

-                echo "Y"

-        else

-                echo "N"

-        fi

-}

-

 TEST glusterd

 TEST pidof glusterd

 TEST $CLI volume create $V0 replica 2 $H0:$B0/${V0}{0,1}

@@ -24,12 +14,15 @@ TEST $CLI volume set $V0 cluster.entry-self-heal off

 TEST $CLI volume set $V0 self-heal-daemon off

 TEST glusterfs --volfile-id=$V0 --volfile-server=$H0 $M0;

-#Create files

+# Create files

 for i in {1..5}

 do

         echo $i > $M0/file$i.txt

 done

+# Metadata changes

+TEST setfattr -n user.test -v qwerty $M0/file5.txt

+

 # Replace brick1

 TEST $CLI volume replace-brick $V0 $H0:$B0/${V0}1 $H0:$B0/${V0}1_new commit force

@@ -45,15 +38,27 @@ EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" afr_child_up_status $V0 1

 TEST $CLI volume set $V0 self-heal-daemon on

 EXPECT_WITHIN $PROCESS_UP_TIMEOUT "Y" glustershd_up_status

+EXPECT_WITHIN $CHILD_UP_TIMEOUT "1" afr_child_up_status_in_shd $V0 0

+EXPECT_WITHIN $CHILD_UP_TIMEOUT "1" afr_child_up_status_in_shd $V0 1

 TEST $CLI volume heal $V0

-# Check if heal has happened

-EXPECT_WITHIN $HEAL_TIMEOUT "Y" match_dirs $B0/${V0}0 $B0/${V0}1_new

+# Wait for heal to complete

+EXPECT_WITHIN $HEAL_TIMEOUT "0" afr_get_pending_heal_count $V0

-# To make sure that data was not lost from brick0

+# Check if entry-heal has happened

+TEST diff <(ls $B0/${V0}0 | sort) <(ls $B0/${V0}1_new | sort)

+

+# To make sure that files were not lost from brick0

 TEST diff <(ls $B0/${V0}0 | sort) <(ls $B0/${V0}1 | sort)

 EXPECT "000000000000000000000000" get_hex_xattr trusted.afr.$V0-client-1 $B0/${V0}0

 # Test if data was healed

+TEST diff $B0/${V0}0/file1.txt $B0/${V0}1_new/file1.txt

+# To make sure that data was not lost from brick0

 TEST diff $B0/${V0}0/file1.txt $B0/${V0}1/file1.txt

+

+# Test if metadata was healed and exists on both the bricks

+EXPECT "qwerty" get_text_xattr user.test $B0/${V0}1_new/file5.txt

+EXPECT "qwerty" get_text_xattr user.test $B0/${V0}0/file5.txt

+

 cleanup;

diff --git a/xlators/cluster/afr/src/afr-inode-write.c b/xlators/cluster/afr/src/afr-inode-write.c

index 5d32927..8800b81 100644

--- a/xlators/cluster/afr/src/afr-inode-write.c

+++ b/xlators/cluster/afr/src/afr-inode-write.c

@@ -38,6 +38,7 @@

 #include "byte-order.h"

 #include "afr-transaction.h"

 #include "afr-self-heal.h"

+#include "afr-messages.h"

 static void

 __afr_inode_write_finalize (call_frame_t *frame, xlator_t *this)

@@ -979,42 +980,26 @@ afr_rb_set_pending_changelog_cbk (call_frame_t *frame, void *cookie,

 {

         afr_local_t *local = NULL;

+        afr_private_t *priv = NULL;

         int i = 0;

         local = frame->local;

+        priv = this->private;

         i = (long) cookie;

         local->replies[i].valid = 1;

         local->replies[i].op_ret = op_ret;

         local->replies[i].op_errno = op_errno;

+        gf_msg (this->name, op_ret ? GF_LOG_ERROR : GF_LOG_INFO,

+                op_ret ? op_errno : 0,

+                AFR_MSG_REPLACE_BRICK_STATUS, "Set of pending xattr %s on"

+                " %s.", op_ret ? "failed" : "succeeded",

+                priv->children[i]->name);

         syncbarrier_wake (&local->barrier);

         return 0;

 }

-char *

-afr_opret_matrix_generate (afr_private_t *priv, afr_local_t *local)

-{

-        char *matrix = NULL;

-        char *ptr    = NULL;

-        int   i      = 0;

-

-        /* Allocate max amount of chars required, including -ve values

-         * and spaces */

-        matrix = GF_CALLOC (priv->child_count, 3 * sizeof (char),

-                            gf_afr_mt_char);

-        if (!matrix)

-                return NULL;

-        ptr = matrix;

-        for (i = 0; i < priv->child_count; i++) {

-                if (local->replies[i].valid)

-                        ptr += sprintf (ptr, "%d ", local->replies[i].op_ret);

-                else

-                        ptr += sprintf (ptr, "-1 ");

-        }

-        return matrix;

-}

-

 int

 afr_rb_set_pending_changelog (call_frame_t *frame, xlator_t *this,

                               unsigned char *locked_nodes)

@@ -1022,7 +1007,6 @@ afr_rb_set_pending_changelog (call_frame_t *frame, xlator_t *this,

         afr_local_t *local = NULL;

         afr_private_t *priv = NULL;

         int ret = 0, i = 0;

-        char *matrix = NULL;

         local = frame->local;

         priv = this->private;

@@ -1033,22 +1017,18 @@ afr_rb_set_pending_changelog (call_frame_t *frame, xlator_t *this,

         /* It is sufficient if xattrop was successful on one child */

         for (i = 0; i < priv->child_count; i++) {

-                if (local->replies[i].valid &&

-                    local->replies[i].op_ret == 0) {

-                        matrix = afr_opret_matrix_generate (priv, local);

-                        gf_log (this->name, GF_LOG_DEBUG, "Successfully set "

-                                "pending changelog. op_ret matrix : [ %s].",

-                                matrix);

+                if (!local->replies[i].valid)

+                        continue;

+

+                if (local->replies[i].op_ret == 0) {

                         ret = 0;

                         goto out;

+                } else {

+                        ret = afr_higher_errno (ret,

+                                                local->replies[i].op_errno);

                 }

-                ret = afr_higher_errno (ret, local->replies[i].op_errno);

         }

-        gf_log (this->name, GF_LOG_ERROR, "Couldn't set pending xattr "

-                "on any child. (%s)", strerror (ret));

 out:

-        if (matrix)

-                GF_FREE (matrix);

         return -ret;

 }

@@ -1060,9 +1040,6 @@ _afr_handle_replace_brick_type (xlator_t *this, call_frame_t *frame,

         afr_local_t     *local            = NULL;

         afr_private_t   *priv             = NULL;

         unsigned char   *locked_nodes     = NULL;

-        struct           gf_flock flock   = {0, };

-        struct           gf_flock unflock = {0, };

-        int              i                = 0;

         int              count            = 0;

         int              ret              = -ENOMEM;

         int              idx              = -1;

@@ -1079,10 +1056,7 @@ _afr_handle_replace_brick_type (xlator_t *this, call_frame_t *frame,

         if (!local->pending)

                 goto out;

-        for (i = 0; i < priv->child_count; i++) {

-                if (i == rb_index)

-                        local->pending[i][idx] = hton32 (1);

-        }

+        local->pending[rb_index][idx] = hton32 (1);

         local->xdata_req = dict_new ();

         if (!local->xdata_req)

@@ -1093,16 +1067,13 @@ _afr_handle_replace_brick_type (xlator_t *this, call_frame_t *frame,

                 goto out;

         if (AFR_ENTRY_TRANSACTION == type) {

-                AFR_SEQ (frame, afr_selfheal_lock_cbk, entrylk, this->name,

-                         loc, NULL, ENTRYLK_LOCK, ENTRYLK_WRLCK, NULL);

+                count = afr_selfheal_entrylk (frame, this, loc->inode,

+                                              this->name, NULL, locked_nodes);

         } else {

-                flock.l_type = F_WRLCK;

-                flock.l_start = LLONG_MAX - 1;

-                flock.l_len = 0;

-                AFR_SEQ (frame, afr_selfheal_lock_cbk, inodelk, this->name,

-                         loc, F_SETLKW, &flock, NULL);

+                count = afr_selfheal_inodelk (frame, this, loc->inode,

+                                              this->name, LLONG_MAX - 1, 0,

+                                              locked_nodes);

         }

-        count = afr_locked_fill (frame, this, locked_nodes);

         if (!count) {

                 gf_log (this->name, GF_LOG_ERROR, "Couldn't acquire lock on"

@@ -1117,15 +1088,11 @@ _afr_handle_replace_brick_type (xlator_t *this, call_frame_t *frame,

         ret = 0;

 unlock:

         if (AFR_ENTRY_TRANSACTION == type) {

-                AFR_ONLIST (locked_nodes, frame, afr_selfheal_lock_cbk,

-                            entrylk, this->name, loc, NULL, ENTRYLK_UNLOCK,

-                            ENTRYLK_WRLCK, NULL);

+                afr_selfheal_unentrylk (frame, this, loc->inode, this->name,

+                                        NULL, locked_nodes);

         } else {

-                unflock.l_type = F_UNLCK;

-                unflock.l_start = LLONG_MAX - 1;

-                unflock.l_len = 0;

-                AFR_ONLIST (locked_nodes, frame, afr_selfheal_lock_cbk,

-                            inodelk, this->name, loc, F_SETLK, &unflock, NULL);

+                afr_selfheal_uninodelk (frame, this, loc->inode, this->name,

+                                        LLONG_MAX - 1, 0, locked_nodes);

         }

 out:

         return ret;

@@ -1162,6 +1129,8 @@ _afr_handle_replace_brick (xlator_t *this, call_frame_t *frame, loc_t *loc,

         dict_unref (local->xdata_req);

         afr_matrix_cleanup (local->pending, priv->child_count);

+        local->pending = NULL;

+        local->xdata_req = NULL;

         ret = _afr_handle_replace_brick_type (this, frame, loc, rb_index,

                                               AFR_ENTRY_TRANSACTION);

@@ -1379,21 +1348,14 @@ int

 afr_handle_replace_brick (xlator_t *this, call_frame_t *frame, loc_t *loc,

                           dict_t *dict)

 {

-        int             len               = 0;

         int             ret               = -1;

         int             rb_index          = -1;

-        int             op_errno          = EPERM;

-        void           *value             = NULL;

         char           *replace_brick     = NULL;

-        ret =  dict_get_ptr_and_len (dict, GF_AFR_REPLACE_BRICK, &value,

-                                     &len;;

-

-        if (value) {

-                replace_brick = alloca0 (len + 1);

-                memcpy (replace_brick, value, len);

+        ret =  dict_get_str (dict, GF_AFR_REPLACE_BRICK, &replace_brick);

-                if (!(frame->root->pid == GF_CLIENT_PID_AFR_SELF_HEALD)) {

+        if (!ret) {

+                if (frame->root->pid != GF_CLIENT_PID_AFR_SELF_HEALD) {

                         ret = 1;

                         goto out;

                 }

@@ -1412,8 +1374,8 @@ out:

         if (ret == 1) {

                 gf_log (this->name, GF_LOG_ERROR, "'%s' is an internal"

                         " extended attribute : %s.",

-                        GF_AFR_REPLACE_BRICK, strerror (op_errno));

-                AFR_STACK_UNWIND (setxattr, frame, -1, op_errno, NULL);

+                        GF_AFR_REPLACE_BRICK, strerror (EPERM));

+                AFR_STACK_UNWIND (setxattr, frame, -1, EPERM, NULL);

                 ret = 0;

         }

         return ret;

diff --git a/xlators/cluster/afr/src/afr-messages.h b/xlators/cluster/afr/src/afr-messages.h

index 52bdead..66b8ed1 100644

--- a/xlators/cluster/afr/src/afr-messages.h

+++ b/xlators/cluster/afr/src/afr-messages.h

@@ -45,7 +45,7 @@

  */

 #define GLFS_COMP_BASE_AFR GLFS_MSGID_COMP_AFR

-#define GLFS_NUM_MESSAGES 10

+#define GLFS_NUM_MESSAGES 11

 #define GLFS_MSGID_END (GLFS_COMP_BASE_AFR + GLFS_NUM_MESSAGES + 1)

 #define glfs_msg_start_x GLFS_COMP_BASE_AFR, "Invalid: Start of messages"

@@ -142,6 +142,16 @@

 #define AFR_MSG_INODE_UNLOCK_FAIL       (GLFS_COMP_BASE_AFR + 10)

+/*!

+ * @messageid 108011

+ * @diagnosis Setting of pending xattrs succeeded/failed during replace-brick

+ * operation.

+ * @recommendedaction In case of failure, error number in the log should give

+ * the reason why it failed. Also observe brick logs for more information.

+*/

+#define AFR_MSG_REPLACE_BRICK_STATUS     (GLFS_COMP_BASE_AFR + 11)

+

+

 #define glfs_msg_end_x GLFS_MSGID_END, "Invalid: End of messages"

 #endif /* !_AFR_MESSAGES_H_ */

diff --git a/xlators/cluster/afr/src/afr-self-heal-entry.c b/xlators/cluster/afr/src/afr-self-heal-entry.c

index e64b6e4..12da920 100644

--- a/xlators/cluster/afr/src/afr-self-heal-entry.c

+++ b/xlators/cluster/afr/src/afr-self-heal-entry.c

@@ -295,6 +295,13 @@ __afr_selfheal_merge_dirent (call_frame_t *frame, xlator_t *this, fd_t *fd,

 		return 0;

 	}

+        /* Set all the sources as 1, otheriwse newentry_mark won't be set */

+	for (i = 0; i < priv->child_count; i++) {

+		if (replies[i].valid && replies[i].op_ret == 0) {

+			sources[i] = 1;

+		}

+	}

+

         /* In case of a gfid or type mismatch on the entry, return -1.*/

         ret = afr_selfheal_detect_gfid_and_type_mismatch (this, replies,

                                                           fd->inode->gfid,

-- 

1.7.1