cluster/afr: Perform new entry mark before creating new entry

There is a chance for the source brick to go down just after
the new entry is created and before source brick is marked with
necessary pending markers. If after this any I/O happens then
new entry will become source and reverse heal will happen.
To prevent this mark the pending xattrs before creating the new
entry.

 >BUG: 1417466
 >Change-Id: I233b87e694d32e5d734df5a83b4d2ca711c17503
 >Signed-off-by: Pranith Kumar K <pkarampu@redhat.com>
 >Reviewed-on: https://review.gluster.org/16474
 >Smoke: Gluster Build System <jenkins@build.gluster.org>
 >NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>
 >CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
 >Reviewed-by: Ravishankar N <ravishankar@redhat.com>
 >Reviewed-by: Krutika Dhananjay <kdhananj@redhat.com>

BUG: 1429312
Change-Id: Ia1bdaf9511acaeff72a336c8185a56a64ea0e2ba
Signed-off-by: Pranith Kumar K <pkarampu@redhat.com>
Reviewed-on: https://review.gluster.org/16850
Smoke: Gluster Build System <jenkins@build.gluster.org>
NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>
Reviewed-by: Ravishankar N <ravishankar@redhat.com>
CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
Reviewed-by: Niels de Vos <ndevos@redhat.com>

4 files changed, 51 insertions, 49 deletions

diff --git a/xlators/cluster/afr/src/afr-self-heal-common.c b/xlators/cluster/afr/src/afr-self-heal-common.c
index a70833174c0..6d1ae1312ea 100644
--- a/xlators/cluster/afr/src/afr-self-heal-common.c
+++ b/xlators/cluster/afr/src/afr-self-heal-common.c
@@ -26,6 +26,8 @@ afr_selfheal_post_op_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
 
 	local = frame->local;
 
+        local->op_ret = op_ret;
+        local->op_errno = op_errno;
 	syncbarrier_wake (&local->barrier);
 
 	return 0;
