cluster/afr: perform list-xattr during lookup

Detect and heal mismatching user extended attributes during lookup.
'Forward' port of http://review.gluster.org/#/c/7444/

Change-Id: Id03c9746f083ffd3014711d0b3a2e5a71a45eed4
BUG: 1134691
Signed-off-by: Ravishankar N <ravishankar@redhat.com>
Reviewed-on: http://review.gluster.org/8558
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Pranith Kumar Karampuri <pkarampu@redhat.com>
Tested-by: Pranith Kumar Karampuri <pkarampu@redhat.com>

6 files changed, 273 insertions, 12 deletions

diff --git a/tests/bugs/bug-1130892.t b/tests/bugs/bug-1130892.t
index 438e795d571..206778f2938 100644
--- a/tests/bugs/bug-1130892.t
+++ b/tests/bugs/bug-1130892.t
@@ -49,7 +49,7 @@ TEST stat $M0/one
 # Check pending xattrs
 EXPECT "00000000" afr_get_specific_changelog_xattr $B0/${V0}-0/one trusted.afr.$V0-client-1 data
 EXPECT_NOT "00000000" afr_get_specific_changelog_xattr $B0/${V0}-0/one trusted.afr.$V0-client-1 entry
-EXPECT_NOT "00000000" afr_get_specific_changelog_xattr $B0/${V0}-0/one trusted.afr.$V0-client-1 metadata
+EXPECT "00000000" afr_get_specific_changelog_xattr $B0/${V0}-0/one trusted.afr.$V0-client-1 metadata
 
 TEST gluster volume set $V0 self-heal-daemon on
 EXPECT_WITHIN $PROCESS_UP_TIMEOUT "Y" glustershd_up_status
diff --git a/tests/bugs/bug-1134691-afr-lookup-metadata-heal.t b/tests/bugs/bug-1134691-afr-lookup-metadata-heal.t
new file mode 100644
index 00000000000..1fb1732a33f
--- /dev/null
+++ b/tests/bugs/bug-1134691-afr-lookup-metadata-heal.t
@@ -0,0 +1,50 @@
+#!/bin/bash
+#### Test iatt and user xattr heal from lookup code path ####
+
+. $(dirname $0)/../include.rc
+. $(dirname $0)/../volume.rc
+cleanup;
+
+TEST glusterd
+TEST pidof glusterd
+TEST $CLI volume create $V0 replica 3 $H0:$B0/brick{0,1,2}
+TEST $CLI volume start $V0
+TEST $CLI volume set $V0 cluster.self-heal-daemon off
+TEST glusterfs --volfile-id=/$V0 --volfile-server=$H0 $M0 --attribute-timeout=0 --entry-timeout=0
+
+cd $M0
+TEST touch file
+TEST setfattr -n user.attribute1 -v "value" $B0/brick0/file
+TEST kill_brick $V0 $H0 $B0/brick2
+TEST chmod +x file
+iatt=$(stat -c "%g:%u:%A" file)
+
+TEST $CLI volume start $V0 force
+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" afr_child_up_status $V0 2
+
+#Trigger metadataheal
+TEST stat file
+
+#iattrs must be matching
+iatt1=$(stat -c "%g:%u:%A" $B0/brick0/file)
+iatt2=$(stat -c "%g:%u:%A" $B0/brick1/file)
+iatt3=$(stat -c "%g:%u:%A" $B0/brick2/file)
+EXPECT $iatt echo $iatt1
+EXPECT $iatt echo $iatt2
+EXPECT $iatt echo $iatt3
+
+#xattrs must be matching
+xatt1_cnt=$(getfattr -d $B0/brick0/file|wc|awk '{print $1}')
+xatt2_cnt=$(getfattr -d $B0/brick1/file|wc|awk '{print $1}')
+xatt3_cnt=$(getfattr -d $B0/brick2/file|wc|awk '{print $1}')
+EXPECT "$xatt1_cnt" echo $xatt2_cnt
+EXPECT "$xatt1_cnt" echo $xatt3_cnt
+
+#changelogs must be zero
+xattr1=$(get_hex_xattr trusted.afr.$V0-client-2 $B0/brick0/file)
+xattr2=$(get_hex_xattr trusted.afr.$V0-client-2 $B0/brick1/file)
+EXPECT "000000000000000000000000" echo $xattr1
+EXPECT "000000000000000000000000" echo $xattr2
+
+cd -
+cleanup;
diff --git a/xlators/cluster/afr/src/afr-common.c b/xlators/cluster/afr/src/afr-common.c
index e9a05de2546..f290c1f8d1c 100644
--- a/xlators/cluster/afr/src/afr-common.c
+++ b/xlators/cluster/afr/src/afr-common.c
@@ -670,6 +670,12 @@ afr_xattr_req_prepare (xlator_t *this, dict_t *xattr_req)
                         "query flag");
         }
 
