Upcall: Read gfid from iatt in case of invalid inode

When any file/dir is looked upon for the first time, inode
created shall be invalid till it gets linked to the inode table.
In such cases, read the gfid from the iatt structure returned
as part of such fops for UPCALL processing.

Change-Id: Ie5eb2f3be18c34cf7ef172e126c9db5ef7a8512b
BUG: 1283983
Signed-off-by: Soumya Koduri <skoduri@redhat.com>
Reviewed-on: http://review.gluster.org/12773
Reviewed-by: Kaleb KEITHLEY <kkeithle@redhat.com>
Tested-by: NetBSD Build System <jenkins@build.gluster.org>
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Niels de Vos <ndevos@redhat.com>

6 files changed, 198 insertions, 22 deletions

diff --git a/api/src/glfs-handleops.c b/api/src/glfs-handleops.c
index 0fe5b35ff11..e8fbae4074e 100644
--- a/api/src/glfs-handleops.c
+++ b/api/src/glfs-handleops.c
@@ -1288,6 +1288,10 @@ pub_glfs_h_create_from_handle (struct glfs *fs, unsigned char *handle, int len,
 
         memcpy (loc.gfid, handle, GFAPI_HANDLE_LENGTH);
 
+        /* make sure the gfid received is valid */
+        GF_VALIDATE_OR_GOTO ("glfs_h_create_from_handle",
+                             !(gf_uuid_is_null (loc.gfid)), out);
+
         newinode = inode_find (subvol->itable, loc.gfid);
         if (newinode) {
                 if (!stat) /* No need of lookup */
diff --git a/tests/basic/gfapi/bug1283983.c b/tests/basic/gfapi/bug1283983.c
new file mode 100644
index 00000000000..76db8d5ca09
--- /dev/null
+++ b/tests/basic/gfapi/bug1283983.c
@@ -0,0 +1,123 @@
+#include <fcntl.h>
+#include <unistd.h>
+#include <time.h>
+#include <limits.h>
+#include <alloca.h>
+#include <string.h>
+#include <stdio.h>
+#include <errno.h>
+#include <stdlib.h>
+#include <glusterfs/api/glfs.h>
+#include <glusterfs/api/glfs-handles.h>
+int gfapi = 1;
+
+#define LOG_ERR(func, ret) do { \
+        if (ret != 0) {            \
+                fprintf (stderr, "%s : returned error ret(%d), errno(%d)\n", \
+                         func, ret, errno); \
+                exit(1); \
+        } else { \
+                fprintf (stderr, "%s : returned %d\n", func, ret); \
+        } \
+        } while (0)
+#define LOG_IF_NO_ERR(func, ret) do { \
+        if (ret == 0) {            \
+                fprintf (stderr, "%s : hasn't returned error %d\n", \
+                         func, ret); \
+                exit(1); \
+        } else { \
+                fprintf (stderr, "%s : returned %d\n", func, ret); \
+        } \
+        } while (0)
+int
+main (int argc, char *argv[])
+{
+        glfs_t    *fs = NULL;
+        int        ret = 0, i;
+        glfs_fd_t *fd = NULL;
+        char      *filename = "/a1";
+        char      *filename2 = "/a2";
+        struct     stat sb = {0, };
+        struct    callback_arg cbk;
+        char      *logfile = NULL;
+        char      *volname = NULL;
+        int       cnt = 1;
+        struct callback_inode_arg *in_arg = NULL;
+        struct glfs_object *root = NULL, *leaf = NULL;
+
+        cbk.reason = 0;
+
+        fprintf (stderr, "Starting libgfapi_fini\n");
+        if (argc != 3) {
+                fprintf (stderr, "Invalid argument\n");
+                exit(1);
+        }
+
+        volname = argv[1];
+        logfile = argv[2];
+
+
+        fs = glfs_new (volname);
+        if (!fs) {
+                fprintf (stderr, "glfs_new: returned NULL\n");
+                return 1;
+        }
+
+        ret = glfs_set_volfile_server (fs, "tcp", "localhost", 24007);
+        LOG_ERR("glfs_set_volfile_server", ret);
+
+        ret = glfs_set_logging (fs, logfile, 7);
+        LOG_ERR("glfs_set_logging", ret);
+
+        ret = glfs_init (fs);
+        LOG_ERR("glfs_init", ret);
+
+        sleep (2);
+        root = glfs_h_lookupat (fs, NULL, "/", &sb, 0);
+        if (!root) {
+                ret = -1;
+                LOG_ERR ("glfs_h_lookupat root", ret);
+        }
+        leaf = glfs_h_lookupat (fs, root, filename, &sb, 0);
+        if (!leaf) {
+                ret = -1;
+                LOG_IF_NO_ERR ("glfs_h_lookupat leaf", ret);
+        }
+
+        leaf = glfs_h_creat (fs, root, filename, O_RDWR, 0644, &sb);
+        if (!leaf) {
+                ret = -1;
+                LOG_ERR ("glfs_h_lookupat leaf", ret);
+        }
+        fprintf (stderr, "glfs_h_create leaf - %p\n", leaf);
+
+        leaf = glfs_h_lookupat (fs, root, filename2, &sb, 0);
+        if (!leaf) {
+                ret = -1;
+                LOG_IF_NO_ERR ("glfs_h_lookupat leaf", ret);
+        }
+
+        ret = glfs_h_rename (fs, root, filename, root, filename2);
+        LOG_ERR("glfs_rename", ret);
+
+        while (cnt++ < 5) {
+                ret = glfs_h_poll_upcall(fs, &cbk);
+                LOG_ERR ("glfs_h_poll_upcall", ret);
+
+                /* There should not be any upcalls sent */
+                if (cbk.reason != GFAPI_CBK_EVENT_NULL) {
+                        fprintf (stderr, "Error: Upcall received(%d)\n",
+                                 cbk.reason);
+                        exit (1);
+                }
+        }
+
+        ret = glfs_fini(fs);
+        LOG_ERR("glfs_fini", ret);
+
+        fprintf (stderr, "End of libgfapi_fini\n");
+
+        exit(0);
+}
+
+
diff --git a/tests/basic/gfapi/bug1283983.sh b/tests/basic/gfapi/bug1283983.sh
new file mode 100755
index 00000000000..97f1b01150c
--- /dev/null
+++ b/tests/basic/gfapi/bug1283983.sh
@@ -0,0 +1,33 @@
+#!/bin/bash
+
+. $(dirname $0)/../../include.rc
+. $(dirname $0)/../../volume.rc
+
+cleanup;
+
+TEST glusterd
+
+TEST $CLI volume create $V0 localhost:$B0/brick1;
+EXPECT 'Created' volinfo_field $V0 'Status';
+
+TEST $CLI volume start $V0;
+EXPECT 'Started' volinfo_field $V0 'Status';
+
+logdir=`gluster --print-logdir`
+
+## Enable Upcall cache-invalidation feature
+TEST $CLI volume set $V0 features.cache-invalidation on;
+
+build_tester $(dirname $0)/bug1283983.c -lgfapi -o $(dirname $0)/bug1283983
+
+TEST ./$(dirname $0)/bug1283983 $V0  $logdir/bug1283983.log
+
+## There shouldn't be any NULL gfid messages logged
+TEST ! cat $logdir/bug1283983.log | grep "upcall" | grep "00000000-0000-0000-0000-000000000000"
+
+cleanup_tester $(dirname $0)/bug1283983
+
+TEST $CLI volume stop $V0
+TEST $CLI volume delete $V0
+
+cleanup;
diff --git a/xlators/features/upcall/src/upcall-internal.c b/xlators/features/upcall/src/upcall-internal.c
index 007a8f854bf..81199eb074c 100644
--- a/xlators/features/upcall/src/upcall-internal.c
+++ b/xlators/features/upcall/src/upcall-internal.c
@@ -88,8 +88,7 @@ get_cache_invalidation_timeout(xlator_t *this) {
  * Allocate and add a new client entry to the given upcall entry
  */
 upcall_client_t*
-add_upcall_client (call_frame_t *frame, uuid_t gfid,
-                   client_t *client,
+add_upcall_client (call_frame_t *frame, client_t *client,
                    upcall_inode_ctx_t *up_inode_ctx)
 {
         upcall_client_t *up_client_entry = NULL;
@@ -97,7 +96,6 @@ add_upcall_client (call_frame_t *frame, uuid_t gfid,
         pthread_mutex_lock (&up_inode_ctx->client_list_lock);
         {
                 up_client_entry = __add_upcall_client (frame,
-                                                       gfid,
                                                        client,
                                                        up_inode_ctx);
         }
@@ -107,8 +105,7 @@ add_upcall_client (call_frame_t *frame, uuid_t gfid,
 }
 
 upcall_client_t*
-__add_upcall_client (call_frame_t *frame, uuid_t gfid,
-                     client_t *client,
+__add_upcall_client (call_frame_t *frame, client_t *client,
                      upcall_inode_ctx_t *up_inode_ctx)
 {
         upcall_client_t *up_client_entry = NULL;
@@ -137,11 +134,11 @@ __add_upcall_client (call_frame_t *frame, uuid_t gfid,
 }
 
 /*
- * Given gfid and client->uid, retrieve the corresponding upcall client entry.
+ * Given client->uid, retrieve the corresponding upcall client entry.
  * If none found, create a new entry.
  */
 upcall_client_t*
-__get_upcall_client (call_frame_t *frame, uuid_t gfid, client_t *client,
+__get_upcall_client (call_frame_t *frame, client_t *client,
                      upcall_inode_ctx_t *up_inode_ctx)
 {
         upcall_client_t *up_client_entry = NULL;
@@ -165,7 +162,7 @@ __get_upcall_client (call_frame_t *frame, uuid_t gfid, client_t *client,
         }
 
         if (!found_client) { /* create one */
-                up_client_entry = __add_upcall_client (frame, gfid, client,
+                up_client_entry = __add_upcall_client (frame, client,
                                                        up_inode_ctx);
         }
 
@@ -200,6 +197,7 @@ __upcall_inode_ctx_set (inode_t *inode, xlator_t *this)
         INIT_LIST_HEAD (&inode_ctx->inode_ctx_list);
         INIT_LIST_HEAD (&inode_ctx->client_list);
         inode_ctx->destroy = 0;
+        gf_uuid_copy (inode_ctx->gfid, inode->gfid);
 
         ctx = (long) inode_ctx;
         ret = __inode_ctx_set (inode, this, &ctx);
@@ -458,7 +456,7 @@ upcall_reaper_thread_init (xlator_t *this)
 }
 
 /*
- * Given a gfid, client, first fetch upcall_entry_t based on gfid.
+ * Given a client, first fetch upcall_entry_t from the inode_ctx client list.
  * Later traverse through the client list of that upcall entry. If this client
  * is not present in the list, create one client entry with this client info.
  * Also check if there are other clients which need to be notified of this
@@ -502,16 +500,31 @@ upcall_cache_invalidate (call_frame_t *frame, xlator_t *this, client_t *client,
                 return;
         }
 
+        /* In case of LOOKUP, if first time, inode created shall be
+         * invalid till it gets linked to inode table. Read gfid from
+         * the stat returned in such cases.
+         */
+        if (gf_uuid_is_null (up_inode_ctx->gfid)) {
+                /* That means inode must have been invalid when this inode_ctx
+                 * is created. Copy the gfid value from stbuf instead.
+                 */
+                gf_uuid_copy (up_inode_ctx->gfid, stbuf->ia_gfid);
+        }
+
+        GF_VALIDATE_OR_GOTO ("upcall_cache_invalidate",
+                             !(gf_uuid_is_null (up_inode_ctx->gfid)), out);
         pthread_mutex_lock (&up_inode_ctx->client_list_lock);
         {
                 list_for_each_entry_safe (up_client_entry, tmp,
                                           &up_inode_ctx->client_list,
                                           client_list) {
 
+                        /* Do not send UPCALL event if same client. */
                         if (!strcmp(client->client_uid,
                                    up_client_entry->client_uid)) {
                                 up_client_entry->access_time = time(NULL);
                                 found = _gf_true;
+                                continue;
                         }
 
                         /*
@@ -532,7 +545,7 @@ upcall_cache_invalidate (call_frame_t *frame, xlator_t *this, client_t *client,
                          *  expire_time_attr to 0.
                          */
                         upcall_client_cache_invalidate(this,
-                                                       inode->gfid,
+                                                       up_inode_ctx->gfid,
                                                        up_client_entry,
                                                        flags, stbuf,
                                                        p_stbuf, oldp_stbuf);
@@ -540,12 +553,13 @@ upcall_cache_invalidate (call_frame_t *frame, xlator_t *this, client_t *client,
 
                 if (!found) {
                         up_client_entry = __add_upcall_client (frame,
-                                                               inode->gfid,
                                                                client,
                                                                up_inode_ctx);
                 }
         }
         pthread_mutex_unlock (&up_inode_ctx->client_list_lock);
+out:
+        return;
 }
 
 /*
@@ -565,6 +579,8 @@ upcall_client_cache_invalidate (xlator_t *this, uuid_t gfid,
         int                                 ret     = -1;
         time_t t_expired = time(NULL) - up_client_entry->access_time;
 
+        GF_VALIDATE_OR_GOTO ("upcall_client_cache_invalidate",
+                             !(gf_uuid_is_null (gfid)), out);
         timeout = get_cache_invalidation_timeout(this);
 
         if (t_expired < timeout) {
@@ -609,6 +625,8 @@ upcall_client_cache_invalidate (xlator_t *this, uuid_t gfid,
                         __upcall_cleanup_client_entry (up_client_entry);
                 }
         }
+out:
+        return;
 }
 
 /*
@@ -640,7 +658,7 @@ upcall_cache_forget (xlator_t *this, inode_t *inode, upcall_inode_ctx_t *up_inod
                         up_client_entry->access_time = time(NULL);
 
                         upcall_client_cache_invalidate(this,
-                                                       inode->gfid,
+                                                       up_inode_ctx->gfid,
                                                        up_client_entry,
                                                        flags, NULL,
                                                        NULL, NULL);
diff --git a/xlators/features/upcall/src/upcall.c b/xlators/features/upcall/src/upcall.c
index fc04d4d5d51..9b724928010 100644
--- a/xlators/features/upcall/src/upcall.c
+++ b/xlators/features/upcall/src/upcall.c
@@ -161,7 +161,7 @@ up_readv_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
         }
         flags = UP_UPDATE_CLIENT;
         upcall_cache_invalidate (frame, this, client, local->inode, flags,
-                                 NULL, NULL, NULL);
+                                 stbuf, NULL, NULL);
 
 out:
         UPCALL_STACK_UNWIND (readv, frame, op_ret, op_errno, vector,
@@ -747,7 +747,7 @@ up_lookup_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
         }
         flags = UP_UPDATE_CLIENT;
         upcall_cache_invalidate (frame, this, client, local->inode, flags,
-                                 NULL, NULL, NULL);
+                                 stbuf, NULL, NULL);
 
 out:
         UPCALL_STACK_UNWIND (lookup, frame, op_ret, op_errno, inode, stbuf,
@@ -804,7 +804,7 @@ up_stat_cbk (call_frame_t *frame, void *cookie,
         }
         flags = UP_UPDATE_CLIENT;
         upcall_cache_invalidate (frame, this, client, local->inode, flags,
-                                 NULL, NULL, NULL);
+                                 buf, NULL, NULL);
 
 out:
         UPCALL_STACK_UNWIND (stat, frame, op_ret, op_errno, buf,
@@ -970,7 +970,7 @@ up_readlink_cbk (call_frame_t *frame, void *cookie, xlator_t *this,
         }
         flags = UP_UPDATE_CLIENT;
         upcall_cache_invalidate (frame, this, client, local->inode, flags,
-                                 NULL, NULL, NULL);
+                                 stbuf, NULL, NULL);
 
 out:
         UPCALL_STACK_UNWIND (readlink, frame, op_ret, op_errno, path, stbuf,
diff --git a/xlators/features/upcall/src/upcall.h b/xlators/features/upcall/src/upcall.h
index eb1c7df89be..344c9c362eb 100644
--- a/xlators/features/upcall/src/upcall.h
+++ b/xlators/features/upcall/src/upcall.h
@@ -72,6 +72,7 @@ struct _upcall_inode_ctx_t {
         pthread_mutex_t client_list_lock; /* mutex for clients list
                                              of this upcall entry */
         int destroy;
+        uuid_t   gfid; /* gfid of the entry */
 };
 typedef struct _upcall_inode_ctx_t upcall_inode_ctx_t;
 
@@ -89,14 +90,11 @@ typedef struct upcall_local upcall_local_t;
 void upcall_local_wipe (xlator_t *this, upcall_local_t *local);
 upcall_local_t *upcall_local_init (call_frame_t *frame, xlator_t *this, inode_t *inode);
 
-upcall_client_t *add_upcall_client (call_frame_t *frame, uuid_t gfid,
-                                    client_t *client,
+upcall_client_t *add_upcall_client (call_frame_t *frame, client_t *client,
                                     upcall_inode_ctx_t *up_inode_ctx);
-upcall_client_t *__add_upcall_client (call_frame_t *frame, uuid_t gfid,
-                                      client_t *client,
+upcall_client_t *__add_upcall_client (call_frame_t *frame, client_t *client,
                                       upcall_inode_ctx_t *up_inode_ctx);
-upcall_client_t *__get_upcall_client (call_frame_t *frame, uuid_t gfid,
-                                      client_t *client,
+upcall_client_t *__get_upcall_client (call_frame_t *frame, client_t *client,
                                       upcall_inode_ctx_t *up_inode_ctx);
 int __upcall_cleanup_client_entry (upcall_client_t *up_client);
 int upcall_cleanup_expired_clients (xlator_t *this,