From cd16967a77d04a8d0c5d5c3ab5346ccdd1df89cb Mon Sep 17 00:00:00 2001
From: Sheetal Pamecha <sheetal.pamecha08@gmail.com>
Date: Fri, 30 Nov 2018 19:49:59 +0530
Subject: afr: Splitbrain with size as policy must not resolve for directory

In automatic Splitbrain resolution when favorite child policy
is set as size, split brain resolution must not work for
directories.

Currently, if a directory is in split brain with both copies
having same size, the source is selected arbitrarily
and healed.

fixes: bz#1655050
Change-Id: I5739498639c17c89874cc577362e543adab55f5d
Signed-off-by: Sheetal Pamecha <sheetal.pamecha08@gmail.com>
---
 .../replicate/bug-1655050-dir-sbrain-size-policy.t | 55 ++++++++++++++++++++++
 xlators/cluster/afr/src/afr-self-heal-common.c     | 14 +++++-
 2 files changed, 67 insertions(+), 2 deletions(-)
 create mode 100644 tests/bugs/replicate/bug-1655050-dir-sbrain-size-policy.t

diff --git a/tests/bugs/replicate/bug-1655050-dir-sbrain-size-policy.t b/tests/bugs/replicate/bug-1655050-dir-sbrain-size-policy.t
new file mode 100644
index 00000000000..63f72e86bf6
--- /dev/null
+++ b/tests/bugs/replicate/bug-1655050-dir-sbrain-size-policy.t
@@ -0,0 +1,55 @@
+#!/bin/bash
+. $(dirname $0)/../../include.rc
+. $(dirname $0)/../../volume.rc
+cleanup;
+
+TEST glusterd
+TEST pidof glusterd
+
+#Create replica 2 volume
+TEST $CLI volume create $V0 replica 2 $H0:$B0/${V0}{0,1}
+TEST $CLI volume set $V0 self-heal-daemon off
+TEST $CLI volume set $V0 data-self-heal off
+TEST $CLI volume set $V0 entry-self-heal off
+TEST $CLI volume set $V0 metadata-self-heal off
+TEST $CLI volume start $V0
+TEST glusterfs --volfile-id=$V0 --volfile-server=$H0 $M0;
+
+cd $M0
+TEST mkdir dir
+
+#Create metadata split-brain
+TEST kill_brick $V0 $H0 $B0/${V0}0
+TEST chmod 757 dir
+TEST $CLI volume start $V0 force
+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" brick_up_status $V0 $H0 $B0/${V0}0
+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" afr_child_up_status $V0 0
+TEST kill_brick $V0 $H0 $B0/${V0}1
+TEST chmod 747 dir
+TEST $CLI volume start $V0 force
+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" brick_up_status $V0 $H0 $B0/${V0}1
+EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" afr_child_up_status $V0 1
+
+#Use size as fav-child policy.
+TEST $CLI volume set $V0 cluster.favorite-child-policy size
+
+#Enable shd and heal the file.
+TEST $CLI volume set $V0 cluster.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
+
+EXPECT_WITHIN $HEAL_TIMEOUT "2" get_pending_heal_count $V0
+
+b1c1dir=$(afr_get_specific_changelog_xattr $B0/${V0}0/dir \
+          trusted.afr.$V0-client-1 "metadata")
+b2c0dir=$(afr_get_specific_changelog_xattr $B0/${V0}1/dir \
+          trusted.afr.$V0-client-0 "metadata")
+
+EXPECT "00000001" echo $b1c1dir
+EXPECT "00000001" echo $b2c0dir
+
+#Finish up
+TEST force_umount $M0
+cleanup;
diff --git a/xlators/cluster/afr/src/afr-self-heal-common.c b/xlators/cluster/afr/src/afr-self-heal-common.c
index 3caa6897424..ccbaca37784 100644
--- a/xlators/cluster/afr/src/afr-self-heal-common.c
+++ b/xlators/cluster/afr/src/afr-self-heal-common.c
@@ -1174,13 +1174,20 @@ afr_sh_fav_by_size(xlator_t *this, struct afr_reply *replies, inode_t *inode)
     uint64_t cmp_sz = 0;
 
     priv = this->private;
-
     for (i = 0; i < priv->child_count; i++) {
         if (replies[i].valid == 1) {
             gf_msg_debug(this->name, 0,
                          "Child:%s file size = %" PRIu64 " for gfid %s",
                          priv->children[i]->name, replies[i].poststat.ia_size,
                          uuid_utoa(inode->gfid));
+            if (replies[i].poststat.ia_type == IA_IFDIR) {
+                gf_msg(this->name, GF_LOG_ERROR, 0,
+                       AFR_MSG_SBRAIN_FAV_CHILD_POLICY,
+                       "Cannot perform selfheal on %s. "
+                       "Size policy is not applicable to directories.",
+                       uuid_utoa(inode->gfid));
+                break;
+            }
             if (replies[i].poststat.ia_size > cmp_sz) {
                 cmp_sz = replies[i].poststat.ia_size;
                 fav_child = i;
@@ -1252,7 +1259,10 @@ afr_mark_split_brain_source_sinks_by_policy(
     priv = this->private;
 
     fav_child = afr_sh_get_fav_by_policy(this, replies, inode, &policy_str);
-    if (fav_child > priv->child_count - 1) {
+    if (fav_child == -1) {
+        gf_msg(this->name, GF_LOG_ERROR, 0, AFR_MSG_SBRAIN_FAV_CHILD_POLICY,
+               "No child selected by favorite-child policy.");
+    } else if (fav_child > priv->child_count - 1) {
         gf_msg(this->name, GF_LOG_ERROR, 0, AFR_MSG_SBRAIN_FAV_CHILD_POLICY,
                "Invalid child (%d) "
                "selected by policy %s.",
-- 
cgit