cluster/afr: Don't let NFS cache stat after writes.

Problem:
Afr does post-ops after write but the stat buffer it unwinds is at the
time of write, so if nfs client caches this, it will see different
ctime when it does stat on it after post-op is done. From NFS client's
perspective it thinks the file is changed. Tar which depends on this
to be correct keeps giving 'file changed as we read it' warning.
If Afr instead has to choose to unwind after post-op, eager-lock,
delayed-post-op will have to be disabled which will lead to bad
performance for all write usecases.

Fix:
Don't let client cache stat after write.

 >Change-Id: Ic6062acc6e5cdd97a9c83c56bd529ec83cee8a23
 >BUG: 1302948
 >Signed-off-by: Pranith Kumar K <pkarampu@redhat.com>
 >Signed-off-by: Anuradha Talur <atalur@redhat.com>
 >Reviewed-on: http://review.gluster.org/13785
 >Smoke: Gluster Build System <jenkins@build.gluster.com>
 >NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>
 >CentOS-regression: Gluster Build System <jenkins@build.gluster.com>
 >Reviewed-by: Niels de Vos <ndevos@redhat.com>

BUG: 1312721
Change-Id: I42a5d524bcf2a2034fe48ee8454812ca26a98c37
Signed-off-by: Pranith Kumar K <pkarampu@redhat.com>
Reviewed-on: http://review.gluster.org/14454
NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>
CentOS-regression: Gluster Build System <jenkins@build.gluster.com>
Smoke: Gluster Build System <jenkins@build.gluster.com>

9 files changed, 100 insertions, 30 deletions

diff --git a/libglusterfs/src/common-utils.c b/libglusterfs/src/common-utils.c
index 40d9b637b7a..337d5ed00fc 100644
--- a/libglusterfs/src/common-utils.c
+++ b/libglusterfs/src/common-utils.c
@@ -4076,3 +4076,26 @@ list_node_del (struct list_node *node)
         GF_FREE (node);
 }
 
+gf_boolean_t
+gf_is_zero_filled_stat (struct iatt *buf)
+{
+        if (!buf)
+                return 1;
+
+        /* Do not use st_dev because it is transformed to store the xlator id
+         * in place of the device number. Do not use st_ino because by this time
+         * we've already mapped the root ino to 1 so it is not guaranteed to be
+         * 0.
+         */
+        if ((buf->ia_nlink == 0) && (buf->ia_ctime == 0))
+                return 1;
+
+        return 0;
+}
+
+void
+gf_zero_fill_stat (struct iatt *buf)
+{
+        buf->ia_nlink = 0;
+        buf->ia_ctime = 0;
+}
diff --git a/libglusterfs/src/common-utils.h b/libglusterfs/src/common-utils.h
index ca9ce78c845..bfcc480293a 100644
--- a/libglusterfs/src/common-utils.h
+++ b/libglusterfs/src/common-utils.h
@@ -790,4 +790,10 @@ gf_nwrite (int fd, const void *buf, size_t count);
 void _mask_cancellation (void);
 void _unmask_cancellation (void);
 
+gf_boolean_t
+gf_is_zero_filled_stat (struct iatt *buf);
+
+void
+gf_zero_fill_stat (struct iatt *buf);
+
 #endif /* _COMMON_UTILS_H */
diff --git a/xlators/cluster/afr/src/afr-dir-write.c b/xlators/cluster/afr/src/afr-dir-write.c
index 3d2b114628b..6d4a35228a1 100644
--- a/xlators/cluster/afr/src/afr-dir-write.c
+++ b/xlators/cluster/afr/src/afr-dir-write.c
@@ -239,7 +239,9 @@ __afr_dir_write_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
         afr_local_t *local = NULL;
         int child_index = (long) cookie;
         int call_count = -1;
+        afr_private_t *priv = NULL;
 
+        priv  = this->private;
         local = frame->local;
 
 	LOCK (&frame->lock);