+        ret = dict_set_int32 (xattr_req, "list-xattr", 1);
+        if (ret) {
+                gf_log (this->name, GF_LOG_DEBUG,
+                        "Unable to set list-xattr in dict ");
+        }
+
 	return ret;
 }
 
@@ -1140,6 +1146,73 @@ afr_handle_quota_size (call_frame_t *frame, xlator_t *this)
 	}
 }
 
+static char *afr_ignore_xattrs[] = {
+        GLUSTERFS_OPEN_FD_COUNT,
+        GLUSTERFS_PARENT_ENTRYLK,
+        GLUSTERFS_ENTRYLK_COUNT,
+        GLUSTERFS_INODELK_COUNT,
+        GF_SELINUX_XATTR_KEY,
+        QUOTA_SIZE_KEY,
+        NULL
+};
+
+static  gf_boolean_t
+afr_lookup_xattr_ignorable (char *key)
+{
+        int i = 0;
+
+        if (!strncmp (key, AFR_XATTR_PREFIX, strlen(AFR_XATTR_PREFIX)))
+                return _gf_true;
+        for (i = 0; afr_ignore_xattrs[i]; i++) {
+                if (!strcmp (key, afr_ignore_xattrs[i]))
+                       return _gf_true;
+        }
+        return _gf_false;
+}
+
+int
+xattr_is_equal (dict_t *this, char *key1, data_t *value1, void *data)
+{
+        dict_t *xattr2 = (dict_t *)data;
+        data_t *value2 = NULL;
+
+        if (afr_lookup_xattr_ignorable (key1))
+                return 0;
+
+        value2 = dict_get (xattr2, key1);
+        if (!value2)
+                return -1;
+
+        if (value1->len != value2->len)
+                return -1;
+        if(memcmp(value1->data, value2->data, value1->len))
+                return -1;
+        else
+                return 0;
+
+}
+
+/* To conclude that both dicts are equal, we need to check if
+ * 1) For every key-val pair in dict1, a match is present in dict2
+ * 2) For every key-val pair in dict2, a match is present in dict1
+ * We need to do both because ignoring glusterfs' internal xattrs
+ * happens only in xattr_is_equal().
+ */
+static gf_boolean_t
+dicts_are_equal (dict_t *dict1, dict_t *dict2)
+{
+        int ret = 0;
+
+        ret = dict_foreach (dict1, xattr_is_equal, dict2);
+        if (ret == -1)
+                return _gf_false;
+
+        ret = dict_foreach (dict2, xattr_is_equal, dict1);
+        if (ret == -1)
+                 return _gf_false;
+
+        return _gf_true;
+}
 
 static void
 afr_lookup_done (call_frame_t *frame, xlator_t *this)
@@ -1437,6 +1510,121 @@ afr_attempt_local_discovery (xlator_t *this, int32_t child_index)
                            &tmploc, GF_XATTR_PATHINFO_KEY, NULL);
 }
 
