From dcfe4ab475c53cec7d117fb8052b26213a4b41db Mon Sep 17 00:00:00 2001 From: Raghavendra Bhat Date: Thu, 25 Jul 2013 01:14:48 +0530 Subject: glusterfsd, libgfapi: destroy the temporary graphs constructed for comparison * The new and the oldgraphs which have been constructed whenever there is a volfile change (either reconfigure of the existing graph or creating a new graph) for comparison should be freed. Otherwise frequent graph changes will lead to huge memory leak Change-Id: I4faddb1aa9393b34cd2de6732e537a60f600026a BUG: 948178 Signed-off-by: Raghavendra Bhat Reviewed-on: http://review.gluster.org/5388 Tested-by: Gluster Build System Reviewed-by: Anand Avati --- libglusterfs/src/graph.c | 185 ++++++++++++++++++++++++++++++++++++++++++++++ libglusterfs/src/graph.y | 2 + libglusterfs/src/xlator.c | 43 +++++++---- libglusterfs/src/xlator.h | 6 +- 4 files changed, 219 insertions(+), 17 deletions(-) (limited to 'libglusterfs') diff --git a/libglusterfs/src/graph.c b/libglusterfs/src/graph.c index 1dba63fc0..e76df1ca5 100644 --- a/libglusterfs/src/graph.c +++ b/libglusterfs/src/graph.c @@ -536,6 +536,184 @@ glusterfs_graph_activate (glusterfs_graph_t *graph, glusterfs_ctx_t *ctx) } +int +xlator_equal_rec (xlator_t *xl1, xlator_t *xl2) +{ + xlator_list_t *trav1 = NULL; + xlator_list_t *trav2 = NULL; + int ret = 0; + + if (xl1 == NULL || xl2 == NULL) { + gf_log ("xlator", GF_LOG_DEBUG, "invalid argument"); + return -1; + } + + trav1 = xl1->children; + trav2 = xl2->children; + + while (trav1 && trav2) { + ret = xlator_equal_rec (trav1->xlator, trav2->xlator); + if (ret) { + gf_log ("glusterfsd-mgmt", GF_LOG_DEBUG, + "xlators children not equal"); + goto out; + } + + trav1 = trav1->next; + trav2 = trav2->next; + } + + if (trav1 || trav2) { + ret = -1; + goto out; + } + + if (strcmp (xl1->name, xl2->name)) { + ret = -1; + goto out; + } + + /* type could have changed even if xlator names match, + e.g cluster/distrubte and cluster/nufa share the same + xlator name + */ + if (strcmp (xl1->type, xl2->type)) { + ret = -1; + goto out; + } +out : + return ret; +} + + +gf_boolean_t +is_graph_topology_equal (glusterfs_graph_t *graph1, glusterfs_graph_t *graph2) +{ + xlator_t *trav1 = NULL; + xlator_t *trav2 = NULL; + gf_boolean_t ret = _gf_true; + + trav1 = graph1->first; + trav2 = graph2->first; + + ret = xlator_equal_rec (trav1, trav2); + + if (ret) { + gf_log ("glusterfsd-mgmt", GF_LOG_DEBUG, + "graphs are not equal"); + ret = _gf_false; + goto out; + } + + ret = _gf_true; + gf_log ("glusterfsd-mgmt", GF_LOG_DEBUG, + "graphs are equal"); + +out: + return ret; +} + + +/* Function has 3types of return value 0, -ve , 1 + * return 0 =======> reconfiguration of options has succeeded + * return 1 =======> the graph has to be reconstructed and all the xlators should be inited + * return -1(or -ve) =======> Some Internal Error occurred during the operation + */ +int +glusterfs_volfile_reconfigure (int oldvollen, FILE *newvolfile_fp, + glusterfs_ctx_t *ctx, const char *oldvolfile) +{ + glusterfs_graph_t *oldvolfile_graph = NULL; + glusterfs_graph_t *newvolfile_graph = NULL; + FILE *oldvolfile_fp = NULL; + gf_boolean_t active_graph_found = _gf_true; + + int ret = -1; + + if (!oldvollen) { + ret = 1; // Has to call INIT for the whole graph + goto out; + } + + if (!ctx) { + gf_log ("glusterfsd-mgmt", GF_LOG_ERROR, + "ctx is NULL"); + goto out; + } + + oldvolfile_graph = ctx->active; + if (!oldvolfile_graph) { + active_graph_found = _gf_false; + gf_log ("glusterfsd-mgmt", GF_LOG_ERROR, + "glusterfs_ctx->active is NULL"); + + oldvolfile_fp = tmpfile (); + if (!oldvolfile_fp) { + gf_log ("glusterfsd-mgmt", GF_LOG_ERROR, "Unable to " + "create temporary volfile: (%s)", + strerror (errno)); + goto out; + } + + fwrite (oldvolfile, oldvollen, 1, oldvolfile_fp); + fflush (oldvolfile_fp); + if (ferror (oldvolfile_fp)) { + goto out; + } + + oldvolfile_graph = glusterfs_graph_construct (oldvolfile_fp); + if (!oldvolfile_graph) + goto out; + } + + newvolfile_graph = glusterfs_graph_construct (newvolfile_fp); + if (!newvolfile_graph) { + goto out; + } + + if (!is_graph_topology_equal (oldvolfile_graph, + newvolfile_graph)) { + + ret = 1; + gf_log ("glusterfsd-mgmt", GF_LOG_DEBUG, + "Graph topology not equal(should call INIT)"); + goto out; + } + + gf_log ("glusterfsd-mgmt", GF_LOG_DEBUG, + "Only options have changed in the new " + "graph"); + + /* */ + ret = glusterfs_graph_reconfigure (oldvolfile_graph, + newvolfile_graph); + if (ret) { + gf_log ("glusterfsd-mgmt", GF_LOG_DEBUG, + "Could not reconfigure new options in old graph"); + goto out; + } + + ret = 0; +out: + if (oldvolfile_fp) + fclose (oldvolfile_fp); + + /* Do not simply destroy the old graph here. If the oldgraph + is constructed here in this function itself instead of getting + it from ctx->active (which happens only of ctx->active is NULL), + then destroy the old graph. If some i/o is still happening in + the old graph and the old graph is obtained from ctx->active, + then destroying the graph will cause problems. + */ + if (!active_graph_found && oldvolfile_graph) + glusterfs_graph_destroy (oldvolfile_graph); + if (newvolfile_graph) + glusterfs_graph_destroy (newvolfile_graph); + + return ret; +} + + int glusterfs_graph_reconfigure (glusterfs_graph_t *oldgraph, glusterfs_graph_t *newgraph) @@ -562,5 +740,12 @@ glusterfs_graph_reconfigure (glusterfs_graph_t *oldgraph, int glusterfs_graph_destroy (glusterfs_graph_t *graph) { + xlator_tree_free (graph->first); + + if (graph) { + list_del_init (&graph->list); + GF_FREE (graph); + } + return 0; } diff --git a/libglusterfs/src/graph.y b/libglusterfs/src/graph.y index ca4301ff0..a220abeb9 100644 --- a/libglusterfs/src/graph.y +++ b/libglusterfs/src/graph.y @@ -482,6 +482,7 @@ preprocess (FILE *srcfp, FILE *dstfp) cmd_buf_size *= 2; cmd = GF_REALLOC (cmd, cmd_buf_size); if (cmd == NULL) { + GF_FREE (result); return -1; } @@ -523,6 +524,7 @@ preprocess (FILE *srcfp, FILE *dstfp) out: fseek (srcfp, 0L, SEEK_SET); fseek (dstfp, 0L, SEEK_SET); + GF_FREE (cmd); GF_FREE (result); diff --git a/libglusterfs/src/xlator.c b/libglusterfs/src/xlator.c index a7caedf02..d02947550 100644 --- a/libglusterfs/src/xlator.c +++ b/libglusterfs/src/xlator.c @@ -538,10 +538,26 @@ out: return; } +int +xlator_list_destroy (xlator_list_t *list) +{ + xlator_list_t *next = NULL; + + while (list) { + next = list->next; + GF_FREE (list); + list = next; + } + + return 0; +} + int xlator_tree_free (xlator_t *tree) { + volume_opt_list_t *vol_opt = NULL; + volume_opt_list_t *tmp = NULL; xlator_t *trav = tree; xlator_t *prev = tree; @@ -552,9 +568,19 @@ xlator_tree_free (xlator_t *tree) while (prev) { trav = prev->next; - dict_destroy (prev->options); + if (prev->dlhandle) + dlclose (prev->dlhandle); + dict_unref (prev->options); GF_FREE (prev->name); GF_FREE (prev->type); + xlator_list_destroy (prev->children); + xlator_list_destroy (prev->parents); + + list_for_each_entry_safe (vol_opt, tmp, &prev->volume_options, + list) { + list_del_init (&vol_opt->list); + GF_FREE (vol_opt); + } GF_FREE (prev); prev = trav; } @@ -695,21 +721,6 @@ loc_is_root (loc_t *loc) return _gf_false; } -int -xlator_list_destroy (xlator_list_t *list) -{ - xlator_list_t *next = NULL; - - while (list) { - next = list->next; - GF_FREE (list); - list = next; - } - - return 0; -} - - int xlator_destroy (xlator_t *xl) { diff --git a/libglusterfs/src/xlator.h b/libglusterfs/src/xlator.h index d23ee0c25..84a028fbc 100644 --- a/libglusterfs/src/xlator.h +++ b/libglusterfs/src/xlator.h @@ -932,5 +932,9 @@ enum gf_hdsk_event_notify_op { GF_EN_DEFRAG_STATUS, GF_EN_MAX, }; - +gf_boolean_t +is_graph_topology_equal (glusterfs_graph_t *graph1, glusterfs_graph_t *graph2); +int +glusterfs_volfile_reconfigure (int oldvollen, FILE *newvolfile_fp, + glusterfs_ctx_t *ctx, const char *oldvolfile); #endif /* _XLATOR_H */ -- cgit