@@ -254,8 +256,13 @@ __afr_dir_write_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
         if (call_count == 0) {
 		__afr_dir_write_finalize (frame, this);
 
-		if (afr_txn_nothing_failed (frame, this))
-			local->transaction.unwind (frame, this);
+		if (afr_txn_nothing_failed (frame, this)) {
+                        /*if it did pre-op, it will do post-op changing ctime*/
+                        if (priv->consistent_metadata &&
+                            afr_needs_changelog_update (local))
+                                afr_zero_fill_stat (local);
+                        local->transaction.unwind (frame, this);
+                }
 
 		afr_mark_entry_pending_changelog (frame, this);
 
diff --git a/xlators/cluster/afr/src/afr-inode-write.c b/xlators/cluster/afr/src/afr-inode-write.c
index 084a78ecf47..4fdf7b5bd0c 100644
--- a/xlators/cluster/afr/src/afr-inode-write.c
+++ b/xlators/cluster/afr/src/afr-inode-write.c
@@ -187,7 +187,9 @@ __afr_inode_write_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
         afr_local_t *local = NULL;
         int child_index = (long) cookie;
         int call_count = -1;
+        afr_private_t *priv = NULL;
 
+        priv = this->private;
         local = frame->local;
 
         LOCK (&frame->lock);
@@ -203,8 +205,13 @@ __afr_inode_write_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
         if (call_count == 0) {
 		__afr_inode_write_finalize (frame, this);
 
-		if (afr_txn_nothing_failed (frame, this))
-			local->transaction.unwind (frame, this);
+		if (afr_txn_nothing_failed (frame, this)) {
+                        /*if it did pre-op, it will do post-op changing ctime*/
+                        if (priv->consistent_metadata &&
+                            afr_needs_changelog_update (local))
+                                afr_zero_fill_stat (local);
+                        local->transaction.unwind (frame, this);
+                }
 
                 local->transaction.resume (frame, this);
         }
@@ -235,8 +242,13 @@ void
 afr_writev_unwind (call_frame_t *frame, xlator_t *this)
 {
         afr_local_t *   local = NULL;
+        afr_private_t *priv = this->private;
+
         local = frame->local;
 
+       if (priv->consistent_metadata)
+               afr_zero_fill_stat (local);
+
         AFR_STACK_UNWIND (writev, frame,
                           local->op_ret, local->op_errno,
                           &local->cont.inode_wfop.prebuf,
diff --git a/xlators/cluster/afr/src/afr-transaction.c b/xlators/cluster/afr/src/afr-transaction.c
index f3e4e9841a1..bbfc3ab9677 100644
--- a/xlators/cluster/afr/src/afr-transaction.c
+++ b/xlators/cluster/afr/src/afr-transaction.c
@@ -30,6 +30,37 @@ int
 afr_changelog_do (call_frame_t *frame, xlator_t *this, dict_t *xattr,
 		  afr_changelog_resume_t changelog_resume);
 
+void
+afr_zero_fill_stat (afr_local_t *local)
+{
+        if (!local)
+                return;
+        if (local->transaction.type == AFR_DATA_TRANSACTION ||
+            local->transaction.type == AFR_METADATA_TRANSACTION) {
+                gf_zero_fill_stat (&local->cont.inode_wfop.prebuf);
+                gf_zero_fill_stat (&local->cont.inode_wfop.postbuf);
+        } else if (local->transaction.type == AFR_ENTRY_TRANSACTION ||
+                   local->transaction.type == AFR_ENTRY_RENAME_TRANSACTION) {
+                gf_zero_fill_stat (&local->cont.dir_fop.buf);
+                gf_zero_fill_stat (&local->cont.dir_fop.preparent);
+                gf_zero_fill_stat (&local->cont.dir_fop.postparent);
+                if (local->transaction.type == AFR_ENTRY_TRANSACTION)
+                        return;
+                gf_zero_fill_stat (&local->cont.dir_fop.prenewparent);
+                gf_zero_fill_stat (&local->cont.dir_fop.postnewparent);
+        }
+}
+
+gf_boolean_t
+afr_needs_changelog_update (afr_local_t *local)
+{
+        if (local->transaction.type == AFR_DATA_TRANSACTION)
+                return _gf_true;
+        if (!local->optimistic_change_log)
+                return _gf_true;
+        return _gf_false;
+}
+
 static int32_t
 afr_quorum_errno (afr_private_t *priv)
 {
@@ -79,9 +110,21 @@ int
 __afr_txn_write_done (call_frame_t *frame, xlator_t *this)
 {
         afr_local_t *local = NULL;
+        afr_private_t *priv = NULL;
+        gf_boolean_t unwind = _gf_false;
 
+        priv  = this->private;
         local = frame->local;
 
+        if (priv->consistent_metadata) {
+                LOCK (&frame->lock);
+                {
+                        unwind = (local->transaction.main_frame != NULL);
+                }
+                UNLOCK (&frame->lock);
+                if (unwind)/*It definitely did post-op*/
+                        afr_zero_fill_stat (local);
+        }
         local->transaction.unwind (frame, this);
 
         AFR_STACK_DESTROY (frame);
@@ -1164,8 +1207,7 @@ afr_changelog_pre_op (call_frame_t *frame, xlator_t *this)
 		goto err;
 	}
 
-	if ((local->transaction.type == AFR_DATA_TRANSACTION ||
-	     !local->optimistic_change_log)) {
+	if (afr_needs_changelog_update (local)) {
 
 		local->dirty[idx] = hton32(1);
 
diff --git a/xlators/cluster/afr/src/afr-transaction.h b/xlators/cluster/afr/src/afr-transaction.h
index 47d43d88991..c58531eff44 100644
--- a/xlators/cluster/afr/src/afr-transaction.h
+++ b/xlators/cluster/afr/src/afr-transaction.h
@@ -52,5 +52,7 @@ int __afr_txn_write_fop (call_frame_t *frame, xlator_t *this);
 int __afr_txn_write_done (call_frame_t *frame, xlator_t *this);
 call_frame_t *afr_transaction_detach_fop_frame (call_frame_t *frame);
 gf_boolean_t afr_has_quorum (unsigned char *subvols, xlator_t *this);
+gf_boolean_t afr_needs_changelog_update (afr_local_t *local);
+void afr_zero_fill_stat (afr_local_t *local);
 
 #endif /* __TRANSACTION_H__ */
diff --git a/xlators/nfs/server/src/nfs-common.c b/xlators/nfs/server/src/nfs-common.c
index cfb0edd1294..5bca4592461 100644
--- a/xlators/nfs/server/src/nfs-common.c
+++ b/xlators/nfs/server/src/nfs-common.c
@@ -111,25 +111,6 @@ nfs_mntpath_to_xlator (xlator_list_t *cl, char *path)
 }
 
 
-/* Returns 1 if the stat seems to be filled with zeroes. */
-int
-nfs_zero_filled_stat (struct iatt *buf)
-{
-        if (!buf)
-                return 1;
-
-        /* Do not use st_dev because it is transformed to store the xlator id
-         * in place of the device number. Do not use st_ino because by this time
-         * we've already mapped the root ino to 1 so it is not guaranteed to be
-         * 0.
-         */
-        if ((buf->ia_nlink == 0) && (buf->ia_ctime == 0))
-                return 1;
-
-        return 0;
-}
-
-
 void
 nfs_loc_wipe (loc_t *loc)
 {
diff --git a/xlators/nfs/server/src/nfs-common.h b/xlators/nfs/server/src/nfs-common.h
index 01b49c1eb7a..58536626a0c 100644
--- a/xlators/nfs/server/src/nfs-common.h
+++ b/xlators/nfs/server/src/nfs-common.h
@@ -42,9 +42,6 @@ nfs_path_to_xlator (xlator_list_t *cl, char *path);
 extern xlator_t *
 nfs_mntpath_to_xlator (xlator_list_t *cl, char *path);
 
-extern int
-nfs_zero_filled_stat (struct iatt *buf);
-
 extern void
 nfs_loc_wipe (loc_t *loc);
 
diff --git a/xlators/nfs/server/src/nfs3-helpers.c b/xlators/nfs/server/src/nfs3-helpers.c
index bf2594f261d..229bfee560a 100644
--- a/xlators/nfs/server/src/nfs3-helpers.c
+++ b/xlators/nfs/server/src/nfs3-helpers.c
@@ -378,7 +378,7 @@ nfs3_stat_to_post_op_attr (struct iatt *buf)
          * returning these zeroed out attrs.
          */
         attr.attributes_follow = FALSE;
-        if (nfs_zero_filled_stat (buf))
+        if (gf_is_zero_filled_stat (buf))
                 goto out;
 
         nfs3_stat_to_fattr3 (buf, &(attr.post_op_attr_u.attributes));
@@ -399,7 +399,7 @@ nfs3_stat_to_pre_op_attr (struct iatt *pre)
          * returning these zeroed out attrs.
          */
         poa.attributes_follow = FALSE;
-        if (nfs_zero_filled_stat (pre))
+        if (gf_is_zero_filled_stat (pre))
                 goto out;
 
         poa.attributes_follow = TRUE;