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>

2 files changed, 70 insertions, 100 deletions

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;