mgtm/core : use sha hash function for volfile check

We are storing the entire volfile and using this to check
volfile change. With brick multiplexing there will be lot
of graphs per process which will increase the memory foot
print of the process. So instead of storing the entire
graph we could use sha256 and we can compare the hash to
see whether volfile change happened or not.

Also with Brick multiplexing, the direct comparison of vol
file is not correct. There are two problems.

Problem 1:

We are currently storing one single graph (the last
updated volfile) whereas, what we need is the entire
graph with all atttached bricks.

If we fix this issue, we have second problem

Problem 2:
With multiplexing we have a graph that contains multiple
bricks. But what we are checking as part of the reconfigure
is, comparing the entire graph with one single graph,
which will always fail.

Solution:
We create list in glusterfs_ctx_t that stores sha256 hash
of individual brick graphs. When a graph changes happens
we compare the stored hash and the current hash. If the
hash matches, then no need for reconfigure. Otherwise we
first do the reconfigure and then update the hash.

For now, gfapi has not changed this way. Meaning when gfapi
volfile fetch or reconfigure happens, we still store the
entire graph and compare, each memory.

This is fine, because libgfapi will not load brick graphs.
But changing the libgfapi will make the code similar in
both glusterfsd-mgmt and api. Also it helps to reduce some
memory.

Change-Id: I9df917a771a52b95622ab8f63af34ec390163a77
BUG: 1467986
Signed-off-by: Mohammed Rafi KC <rkavunga@redhat.com>
Reviewed-on: https://review.gluster.org/17709
Smoke: Gluster Build System <jenkins@build.gluster.org>
Reviewed-by: Pranith Kumar Karampuri <pkarampu@redhat.com>
CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
Reviewed-by: Atin Mukherjee <amukherj@redhat.com>
Reviewed-by: Amar Tumballi <amarts@redhat.com>

11 files changed, 257 insertions, 44 deletions

