summaryrefslogtreecommitdiffstats
path: root/xlators/features/changelog
diff options
context:
space:
mode:
authorVenky Shankar <vshankar@redhat.com>2015-03-19 10:09:04 +0530
committerVijay Bellur <vbellur@redhat.com>2015-03-24 06:25:15 -0700
commit9c9f3f368c693b1cf5f67f3d8d4e599d4ba61383 (patch)
tree8927e9270a0f3fd27a42d3af7eab1287f4772ffa /xlators/features/changelog
parent191ab5693d3c9a0cdedc66bb24dd5efa535963d9 (diff)
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>
Diffstat (limited to 'xlators/features/changelog')
-rw-r--r--xlators/features/changelog/lib/examples/c/get-changes-multi.c4
-rw-r--r--xlators/features/changelog/lib/src/changelog.h3
-rw-r--r--xlators/features/changelog/lib/src/gf-changelog-helpers.h3
-rw-r--r--xlators/features/changelog/lib/src/gf-changelog.c143
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 8f23c81..ae404bc 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 d7048ff..1e0df05 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 17b8862..4247cb4 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 8f33eb0..e1cfdb0 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)