glusterd : Fixes for breakages caused by volume-id validation

Fixes glusterd_op_build_payload() to,
1. take account of status cmd type when building payload for "volume status" to
   prevent "volume status all" from failing.
2. take account of volname being "help/help-xml" in volume set to prevent
   "volume set help/help-xml" from failing
3. obtain volname using key "master" prevent "volume geo-replication" commands
   from failing

Also, fails op and sets correct op_errstr if volume not found during
glusterd_dict_set_volid(), to make sure cli displays proper message.

Change-Id: I40ded15c50b54a82ee61bf6d6e9d07f571679c8c
BUG: 812801
Signed-off-by: Kaushal M <kaushal@redhat.com>
Reviewed-on: http://review.gluster.com/3157
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Krishnan Parthasarathi <kp@gluster.com>

4 files changed, 142 insertions, 47 deletions

diff --git a/xlators/mgmt/glusterd/src/glusterd-geo-rep.c b/xlators/mgmt/glusterd/src/glusterd-geo-rep.c
index 5e12e1996..4dc939666 100644
--- a/xlators/mgmt/glusterd/src/glusterd-geo-rep.c
+++ b/xlators/mgmt/glusterd/src/glusterd-geo-rep.c
@@ -987,7 +987,7 @@ glusterd_verify_gsync_status_opts (dict_t *dict, char **op_errstr)
 }
 
 
