gfapi: redesign the public interface for upcall consumers

The glfs_callback_arg and glfs_callback_inode_arg were allocated by
gfapi, and expected to be free()'d by the application. However it is not
reasonable to expect that applications use the same memory allocator to
as the compiled libgfapi.so. For instance, it is possible that gfapi
uses glibc malloc/free, and an application like NFS-Ganesha the versions
from jemalloc. Mismatching of the malloc() and free() functions causes
segmentation faults at best.

In order to prevent problems like this in the future, the API for
applications that consume upcalls has been remodeled. Any of the
structures that gfapi allocates, should be free'd with glfs_free(). The
members of the structures can not be accessed directly anymore, each
has its own function to access now.

Correcting the naming of the functions, structures and constants is a
continuation of commit 2775dc64101ed37c8d9809bf9852dbf0746ee2b6. These
new improvements not only have correct prefixes for the functions and
structures, the naming also reflects more to the upcall framework and
does not use "callback" anymore.

Change-Id: I2b8bd5a0a82036d2abea1a217f5e5975a1d4fe93
BUG: 1344714
Signed-off-by: Niels de Vos <ndevos@redhat.com>
Reviewed-on: http://review.gluster.org/14701
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: Kaleb KEITHLEY <kkeithle@redhat.com>
Reviewed-by: soumya k <skoduri@redhat.com>
Reviewed-by: jiffin tony Thottan <jthottan@redhat.com>

12 files changed, 520 insertions, 178 deletions

diff --git a/api/src/gfapi.aliases b/api/src/gfapi.aliases
index 365d170230b..ac89d35df6b 100644
--- a/api/src/gfapi.aliases
+++ b/api/src/gfapi.aliases
@@ -140,4 +140,18 @@ _priv_glfs_resolve _glfs_resolve$GFAPI_PRIVATE_3.7.0
 _priv_glfs_process_upcall_event _glfs_process_upcall_event$GFAPI_PRIVATE_3.7.0
 
 _pub_glfs_h_lookupat _glfs_h_lookupat$GFAPI_3.7.4
+
+_pub_glfs_h_poll_upcall _glfs_h_poll_upcall$GFAPI_3.7.16
+_pub_glfs_upcall_get_fs _glfs_upcall_get_fs$GFAPI_3.7.16
+_pub_glfs_upcall_get_reason _glfs_upcall_get_reason$GFAPI_3.7.16
+_pub_glfs_upcall_inode_get_event _glfs_upcall_inode_get_event$GFAPI_3.7.16
+_pub_glfs_upcall_inode_get_object _glfs_upcall_inode_get_object$GFAPI_3.7.16
+_pub_glfs_upcall_inode_get_flags _glfs_upcall_inode_get_flags$GFAPI_3.7.16
+_pub_glfs_upcall_inode_get_stat _glfs_upcall_inode_get_stat$GFAPI_3.7.16
+_pub_glfs_upcall_inode_get_expire _glfs_upcall_inode_get_expire$GFAPI_3.7.16
+_pub_glfs_upcall_inode_get_pobject _glfs_upcall_inode_get_pobject$GFAPI_3.7.16
+_pub_glfs_upcall_inode_get_pstat _glfs_upcall_inode_get_pstat$GFAPI_3.7.16
+_pub_glfs_upcall_inode_get_oldpobject _glfs_upcall_inode_get_oldpobject$GFAPI_3.7.16
+_pub_glfs_upcall_inode_get_oldpstat _glfs_upcall_inode_get_oldpstat$GFAPI_3.7.16
+
 _pub_glfs_ipc _glfs_ipc$GFAPI_4.0.0
diff --git a/api/src/gfapi.map b/api/src/gfapi.map
index b03e6212694..bd7ff13fe41 100644
--- a/api/src/gfapi.map
+++ b/api/src/gfapi.map
@@ -173,7 +173,22 @@ GFAPI_3.7.15 {
 		glfs_truncate;
 } GFAPI_3.7.4;
 
+GFAPI_3.7.16 {
+	global:
+		glfs_upcall_get_fs;
+		glfs_upcall_get_reason;
+		glfs_upcall_inode_get_event;
+		glfs_upcall_inode_get_object;
+		glfs_upcall_inode_get_flags;
+		glfs_upcall_inode_get_stat;
+		glfs_upcall_inode_get_expire;
+		glfs_upcall_inode_get_pobject;
+		glfs_upcall_inode_get_pstat;
+		glfs_upcall_inode_get_oldpobject;
+		glfs_upcall_inode_get_oldpstat;
+} GFAPI_3.7.15;
+
 GFAPI_4.0.0 {
 	global:
 		glfs_ipc;
-} GFAPI_3.7.15;
+} GFAPI_3.7.16;
diff --git a/api/src/glfs-handleops.c b/api/src/glfs-handleops.c
index 47bdbcbec52..84dba5b82c5 100644
--- a/api/src/glfs-handleops.c
+++ b/api/src/glfs-handleops.c
@@ -1854,9 +1854,27 @@ invalid_fs:
 
 }
 
