libgfchangelog: Fix faulty reference to xlator context

Problem:
libgfchangelog initializes global xlator on library load (via
constructor: _ctor) and mangles it's xlator context thereby
messing with certain important members of the command structure.
On receiving an RPC disconnection event, if the point-of-execution
was in libgfchangelogs context, accessing ->cmd_args during RPC
notify resulted in a segfault.

Fix:
Since the libarary needs to be able to work with processes that
have a notion of an xlator (THIS in particular) and without it,
care needs to be taken to allocate the global xlator when needed.
Moreover, the actual fix is to use the correct xlator context
in both cases. A new API is introduces when needs to be invoked
by the conusmer (although this could have been done during
register() call, keeping it a separate API makes thing flexible
and easy).

Test:
The issue is observed when a brick process goes offline. This is
triggered when test cases (.t's) are run in bulk, since each
test essestially spawns bricks processes (on volume start) and
terminates them (volume stop). Since bitrot daemon, as of now,
spawns upon volume start, the issue is much observed when the
volume is taken offline at the end of each test case. With this
fix, running the basic and core test cases along with building
the linux kernel has passed without daemon segfaults.

Thanks to Johnny (rabhat@) for helping in debugging the issue
(and with the fix :)).

Change-Id: I8d3022bf749590b2ee816504ed9b1dfccc65559a
BUG: 1170075
Signed-off-by: Venky Shankar <vshankar@redhat.com>
Reviewed-on: http://review.gluster.org/9953
Reviewed-by: Vijay Bellur <vbellur@redhat.com>
Tested-by: Vijay Bellur <vbellur@redhat.com>

4 files changed, 98 insertions, 55 deletions

diff --git a/xlators/features/changelog/lib/examples/c/get-changes-multi.c b/xlators/features/changelog/lib/examples/c/get-changes-multi.c
index 8f23c81c2a0..ae404bc7ad6 100644
--- a/xlators/features/changelog/lib/examples/c/get-changes-multi.c
+++ b/xlators/features/changelog/lib/examples/c/get-changes-multi.c
@@ -70,6 +70,10 @@ main (int argc, char **argv)
         brick++;
         fill_brick_spec (brick, "/export/z2/zwoop");
 
+        ret = gf_changelog_init (NULL);
+        if (ret)
+                goto error_return;
+
         ret = gf_changelog_register_generic ((struct gf_brick_spec *)bricks, 2,
                                              1, "/tmp/multi-changes.log", 9,
                                              NULL);
diff --git a/xlators/features/changelog/lib/src/changelog.h b/xlators/features/changelog/lib/src/changelog.h
index d7048ff2508..1e0df053a96 100644
--- a/xlators/features/changelog/lib/src/changelog.h
+++ b/xlators/features/changelog/lib/src/changelog.h
@@ -97,6 +97,9 @@ gf_changelog_done (char *file);
 
 /* newer flexible API */
 int
+gf_changelog_init (void *xl);
+
+int
 gf_changelog_register_generic (struct gf_brick_spec *bricks, int count,
                                int ordered, char *logfile, int lvl, void *xl);
 
diff --git a/xlators/features/changelog/lib/src/gf-changelog-helpers.h b/xlators/features/changelog/lib/src/gf-changelog-helpers.h
index 17b8862a89b..4247cb46718 100644
--- a/xlators/features/changelog/lib/src/gf-changelog-helpers.h
+++ b/xlators/features/changelog/lib/src/gf-changelog-helpers.h
@@ -179,7 +179,8 @@ typedef struct gf_private {
 
 #define RESTORE_THIS()                          \
         do {                                    \
-                THIS = old_this;                \
+                if (old_this)                   \
+                        THIS = old_this;        \
         } while (0)
 
 /** APIs and the rest */
diff --git a/xlators/features/changelog/lib/src/gf-changelog.c b/xlators/features/changelog/lib/src/gf-changelog.c
index 8f33eb01013..e1cfdb038fa 100644
--- a/xlators/features/changelog/lib/src/gf-changelog.c
+++ b/xlators/features/changelog/lib/src/gf-changelog.c
@@ -50,7 +50,7 @@ gf_private_t *gf_changelog_alloc_priv ()
         int ret = 0;
         gf_private_t *priv = NULL;
 
-        priv = calloc (1, sizeof (gf_private_t));
+        priv = GF_CALLOC (1, sizeof (*priv), gf_changelog_mt_priv_t);
         if (!priv)
                 goto error_return;
         INIT_LIST_HEAD (&priv->connections);
@@ -63,7 +63,7 @@ gf_private_t *gf_changelog_alloc_priv ()
         return priv;
 
  free_priv:
-        free (priv);
+        GF_FREE (priv);
  error_return:
         return NULL;
 }