-static int
+int
 glusterd_op_gsync_args_get (dict_t *dict, char **op_errstr,
                             char **master, char **slave)
 {
@@ -995,21 +995,23 @@ glusterd_op_gsync_args_get (dict_t *dict, char **op_errstr,
         int             ret = -1;
         GF_ASSERT (dict);
         GF_ASSERT (op_errstr);
-        GF_ASSERT (master);
-        GF_ASSERT (slave);
 
-        ret = dict_get_str (dict, "master", master);
-        if (ret < 0) {
-                gf_log ("", GF_LOG_WARNING, "master not found");
-                *op_errstr = gf_strdup ("master not found");
-                goto out;
+        if (master) {
+                ret = dict_get_str (dict, "master", master);
+                if (ret < 0) {
+                        gf_log ("", GF_LOG_WARNING, "master not found");
+                        *op_errstr = gf_strdup ("master not found");
+                        goto out;
+                }
         }
 
-        ret = dict_get_str (dict, "slave", slave);
-        if (ret < 0) {
-                gf_log ("", GF_LOG_WARNING, "slave not found");
-                *op_errstr = gf_strdup ("slave not found");
-                goto out;
+        if (slave) {
+                ret = dict_get_str (dict, "slave", slave);
+                if (ret < 0) {
+                        gf_log ("", GF_LOG_WARNING, "slave not found");
+                        *op_errstr = gf_strdup ("slave not found");
+                        goto out;
+                }
         }
 
 
diff --git a/xlators/mgmt/glusterd/src/glusterd-op-sm.c b/xlators/mgmt/glusterd/src/glusterd-op-sm.c
index 6d646c111..b243df433 100644
--- a/xlators/mgmt/glusterd/src/glusterd-op-sm.c
+++ b/xlators/mgmt/glusterd/src/glusterd-op-sm.c
@@ -387,6 +387,10 @@ glusterd_op_stage_set_volume (dict_t *dict, char **op_errstr)
                 goto out;
         }
 
+        ret = glusterd_validate_volume_id (dict, volinfo);
+        if (ret)
+                goto out;
+
         for ( count = 1; ret != 1 ; count++ ) {
                 global_opt = _gf_false;
                 sprintf (str, "key%d", count);
@@ -1788,16 +1792,50 @@ out:
         return ret;
 }
 
+static int
+glusterd_dict_set_volid (dict_t *dict, char *volname, char **op_errstr)
+{
+        int                     ret = -1;
+        glusterd_volinfo_t      *volinfo = NULL;
+        char                    *volid = NULL;
+        char                    msg[1024] = {0,};
+
+        if (!dict || !volname)
+                goto out;
+
+        ret = glusterd_volinfo_find (volname, &volinfo);
+        if (ret) {
+                snprintf (msg, sizeof (msg), "Volume %s does not exist",
+                          volname);
+                gf_log (THIS->name, GF_LOG_ERROR, "%s", msg);
+                *op_errstr = gf_strdup (msg);
+                goto out;
+        }
+        volid = gf_strdup (uuid_utoa (volinfo->volume_id));
+        if (!volid) {
+                ret = -1;
+                goto out;
+        }
+        ret = dict_set_dynstr (dict, "vol-id", volid);
+        if (ret) {
+                gf_log (THIS->name, GF_LOG_ERROR,
+                        "Failed to set volume id in dictionary");
+                goto out;
+        }
+out:
+        return ret;
+}
+
 int
-glusterd_op_build_payload (dict_t **req)
+glusterd_op_build_payload (dict_t **req, char **op_errstr)
 {
         int                     ret = -1;
         void                    *ctx = NULL;
         dict_t                  *req_dict = NULL;
         glusterd_op_t           op = GD_OP_NONE;
         char                    *volname = NULL;
-        char                    *volid = NULL;
-        glusterd_volinfo_t      *volinfo = NULL;
+        uint32_t                status_cmd = GF_CLI_STATUS_NONE;
+        char                    *errstr = NULL;
 
         GF_ASSERT (req);
 
@@ -1819,34 +1857,91 @@ glusterd_op_build_payload (dict_t **req)
                         {
                                 dict_t  *dict = ctx;
                                 ++glusterfs_port;
-                                ret = dict_set_int32 (dict, "port", glusterfs_port);
+                                ret = dict_set_int32 (dict, "port",
+                                                      glusterfs_port);
+                                if (ret)
+                                        goto out;
+                                dict_copy (dict, req_dict);
+                        }
+                        break;
+
+                case GD_OP_GSYNC_SET:
+                        {
+                                dict_t *dict = ctx;
+                                ret = glusterd_op_gsync_args_get (dict,
+                                                                  &errstr,
+                                                                  &volname,
+                                                                  NULL);
+                                if (ret) {
+                                        gf_log (THIS->name, GF_LOG_ERROR,
+                                                "%s", errstr);
+                                        goto out;
+                                }
+
+                                ret = glusterd_dict_set_volid (dict, volname,
+                                                               op_errstr);
                                 if (ret)
                                         goto out;
                                 dict_copy (dict, req_dict);
                         }
                         break;
 
+                case GD_OP_SET_VOLUME:
+                        {
+                                dict_t *dict = ctx;
+                                ret = dict_get_str (dict, "volname", &volname);
+                                if (ret) {
+                                        gf_log (THIS->name, GF_LOG_CRITICAL,
+                                                "volname is not present in "
+                                                "operation ctx");
+                                        goto out;
+                                }
+                                if (strcmp (volname, "help") &&
+                                    strcmp (volname, "help-xml")) {
+                                        ret = glusterd_dict_set_volid
+                                                (dict, volname, op_errstr);
+                                        if (ret)
+                                                goto out;
+                                }
+                                dict_copy (dict, req_dict);
+                        }
+                        break;
+
+                case GD_OP_STATUS_VOLUME:
+                        {
+                                dict_t *dict = ctx;
+                                ret = dict_get_uint32 (dict, "cmd",
+                                                       &status_cmd);
+                                if (ret) {
+                                        gf_log (THIS->name, GF_LOG_ERROR,
+                                                "Status command not present "
+                                                "in op ctx");
+                                        goto out;
+                                }
+                                if (GF_CLI_STATUS_ALL & status_cmd) {
+                                        dict_copy (dict, req_dict);
+                                        break;
+                                }
+                        }
+
                 case GD_OP_DELETE_VOLUME:
                 case GD_OP_START_VOLUME:
                 case GD_OP_STOP_VOLUME:
                 case GD_OP_ADD_BRICK:
                 case GD_OP_REPLACE_BRICK:
-                case GD_OP_SET_VOLUME:
                 case GD_OP_RESET_VOLUME:
                 case GD_OP_REMOVE_BRICK:
                 case GD_OP_LOG_ROTATE:
                 case GD_OP_SYNC_VOLUME:
                 case GD_OP_QUOTA:
-                case GD_OP_GSYNC_SET:
                 case GD_OP_PROFILE_VOLUME:
-                case GD_OP_STATUS_VOLUME:
                 case GD_OP_REBALANCE:
                 case GD_OP_HEAL_VOLUME:
                 case GD_OP_STATEDUMP_VOLUME:
                 case GD_OP_CLEARLOCKS_VOLUME:
                 case GD_OP_DEFRAG_BRICK_VOLUME:
                         {
-                                dict_t  *dict = ctx;
+                                dict_t *dict = ctx;
                                 ret = dict_get_str (dict, "volname", &volname);
                                 if (ret) {
                                         gf_log (THIS->name, GF_LOG_CRITICAL,
@@ -1854,25 +1949,11 @@ glusterd_op_build_payload (dict_t **req)
                                                 "operation ctx");
                                         goto out;
                                 }
-                                ret = glusterd_volinfo_find (volname, &volinfo);
-                                if (ret) {
-                                        gf_log (THIS->name, GF_LOG_ERROR,
-                                                "volume %s not present in "
-                                                "the cluster", volname);
-                                        goto out;
-                                }
-                                volid = gf_strdup (uuid_utoa (volinfo->volume_id));
-                                if (!volid) {
-                                        ret = -1;
-                                        goto out;
-                                }
-                                ret = dict_set_dynstr (dict, "vol-id", volid);
-                                if (ret) {
-                                        gf_log (THIS->name, GF_LOG_ERROR,
-                                                "Failed to set volume id in "
-                                                "dictionary");
+
+                                ret = glusterd_dict_set_volid (dict, volname,
+                                                               op_errstr);
+                                if (ret)
                                         goto out;
-                                }
                                 dict_copy (dict, req_dict);
                         }
                         break;
@@ -1908,9 +1989,12 @@ glusterd_op_ac_send_stage_op (glusterd_op_sm_event_t *event, void *ctx)
 
         op = glusterd_op_get_op ();
 
-        ret = glusterd_op_build_payload (&dict);
-        if (ret)
+        ret = glusterd_op_build_payload (&dict, &op_errstr);
+        if (ret) {
+                gf_log (THIS->name, GF_LOG_ERROR, "Building payload failed");
+                opinfo.op_errstr = op_errstr;
                 goto out;
+        }
 
         /* rsp_dict NULL from source */
         ret = glusterd_op_stage_validate (op, dict, &op_errstr, NULL);
@@ -2230,10 +2314,12 @@ glusterd_op_ac_send_commit_op (glusterd_op_sm_event_t *event, void *ctx)
         op      = glusterd_op_get_op ();
         op_dict = glusterd_op_get_ctx ();
 
-        ret = glusterd_op_build_payload (&dict);
-
-        if (ret)
+        ret = glusterd_op_build_payload (&dict, &op_errstr);
+        if (ret) {
+                gf_log (THIS->name, GF_LOG_ERROR, "Building payload failed");
+                opinfo.op_errstr = op_errstr;
                 goto out;
+        }
 
         glusterd_op_commit_hook (op, op_dict, GD_COMMIT_HOOK_PRE);
         ret = glusterd_op_commit_perform (op, dict, &op_errstr, NULL); //rsp_dict invalid for source
@@ -4003,6 +4089,7 @@ glusterd_op_ac_send_brick_op (glusterd_op_sm_event_t *event, void *ctx)
         xlator_t                        *this = NULL;
         glusterd_op_t                   op = GD_OP_NONE;
         glusterd_req_ctx_t              *req_ctx = NULL;
+        char                            *op_errstr = NULL;
 
         this = THIS;
         priv = this->private;
@@ -4015,9 +4102,13 @@ glusterd_op_ac_send_brick_op (glusterd_op_sm_event_t *event, void *ctx)
                 op = glusterd_op_get_op ();
                 req_ctx->op = op;
                 uuid_copy (req_ctx->uuid, priv->uuid);
-                ret = glusterd_op_build_payload (&req_ctx->dict);
-                if (ret)//TODO:what to do??
+                ret = glusterd_op_build_payload (&req_ctx->dict, &op_errstr);
+                if (ret) {
+                        gf_log (this->name, GF_LOG_ERROR,
+                                "Building payload failed");
+                        opinfo.op_errstr = op_errstr;
                         goto out;
+                }
         }
 
         proc = &priv->gfs_mgmt->proctable[GLUSTERD_BRICK_OP];
diff --git a/xlators/mgmt/glusterd/src/glusterd-op-sm.h b/xlators/mgmt/glusterd/src/glusterd-op-sm.h
index 047ff2f3d..7ea2995d0 100644
--- a/xlators/mgmt/glusterd/src/glusterd-op-sm.h
+++ b/xlators/mgmt/glusterd/src/glusterd-op-sm.h
@@ -191,7 +191,7 @@ int32_t
 glusterd_op_set_op (glusterd_op_t op);
 
 int
-glusterd_op_build_payload (dict_t **req);
+glusterd_op_build_payload (dict_t **req, char **op_errstr);
 
 int32_t
 glusterd_op_stage_validate (glusterd_op_t op, dict_t *req, char **op_errstr,
diff --git a/xlators/mgmt/glusterd/src/glusterd.h b/xlators/mgmt/glusterd/src/glusterd.h
index 1b958d105..7484fc695 100644
--- a/xlators/mgmt/glusterd/src/glusterd.h
+++ b/xlators/mgmt/glusterd/src/glusterd.h
@@ -637,6 +637,8 @@ int glusterd_op_stop_volume_args_get (dict_t *dict, char** volname, int *flags);
 int glusterd_op_statedump_volume_args_get (dict_t *dict, char **volname,
                                            char **options, int *option_cnt);
 
+int glusterd_op_gsync_args_get (dict_t *dict, char **op_errstr,
+                                char **master, char **slave);
 /* Synctask part */
 int32_t glusterd_op_begin_synctask (rpcsvc_request_t *req, glusterd_op_t op,
                                     void *dict);