+static void
+glfs_free_upcall_inode (void *to_free)
+{
+        struct glfs_upcall_inode *arg = to_free;
+
+        if (!arg)
+                return;
+
+        if (arg->object)
+                glfs_h_close (arg->object);
+        if (arg->p_object)
+                glfs_h_close (arg->p_object);
+        if (arg->oldp_object)
+                glfs_h_close (arg->oldp_object);
+
+        GF_FREE (arg);
+}
+
 int
 glfs_h_poll_cache_invalidation (struct glfs *fs,
-                                struct glfs_callback_arg *up_arg,
+                                struct glfs_upcall *up_arg,
                                 struct gf_upcall *upcall_data)
 {
         int                                 ret           = -1;
@@ -1864,7 +1882,7 @@ glfs_h_poll_cache_invalidation (struct glfs *fs,
         struct glfs_object                  *oldp_object  = NULL;
         struct glfs_object                  *object       = NULL;
         struct gf_upcall_cache_invalidation *ca_data      = NULL;
-        struct glfs_callback_inode_arg      *up_inode_arg = NULL;
+        struct glfs_upcall_inode            *up_inode_arg = NULL;
 
         ca_data = upcall_data->data;
         GF_VALIDATE_OR_GOTO ("glfs_h_poll_cache_invalidation",
@@ -1889,13 +1907,11 @@ glfs_h_poll_cache_invalidation (struct glfs *fs,
                 goto out;
         }
 
-        up_inode_arg = GF_CALLOC (1, sizeof (struct glfs_callback_inode_arg),
-                                  glfs_mt_upcall_entry_t);
+        up_inode_arg = GF_CALLOC (1, sizeof (struct glfs_upcall_inode),
+                                  glfs_mt_upcall_inode_t);
         GF_VALIDATE_OR_GOTO ("glfs_h_poll_cache_invalidation",
                              up_inode_arg, out);
 
-        up_arg->event_arg = up_inode_arg;
-
         up_inode_arg->object = object;
         up_inode_arg->flags = ca_data->flags;
         up_inode_arg->expire_time_attr = ca_data->expire_time_attr;
@@ -1946,6 +1962,10 @@ glfs_h_poll_cache_invalidation (struct glfs *fs,
         }
         up_inode_arg->oldp_object = oldp_object;
 
+        up_arg->reason = GLFS_UPCALL_INODE_INVALIDATE;
+        up_arg->event = up_inode_arg;
+        up_arg->free_event = glfs_free_upcall_inode;
+
         ret = 0;
 
 out:
@@ -1954,47 +1974,42 @@ out:
                 if (object)
                         glfs_h_close (object);
 
-                /* Reset event_arg as well*/
-                up_arg->event_arg = NULL;
+                /* Set reason to prevent applications from using ->event */
+                up_arg->reason = GLFS_UPCALL_EVENT_NULL;
                 GF_FREE (up_inode_arg);
         }
         return ret;
 }
 
 /*
- * This API is used to poll for upcall events stored in the
- * upcall list. Current users of this API is NFS-Ganesha.
- * Incase of any event received, it will be mapped appropriately
- * into 'glfs_callback_arg' along with the handle object  to be passed
- * to NFS-Ganesha.
- *
- * On success, applications need to check for 'reason' to decide
- * if any upcall event is received.
+ * This API is used to poll for upcall events stored in the upcall list.
+ * Current users of this API is NFS-Ganesha. Incase of any event received, it
+ * will be mapped appropriately into 'glfs_upcall' along with the handle object
+ * to be passed to NFS-Ganesha.
  *
- * Current supported upcall_events -
- *      GFAPI_INODE_INVALIDATE -
- *              'arg - glfs_callback_inode_arg
+ * On success, applications need to check if up_arg is not-NULL or errno is not
+ * ENOENT. glfs_upcall_get_reason() can be used to decide what kind of event
+ * has been received.
  *
- * After processing the event, applications need to free 'event_arg'.
+ * Current supported upcall_events:
+ *      GLFS_UPCALL_INODE_INVALIDATE
  *
- * Incase of INODE_INVALIDATE, applications need to free "object",
- * "p_object" and "oldp_object" using glfs_h_close(..).
+ * After processing the event, applications need to free 'up_arg' by calling
+ * glfs_free().
  *
- * Also similar to I/Os, the application should ideally stop polling
- * before calling glfs_fini(..). Hence making an assumption that
- * 'fs' & ctx structures cannot be freed while in this routine.
+ * Also similar to I/Os, the application should ideally stop polling before
+ * calling glfs_fini(..). Hence making an assumption that 'fs' & ctx structures
+ * cannot be freed while in this routine.
  */
 int
-pub_glfs_h_poll_upcall (struct glfs *fs, struct glfs_callback_arg *up_arg)
+pub_glfs_h_poll_upcall (struct glfs *fs, struct glfs_upcall **up_arg)
 {
-        upcall_entry                        *u_list         = NULL;
-        upcall_entry                        *tmp            = NULL;
-        xlator_t                            *subvol         = NULL;
-        int                                 found           = 0;
-        int                                 reason          = 0;
-        glusterfs_ctx_t                     *ctx            = NULL;
-        int                                 ret             = -1;
-        struct gf_upcall                    *upcall_data    = NULL;
+        upcall_entry       *u_list         = NULL;
+        upcall_entry       *tmp            = NULL;
+        xlator_t           *subvol         = NULL;
+        glusterfs_ctx_t    *ctx            = NULL;
+        int                 ret            = -1;
+        struct gf_upcall   *upcall_data    = NULL;
 
         DECLARE_OLD_THIS;
 
@@ -2007,15 +2022,13 @@ pub_glfs_h_poll_upcall (struct glfs *fs, struct glfs_callback_arg *up_arg)
 
         /* get the active volume */
         subvol = glfs_active_subvol (fs);
-
         if (!subvol) {
                 errno = EIO;
                 goto restore;
         }
 
         /* Ideally applications should stop polling before calling
-         * 'glfs_fini'. Yet cross check if cleanup has started
-         */
+         * 'glfs_fini'. Yet cross check if cleanup has started. */
         pthread_mutex_lock (&fs->mutex);
         {
                 ctx = fs->ctx;
@@ -2038,46 +2051,55 @@ pub_glfs_h_poll_upcall (struct glfs *fs, struct glfs_callback_arg *up_arg)
                 list_for_each_entry_safe (u_list, tmp,
                                           &fs->upcall_list,
                                           upcall_list) {
-                        found = 1;
                         list_del_init (&u_list->upcall_list);
+                        upcall_data = &u_list->upcall_data;
                         break;
                 }
         }
         /* No other thread can delete this entry. So unlock it */
         pthread_mutex_unlock (&fs->upcall_list_mutex);
 
-        if (found) {
-                upcall_data = &u_list->upcall_data;
-
+        if (upcall_data) {
                 switch (upcall_data->event_type) {
                 case GF_UPCALL_CACHE_INVALIDATION:
+                        *up_arg = GF_CALLOC (1, sizeof (struct gf_upcall),
+                                             glfs_mt_upcall_entry_t);
+                        if (!*up_arg) {
+                                errno = ENOMEM;
+                                break; /* goto free u_list */
+                        }
+
                         /* XXX: Need to revisit this to support
-                         * GFAPI_INODE_UPDATE if required.
-                         */
-                        reason = GFAPI_INODE_INVALIDATE;
-                        ret = glfs_h_poll_cache_invalidation (fs,
-                                                              up_arg,
+                         * GLFS_UPCALL_INODE_UPDATE if required. */
+                        ret = glfs_h_poll_cache_invalidation (fs, *up_arg,
                                                               upcall_data);
-                        if (!ret) {
-                                break;
+                        if (ret
+                            || (*up_arg)->reason == GLFS_UPCALL_EVENT_NULL) {
+                                /* It could so happen that the file which got
+                                 * upcall notification may have got deleted by
+                                 * the same client. Irrespective of the error,
+                                 * return with an error or success+ENOENT. */
+                                if ((*up_arg)->reason == GLFS_UPCALL_EVENT_NULL)
+                                        errno = ENOENT;
+
+                                GF_FREE (*up_arg);
+                                *up_arg = NULL;
                         }
-                        /* It could so happen that the file which got
-                         * upcall notification may have got deleted
-                         * by the same client. Irrespective of the error,
-                         * return with CBK_NULL reason.
-                         *
-                         * Applications will ignore this notification
-                         * as up_arg->object will be NULL */
-                        reason = GFAPI_CBK_EVENT_NULL;
                         break;
-                default:
+                case GF_UPCALL_RECALL_LEASE:
+                        gf_log ("glfs_h_poll_upcall", GF_LOG_DEBUG,
+                                "UPCALL_RECALL_LEASE is not implemented yet");
+                case GF_UPCALL_EVENT_NULL:
+                /* no 'default:' label, to force handling all upcall events */
+                        errno = ENOENT;
                         break;
                 }
 
-                up_arg->reason = reason;
-
                 GF_FREE (u_list->upcall_data.data);
                 GF_FREE (u_list);
+        } else {
+                /* fs->upcall_list was empty, no upcall events cached */
+                errno = ENOENT;
         }
 
         ret = 0;
@@ -2097,7 +2119,91 @@ err:
         return ret;
 }
 
-GFAPI_SYMVER_PUBLIC_DEFAULT(glfs_h_poll_upcall, 3.7.0);
+GFAPI_SYMVER_PUBLIC_DEFAULT(glfs_h_poll_upcall, 3.7.16);
+
+static gf_boolean_t log_upcall370 = _gf_true; /* log once */
+
+/* The old glfs_h_poll_upcall interface requires intimite knowledge of the
+ * structures that are returned to the calling application. This is not
+ * recommended, as the returned structures need to returned correctly (handles
+ * closed, memory free'd with the unavailable GF_FREE(), and possibly more.)
+ *
+ * To the best of our knowledge, only NFS-Ganesha uses the upcall events
+ * through gfapi. We keep this backwards compatability function around so that
+ * applications using the existing implementation do not break.
+ *
+ * WARNING: this function will be removed in the future.
+ */
+int
+pub_glfs_h_poll_upcall370 (struct glfs *fs, struct glfs_callback_arg *up_arg)
+{
+        struct glfs_upcall    *upcall        = NULL;
+        int                    ret           = -1;
+
+        if (log_upcall370) {
+                log_upcall370 = _gf_false;
+                gf_log (THIS->name, GF_LOG_WARNING, "this application is "
+                        "compiled against an old version of libgfapi, it "
+                        "should use glfs_free() to release the structure "
+                        "returned by glfs_h_poll_upcall() - for more details, "
+                        "see http://review.gluster.org/14701");
+        }
+
+        ret = pub_glfs_h_poll_upcall (fs, &upcall);
+        if (ret == 0) {
+                up_arg->fs = fs;
+                if (errno == ENOENT || upcall->event == NULL) {
+                        up_arg->reason = GLFS_UPCALL_EVENT_NULL;
+                        goto out;
+                }
+
+                up_arg->reason = upcall->reason;
+
+                if (upcall->reason == GLFS_UPCALL_INODE_INVALIDATE) {
+                        struct glfs_callback_inode_arg *cb_inode = NULL;
+                        struct glfs_upcall_inode       *up_inode = NULL;
+
+                        cb_inode = GF_CALLOC (1,
+                                              sizeof (struct glfs_callback_inode_arg),
+                                              glfs_mt_upcall_inode_t);
+                        if (!cb_inode) {
+                                errno = ENOMEM;
+                                ret = -1;
+                                goto out;
+                        }
+
+                        up_inode = upcall->event;
+
+                        /* copy attributes one by one, the memory layout might
+                         * be different between the old glfs_callback_inode_arg
+                         * and new glfs_upcall_inode */
+                        cb_inode->object = up_inode->object;
+                        cb_inode->flags = up_inode->flags;
+                        memcpy (&cb_inode->buf, &up_inode->buf,
+                                sizeof (struct stat));
+                        cb_inode->expire_time_attr = up_inode->expire_time_attr;
+                        cb_inode->p_object = up_inode->p_object;
+                        memcpy (&cb_inode->p_buf, &up_inode->p_buf,
+                                sizeof (struct stat));
+                        cb_inode->oldp_object = up_inode->oldp_object;
+                        memcpy (&cb_inode->oldp_buf, &up_inode->oldp_buf,
+                                sizeof (struct stat));
+
+                        up_arg->event_arg = cb_inode;
+                }
+        }
+
+out:
+        if (upcall) {
+                /* we can not use glfs_free() here, objects need to stay */
+                GF_FREE (upcall->event);
+                GF_FREE (upcall);
+        }
+
+        return ret;
+}
+
+GFAPI_SYMVER_PUBLIC(glfs_h_poll_upcall370, glfs_h_poll_upcall, 3.7.0);
 
 #ifdef HAVE_ACL_LIBACL_H
 #include "glusterfs-acl.h"
diff --git a/api/src/glfs-handles.h b/api/src/glfs-handles.h
index 557934503d2..740b759d0a0 100644
--- a/api/src/glfs-handles.h
+++ b/api/src/glfs-handles.h
@@ -12,6 +12,7 @@
 #define _GLFS_HANDLES_H
 
 #include "glfs.h"
+#include <inttypes.h>
 
 /* GLFS OBJECT BASED OPERATIONS
  *
@@ -110,45 +111,82 @@ typedef struct glfs_object glfs_object_t;
  *
  * Currently supported upcall_events -
  *      GFAPI_INODE_INVALIDATE -
- *              'event_arg' - glfs_callback_inode_arg
+ *              'event_arg' - glfs_upcall_inode
  *
- * After processing the event, applications need to free 'event_arg'.
+ * After processing the event, applications need to free 'event_arg' with
+ * glfs_free().
  *
  * Also similar to I/Os, the application should ideally stop polling
  * before calling glfs_fini(..). Hence making an assumption that
  * 'fs' & ctx structures cannot be freed while in this routine.
  */
-struct glfs_callback_arg {
-        struct glfs             *fs; /* glfs object */
-        int                     reason;  /* Upcall event type */
-        void                    *event_arg; /* changes based in the event type */
+struct glfs_upcall;
+
+struct glfs*
+glfs_upcall_get_fs (struct glfs_upcall *arg) __THROW
+        GFAPI_PUBLIC(glfs_upcall_get_fs, 3.7.16);
+
+enum glfs_upcall_reason {
+        GLFS_UPCALL_EVENT_NULL = 0,
+        GLFS_UPCALL_INODE_INVALIDATE,    /* invalidate cache entry */
 };
 
+enum glfs_upcall_reason
+glfs_upcall_get_reason (struct glfs_upcall *arg) __THROW
+        GFAPI_PUBLIC(glfs_upcall_get_reason, 3.7.16);
+
+
 /*
- * After processing upcall event, they need to free "object" , "p_object",
- * "oldp_object" using glfs_h_close(..).
+ * After processing upcall event, glfs_free() should be called on the
+ * glfs_upcall.
  */
-struct glfs_callback_inode_arg {
-        struct glfs_object      *object; /* Object which need to be acted upon */
-        int                     flags; /* Cache UPDATE/INVALIDATE flags */
-        struct stat             buf; /* Latest stat of this entry */
-        unsigned int            expire_time_attr; /* the amount of time for which
-                                                   * the application need to cache
-                                                   * this entry
-                                                   */
-        struct glfs_object      *p_object; /* parent Object to be updated */
-        struct stat             p_buf; /* Latest stat of parent dir handle */
-        struct glfs_object      *oldp_object; /* Old parent Object
-                                               * to be updated */
-        struct stat             oldp_buf; /* Latest stat of old parent
-                                           * dir handle */
-};
+void*
+glfs_upcall_get_event (struct glfs_upcall *arg) __THROW
+        GFAPI_PUBLIC(glfs_upcall_get_event, 3.7.16);
+
+
+/* Functions for getting details about the glfs_upcall_inode
+ *
+ * None of the pointers returned by the below functions should be free()'d,
+ * glfs_free()'d or glfs_h_close()'d by the application.
+ *
+ * Releasing of the structures is done by passing the glfs_upcall pointer
+ * to glfs_free().
+ */
+struct glfs_upcall_inode;
+
+struct glfs_object*
+glfs_upcall_inode_get_object (struct glfs_upcall_inode *arg) __THROW
+        GFAPI_PUBLIC(glfs_upcall_inode_get_object, 3.7.16);
+
+uint64_t
+glfs_upcall_inode_get_flags (struct glfs_upcall_inode *arg) __THROW
+        GFAPI_PUBLIC(glfs_upcall_inode_get_flags, 3.7.16);
+
+struct stat*
+glfs_upcall_inode_get_stat (struct glfs_upcall_inode *arg) __THROW
+        GFAPI_PUBLIC(glfs_upcall_inode_get_stat, 3.7.16);
+
+uint64_t
+glfs_upcall_inode_get_expire (struct glfs_upcall_inode *arg) __THROW
+        GFAPI_PUBLIC(glfs_upcall_inode_get_expire, 3.7.16);
+
+struct glfs_object*
+glfs_upcall_inode_get_pobject (struct glfs_upcall_inode *arg) __THROW
+        GFAPI_PUBLIC(glfs_upcall_inode_get_pobject, 3.7.16);
+
+struct stat*
+glfs_upcall_inode_get_pstat (struct glfs_upcall_inode *arg) __THROW
+        GFAPI_PUBLIC(glfs_upcall_inode_get_pstat, 3.7.16);
+
+struct glfs_object*
+glfs_upcall_inode_get_oldpobject (struct glfs_upcall_inode *arg) __THROW
+        GFAPI_PUBLIC(glfs_upcall_inode_get_oldpobject, 3.7.16);
+
+struct stat*
+glfs_upcall_inode_get_oldpstat (struct glfs_upcall_inode *arg) __THROW
+        GFAPI_PUBLIC(glfs_upcall_inode_get_oldpstat, 3.7.16);
 
-/* reason list in glfs_callback_arg */
-enum gfapi_callback_type {
-        GFAPI_CBK_EVENT_NULL,
-        GFAPI_INODE_INVALIDATE, /* invalidate cache entry */
-};
 
 /* Handle based operations */
 /* Operations that generate handles */
@@ -273,7 +311,7 @@ glfs_h_access (struct glfs *fs, struct glfs_object *object, int mask) __THROW
   This API is used to poll for upcall events stored in the
   upcall list. Current users of this API is NFS-Ganesha.
   Incase of any event received, it will be mapped appropriately
-  into 'glfs_callback_arg' along with the handle('glfs_object') to be
+  into 'glfs_upcall' along with the handle('glfs_object') to be
   passed to NFS-Ganesha.
 
   In case of success, applications need to check the value of
@@ -283,11 +321,8 @@ glfs_h_access (struct glfs *fs, struct glfs_object *object, int mask) __THROW
   PARAMETERS
 
   @fs: glfs object to poll the upcall events for
-  @cbk: Structure to store upcall events as desired by the application.
-        Application is responsible for allocating and passing the
-        references of all the pointers of this structure except for
-        "handle". In case of any events received, it needs to free
-        "handle"
+  @cbk: Pointer that will contain an upcall event for use by the application.
+        Application is responsible for free'ing the structure with glfs_free().
 
   RETURN VALUES
 
@@ -297,8 +332,8 @@ glfs_h_access (struct glfs *fs, struct glfs_object *object, int mask) __THROW
 */
 
 int
-glfs_h_poll_upcall (struct glfs *fs, struct glfs_callback_arg *cbk) __THROW
-        GFAPI_PUBLIC(glfs_h_poll_upcall, 3.7.0);
+glfs_h_poll_upcall (struct glfs *fs, struct glfs_upcall **cbk) __THROW
+        GFAPI_PUBLIC(glfs_h_poll_upcall, 3.7.16);
 
 int
 glfs_h_acl_set (struct glfs *fs, struct glfs_object *object,
diff --git a/api/src/glfs-internal.h b/api/src/glfs-internal.h
index a10c7289115..fc62bbf027d 100644
--- a/api/src/glfs-internal.h
+++ b/api/src/glfs-internal.h
@@ -225,6 +225,26 @@ struct glfs_object {
         uuid_t          gfid;
 };
 
+struct glfs_upcall {
+        struct glfs             *fs;     /* glfs object */
+        enum glfs_upcall_reason  reason; /* Upcall event type */
+        void                    *event;  /* changes based in the event type */
+        void (*free_event)(void *);      /* free event after the usage */
+};
+
+struct glfs_upcall_inode {
+        struct glfs_object   *object;  /* Object which need to be acted upon */
+        int                   flags;   /* Cache UPDATE/INVALIDATE flags */
+        struct stat           buf;     /* Latest stat of this entry */
+        unsigned int          expire_time_attr; /* the amount of time for which
+                                                 * the application need to cache
+                                                 * this entry */
+        struct glfs_object   *p_object; /* parent Object to be updated */
+        struct stat           p_buf;    /* Latest stat of parent dir handle */
+        struct glfs_object   *oldp_object; /* Old parent Object to be updated */
+        struct stat           oldp_buf; /* Latest stat of old parent dir handle */
+};
+
 #define DEFAULT_EVENT_POOL_SIZE           16384
 #define GF_MEMPOOL_COUNT_OF_DICT_T        4096
 #define GF_MEMPOOL_COUNT_OF_DATA_T        (GF_MEMPOOL_COUNT_OF_DICT_T * 4)
@@ -401,7 +421,7 @@ int glfs_get_upcall_cache_invalidation (struct gf_upcall *to_up_data,
                                         struct gf_upcall *from_up_data);
 int
 glfs_h_poll_cache_invalidation (struct glfs *fs,
-                                struct glfs_callback_arg *up_arg,
+                                struct glfs_upcall *up_arg,
                                 struct gf_upcall *upcall_data);
 
 ssize_t
@@ -416,4 +436,35 @@ glfs_anonymous_pwritev (struct glfs *fs, struct glfs_object *object,
 struct glfs_object *
 glfs_h_resolve_symlink (struct glfs *fs, struct glfs_object *object);
 
+
+/* Deprecated structures that were passed to client applications, replaced by
+ * accessor functions. Do not use these in new applications, and update older
+ * usage.
+ *
+ * See http://review.gluster.org/14701 for more details.
+ *
+ * WARNING: These structures will be removed in the future.
+ */
+struct glfs_callback_arg {
+        struct glfs             *fs;
+        enum glfs_upcall_reason  reason;
+        void                    *event_arg;
+};
+
+struct glfs_callback_inode_arg {
+        struct glfs_object      *object; /* Object which need to be acted upon */
+        int                     flags; /* Cache UPDATE/INVALIDATE flags */
+        struct stat             buf; /* Latest stat of this entry */
+        unsigned int            expire_time_attr; /* the amount of time for which
+                                                   * the application need to cache
+                                                   * this entry
+                                                   */
+        struct glfs_object      *p_object; /* parent Object to be updated */
+        struct stat             p_buf; /* Latest stat of parent dir handle */
+        struct glfs_object      *oldp_object; /* Old parent Object
+                                               * to be updated */
+        struct stat             oldp_buf; /* Latest stat of old parent
+                                           * dir handle */
+};
+
 #endif /* !_GLFS_INTERNAL_H */
diff --git a/api/src/glfs-mem-types.h b/api/src/glfs-mem-types.h
index cad1ca95d4f..d32d63f0f13 100644
--- a/api/src/glfs-mem-types.h
+++ b/api/src/glfs-mem-types.h
@@ -27,6 +27,7 @@ enum glfs_mem_types_ {
 	glfs_mt_readdirbuf_t,
         glfs_mt_upcall_entry_t,
 	glfs_mt_acl_t,
+        glfs_mt_upcall_inode_t,
 	glfs_mt_end
 };
 #endif
diff --git a/api/src/glfs.c b/api/src/glfs.c
index e46c875bb79..a9d56071743 100644
--- a/api/src/glfs.c
+++ b/api/src/glfs.c
@@ -1360,3 +1360,107 @@ pub_glfs_ipc37 (struct glfs *fs, int opcode)
 }
 
 GFAPI_SYMVER_PUBLIC(glfs_ipc37, glfs_ipc, 3.7.0);
+
+
+void
+pub_glfs_free (void *ptr)
+{
+        int mem_type = 0;
+
+        mem_type = gf_get_mem_type (ptr);
+
+        switch (mem_type) {
+        case glfs_mt_upcall_entry_t:
+        {
+                struct glfs_upcall *to_free = ptr;
+
+                if (to_free->event)
+                        to_free->free_event (to_free->event);
+
+                GF_FREE (ptr);
+                break;
+        }
+        default:
+                GF_FREE (ptr);
+        }
+}
+
+GFAPI_SYMVER_PUBLIC_DEFAULT(glfs_free, 3.7.16);
+
+
+struct glfs*
+pub_glfs_upcall_get_fs (struct glfs_upcall *arg)
+{
+        return arg->fs;
+}
+GFAPI_SYMVER_PUBLIC_DEFAULT(glfs_upcall_get_fs, 3.7.16);
+
+enum glfs_upcall_reason
+pub_glfs_upcall_get_reason (struct glfs_upcall *arg)
+{
+        return arg->reason;
+}
+GFAPI_SYMVER_PUBLIC_DEFAULT(glfs_upcall_get_reason, 3.7.16);
+
+void*
+pub_glfs_upcall_get_event (struct glfs_upcall *arg)
+{
+        return arg->event;
+}
+GFAPI_SYMVER_PUBLIC_DEFAULT(glfs_upcall_get_event, 3.7.16);
+
+struct glfs_object*
+pub_glfs_upcall_inode_get_object (struct glfs_upcall_inode *arg)
+{
+        return arg->object;
+}
+GFAPI_SYMVER_PUBLIC_DEFAULT(glfs_upcall_inode_get_object, 3.7.16);
+
+uint64_t
+pub_glfs_upcall_inode_get_flags (struct glfs_upcall_inode *arg)
+{
+        return arg->flags;
+}
+GFAPI_SYMVER_PUBLIC_DEFAULT(glfs_upcall_inode_get_flags, 3.7.16);
+
+struct stat*
+pub_glfs_upcall_inode_get_stat (struct glfs_upcall_inode *arg)
+{
+        return &arg->buf;
+}
+GFAPI_SYMVER_PUBLIC_DEFAULT(glfs_upcall_inode_get_stat, 3.7.16);
+
+uint64_t
+pub_glfs_upcall_inode_get_expire (struct glfs_upcall_inode *arg)
+{
+        return arg->expire_time_attr;
+}
+GFAPI_SYMVER_PUBLIC_DEFAULT(glfs_upcall_inode_get_expire, 3.7.16);
+
+struct glfs_object*
+pub_glfs_upcall_inode_get_pobject (struct glfs_upcall_inode *arg)
+{
+        return arg->p_object;
+}
+GFAPI_SYMVER_PUBLIC_DEFAULT(glfs_upcall_inode_get_pobject, 3.7.16);
+
+struct stat*
+pub_glfs_upcall_inode_get_pstat (struct glfs_upcall_inode *arg)
+{
+        return &arg->p_buf;
+}
+GFAPI_SYMVER_PUBLIC_DEFAULT(glfs_upcall_get_pstat, 3.7.16);
+
+struct glfs_object*
+pub_glfs_upcall_inode_get_oldpobject (struct glfs_upcall_inode *arg)
+{
+        return arg->oldp_object;
+}
+GFAPI_SYMVER_PUBLIC_DEFAULT(glfs_upcall_get_oldpobject, 3.7.16);
+
+struct stat*
+pub_glfs_upcall_inode_get_oldpstat (struct glfs_upcall_inode *arg)
+{
+        return &arg->oldp_buf;
+}
+GFAPI_SYMVER_PUBLIC_DEFAULT(glfs_upcall_get_oldpstat, 3.7.16);
diff --git a/tests/basic/gfapi/bug1283983.c b/tests/basic/gfapi/bug1283983.c
index 6583097697b..3334b290d9e 100644
--- a/tests/basic/gfapi/bug1283983.c
+++ b/tests/basic/gfapi/bug1283983.c
@@ -31,20 +31,18 @@ int gfapi = 1;
 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    glfs_callback_arg cbk;
-        char      *logfile = NULL;
-        char      *volname = NULL;
-        int       cnt = 1;
-        struct glfs_callback_inode_arg *in_arg = NULL;
-        struct glfs_object *root = NULL, *leaf = NULL;
-
-        cbk.reason = 0;
+        glfs_t                   *fs = NULL;
+        int                       ret = 0, i;
+        glfs_fd_t                *fd = NULL;
+        char                     *filename = "/a1";
+        char                     *filename2 = "/a2";
+        struct stat               sb = {0, };
+        struct glfs_upcall       *cbk = NULL;
+        char                     *logfile = NULL;
+        char                     *volname = NULL;
+        int                       cnt = 1;
+        struct glfs_upcall_inode *in_arg = NULL;
+        struct glfs_object       *root = NULL, *leaf = NULL;
 
         fprintf (stderr, "Starting libgfapi_fini\n");
         if (argc != 4) {
@@ -105,11 +103,13 @@ main (int argc, char *argv[])
                 LOG_ERR ("glfs_h_poll_upcall", ret);
 
                 /* There should not be any upcalls sent */
-                if (cbk.reason != GFAPI_CBK_EVENT_NULL) {
+                if (glfs_upcall_get_reason(cbk) != GLFS_UPCALL_EVENT_NULL) {
                         fprintf (stderr, "Error: Upcall received(%d)\n",
-                                 cbk.reason);
+                                 glfs_upcall_get_reason(cbk));
                         exit (1);
                 }
+
+                glfs_free (cbk);
         }
 
         ret = glfs_fini(fs);
diff --git a/tests/basic/gfapi/bug1291259.c b/tests/basic/gfapi/bug1291259.c
index 9fbe02ba34d..78b95c6dcfd 100644
--- a/tests/basic/gfapi/bug1291259.c
+++ b/tests/basic/gfapi/bug1291259.c
@@ -33,23 +33,22 @@ int gfapi = 1;
 int
 main (int argc, char *argv[])
 {
-        glfs_t    *fs = NULL;
-        glfs_t    *fs2 = NULL;
-        int        ret = 0, i;
-        glfs_fd_t *fd = NULL;
-        char      *filename = "/a1";
-        char      *filename2 = "/a2";
-        struct     stat sb = {0, };
-        struct    glfs_callback_arg cbk;
-        char      *logfile = NULL;
-        char      *volname = NULL;
-        char      *hostname = NULL;
-        int        cnt = 1;
-        int       upcall_received = 0;
-        struct glfs_callback_inode_arg *in_arg = NULL;
-        struct glfs_object *root = NULL, *leaf = NULL;
-        unsigned char   globjhdl[GFAPI_HANDLE_LENGTH];
-        unsigned char   globjhdl2[GFAPI_HANDLE_LENGTH];
+        glfs_t                     *fs = NULL;
+        glfs_t                     *fs2 = NULL;
+        int                         ret = 0, i;
+        glfs_fd_t                  *fd = NULL;
+        char                       *filename = "/a1";
+        char                       *filename2 = "/a2";
+        struct                      stat sb = {0, };
+        char                       *logfile = NULL;
+        char                       *volname = NULL;
+        char                       *hostname = NULL;
+        int                         cnt = 1;
+        int                         upcall_received = 0;
+        struct glfs_upcall         *cbk = NULL;
+        struct glfs_object         *root = NULL, *leaf = NULL;
+        unsigned char               globjhdl[GFAPI_HANDLE_LENGTH];
+        unsigned char               globjhdl2[GFAPI_HANDLE_LENGTH];
 
         fprintf (stderr, "Starting libgfapi_fini\n");
         if (argc != 4) {
@@ -82,7 +81,6 @@ main (int argc, char *argv[])
          * on the fs (through this instance) happens. */
         ret = glfs_h_poll_upcall(fs, &cbk);
         LOG_ERR ("glfs_h_poll_upcall", ret);
-        cbk.reason = 0;
 
         fs2 = glfs_new (volname);
         if (!fs) {
@@ -123,21 +121,30 @@ main (int argc, char *argv[])
         }
         fprintf (stderr, "glfs_h_create leaf - %p\n", leaf);
 
-        while (cnt++ < 5) {
+        while (cnt++ < 5 && !upcall_received) {
+                enum glfs_upcall_reason     reason = 0;
+                struct glfs_upcall_inode   *in_arg = NULL;
+
                 ret = glfs_h_poll_upcall(fs, &cbk);
                 LOG_ERR ("glfs_h_poll_upcall", ret);
+                if (ret)
+                        goto retry;
+
+                reason = glfs_upcall_get_reason (cbk);
+                fprintf (stderr, "Upcall received(%d)\n", reason);
+
+                if (reason == GLFS_UPCALL_INODE_INVALIDATE) {
+                        struct glfs_object *object = NULL;
 
-                if (cbk.reason == GFAPI_INODE_INVALIDATE) {
-                        fprintf (stderr, "Upcall received(%d)\n",
-                                 cbk.reason);
-                        in_arg = (struct glfs_callback_inode_arg *)(cbk.event_arg);
+                        in_arg = glfs_upcall_get_event (cbk);
+                        object = glfs_upcall_inode_get_object (in_arg);
 
                         ret = glfs_h_extract_handle (root,
                                                      globjhdl+GLAPI_UUID_LENGTH,
                                                      GFAPI_HANDLE_LENGTH);
                         LOG_ERR("glfs_h_extract_handle", (ret != 16));
 
-                        ret = glfs_h_extract_handle (in_arg->object,
+                        ret = glfs_h_extract_handle (object,
                                                   globjhdl2+GLAPI_UUID_LENGTH,
                                                   GFAPI_HANDLE_LENGTH);
                         LOG_ERR("glfs_h_extract_handle", (ret != 16));
@@ -149,6 +156,13 @@ main (int argc, char *argv[])
                         }
                         upcall_received = 1;
                 }
+
+retry:
+                if (!upcall_received)
+                        sleep (1); /* glfs_h_poll_upcall() does not block */
+
+                glfs_free (cbk);
+                cbk = NULL;
         }
 
         if (!upcall_received) {
diff --git a/tests/basic/gfapi/libgfapi-fini-hang.t b/tests/basic/gfapi/libgfapi-fini-hang.t
index 7c939872574..ba262a943ee 100755
--- a/tests/basic/gfapi/libgfapi-fini-hang.t
+++ b/tests/basic/gfapi/libgfapi-fini-hang.t
@@ -21,9 +21,9 @@ EXPECT 'Created' volinfo_field $V0 'Status';
 TEST $CLI volume start $V0;
 EXPECT 'Started' volinfo_field $V0 'Status';
 
-logdir=`gluster --print-logdir`
+logdir=$(gluster --print-logdir)
 
-TEST build_tester -lgfapi $(dirname $0)/libgfapi-fini-hang.c -o $M0/libgfapi-fini-hang
+TEST build_tester $(dirname $0)/libgfapi-fini-hang.c -o $M0/libgfapi-fini-hang -lgfapi
 TEST cd $M0
  ./libgfapi-fini-hang $H0 $V0 $logdir/libgfapi-fini-hang.log &
 PID=$!
diff --git a/tests/basic/gfapi/upcall-cache-invalidate.c b/tests/basic/gfapi/upcall-cache-invalidate.c
index 44e186955e7..9add3381278 100644
--- a/tests/basic/gfapi/upcall-cache-invalidate.c
+++ b/tests/basic/gfapi/upcall-cache-invalidate.c
@@ -8,7 +8,6 @@
 #include <errno.h>
 #include <glusterfs/api/glfs.h>
 #include <glusterfs/api/glfs-handles.h>
-int gfapi = 1;
 
 #define LOG_ERR(func, ret) do { \
         if (ret != 0) {            \
@@ -23,25 +22,24 @@ int gfapi = 1;
 int
 main (int argc, char *argv[])
 {
-        glfs_t    *fs = NULL;
-        glfs_t    *fs2 = NULL;
-        glfs_t    *fs_tmp = NULL;
-        glfs_t    *fs_tmp2 = NULL;
-        int        ret = 0, i;
-        glfs_fd_t *fd = NULL;
-        glfs_fd_t *fd2 = NULL;
-        glfs_fd_t *fd_tmp = NULL;
-        glfs_fd_t *fd_tmp2 = NULL;
-        char       readbuf[32];
-        char      *filename = "file_tmp";
-        char      *writebuf = NULL;
-        char      *vol_id  = NULL;
-        unsigned int       cnt = 1;
-        struct    glfs_callback_arg cbk;
-        char      *logfile = NULL;
-        char      *volname = NULL;
-        char      *hostname = NULL;
-        struct glfs_callback_inode_arg *in_arg = NULL;
+        glfs_t                   *fs = NULL;
+        glfs_t                   *fs2 = NULL;
+        glfs_t                   *fs_tmp = NULL;
+        glfs_t                   *fs_tmp2 = NULL;
+        int                       ret = 0, i;
+        glfs_fd_t                *fd = NULL;
+        glfs_fd_t                *fd2 = NULL;
+        glfs_fd_t                *fd_tmp = NULL;
+        glfs_fd_t                *fd_tmp2 = NULL;
+        char                      readbuf[32];
+        char                     *filename = "file_tmp";
+        char                     *writebuf = NULL;
+        char                     *vol_id  = NULL;
+        unsigned int              cnt = 1;
+        struct glfs_upcall       *cbk = NULL;
+        char                     *logfile = NULL;
+        char                     *volname = NULL;
+        char                     *hostname = NULL;
 
         if (argc != 4) {
                 fprintf (stderr, "Invalid argument\n");
@@ -72,7 +70,6 @@ main (int argc, char *argv[])
          * on the fs (through this instance) happens. */
         ret = glfs_h_poll_upcall(fs_tmp, &cbk);
         LOG_ERR ("glfs_h_poll_upcall", ret);
-        cbk.reason = 0;
 
         fs2 = glfs_new (volname);
         if (!fs2) {
@@ -139,6 +136,7 @@ main (int argc, char *argv[])
                 ret = glfs_lseek (fd_tmp2, 0, SEEK_SET);
                 LOG_ERR ("glfs_lseek", ret);
 
+                memset (readbuf, 0, sizeof(readbuf));
                 ret = glfs_pread (fd_tmp2, readbuf, 4, 0, 0);
 
                 if (ret <= 0) {
@@ -152,26 +150,37 @@ main (int argc, char *argv[])
                  * there are I/Os on that fd
                  */
                 if (cnt > 2) {
+                        struct glfs_upcall_inode *in_arg = NULL;
+                        enum glfs_upcall_reason   reason = 0;
+                        struct glfs_object       *object = NULL;
+                        uint64_t                  flags = 0;
+                        uint64_t                  expire = 0;
+
                         ret = glfs_h_poll_upcall(fs_tmp, &cbk);
                         LOG_ERR ("glfs_h_poll_upcall", ret);
-                        /* Expect 'GFAPI_INODE_INVALIDATE' upcall event. */
-                        if (cbk.reason == GFAPI_INODE_INVALIDATE) {
-                                in_arg = cbk.event_arg;
+
+                        reason = glfs_upcall_get_reason (cbk);
+
+                        /* Expect 'GLFS_INODE_INVALIDATE' upcall event. */
+                        if (reason == GLFS_UPCALL_INODE_INVALIDATE) {
+                                in_arg = glfs_upcall_get_event (cbk);
+
+                                object = glfs_upcall_inode_get_object (in_arg);
+                                flags = glfs_upcall_inode_get_flags (in_arg);
+                                expire = glfs_upcall_inode_get_expire (in_arg);
+
                                 fprintf (stderr, " upcall event type - %d,"
                                          " object(%p), flags(%d), "
                                          " expire_time_attr(%d)\n" ,
-                                         cbk.reason, in_arg->object,
-                                         in_arg->flags,
-                                         in_arg->expire_time_attr);
-                                ret = glfs_h_close (in_arg->object);
-                                LOG_ERR ("glfs_h_close", ret);
-                                free (in_arg);
+                                         reason, object, flags, expire);
                         } else {
                                 fprintf (stderr,
-                                         "Dint receive upcall notify event");
+                                         "Didnt receive upcall notify event");
                                 ret = -1;
                                 goto err;
                         }
+
+                        glfs_free (cbk);
                 }
 
                 sleep(5);
diff --git a/tests/basic/gfapi/upcall-cache-invalidate.t b/tests/basic/gfapi/upcall-cache-invalidate.t
index 6acbab2c51a..5fd6a3332e7 100755
--- a/tests/basic/gfapi/upcall-cache-invalidate.t
+++ b/tests/basic/gfapi/upcall-cache-invalidate.t
@@ -5,13 +5,6 @@
 
 cleanup;
 
-# Upcall feature is disable for now. A new xlator option
-# will be introduced to turn it on. Skipping this test
-# till then.
-
-SKIP_TESTS;
-exit 0
-
 TEST glusterd
 
 TEST $CLI volume create $V0 $H0:$B0/brick1;