@@ -80,9 +80,8 @@ gf_changelog_ctx_defaults_init (glusterfs_ctx_t *ctx)
         int            ret         = -1;
 
         ret = xlator_mem_acct_init (THIS, gf_changelog_mt_end);
-        if (ret != 0) {
-                return ret;
-        }
+        if (ret != 0)
+                return -1;
 
         ctx->process_uuid = generate_glusterfs_ctx_id ();
         if (!ctx->process_uuid)
@@ -149,7 +148,7 @@ gf_changelog_ctx_defaults_init (glusterfs_ctx_t *ctx)
 }
 
 /* TODO: cleanup ctx defaults */
-static void
+void
 gf_changelog_cleanup_this (xlator_t *this)
 {
         glusterfs_ctx_t *ctx = NULL;
@@ -161,15 +160,12 @@ gf_changelog_cleanup_this (xlator_t *this)
         syncenv_destroy (ctx->env);
         free (ctx);
 
-        if (this->private)
-                free (this->private);
-
         this->private = NULL;
         this->ctx = NULL;
 }
 
 static int
-gf_changelog_init_this ()
+gf_changelog_init_context ()
 {
         glusterfs_ctx_t *ctx = NULL;
 
@@ -199,50 +195,7 @@ gf_changelog_init_this ()
 static int
 gf_changelog_init_master ()
 {
-        int              ret  = 0;
-        gf_private_t    *priv = NULL;
-        glusterfs_ctx_t *ctx  = NULL;
-
-        ret = gf_changelog_init_this ();
-        if (ret != 0)
-                goto error_return;
-        master = THIS;
-
-        priv = gf_changelog_alloc_priv ();
-        if (!priv)
-                goto cleanup_master;
-        master->private = priv;
-
-        /* poller thread */
-        ret = pthread_create (&priv->poller,
-                              NULL, changelog_rpc_poller, master);
-        if (ret != 0) {
-                gf_log (master->name, GF_LOG_ERROR,
-                        "failed to spawn poller thread");
-                goto cleanup_master;
-        }
-
-        return 0;
-
- cleanup_master:
-        master->private = NULL;
-        gf_changelog_cleanup_this (master);
- error_return:
-        return -1;
-}
-
-/* ctor/dtor */
-
-void
-__attribute__ ((constructor)) gf_changelog_ctor (void)
-{
-        (void) gf_changelog_init_master ();
-}
-
-void
-__attribute__ ((destructor)) gf_changelog_dtor (void)
-{
-        gf_changelog_cleanup_this (master);
+        return gf_changelog_init_context ();
 }
 
 /* TODO: cleanup clnt/svc on failure */
@@ -450,6 +403,88 @@ gf_changelog_setup_logging (xlator_t *this, char *logfile, int loglevel)
         return 0;
 }
 
+static int
+gf_changelog_set_master (xlator_t *master, void *xl)
+{
+        int32_t ret = 0;
+        xlator_t *this = NULL;
+        xlator_t *old_this = NULL;
+        gf_private_t *priv = NULL;
+
+        this = xl;
+        if (!this || !this->ctx) {
+                ret = gf_changelog_init_master ();
+                if (ret)
+                        return -1;
+                this = THIS;
+        }
+
+        master->ctx = this->ctx;
+
+        INIT_LIST_HEAD (&master->volume_options);
+        SAVE_THIS (THIS);
+
+        ret = xlator_mem_acct_init (THIS, gf_changelog_mt_end);
+        if (ret != 0)
+                goto restore_this;
+
+        priv = gf_changelog_alloc_priv ();
+        if (!priv) {
+                ret = -1;
+                goto restore_this;
+        }
+
+        if (!xl) {
+                /* poller thread */
+                ret = pthread_create (&priv->poller,
+                                      NULL, changelog_rpc_poller, THIS);
+                if (ret != 0) {
+                        GF_FREE (priv);
+                        gf_log (master->name, GF_LOG_ERROR,
+                                "failed to spawn poller thread");
+                        goto restore_this;
+                }
+        }
+
+        master->private = priv;
+
+ restore_this:
+        RESTORE_THIS ();
+
+        return ret;
+}
+
+int
+gf_changelog_init (void *xl)
+{
+        int ret = 0;
+
+        if (master)
+                return 0;
+
+        master = calloc (1, sizeof (*master));
+        if (!master)
+                goto error_return;
+
+        master->name = strdup ("gfchangelog");
+        if (!master->name)
+                goto dealloc_master;
+
+        ret = gf_changelog_set_master (master, xl);
+        if (ret)
+                goto dealloc_name;
+
+        return 0;
+
+ dealloc_name:
+        free (master->name);
+ dealloc_master:
+        free (master);
+        master = NULL;
+ error_return:
+        return -1;
+}
+
 int
 gf_changelog_register_generic (struct gf_brick_spec *bricks, int count,
                                int ordered, char *logfile, int lvl, void *xl)