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 <raghavendra@redhat.com>
Reviewed-on: http://review.gluster.org/5388
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Anand Avati <avati@redhat.com>

4 files changed, 219 insertions, 17 deletions

diff --git a/libglusterfs/src/graph.c b/libglusterfs/src/graph.c
index 1dba63fc052..e76df1ca560 100644
--- a/libglusterfs/src/graph.c
+++ b/libglusterfs/src/graph.c
@@ -537,6 +537,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 ca4301ff0ac..a220abeb9ce 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 a7caedf02e7..d029475504a 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;
         }
@@ -696,21 +722,6 @@ loc_is_root (loc_t *loc)
 }
 
 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)
 {
         volume_opt_list_t *vol_opt = NULL;
diff --git a/libglusterfs/src/xlator.h b/libglusterfs/src/xlator.h
index d23ee0c25d7..84a028fbcab 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 */