+int
+afr_lookup_sh_metadata_wrap (void *opaque)
+{
+        call_frame_t *frame       = opaque;
+        afr_local_t  *local       = NULL;
+        xlator_t     *this        = NULL;
+        inode_t      *inode       = NULL;
+        afr_private_t *priv       = NULL;
+        struct afr_reply *replies = NULL;
+        int i= 0, first = -1;
+
+        local = frame->local;
+        this  = frame->this;
+        priv  = this->private;
+        replies = local->replies;
+
+        for (i =0; i < priv->child_count; i++) {
+                if(!replies[i].valid || replies[i].op_ret == -1)
+                        continue;
+                first = i;
+                break;
+        }
+        if (first == -1)
+                goto out;
+
+        inode = afr_inode_link (local->inode,&replies[first].poststat);
+        if(!inode)
+                goto out;
+
+        afr_selfheal_metadata (frame, this, inode);
+        inode_forget (inode, 1);
+        inode_unref (inode);
+
+        afr_local_replies_wipe (local, this->private);
+        inode = afr_selfheal_unlocked_lookup_on (frame, local->loc.parent,
+                                                 local->loc.name, local->replies,
+                                                 local->child_up, NULL);
+        if (inode)
+                inode_unref (inode);
+out:
+        afr_lookup_done (frame, this);
+
+        return 0;
+}
+
+static gf_boolean_t
+afr_can_start_metadata_self_heal(call_frame_t *frame, xlator_t *this)
+{
+        afr_local_t *local = NULL;
+        afr_private_t *priv = NULL;
+        struct afr_reply *replies = NULL;
+        int i = 0, first = -1;
+        gf_boolean_t start = _gf_false;
+        struct iatt stbuf = {0, };
+
+        local = frame->local;
+        replies = local->replies;
+        priv = this->private;
+
+        for (i = 0; i < priv->child_count; i++) {
+                if(!replies[i].valid || replies[i].op_ret == -1)
+                        continue;
+                if (first == -1) {
+                        first = i;
+                        stbuf = replies[i].poststat;
+                        continue;
+                }
+
+                if (uuid_compare (stbuf.ia_gfid, replies[i].poststat.ia_gfid)) {
+                        start = _gf_false;
+                        break;
+                }
+                if (!IA_EQUAL (stbuf, replies[i].poststat, type)) {
+                        start = _gf_false;
+                        break;
+                }
+
+                /*Check if iattrs need heal*/
+                if ((!IA_EQUAL (stbuf, replies[i].poststat, uid)) ||
+                    (!IA_EQUAL (stbuf, replies[i].poststat, gid)) ||
+                    (!IA_EQUAL (stbuf, replies[i].poststat, prot))) {
+                        start = _gf_true;
+                        continue;
+                }
+
+                /*Check if xattrs need heal*/
+                if (!dicts_are_equal (replies[first].xdata, replies[i].xdata))
+                        start = _gf_true;
+        }
+
+        return start;
+}
+
+int
+afr_lookup_metadata_heal_check (call_frame_t *frame, xlator_t *this)
+
+{
+        call_frame_t *heal = NULL;
+        int ret            = 0;
+
+        if (!afr_can_start_metadata_self_heal (frame, this))
+                goto out;
+
+        heal = copy_frame (frame);
+        if (heal)
+                heal->root->pid = GF_CLIENT_PID_AFR_SELF_HEALD;
+        ret = synctask_new (this->ctx->env, afr_lookup_sh_metadata_wrap,
+                            afr_refresh_selfheal_done, heal, frame);
+        if(ret)
+                goto out;
+        return ret;
+out:
+        afr_lookup_done (frame, this);
+        return ret;
+}
 
 int
 afr_lookup_selfheal_wrap (void *opaque)
@@ -1463,7 +1651,7 @@ afr_lookup_selfheal_wrap (void *opaque)
 	if (inode)
 		inode_unref (inode);
 
-	afr_lookup_done (frame, this);
+        afr_lookup_metadata_heal_check(frame, this);
         return 0;
 
 unwind:
@@ -1519,11 +1707,11 @@ afr_lookup_entry_heal (call_frame_t *frame, xlator_t *this)
 		ret = synctask_new (this->ctx->env, afr_lookup_selfheal_wrap,
 				    afr_refresh_selfheal_done, heal, frame);
 		if (ret)
-			goto lookup_done;
-	} else {
-	lookup_done:
-		afr_lookup_done (frame, this);
+			goto metadata_heal;
+                return ret;
 	}
+metadata_heal:
+        ret = afr_lookup_metadata_heal_check (frame, this);
 
 	return ret;
 }
