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.

Cherry picked from commit 4721188a154acd9a0a4c096d8d73e97f3bf1b2a9:
> 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>

Once difference with the version of this change in other branches is
that leases are not included in glusterfs-3.7. Hence there is a little
change that drops the handling of GF_UPCALL_RECALL_LEASE.

In addition, this backport contains commit 2775dc6410:
> libgfapi/upcall : prepend "glfs_" to callback_arg, callback_inode_arg
> Reviewed-on: http://review.gluster.org/14702

Change-Id: I2b8bd5a0a82036d2abea1a217f5e5975a1d4fe93
BUG: 1347715
Signed-off-by: Niels de Vos <ndevos@redhat.com>
Reviewed-on: http://review.gluster.org/15602
NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>
Smoke: Gluster Build System <jenkins@build.gluster.org>
CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
Reviewed-by: Kaushal M <kaushal@redhat.com>

1 files changed, 163 insertions, 60 deletions

diff --git a/api/src/glfs-handleops.c b/api/src/glfs-handleops.c
index f9b4ee90267..bc2c43b1f39 100644
--- a/api/src/glfs-handleops.c
+++ b/api/src/glfs-handleops.c
@@ -1856,9 +1856,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 callback_arg *up_arg,
+                                struct glfs_upcall *up_arg,
                                 struct gf_upcall *upcall_data)
 {
         int                                 ret           = -1;
@@ -1866,7 +1884,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 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",
@@ -1891,13 +1909,11 @@ glfs_h_poll_cache_invalidation (struct glfs *fs,
                 goto out;
         }
 
-        up_inode_arg = GF_CALLOC (1, sizeof (struct 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;
@@ -1948,6 +1964,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:
@@ -1956,47 +1976,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 '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 - 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 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;
 
@@ -2009,15 +2024,13 @@ pub_glfs_h_poll_upcall (struct glfs *fs, struct 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;
@@ -2040,46 +2053,52 @@ pub_glfs_h_poll_upcall (struct glfs *fs, struct 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_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;
@@ -2099,7 +2118,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"