@@ -39,6 +41,7 @@ afr_selfheal_post_op (call_frame_t *frame, xlator_t *this, inode_t *inode,
 	afr_private_t *priv = NULL;
 	afr_local_t *local = NULL;
 	loc_t loc = {0, };
+        int ret = 0;
 
 	priv = this->private;
 	local = frame->local;
@@ -46,15 +49,20 @@ afr_selfheal_post_op (call_frame_t *frame, xlator_t *this, inode_t *inode,
 	loc.inode = inode_ref (inode);
 	gf_uuid_copy (loc.gfid, inode->gfid);
 
+        local->op_ret = 0;
+
 	STACK_WIND (frame, afr_selfheal_post_op_cbk, priv->children[subvol],
 		    priv->children[subvol]->fops->xattrop, &loc,
 		    GF_XATTROP_ADD_ARRAY, xattr, xdata);
 
 	syncbarrier_wait (&local->barrier, 1);
+        if (local->op_ret < 0)
+                ret = -local->op_errno;
 
         loc_wipe (&loc);
+        local->op_ret = 0;
 
-	return 0;
+	return ret;
 }
 
 int
@@ -1977,13 +1985,16 @@ afr_selfheal_newentry_mark (call_frame_t *frame, xlator_t *this, inode_t *inode,
         changelog = afr_mark_pending_changelog (priv, newentry, xattr,
                                             replies[source].poststat.ia_type);
 
-        if (!changelog)
+        if (!changelog) {
+                ret = -ENOMEM;
                 goto out;
+        }
 
 	for (i = 0; i < priv->child_count; i++) {
 		if (!sources[i])
 			continue;
-		afr_selfheal_post_op (frame, this, inode, i, xattr, NULL);
+		ret |= afr_selfheal_post_op (frame, this, inode, i, xattr,
+                                             NULL);
 	}
 out:
         if (changelog)
diff --git a/xlators/cluster/afr/src/afr-self-heal-entry.c b/xlators/cluster/afr/src/afr-self-heal-entry.c
index 9ab566b8242..5b536b0ded8 100644
--- a/xlators/cluster/afr/src/afr-self-heal-entry.c
+++ b/xlators/cluster/afr/src/afr-self-heal-entry.c
@@ -75,26 +75,29 @@ afr_selfheal_entry_delete (xlator_t *this, inode_t *dir, const char *name,
 
 
 int
-afr_selfheal_recreate_entry (xlator_t *this, int dst, int source, inode_t *dir,
+afr_selfheal_recreate_entry (call_frame_t *frame, int dst, int source,
+                             unsigned char *sources, inode_t *dir,
                              const char *name, inode_t *inode,
-                             struct afr_reply *replies,
-                             unsigned char *newentry)
+                             struct afr_reply *replies)
 {
 	int ret = 0;
 	loc_t loc = {0,};
 	loc_t srcloc = {0,};
+        xlator_t *this = frame->this;
 	afr_private_t *priv = NULL;
 	dict_t *xdata = NULL;
 	struct iatt *iatt = NULL;
 	char *linkname = NULL;
 	mode_t mode = 0;
 	struct iatt newent = {0,};
-	priv = this->private;
+        unsigned char *newentry = NULL;
 
+        priv = this->private;
 	xdata = dict_new();
 	if (!xdata)
 		return -ENOMEM;
 
+        newentry = alloca0 (priv->child_count);
 	loc.parent = inode_ref (dir);
 	gf_uuid_copy (loc.pargfid, dir->gfid);
 	loc.name = name;
@@ -113,6 +116,15 @@ afr_selfheal_recreate_entry (xlator_t *this, int dst, int source, inode_t *dir,
 
 	srcloc.inode = inode_ref (inode);
 	gf_uuid_copy (srcloc.gfid, iatt->ia_gfid);
+        if (iatt->ia_type != IA_IFDIR)
+                ret = syncop_lookup (priv->children[dst], &srcloc, 0, 0, 0, 0);
+        if (iatt->ia_type == IA_IFDIR || ret == -ENOENT || ret == -ESTALE) {
+                newentry[dst] = 1;
+                ret = afr_selfheal_newentry_mark (frame, this, inode, source,
+                                                  replies, sources, newentry);
+                if (ret)
+                        goto out;
+        }
 
 	mode = st_mode_from_ia (iatt->ia_prot, iatt->ia_type);
 
@@ -120,12 +132,9 @@ afr_selfheal_recreate_entry (xlator_t *this, int dst, int source, inode_t *dir,
 	case IA_IFDIR:
 		ret = syncop_mkdir (priv->children[dst], &loc, mode, 0,
                                     xdata, NULL);
-                if (ret == 0)
-                        newentry[dst] = 1;
 		break;
 	case IA_IFLNK:
-		ret = syncop_lookup (priv->children[dst], &srcloc, 0, 0, 0, 0);
-		if (ret == 0) {
+		if (!newentry[dst]) {
 			ret = syncop_link (priv->children[dst], &srcloc, &loc,
 					   &newent, NULL, NULL);
 		} else {
@@ -135,8 +144,6 @@ afr_selfheal_recreate_entry (xlator_t *this, int dst, int source, inode_t *dir,
 				goto out;
 			ret = syncop_symlink (priv->children[dst], &loc,
                                               linkname, NULL, xdata, NULL);
-                        if (ret == 0)
-                                newentry[dst] = 1;
                 }
 		break;
 	default:
@@ -146,10 +153,6 @@ afr_selfheal_recreate_entry (xlator_t *this, int dst, int source, inode_t *dir,
 		ret = syncop_mknod (priv->children[dst], &loc, mode,
                     makedev (ia_major(iatt->ia_rdev), ia_minor (iatt->ia_rdev)),
                     &newent, xdata, NULL);
-		if (ret == 0 && newent.ia_nlink == 1) {
-			/* New entry created. Mark @dst pending on all sources */
-                        newentry[dst] = 1;
-		}
 		break;
 	}
 
@@ -172,12 +175,9 @@ __afr_selfheal_heal_dirent (call_frame_t *frame, xlator_t *this, fd_t *fd,
 	int ret = 0;
 	afr_private_t *priv = NULL;
 	int i = 0;
-	unsigned char *newentry = NULL;
 
 	priv = this->private;
 
-	newentry = alloca0 (priv->child_count);
-
 	if (!replies[source].valid)
 		return -EIO;
 
@@ -200,17 +200,15 @@ __afr_selfheal_heal_dirent (call_frame_t *frame, xlator_t *this, fd_t *fd,
 					   replies[source].poststat.ia_gfid))
 				continue;
 
-			ret = afr_selfheal_recreate_entry (this, i, source,
-							   fd->inode, name, inode,
-							   replies, newentry);
+			ret = afr_selfheal_recreate_entry (frame, i, source,
+                                                           sources, fd->inode,
+                                                           name, inode,
+							   replies);
                 }
 		if (ret < 0)
 			break;
 	}
 
-	if (AFR_COUNT (newentry, priv->child_count))
-		afr_selfheal_newentry_mark (frame, this, inode, source, replies,
-					    sources, newentry);
 	return ret;
 }
 
@@ -278,13 +276,10 @@ __afr_selfheal_merge_dirent (call_frame_t *frame, xlator_t *this, fd_t *fd,
         int             ret       = 0;
         int             i         = 0;
         int             source    = -1;
-        unsigned char  *newentry  = NULL;
         afr_private_t  *priv      = NULL;
 
 	priv = this->private;
 
-	newentry = alloca0 (priv->child_count);
-
 	for (i = 0; i < priv->child_count; i++) {
 		if (replies[i].valid && replies[i].op_ret == 0) {
 			source = i;
@@ -319,14 +314,11 @@ __afr_selfheal_merge_dirent (call_frame_t *frame, xlator_t *this, fd_t *fd,
 		if (replies[i].op_errno != ENOENT)
 			continue;
 
-		ret = afr_selfheal_recreate_entry (this, i, source, fd->inode,
-                                                   name, inode, replies,
-                                                   newentry);
+		ret = afr_selfheal_recreate_entry (frame, i, source, sources,
+                                                   fd->inode, name, inode,
+                                                   replies);
 	}
 
-	if (AFR_COUNT (newentry, priv->child_count))
-		afr_selfheal_newentry_mark (frame, this, inode, source, replies,
-					    sources, newentry);
 	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 3445ecccf9c..b28ce4170f1 100644
--- a/xlators/cluster/afr/src/afr-self-heal-name.c
+++ b/xlators/cluster/afr/src/afr-self-heal-name.c
@@ -109,12 +109,10 @@ __afr_selfheal_name_impunge (call_frame_t *frame, xlator_t *this,
 	int i = 0;
 	afr_private_t *priv = NULL;
         int ret = 0;
-        unsigned char *newentry = NULL;
         unsigned char *sources = NULL;
 
         priv = this->private;
 
-	newentry = alloca0 (priv->child_count);
 	sources = alloca0 (priv->child_count);
 
 	gf_uuid_copy (parent->gfid, pargfid);
@@ -128,15 +126,17 @@ __afr_selfheal_name_impunge (call_frame_t *frame, xlator_t *this,
                         sources[i] = 1;
 			continue;
                 }
+        }
+
+        for (i = 0; i < priv->child_count; i++) {
+                if (sources[i])
+                        continue;
 
-		ret |= afr_selfheal_recreate_entry (this, i, gfid_idx, parent,
-                                                    bname, inode, replies,
-                                                    newentry);
+		ret |= afr_selfheal_recreate_entry (frame, i, gfid_idx, sources,
+                                                    parent, bname, inode,
+                                                    replies);
 	}
 
-	if (AFR_COUNT (newentry, priv->child_count))
-		afr_selfheal_newentry_mark (frame, this, inode, gfid_idx, replies,
-					    sources, newentry);
 	return ret;
 }
 
@@ -457,8 +457,7 @@ __afr_selfheal_name_do (call_frame_t *frame, xlator_t *this, inode_t *parent,
         }
 
 	ret = __afr_selfheal_name_impunge (frame, this, parent, pargfid,
-                                           bname, inode,
-                                           replies, gfid_idx);
+                                           bname, inode, replies, gfid_idx);
         if (ret == -EIO)
                 ret = -1;
 
diff --git a/xlators/cluster/afr/src/afr-self-heal.h b/xlators/cluster/afr/src/afr-self-heal.h
index c7092c7a7da..0a3d6482ca3 100644
--- a/xlators/cluster/afr/src/afr-self-heal.h
+++ b/xlators/cluster/afr/src/afr-self-heal.h
@@ -183,10 +183,10 @@ afr_selfheal_undo_pending (call_frame_t *frame, xlator_t *this, inode_t *inode,
                            unsigned char *locked_on);
 
 int
-afr_selfheal_recreate_entry (xlator_t *this, int dst, int source, inode_t *dir,
-                             const char *name, inode_t *inode,
-                             struct afr_reply *replies,
-                             unsigned char *newentry);
+afr_selfheal_recreate_entry (call_frame_t *frame, int dst, int source,
+                             unsigned char *sources,
+                             inode_t *dir, const char *name, inode_t *inode,
+                             struct afr_reply *replies);
 
 int
 afr_selfheal_post_op (call_frame_t *frame, xlator_t *this, inode_t *inode,