diff --git a/xlators/cluster/afr/src/afr-self-heal-metadata.c b/xlators/cluster/afr/src/afr-self-heal-metadata.c
index dc7825d3d16..e98728ba54f 100644
--- a/xlators/cluster/afr/src/afr-self-heal-metadata.c
+++ b/xlators/cluster/afr/src/afr-self-heal-metadata.c
@@ -82,12 +82,12 @@ __afr_selfheal_metadata_do (call_frame_t *frame, xlator_t *this, inode_t *inode,
 
 
 /*
- * Look for mismatching uid/gid or mode even if xattrs don't say so, and
- * pick one arbitrarily as winner.
- */
+ * Look for mismatching uid/gid or mode or user xattrs even if
+ * AFR xattrs don't say so, and pick one arbitrarily as winner. */
 
 static int
-__afr_selfheal_metadata_finalize_source (xlator_t *this, unsigned char *sources,
+__afr_selfheal_metadata_finalize_source (call_frame_t *frame, xlator_t *this,
+                                         unsigned char *sources,
 					 unsigned char *healed_sinks,
 					 unsigned char *locked_on,
 					 struct afr_reply *replies)
@@ -97,6 +97,7 @@ __afr_selfheal_metadata_finalize_source (xlator_t *this, unsigned char *sources,
 	struct iatt first = {0, };
 	int source = -1;
 	int sources_count = 0;
+        int ret = 0;
 
 	priv = this->private;
 
@@ -124,11 +125,12 @@ __afr_selfheal_metadata_finalize_source (xlator_t *this, unsigned char *sources,
 		if (source == -1) {
 			source = i;
 			first = replies[i].poststat;
+                        break;
 		}
 	}
 
 	for (i = 0; i < priv->child_count; i++) {
-		if (!sources[i])
+		if (!sources[i] || i == source)
 			continue;
 		if (!IA_EQUAL (first, replies[i].poststat, type) ||
 		    !IA_EQUAL (first, replies[i].poststat, uid) ||
@@ -139,6 +141,22 @@ __afr_selfheal_metadata_finalize_source (xlator_t *this, unsigned char *sources,
 		}
 	}
 
+        for (i =0; i < priv->child_count; i++) {
+		if (!sources[i] || i == source)
+			continue;
+                if (replies[source].xdata->count != replies[i].xdata->count) {
+                        sources[i] = 0;
+                        healed_sinks[i] = 1;
+                        continue;
+                }
+                ret = dict_foreach(replies[source].xdata, xattr_is_equal,
+                                        (void*)replies[i].xdata);
+                if  (ret == -1) {
+                        sources[i] = 0;
+                        healed_sinks[i] = 1;
+                }
+        }
+
 	return source;
 }
 
@@ -176,7 +194,7 @@ __afr_selfheal_metadata_prepare (call_frame_t *frame, xlator_t *this, inode_t *i
         */
         AFR_INTERSECT (healed_sinks, sinks, locked_on, priv->child_count);
 
-	source = __afr_selfheal_metadata_finalize_source (this, sources,
+	source = __afr_selfheal_metadata_finalize_source (frame, this, sources,
                                                           healed_sinks,
 							  locked_on, replies);
 	if (source < 0)
diff --git a/xlators/cluster/afr/src/afr-self-heal.h b/xlators/cluster/afr/src/afr-self-heal.h
index 2713ffa09bf..31f12a4e74a 100644
--- a/xlators/cluster/afr/src/afr-self-heal.h
+++ b/xlators/cluster/afr/src/afr-self-heal.h
@@ -179,4 +179,6 @@ afr_selfheal_newentry_mark (call_frame_t *frame, xlator_t *this, inode_t *inode,
                             int source, struct afr_reply *replies,
                             unsigned char *sources, unsigned char *newentry);
 
+inode_t*
+afr_inode_link (inode_t *inode, struct iatt *iatt);
 #endif /* !_AFR_SELFHEAL_H */
diff --git a/xlators/cluster/afr/src/afr.h b/xlators/cluster/afr/src/afr.h
index 4adbd5d5c83..c8224e16ab7 100644
--- a/xlators/cluster/afr/src/afr.h
+++ b/xlators/cluster/afr/src/afr.h
@@ -776,6 +776,9 @@ int32_t
 afr_notify (xlator_t *this, int32_t event, void *data, void *data2);
 
 int
+xattr_is_equal (dict_t *this, char *key1, data_t *value1, void *data);
+
+int
 afr_init_entry_lockee (afr_entry_lockee_t *lockee, afr_local_t *local,
                        loc_t *loc, char *basename, int child_count);