afr: assign gfid during name heal when no 'source' is present

Backport of https://review.gluster.org/#/c/glusterfs/+/21297/

Problem:
If parent dir is in split-brain or has dirty xattrs set, and the file
has gfid missing on one of the bricks, then name heal won't assign the
gfid.

Fix:
Use the brick we select the gfid from as the 'source'.

Note: Problem was found while trying to debug a split-brain issue on
Cynthia Zhou's setup.

fixes: bz#1655561
Change-Id: Id088d4f0fb017aa35122de426654194e581ed742
Reported-by: Cynthia Zhou <cynthia.zhou@nokia-sbell.com>
Signed-off-by: Ravishankar N <ravishankar@redhat.com>

4 files changed, 55 insertions, 51 deletions

diff --git a/xlators/cluster/afr/src/afr-self-heal-common.c b/xlators/cluster/afr/src/afr-self-heal-common.c
index 44d724112a9..a05c90f1b33 100644
--- a/xlators/cluster/afr/src/afr-self-heal-common.c
+++ b/xlators/cluster/afr/src/afr-self-heal-common.c
@@ -22,7 +22,8 @@ afr_heal_synctask (xlator_t *this, afr_local_t *local);
 int
 afr_lookup_and_heal_gfid (xlator_t *this, inode_t *parent, const char *name,
                           inode_t *inode, struct afr_reply *replies,
-                          int source,  unsigned char *sources, void *gfid)
+                          int source,  unsigned char *sources, void *gfid,
+                          int *gfid_idx)
 {
         afr_private_t *priv = NULL;
         call_frame_t   *frame    = NULL;
@@ -36,15 +37,30 @@ afr_lookup_and_heal_gfid (xlator_t *this, inode_t *parent, const char *name,
 
         priv = this->private;
         wind_on = alloca0 (priv->child_count);
-        ia_type = replies[source].poststat.ia_type;
-        if ((ia_type == IA_INVAL) &&
-            (AFR_COUNT(sources, priv->child_count) == priv->child_count)) {
-                /* If a file is present on some bricks of the replica but parent
-                 * dir does not have pending xattrs, all bricks are sources and
-                 * the 'source' we  selected earlier might be one where the file
-                 * is not actually present. Hence check if file is present in
-                 * any of the sources.*/
-                for (i = 0; i < priv->child_count; i++) {
+        if (source >= 0 && replies[source].valid && replies[source].op_ret == 0)
+                ia_type = replies[source].poststat.ia_type;
+
+        if (ia_type != IA_INVAL)
+                goto heal;
+
+        /* If ia_type is still invalid, it means either
+         * (a)'source' was -1, i.e. parent dir pending xattrs are in split-brain
+         * (or) (b) The parent dir pending xattrs are all zeroes (i.e. all bricks
+         * are sources) and the 'source' we selected earlier might be the one where
+         * the file is not actually present.
+         *
+         * In both cases, let us pick a brick with a successful reply and use its
+         * ia_type.
+         */
+        for (i = 0; i < priv->child_count; i++) {
+                if (source == -1) {
+                        /* case (a) above. */
+                        if (replies[i].valid && replies[i].op_ret == 0) {
+                                ia_type = replies[i].poststat.ia_type;
+                                break;
+                        }
+                } else {
+                        /* case (b) above. */
                         if (i == source)
                                 continue;
                         if (sources[i] && replies[i].valid &&
@@ -55,6 +71,7 @@ afr_lookup_and_heal_gfid (xlator_t *this, inode_t *parent, const char *name,
                 }
         }
 
+heal:
         /* gfid heal on those subvolumes that do not have gfid associated
          * with the inode and update those replies.
          */
@@ -104,7 +121,22 @@ afr_lookup_and_heal_gfid (xlator_t *this, inode_t *parent, const char *name,
                 afr_reply_wipe (&replies[i]);
                 afr_reply_copy (&replies[i], &local->replies[i]);
         }
+        if (gfid_idx && (*gfid_idx == -1)) {
+                /*Pick a brick where the gifd heal was successful.*/
+                for (i = 0; i < priv->child_count; i++) {
+                        if (!wind_on[i])
+                                continue;
+                        if (replies[i].valid && replies[i].op_ret == 0 &&
+                            !gf_uuid_is_null(replies[i].poststat.ia_gfid)) {
+                                *gfid_idx = i;
+                                break;
+                        }
+                }
+        }
 out:
+        if (gfid_idx && (*gfid_idx == -1) && (ret == 0)) {
+                ret = -afr_final_errno(local, priv);
+        }
         loc_wipe (&loc);
         if (frame)
                 AFR_STACK_DESTROY (frame);
diff --git a/xlators/cluster/afr/src/afr-self-heal-entry.c b/xlators/cluster/afr/src/afr-self-heal-entry.c
index 8514f0df595..cb03a3f9717 100644
--- a/xlators/cluster/afr/src/afr-self-heal-entry.c
+++ b/xlators/cluster/afr/src/afr-self-heal-entry.c
@@ -188,7 +188,8 @@ __afr_selfheal_heal_dirent (call_frame_t *frame, xlator_t *this, fd_t *fd,
         if (replies[source].op_ret == 0) {
                 ret = afr_lookup_and_heal_gfid (this, fd->inode, name,
                                                 inode, replies, source, sources,
-                                             &replies[source].poststat.ia_gfid);
+                                              &replies[source].poststat.ia_gfid,
+                                              NULL);
                 if (ret)
                         return ret;
         }
@@ -321,7 +322,8 @@ __afr_selfheal_merge_dirent (call_frame_t *frame, xlator_t *this, fd_t *fd,
 
         ret = afr_lookup_and_heal_gfid (this, fd->inode, name, inode, replies,
                                         source, sources,
-                                        &replies[source].poststat.ia_gfid);
+                                        &replies[source].poststat.ia_gfid,
+                                        NULL);
         if (ret)
                 return ret;
 
diff --git a/xlators/cluster/afr/src/afr-self-heal-name.c b/xlators/cluster/afr/src/afr-self-heal-name.c
index 0a5be29d5ee..869bdbd49c6 100644
--- a/xlators/cluster/afr/src/afr-self-heal-name.c
+++ b/xlators/cluster/afr/src/afr-self-heal-name.c
@@ -19,7 +19,8 @@ __afr_selfheal_assign_gfid (xlator_t *this, inode_t *parent, uuid_t pargfid,
                             const char *bname, inode_t *inode,
                             struct afr_reply *replies, void *gfid,
                             unsigned char *locked_on, int source,
-                            unsigned char *sources, gf_boolean_t is_gfid_absent)
+                            unsigned char *sources, gf_boolean_t is_gfid_absent,
+                            int *gfid_idx)
 {
 	int             ret          = 0;
         int             up_count     = 0;
@@ -48,7 +49,7 @@ __afr_selfheal_assign_gfid (xlator_t *this, inode_t *parent, uuid_t pargfid,
         }
 
         afr_lookup_and_heal_gfid (this, parent, bname, inode, replies, source,
-                                  sources, gfid);
+                                  sources, gfid, gfid_idx);
 
 out:
 	return ret;
@@ -152,35 +153,6 @@ __afr_selfheal_name_expunge (xlator_t *this, inode_t *parent, uuid_t pargfid,
 
 }
 
-/* This function is to be called after ensuring that there is no gfid mismatch
- * for the inode across multiple sources
- */
-static int
-afr_selfheal_gfid_idx_get (xlator_t *this, struct afr_reply *replies,
-                           unsigned char *sources)
-{
-        int             i        =  0;
-        int             gfid_idx = -1;
-        afr_private_t  *priv     =  NULL;
-
-        priv = this->private;
-
-        for (i = 0; i < priv->child_count; i++) {
-                if (!replies[i].valid || replies[i].op_ret != 0)
-                        continue;
-
-                if (!sources[i])
-                        continue;
-
-                if (gf_uuid_is_null (replies[i].poststat.ia_gfid))
-                        continue;
-
-                gfid_idx = i;
-                break;
-        }
-        return gfid_idx;
-}
-
 static gf_boolean_t
 afr_selfheal_name_need_heal_check (xlator_t *this, struct afr_reply *replies)
 {
@@ -421,21 +393,19 @@ __afr_selfheal_name_do (call_frame_t *frame, xlator_t *this, inode_t *parent,
                 gfid = gfid_req;
         } else {
                 gfid = &replies[gfid_idx].poststat.ia_gfid;
+                if (source == -1)
+                        /* Either entry split-brain or dirty xattrs are
+                        * present on parent.*/
+                        source = gfid_idx;
         }
 
         is_gfid_absent = (gfid_idx == -1) ? _gf_true : _gf_false;
 	ret = __afr_selfheal_assign_gfid (this, parent, pargfid, bname, inode,
                                           replies, gfid, locked_on, source,
-                                          sources, is_gfid_absent);
+                                          sources, is_gfid_absent, &gfid_idx);
         if (ret)
                 return ret;
 
-        if (gfid_idx == -1) {
-                gfid_idx = afr_selfheal_gfid_idx_get (this, replies, sources);
-                if (gfid_idx == -1)
-                        return -1;
-        }
-
 	ret = __afr_selfheal_name_impunge (frame, this, parent, pargfid,
                                            bname, inode, replies, gfid_idx);
         if (ret == -EIO)
diff --git a/xlators/cluster/afr/src/afr-self-heal.h b/xlators/cluster/afr/src/afr-self-heal.h
index bac5e51341e..d7491e1522d 100644
--- a/xlators/cluster/afr/src/afr-self-heal.h
+++ b/xlators/cluster/afr/src/afr-self-heal.h
@@ -113,7 +113,7 @@ afr_selfheal_entry (call_frame_t *frame, xlator_t *this, inode_t *inode);
 int
 afr_lookup_and_heal_gfid (xlator_t *this, inode_t *parent, const char *name,
                           inode_t *inode, struct afr_reply *replies, int source,
-                          unsigned char *sources, void *gfid);
+                          unsigned char *sources, void *gfid, int *gfid_idx);
 
 int
 afr_selfheal_inodelk (call_frame_t *frame, xlator_t *this, inode_t *inode,