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>

1 files changed, 66 insertions, 32 deletions

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;
 }