glusterd: Clean up and fix glusterd_op_quota()

... and also fix cli logging

In glusterd_op_quota(),

* do not modify ret after going to 'out' as this causes the failure
  status (-1) to be overwritten, thereby causing the command to return
  0 even on failure.

* knock off additional labels like create_vol.

* replace 'if' statements with 'switch case' statement.

* delete only the 3 timeouts and the defaul-soft-limit, if present,
  from volinfo->dict, upon disablement of quota.

Change-Id: If486a5373b66f2379d6d041a974d9b824fcb8518
Signed-off-by: Krutika Dhananjay <kdhananj@redhat.com>

4 files changed, 85 insertions, 130 deletions

diff --git a/cli/src/cli-cmd-volume.c b/cli/src/cli-cmd-volume.c
index 51cfa251..2fd74554 100644
--- a/cli/src/cli-cmd-volume.c
+++ b/cli/src/cli-cmd-volume.c
@@ -1020,6 +1020,7 @@ cli_cmd_quota_cbk (struct cli_state *state, struct cli_cmd_word *word,
         dict_t                  *options   = NULL;
         gf_answer_t              answer    = GF_ANSWER_NO;
         cli_local_t             *local     = NULL;
+        int                      sent      = 0;
         const char *question = "Disabling quota will delete all the quota "
                                "configuration. Do you want to continue?";
 
@@ -1054,13 +1055,15 @@ cli_cmd_quota_cbk (struct cli_state *state, struct cli_cmd_word *word,
                 ret = proc->fn (frame, THIS, options);
 
 out:
-        if (ret && parse_err == 0)
-                cli_out ("Quota command failed");
+        if (ret) {
+                cli_cmd_sent_status_get (&sent);
+                if (sent == 0 && parse_err == 0)
+                        cli_out ("Quota command failed");
+        }
 
         CLI_STACK_DESTROY (frame);
 
         return ret;
-
 }
 
 int
diff --git a/cli/src/cli-rpc-ops.c b/cli/src/cli-rpc-ops.c
index cd1e0047..80e2f24d 100644
--- a/cli/src/cli-rpc-ops.c
+++ b/cli/src/cli-rpc-ops.c
@@ -2699,7 +2699,6 @@ gf_cli_quota_cbk (struct rpc_req *req, struct iovec *iov,
         char              *volname     = NULL;
         char              *limit_list  = NULL;
         int32_t            type        = 0;
-        char               msg[5120]   = {0,};
         call_frame_t      *frame       = NULL;
         uint32_t           op_version  = 1;
         char              *default_sl  = NULL;
@@ -2718,15 +2717,14 @@ gf_cli_quota_cbk (struct rpc_req *req, struct iovec *iov,
                 goto out;
         }
 
-        if (rsp.op_ret &&
-            strcmp (rsp.op_errstr, "") == 0) {
-                snprintf (msg, sizeof (msg), "command unsuccessful %s",
-                          rsp.op_errstr);
+        if (rsp.op_ret && strcmp (rsp.op_errstr, "") == 0) {
+                cli_err ("quota command : failed");
 
                 if (global_state->mode & GLUSTER_MODE_XML)
                         goto xml_output;
                 goto out;
-        }
+        } else if (strcmp (rsp.op_errstr, ""))
+                        cli_err ("quota command failed : %s", rsp.op_errstr);
 
         if (rsp.dict.dict_len) {
                 /* Unserialize the dictionary */
@@ -2743,6 +2741,8 @@ gf_cli_quota_cbk (struct rpc_req *req, struct iovec *iov,
                 }
         }
 
+        gf_log ("cli", GF_LOG_DEBUG, "Received resp to quota command");
+
         ret = dict_get_str (dict, "volname", &volname);
         if (ret)
                 gf_log (frame->this->name, GF_LOG_TRACE,
@@ -2801,25 +2801,11 @@ gf_cli_quota_cbk (struct rpc_req *req, struct iovec *iov,
                 gf_cli_quota_list_run_crawler (volname, limit_list, dict, count,
                                                rsp.op_errstr, op_version,
                                                default_sl, entry_count);
-                if (rsp.op_errstr)
-                        snprintf (msg, sizeof (msg)-1, "%s", rsp.op_errstr);
-        }
-
-        gf_log ("cli", GF_LOG_DEBUG, "Received resp to quota command");
-
-        if (rsp.op_errstr) {
-                snprintf (msg, sizeof (msg)-1, "%s", rsp.op_errstr);
-        } else {
-                if (!rsp.op_ret)
-                        snprintf (msg, sizeof (msg)-1, "Quota command "
-                                                     "successful");
-                else
-                        snprintf (msg, sizeof (msg)-1, "Quota command failed");
         }
 
 xml_output:
         if (global_state->mode & GLUSTER_MODE_XML) {
-                ret = cli_xml_output_str ("volQuota", msg, rsp.op_ret,
+                ret = cli_xml_output_str ("volQuota", NULL, rsp.op_ret,
                                           rsp.op_errno, rsp.op_errstr);
                 if (ret)
                         gf_log ("cli", GF_LOG_ERROR,
@@ -2827,12 +2813,8 @@ xml_output:
                 goto out;
         }
 
-        if (strlen (msg) > 0) {
-                if (rsp.op_ret)
-                        cli_err ("%s", msg);
-                else
-                        cli_out ("%s", msg);
-        }
+        if (!rsp.op_ret && type != GF_QUOTA_OPTION_TYPE_LIST)
+                cli_out ("volume quota : success");
 
         ret = rsp.op_ret;
 out:
diff --git a/xlators/mgmt/glusterd/src/glusterd-quota.c b/xlators/mgmt/glusterd/src/glusterd-quota.c
index 5a8a2a63..0db26f4b 100644
--- a/xlators/mgmt/glusterd/src/glusterd-quota.c
+++ b/xlators/mgmt/glusterd/src/glusterd-quota.c
@@ -503,12 +503,14 @@ glusterd_quota_enable (glusterd_volinfo_t *volinfo, char **op_errstr,
         if (glusterd_is_volume_started (volinfo) == 0) {
                 *op_errstr = gf_strdup ("Volume is stopped, start volume "
                                         "to enable quota.");
+                ret = -1;
                 goto out;
         }
 
         ret = glusterd_check_if_quota_trans_enabled (volinfo);
         if (ret == 0) {
                 *op_errstr = gf_strdup ("Quota is already enabled");
+                ret = -1;
                 goto out;
         }
 
@@ -526,8 +528,6 @@ glusterd_quota_enable (glusterd_volinfo_t *volinfo, char **op_errstr,
                 goto out;
         }
 
-        *op_errstr = gf_strdup ("Enabling quota has been successful");
-
         *crawl = _gf_true;
 
         ret = 0;
@@ -547,8 +547,7 @@ glusterd_quota_disable (glusterd_volinfo_t *volinfo, char **op_errstr)
         char      *value          = NULL;
         xlator_t  *this           = NULL;
         glusterd_conf_t *conf     = NULL;
-        char *quota_options[]     = {VKEY_FEATURES_LIMIT_USAGE,
-                                     "features.soft-timeout",
+        char *quota_options[]     = {"features.soft-timeout",
                                      "features.hard-timeout",
                                      "features.alert-time",
                                      "features.default-soft-limit", NULL};
@@ -589,11 +588,8 @@ glusterd_quota_disable (glusterd_volinfo_t *volinfo, char **op_errstr)
                 } else {
                 dict_del (volinfo->dict, quota_options[i]);
                 }
-                if ((i == 0) && (conf->op_version == GD_OP_VERSION_MIN))
-                        break;
          }
 
-        *op_errstr = gf_strdup ("Disabling quota has been successful");
         ret = 0;
 out:
         if (ret && op_errstr && !*op_errstr)
@@ -684,7 +680,6 @@ glusterd_quota_limit_usage (glusterd_volinfo_t *volinfo, dict_t *dict,
         char            *path               = NULL;
         char            *hard_limit         = NULL;
         char            *soft_limit         = NULL;
-        char             msg[5120]          = {0,};
         xlator_t        *this               = NULL;
 
         this = THIS;
@@ -732,9 +727,6 @@ glusterd_quota_limit_usage (glusterd_volinfo_t *volinfo, dict_t *dict,
                 if (ret)
                         goto out;
         }
-
-        snprintf (msg, sizeof (msg)-1, "Quota limit set on %s", path);
-        *op_errstr = gf_strdup (msg);
         ret = 0;
 out:
 
@@ -816,8 +808,6 @@ glusterd_quota_remove_limits (glusterd_volinfo_t *volinfo, dict_t *dict,
                 if (ret)
                         goto out;
         }
-
-        gf_asprintf (op_errstr, "Removed quota limit on %s", path);
         ret = 0;
 
 out:
@@ -856,7 +846,6 @@ glusterd_set_quota_option (glusterd_volinfo_t *volinfo, dict_t *dict,
                         key);
                 return -1;
         }
-        gf_asprintf (op_errstr, "%s on volume %s set", key, volinfo->volname);
 
         return 0;
 }
@@ -935,88 +924,83 @@ glusterd_op_quota (dict_t *dict, char **op_errstr, dict_t *rsp_dict)
                 goto out;
         }
 
-        if (type == GF_QUOTA_OPTION_TYPE_ENABLE) {
-                ret = glusterd_quota_enable (volinfo, op_errstr, &start_crawl);
-                if (ret < 0)
-                        goto out;
+        switch (type) {
+                case GF_QUOTA_OPTION_TYPE_ENABLE:
+                        ret = glusterd_quota_enable (volinfo, op_errstr,
+                                                     &start_crawl);
+                        if (ret < 0)
+                                goto out;
+                        break;
 
-                goto create_vol;
-        }
-        if (type == GF_QUOTA_OPTION_TYPE_DISABLE) {
-                ret = glusterd_quota_disable (volinfo, op_errstr);
-                if (ret < 0)
-                        goto out;
+                case GF_QUOTA_OPTION_TYPE_DISABLE:
+                        ret = glusterd_quota_disable (volinfo, op_errstr);
+                        if (ret < 0)
+                                goto out;
 
-                goto create_vol;
-        }
+                        break;
 
-        if (type == GF_QUOTA_OPTION_TYPE_LIMIT_USAGE) {
-                ret = glusterd_quota_limit_usage (volinfo, dict, op_errstr);
-                if (ret < 0)
+                case GF_QUOTA_OPTION_TYPE_LIMIT_USAGE:
+                        ret = glusterd_quota_limit_usage (volinfo, dict,
+                                                          op_errstr);
                         goto out;
 
-                goto create_vol;
-        }
-
-        if (type == GF_QUOTA_OPTION_TYPE_REMOVE) {
-                ret = glusterd_quota_remove_limits (volinfo, dict, op_errstr);
-                if (ret < 0)
+                case GF_QUOTA_OPTION_TYPE_REMOVE:
+                        ret = glusterd_quota_remove_limits (volinfo, dict,
+                                                            op_errstr);
                         goto out;
 
-                goto create_vol;
-        }
-
-        if (type == GF_QUOTA_OPTION_TYPE_LIST) {
-                ret = glusterd_check_if_quota_trans_enabled (volinfo);
-                if (ret == -1) {
-                        *op_errstr = gf_strdup ("cannot list the limits, "
-                                                "quota is disabled");
+                case GF_QUOTA_OPTION_TYPE_LIST:
+                        ret = glusterd_check_if_quota_trans_enabled (volinfo);
+                        if (ret == -1) {
+                                *op_errstr = gf_strdup ("Cannot list limits, "
+                                                        "quota is disabled");
+                                goto out;
+                        }
+                        ret = glusterd_quota_get_limit_usages (priv, volinfo,
+                                                               volname, dict,
+                                                               op_errstr,
+                                                               rsp_dict);
                         goto out;
-                }
 
-                ret = glusterd_quota_get_limit_usages (priv, volinfo, volname,
-                                                       dict, op_errstr,
-                                                       rsp_dict);
+                case GF_QUOTA_OPTION_TYPE_SOFT_TIMEOUT:
+                        ret = glusterd_set_quota_option (volinfo, dict,
+                                                        "features.soft-timeout",
+                                                         op_errstr);
+                        if (ret)
+                                goto out;
+                        break;
 
-                goto out;
-        }
+                case GF_QUOTA_OPTION_TYPE_HARD_TIMEOUT:
+                        ret = glusterd_set_quota_option (volinfo, dict,
+                                                        "features.hard-timeout",
+                                                         op_errstr);
+                        if (ret)
+                                goto out;
+                        break;
 
-        if (type == GF_QUOTA_OPTION_TYPE_SOFT_TIMEOUT) {
-                ret = glusterd_set_quota_option (volinfo, dict,
-                                                 "features.soft-timeout",
-                                                 op_errstr);
-                if (ret)
-                        goto out;
-                goto create_vol;
-        }
+                case GF_QUOTA_OPTION_TYPE_ALERT_TIME:
+                        ret = glusterd_set_quota_option (volinfo, dict,
+                                                         "features.alert-time",
+                                                         op_errstr);
+                        if (ret)
+                                goto out;
+                        break;
 
-        if (type == GF_QUOTA_OPTION_TYPE_HARD_TIMEOUT) {
-                ret = glusterd_set_quota_option (volinfo, dict,
-                                                 "features.hard-timeout",
-                                                 op_errstr);
-                if (ret)
-                        goto out;
-                goto create_vol;
-        }
+                case GF_QUOTA_OPTION_TYPE_DEFAULT_SOFT_LIMIT:
+                        ret = glusterd_set_quota_option (volinfo, dict,
+                                                  "features.default-soft-limit",
+                                                  op_errstr);
+                        if (ret)
+                                goto out;
+                        break;
 
-        if (type == GF_QUOTA_OPTION_TYPE_ALERT_TIME) {
-                ret = glusterd_set_quota_option (volinfo, dict,
-                                                 "features.alert-time",
-                                                 op_errstr);
-                if (ret)
-                        goto out;
-                goto create_vol;
-        }
-        if (type == GF_QUOTA_OPTION_TYPE_DEFAULT_SOFT_LIMIT) {
-                ret = glusterd_set_quota_option (volinfo, dict,
-                                                 "features.default-soft-limit",
-                                                 op_errstr);
-                if (ret)
+                default:
+                        gf_asprintf (op_errstr, "Quota command failed. Invalid "
+                                     "opcode");
+                        ret = -1;
                         goto out;
-                goto create_vol;
         }
 
-create_vol:
         ret = glusterd_create_volfiles_and_notify_services (volinfo);
         if (ret) {
                 gf_log (this->name, GF_LOG_ERROR, "Unable to re-create "
@@ -1033,25 +1017,17 @@ create_vol:
                 if (priv->op_version == GD_OP_VERSION_MIN)
                         ret = glusterd_check_generate_start_nfs ();
         }
-        ret = 0;
 
-out:
         if (rsp_dict && start_crawl == _gf_true)
                 glusterd_quota_initiate_fs_crawl (priv, volname);
 
-        if (priv->op_version > GD_OP_VERSION_MIN)
+        if (priv->op_version > GD_OP_VERSION_MIN) {
                 ret = glusterd_quotad_op (type);
-
-        if (rsp_dict && *op_errstr) {
-                ret = dict_set_dynstr (rsp_dict, "errstr", *op_errstr);
-                if (ret) {
-                        GF_FREE (*op_errstr);
-                        gf_log (this->name, GF_LOG_DEBUG,
-                                "failed to set error message in ctx");
-                }
-                *op_errstr = NULL;
+                if (ret)
+                        goto out;
         }
-
+        ret = 0;
+out:
         return ret;
 }
 
diff --git a/xlators/mgmt/glusterd/src/glusterd-rpc-ops.c b/xlators/mgmt/glusterd/src/glusterd-rpc-ops.c
index 9d10bee5..9a9f16c6 100644
--- a/xlators/mgmt/glusterd/src/glusterd-rpc-ops.c
+++ b/xlators/mgmt/glusterd/src/glusterd-rpc-ops.c
@@ -96,13 +96,6 @@ glusterd_op_send_cli_response (glusterd_op_t op, int32_t op_ret,
                break;
 
         }
-        case GD_OP_QUOTA:
-        {
-                if (ctx && !op_errstr) {
-                        ret = dict_get_str (ctx, "errstr", &errstr);
-                }
-                break;
-        }
         case GD_OP_PROFILE_VOLUME:
         {
                 if (ctx && dict_get_int32 (ctx, "count", &count)) {
@@ -143,6 +136,7 @@ glusterd_op_send_cli_response (glusterd_op_t op, int32_t op_ret,
         case GD_OP_CLEARLOCKS_VOLUME:
         case GD_OP_HEAL_VOLUME:
         case GD_OP_BD_OP:
+        case GD_OP_QUOTA:
         {
                 /*nothing specific to be done*/
                 break;