Upcall/cache-invalidation: Ignore fops with frame->root->client not set

Server-side internally generated fops like 'quota/marker' will
not have any client associated with the frame. Hence we need a
check for clients to be valid before processing for upcall cache
invalidation. Also fixed an issue with initializing reaper-thread.

Added a testcase to test the fix.

Change-Id: If7419b98aca383f4b80711c10fef2e0b32498c57
BUG: 1227204
Signed-off-by: Soumya Koduri <skoduri@redhat.com>
Reviewed-on: http://review.gluster.org/10909
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Kaleb KEITHLEY <kkeithle@redhat.com>
Reviewed-by: jiffin tony Thottan <jthottan@redhat.com>
Tested-by: NetBSD Build System <jenkins@build.gluster.org>
Reviewed-by: Niels de Vos <ndevos@redhat.com>

4 files changed, 53 insertions, 7 deletions

diff --git a/tests/bugs/upcall/bug-1227204.t b/tests/bugs/upcall/bug-1227204.t
new file mode 100755
index 00000000000..fc393b1837f
--- /dev/null
+++ b/tests/bugs/upcall/bug-1227204.t
@@ -0,0 +1,29 @@
+#!/bin/bash
+
+# This regression test tries to ensure that quota limit-usage set work with
+# features.cache-invalidation on.
+
+. $(dirname $0)/../../include.rc
+. $(dirname $0)/../../volume.rc
+
+cleanup;
+
+TEST glusterd
+TEST pidof glusterd;
+TEST $CLI volume info;
+
+TEST $CLI volume create $V0 replica 2 $H0:$B0/${V0}{1,2,3,4,5,6};
+TEST $CLI volume start $V0;
+
+TEST $CLI volume set $V0 features.cache-invalidation on;
+TEST $CLI volume quota $V0 enable;
+
+TEST glusterfs --volfile-id=$V0 --volfile-server=$H0 $M0;
+
+TEST mkdir -p $M0/1/2;
+TEST $CLI volume quota $V0 limit-usage /1/2 100MB 70%;
+
+TEST $CLI volume status $V0
+TEST $CLI volume stop $V0
+
+cleanup;
diff --git a/xlators/features/upcall/src/upcall-internal.c b/xlators/features/upcall/src/upcall-internal.c
index f618e2655fc..3b6186ea943 100644
--- a/xlators/features/upcall/src/upcall-internal.c
+++ b/xlators/features/upcall/src/upcall-internal.c
@@ -201,10 +201,13 @@ __upcall_inode_ctx_set (inode_t *inode, xlator_t *this)
         INIT_LIST_HEAD (&inode_ctx->client_list);
         inode_ctx->destroy = 0;
 
-        ret = __inode_ctx_set (inode, this, (uint64_t *) inode_ctx);
-        if (ret)
+        ctx = (long) inode_ctx;
+        ret = __inode_ctx_set (inode, this, &ctx);
+        if (ret) {
                 gf_log (this->name, GF_LOG_DEBUG,
                         "failed to set inode ctx (%p)", inode);
+                goto out;
+        }
 
         /* add this inode_ctx to the global list */
         LOCK (&priv->inode_ctx_lk);
@@ -236,7 +239,7 @@ __upcall_inode_ctx_get (inode_t *inode, xlator_t *this)
                         goto out;
         }
 
-        inode_ctx = (upcall_inode_ctx_t *)(long) ctx;
+        inode_ctx = (upcall_inode_ctx_t *) (long) (ctx);
 
 out:
         return inode_ctx;
@@ -372,10 +375,10 @@ upcall_cleanup_inode_ctx (xlator_t *this, inode_t *inode)
                 }
                 pthread_mutex_unlock (&inode_ctx->client_list_lock);
 
-                pthread_mutex_destroy (&inode_ctx->client_list_lock);
-
                 /* Mark the inode_ctx to be destroyed */
                 inode_ctx->destroy = 1;
+                gf_msg_debug ("upcall", 0, "set upcall_inode_ctx (%p) to destroy mode",
+                              inode_ctx);
         }
 
 out:
@@ -417,8 +420,12 @@ upcall_reaper_thread (void *data)
                         LOCK (&priv->inode_ctx_lk);
                         {
                                 /* client list would have been cleaned up*/
+                                gf_msg_debug ("upcall", 0, "Freeing upcall_inode_ctx (%p)",
+                                              inode_ctx);
                                 list_del_init (&inode_ctx->inode_ctx_list);
+                                pthread_mutex_destroy (&inode_ctx->client_list_lock);
                                 GF_FREE (inode_ctx);
+                                inode_ctx = NULL;
                         }
                         UNLOCK (&priv->inode_ctx_lk);
                 }
@@ -469,6 +476,14 @@ upcall_cache_invalidate (call_frame_t *frame, xlator_t *this, client_t *client,
         if (!is_cache_invalidation_enabled(this))
                 return;
 
+        /* server-side generated fops like quota/marker will not have any
+         * client associated with them. Ignore such fops.
+         */
+        if (!client) {
+                gf_msg_debug ("upcall", 0, "Internal fop - client NULL");
+                return;
+        }
+
         up_inode_ctx = ((upcall_local_t *)frame->local)->upcall_inode_ctx;
 
         if (!up_inode_ctx)
diff --git a/xlators/features/upcall/src/upcall.c b/xlators/features/upcall/src/upcall.c
index 50e621a1127..0e2a75473dc 100644
--- a/xlators/features/upcall/src/upcall.c
+++ b/xlators/features/upcall/src/upcall.c
@@ -1576,8 +1576,8 @@ upcall_local_wipe (xlator_t *this, upcall_local_t *local)
 {
         if (local) {
                 inode_unref (local->inode);
-                mem_put (local);
                 loc_wipe (&local->rename_oldloc);
+                mem_put (local);
         }
 }
 
@@ -1629,6 +1629,7 @@ reconfigure (xlator_t *this, dict_t *options)
                                 " Disabling cache_invalidation",
                                 strerror(errno));
                 }
+                priv->reaper_init_done = 1;
         }
 
 out:
@@ -1675,6 +1676,7 @@ init (xlator_t *this)
                                 " Disabling cache_invalidation",
                                 strerror(errno));
                 }
+                priv->reaper_init_done = 1;
         }
 out:
         if (ret) {
diff --git a/xlators/mgmt/glusterd/src/glusterd-volume-set.c b/xlators/mgmt/glusterd/src/glusterd-volume-set.c
index 3e49fab37d4..ed78be9ba6a 100644
--- a/xlators/mgmt/glusterd/src/glusterd-volume-set.c
+++ b/xlators/mgmt/glusterd/src/glusterd-volume-set.c
@@ -1858,7 +1858,7 @@ struct volopt_map_entry glusterd_volopt_map[] = {
         /* Upcall translator options */
         { .key         = "features.cache-invalidation",
           .voltype     = "features/upcall",
-          .value      = "off",
+          .value       = "off",
           .op_version  = GD_OP_VERSION_3_7_0,
         },
         { .key         = "features.cache-invalidation-timeout",