diff --git a/api/src/glfs-mgmt.c b/api/src/glfs-mgmt.c
index bb4e77a1467..32b9dbd4e31 100644
--- a/api/src/glfs-mgmt.c
+++ b/api/src/glfs-mgmt.c
@@ -650,8 +650,8 @@ glfs_mgmt_getspec_cbk (struct rpc_req *req, struct iovec *iov, int count,
 	*  return -1(or -ve) =======> Some Internal Error occurred during the operation
 	*/
 
-	ret = glusterfs_volfile_reconfigure (fs->oldvollen, tmpfp, fs->ctx,
-					     fs->oldvolfile);
+        ret = gf_volfile_reconfigure (fs->oldvollen, tmpfp, fs->ctx,
+                                      fs->oldvolfile);
 	if (ret == 0) {
 		gf_msg_debug ("glusterfsd-mgmt", 0, "No need to re-load "
                               "volfile, reconfigure done");
diff --git a/glusterfsd/src/glusterfsd-mgmt.c b/glusterfsd/src/glusterfsd-mgmt.c
index c17bf3bb6fc..ca706d1020d 100644
--- a/glusterfsd/src/glusterfsd-mgmt.c
+++ b/glusterfsd/src/glusterfsd-mgmt.c
@@ -1761,12 +1761,6 @@ out:
         return ret;
 }
 
-
-/* XXX: move these into @ctx */
-static char *oldvolfile = NULL;
-static int   oldvollen;
-
-
 int
 mgmt_getspec_cbk (struct rpc_req *req, struct iovec *iov, int count,
                   void *myframe)
@@ -1777,7 +1771,10 @@ mgmt_getspec_cbk (struct rpc_req *req, struct iovec *iov, int count,
         int                      ret   = 0, locked = 0;
         ssize_t                  size = 0;
         FILE                    *tmpfp = NULL;
-        char                    *volfilebuf = NULL;
+        char                    *volfile_id = NULL;
+        gf_volfile_t            *volfile_obj = NULL;
+        gf_volfile_t            *volfile_tmp = NULL;
+        char                     sha256_hash[SHA256_DIGEST_LENGTH] = {0, };
 
         frame = myframe;
         ctx = frame->this->ctx;
@@ -1804,14 +1801,29 @@ mgmt_getspec_cbk (struct rpc_req *req, struct iovec *iov, int count,
         ret = 0;
         size = rsp.op_ret;
 
+        glusterfs_compute_sha256 ((const unsigned char *) rsp.spec, size,
+                                  sha256_hash);
+
+        volfile_id = frame->local;
+
         LOCK (&ctx->volfile_lock);
         {
                 locked = 1;
 
-                if (size == oldvollen && (memcmp (oldvolfile, rsp.spec, size) == 0)) {
-                        gf_log (frame->this->name, GF_LOG_INFO,
-                                "No change in volfile, continuing");
-                        goto out;
+                list_for_each_entry (volfile_obj,  &ctx->volfile_list,
+                                     volfile_list) {
+                        if (!strcmp (volfile_id, volfile_obj->vol_id)) {
+                               if (!strncmp (sha256_hash,
+                                      volfile_obj->volfile_checksum,
+                                      sizeof (volfile_obj->volfile_checksum))) {
+                                        gf_log (frame->this->name, GF_LOG_INFO,
+                                                "No change in volfile,"
+                                                "continuing");
+                                        goto out;
+                               }
+                               volfile_tmp = volfile_obj;
+                               break;
+                        }
                 }
 
                 tmpfp = tmpfile ();
@@ -1835,21 +1847,19 @@ mgmt_getspec_cbk (struct rpc_req *req, struct iovec *iov, int count,
                  *  return -1(or -ve) =======> Some Internal Error occurred during the operation
                  */
 
-                ret = glusterfs_volfile_reconfigure (oldvollen, tmpfp, ctx, oldvolfile);
+                ret = glusterfs_volfile_reconfigure (tmpfp, ctx);
                 if (ret == 0) {
                         gf_log ("glusterfsd-mgmt", GF_LOG_DEBUG,
                                 "No need to re-load volfile, reconfigure done");
-                        if (oldvolfile)
-                                volfilebuf = GF_REALLOC (oldvolfile, size);
-                        else
-                                volfilebuf = GF_CALLOC (1, size, gf_common_mt_char);
-                        if (!volfilebuf) {
+                        if (!volfile_tmp) {
                                 ret = -1;
+                                gf_log ("mgmt", GF_LOG_ERROR, "Graph "
+                                        "reconfigure succeeded with out having "
+                                        "checksum.");
                                 goto out;
                         }
-                        oldvolfile = volfilebuf;
-                        oldvollen = size;
-                        memcpy (oldvolfile, rsp.spec, size);
+                        strncpy (volfile_tmp->volfile_checksum, sha256_hash,
+                                 sizeof (volfile_tmp->volfile_checksum));
                         goto out;
                 }
 
@@ -1865,19 +1875,23 @@ mgmt_getspec_cbk (struct rpc_req *req, struct iovec *iov, int count,
                 if (ret)
                         goto out;
 
-                if (oldvolfile)
-                        volfilebuf = GF_REALLOC (oldvolfile, size);
-                else
-                        volfilebuf = GF_CALLOC (1, size, gf_common_mt_char);
+                if (!volfile_tmp) {
+                        volfile_tmp = GF_CALLOC (1, sizeof (gf_volfile_t),
+                                                 gf_common_volfile_t);
+                        if (!volfile_tmp) {
+                                ret = -1;
+                                goto out;
+                        }
 
-                if (!volfilebuf) {
-                        ret = -1;
-                        goto out;
+                        INIT_LIST_HEAD (&volfile_tmp->volfile_list);
+                        list_add (&volfile_tmp->volfile_list,
+                                  &ctx->volfile_list);
+                        snprintf (volfile_tmp->vol_id,
+                                  sizeof (volfile_tmp->vol_id), "%s",
+                                  volfile_id);
                 }
-
-                oldvolfile = volfilebuf;
-                oldvollen = size;
-                memcpy (oldvolfile, rsp.spec, size);
+                strncpy (volfile_tmp->volfile_checksum, sha256_hash,
+                         sizeof (volfile_tmp->volfile_checksum));
         }
         UNLOCK (&ctx->volfile_lock);
 
@@ -1894,7 +1908,11 @@ out:
         if (locked)
                 UNLOCK (&ctx->volfile_lock);
 
-        STACK_DESTROY (frame->root);
+        if (frame) {
+                GF_FREE (frame->local);
+                frame->local = NULL;
+                STACK_DESTROY (frame->root);
+        }
 
         free (rsp.spec);
 
@@ -1941,6 +1959,15 @@ glusterfs_volfile_fetch_one (glusterfs_ctx_t *ctx, char *volfile_id)
 
         req.key = volfile_id;
         req.flags = 0;
+        /*
+         * We are only storing one variable in local, hence using the same
+         * variable. If multiple local variable is required, create a struct.
+         */
+        frame->local = gf_strdup (volfile_id);
+        if (!frame->local) {
+                ret = -1;
+                goto out;
+        }
 
         dict = dict_new ();
         if (!dict) {
@@ -1990,6 +2017,13 @@ out:
         GF_FREE (req.xdata.xdata_val);
         if (dict)
                 dict_unref (dict);
+        if (ret && frame) {
+                /* Free the frame->local fast, because we have not used memget
+                 */
+                GF_FREE (frame->local);
+                frame->local = NULL;
+                STACK_DESTROY (frame->root);
+        }
 
         return ret;
 }
diff --git a/libglusterfs/src/common-utils.c b/libglusterfs/src/common-utils.c
index e5e08909ac7..c93b3667ad9 100644
--- a/libglusterfs/src/common-utils.c
+++ b/libglusterfs/src/common-utils.c
@@ -4950,3 +4950,15 @@ gf_getgrouplist (const char *user, gid_t group, gid_t **groups)
         }
         return ret;
 }
+
+int
+glusterfs_compute_sha256 (const unsigned char *content, size_t size,
+                          char *sha256_hash) {
+        SHA256_CTX               sha256;
+
+        SHA256_Init (&sha256);
+        SHA256_Update (&sha256, (const unsigned char *) (content), size);
+        SHA256_Final ((unsigned char *) sha256_hash, &sha256);
+
+        return 0;
+}
diff --git a/libglusterfs/src/common-utils.h b/libglusterfs/src/common-utils.h
index 6a260c090c8..86b7ec6c3d6 100644
--- a/libglusterfs/src/common-utils.h
+++ b/libglusterfs/src/common-utils.h
@@ -921,4 +921,9 @@ close_fds_except (int *fdv, size_t count);
 
 int
 gf_getgrouplist (const char *user, gid_t group, gid_t **groups);
+
+int
+glusterfs_compute_sha256 (const unsigned char *content, size_t size,
+                          char *sha256_hash);
+
 #endif /* _COMMON_UTILS_H */
diff --git a/libglusterfs/src/ctx.c b/libglusterfs/src/ctx.c
index 1cf1b988590..365c6c087ee 100644
--- a/libglusterfs/src/ctx.c
+++ b/libglusterfs/src/ctx.c
@@ -32,6 +32,7 @@ glusterfs_ctx_new ()
 
         INIT_LIST_HEAD (&ctx->graphs);
 	INIT_LIST_HEAD (&ctx->mempool_list);
+        INIT_LIST_HEAD (&ctx->volfile_list);
 
 	ctx->daemon_pipe[0] = -1;
 	ctx->daemon_pipe[1] = -1;
diff --git a/libglusterfs/src/glusterfs.h b/libglusterfs/src/glusterfs.h
index 2cd1f363b3b..fa3828365f7 100644
--- a/libglusterfs/src/glusterfs.h
+++ b/libglusterfs/src/glusterfs.h
@@ -28,6 +28,7 @@
 #include <sys/poll.h>
 #include <pthread.h>
 #include <limits.h> /* For PATH_MAX */
+#include <openssl/sha.h>
 
 #include "glusterfs-fops.h" /* generated XDR values for FOPs */
 
@@ -532,9 +533,19 @@ struct _glusterfs_ctx {
                 gf_atomic_t total_pairs_used;
                 gf_atomic_t total_dicts_used;
         } stats;
+
+        struct list_head volfile_list;
 };
 typedef struct _glusterfs_ctx glusterfs_ctx_t;
 
+typedef struct {
+        char volfile_checksum[SHA256_DIGEST_LENGTH];
+        char vol_id[NAME_MAX+1];
+        struct list_head volfile_list;
+
+} gf_volfile_t;
+
+
 glusterfs_ctx_t *glusterfs_ctx_new (void);
 
 struct gf_flock {
diff --git a/libglusterfs/src/graph.c b/libglusterfs/src/graph.c
index 11d2a0adf1e..738cd9688db 100644
--- a/libglusterfs/src/graph.c
+++ b/libglusterfs/src/graph.c
@@ -832,8 +832,70 @@ out:
  *   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_volfile_reconfigure (FILE *newvolfile_fp, glusterfs_ctx_t *ctx)
+{
+        glusterfs_graph_t *oldvolfile_graph = NULL;
+        glusterfs_graph_t *newvolfile_graph = NULL;
+
+        int ret = -1;
+
+        if (!ctx) {
+                gf_msg ("glusterfsd-mgmt", GF_LOG_ERROR, 0, LG_MSG_CTX_NULL,
+                        "ctx is NULL");
+                goto out;
+        }
+
+        oldvolfile_graph = ctx->active;
+        if (!oldvolfile_graph) {
+                ret = 1;
+                goto out;
+        }
+
+        newvolfile_graph = glusterfs_graph_construct (newvolfile_fp);
+
+        if (!newvolfile_graph) {
+                goto out;
+        }
+
+        glusterfs_graph_prepare (newvolfile_graph, ctx,
+                                 ctx->cmd_args.volume_name);
+
+        if (!is_graph_topology_equal (oldvolfile_graph,
+                                      newvolfile_graph)) {
+
+                ret = 1;
+                gf_msg_debug ("glusterfsd-mgmt", 0, "Graph topology not "
+                              "equal(should call INIT)");
+                goto out;
+        }
+
+        gf_msg_debug ("glusterfsd-mgmt", 0, "Only options have changed in the"
+                      " new graph");
+
+        ret = glusterfs_graph_reconfigure (oldvolfile_graph,
+                                           newvolfile_graph);
+        if (ret) {
+                gf_msg_debug ("glusterfsd-mgmt", 0, "Could not reconfigure "
+                              "new options in old graph");
+                goto out;
+        }
+
+        ret = 0;
+out:
+
+        if (newvolfile_graph)
+                glusterfs_graph_destroy (newvolfile_graph);
+
+        return ret;
+}
+
+/* This function need to remove. This added to support gfapi volfile
+ * reconfigure.
+ */
+
+int
+gf_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;
@@ -855,9 +917,9 @@ glusterfs_volfile_reconfigure (int oldvollen, FILE *newvolfile_fp,
 
         if (!ctx) {
                 gf_msg ("glusterfsd-mgmt", GF_LOG_ERROR, 0, LG_MSG_CTX_NULL,
-			"ctx is NULL");
-		goto out;
-	}
+                        "ctx is NULL");
+                goto out;
+        }
 
         oldvolfile_graph = ctx->active;
         if (!oldvolfile_graph) {
@@ -908,7 +970,7 @@ glusterfs_volfile_reconfigure (int oldvollen, FILE *newvolfile_fp,
                 goto out;
         }
 
-	glusterfs_graph_prepare (newvolfile_graph, ctx,
+        glusterfs_graph_prepare (newvolfile_graph, ctx,
                                  ctx->cmd_args.volume_name);
 
         if (!is_graph_topology_equal (oldvolfile_graph,
@@ -952,7 +1014,6 @@ out:
         return ret;
 }
 
-
 int
 glusterfs_graph_reconfigure (glusterfs_graph_t *oldgraph,
                              glusterfs_graph_t *newgraph)
@@ -1056,19 +1117,56 @@ glusterfs_graph_attach (glusterfs_graph_t *orig_graph, char *path,
         FILE                    *fp;
         glusterfs_graph_t       *graph;
         xlator_t                *xl;
-        char                    *volfile_id;
+        char                    *volfile_id                        = NULL;
+        char                    *volfile_content                   = NULL;
+        struct stat              stbuf                             = {0,};
+        size_t                   file_len                          = -1;
+        gf_volfile_t            *volfile_obj                       = NULL;
+        int                      ret                               = -1;
+        char                     sha256_hash[SHA256_DIGEST_LENGTH] = {0, };
 
         if (!orig_graph) {
                 return -EINVAL;
         }
 
+        ret = sys_stat (path, &stbuf);
+        if (ret < 0) {
+                gf_log (THIS->name, GF_LOG_ERROR, "Unable to stat %s (%s)",
+                        path, strerror (errno));
+                return -EINVAL;
+        }
+
+        file_len = stbuf.st_size;
+        if (file_len) {
+                volfile_content = GF_CALLOC (file_len+1, sizeof (char),
+                                             gf_common_mt_char);
+                if (!volfile_content) {
+                        return -ENOMEM;
+                }
+        }
+
         fp = fopen (path, "r");
         if (!fp) {
                 gf_log (THIS->name, GF_LOG_WARNING,
                         "oops, %s disappeared on us", path);
+                GF_FREE (volfile_content);
                 return -EIO;
         }
 
+        ret = fread (volfile_content, sizeof (char), file_len, fp);
+        if (ret == file_len) {
+              glusterfs_compute_sha256 ((const unsigned char *) volfile_content,
+                                         file_len, sha256_hash);
+        } else {
+                gf_log (THIS->name, GF_LOG_ERROR,
+                        "read failed on path %s. File size=%"GF_PRI_SIZET
+                        "read size=%d", path, file_len, ret);
+                GF_FREE (volfile_content);
+                return -EIO;
+        }
+
+        GF_FREE (volfile_content);
+
         graph = glusterfs_graph_construct (fp);
         fclose(fp);
         if (!graph) {
@@ -1117,5 +1215,20 @@ glusterfs_graph_attach (glusterfs_graph_t *orig_graph, char *path,
                 return -EIO;
         }
 
+        if (!volfile_obj) {
+                volfile_obj = GF_CALLOC (1, sizeof (gf_volfile_t),
+                                         gf_common_volfile_t);
+                if (!volfile_obj) {
+                        return -EIO;
+                }
+        }
+
+        INIT_LIST_HEAD (&volfile_obj->volfile_list);
+        snprintf (volfile_obj->vol_id, sizeof (volfile_obj->vol_id),
+                  "%s", xl->volfile_id);
+        strncpy (volfile_obj->volfile_checksum, sha256_hash,
+                 sizeof (volfile_obj->volfile_checksum));
+        list_add (&volfile_obj->volfile_list, &this->ctx->volfile_list);
+
         return 0;
 }
diff --git a/libglusterfs/src/mem-types.h b/libglusterfs/src/mem-types.h
index d5b2fcd64bd..c2ac088f639 100644
--- a/libglusterfs/src/mem-types.h
+++ b/libglusterfs/src/mem-types.h
@@ -174,6 +174,7 @@ enum gf_common_mem_types_ {
         gf_common_mt_tbf_throttle_t,
         gf_common_mt_pthread_t,
         gf_common_ping_local_t,
+        gf_common_volfile_t,
         gf_common_mt_end
 };
 #endif
diff --git a/libglusterfs/src/xlator.c b/libglusterfs/src/xlator.c
index 9da6d657707..de97dff6dfe 100644
--- a/libglusterfs/src/xlator.c
+++ b/libglusterfs/src/xlator.c
@@ -1195,3 +1195,29 @@ copy_opts_to_child (xlator_t *src, xlator_t *dst, char *glob)
         return dict_foreach_fnmatch (src->options, glob,
                                      _copy_opt_to_child, dst);
 }
+
+int
+glusterfs_delete_volfile_checksum (glusterfs_ctx_t *ctx,
+                                   const char *volfile_id) {
+
+        gf_volfile_t            *volfile_tmp      = NULL;
+        gf_volfile_t            *volfile_obj      = NULL;
+
+        list_for_each_entry (volfile_tmp,  &ctx->volfile_list,
+                             volfile_list) {
+                if (!strcmp (volfile_id, volfile_tmp->vol_id)) {
+                        list_del_init (&volfile_tmp->volfile_list);
+                        volfile_obj = volfile_tmp;
+                        break;
+                }
+        }
+
+        if (volfile_obj) {
+                GF_FREE (volfile_obj);
+        } else {
+                gf_log (THIS->name, GF_LOG_ERROR, "failed to get volfile "
+                        "checksum for volfile id %s.", volfile_id);
+        }
+
+        return 0;
+}
diff --git a/libglusterfs/src/xlator.h b/libglusterfs/src/xlator.h
index 06e7241bb8e..26d2cc5b595 100644
--- a/libglusterfs/src/xlator.h
+++ b/libglusterfs/src/xlator.h
@@ -1043,8 +1043,11 @@ enum gf_hdsk_event_notify_op {
 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);
+glusterfs_volfile_reconfigure (FILE *newvolfile_fp, glusterfs_ctx_t *ctx);
+
+int
+gf_volfile_reconfigure (int oldvollen, FILE *newvolfile_fp,
+                        glusterfs_ctx_t *ctx, const char *oldvolfile);
 
 int
 loc_touchup (loc_t *loc, const char *name);
@@ -1063,4 +1066,8 @@ void xlator_init_unlock (void);
 int
 copy_opts_to_child (xlator_t *src, xlator_t *dst, char *glob);
 
+int
+glusterfs_delete_volfile_checksum (glusterfs_ctx_t *ctx,
+                                   const char *volfile_id);
+
 #endif /* _XLATOR_H */
diff --git a/xlators/protocol/server/src/server.c b/xlators/protocol/server/src/server.c
index b714d54dc97..e59941e4ca1 100644
--- a/xlators/protocol/server/src/server.c
+++ b/xlators/protocol/server/src/server.c
@@ -1559,6 +1559,9 @@ notify (xlator_t *this, int32_t event, void *data, ...)
                                                 break;
                                         }
                                 }
+                                if (victim_found)
+                                        glusterfs_delete_volfile_checksum (ctx,
+                                                 victim->volfile_id);
                         UNLOCK (&ctx->volfile_lock);
                         if (victim_found)
                                 (*trav_p) = (*trav_p)->next;