glusterd: Added validation function for performance cache max and min size.

Change-Id: I0b8dbc4b65412b8aff24873f030c03e3dcfcb988
BUG: 782095
Signed-off-by: Avra Sengupta <asengupt@redhat.com>
Reviewed-on: http://review.gluster.org/4541
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Anand Avati <avati@redhat.com>

5 files changed, 222 insertions, 10 deletions

diff --git a/cli/src/cli-rpc-ops.c b/cli/src/cli-rpc-ops.c
index 69496492217..62b949b1673 100644
--- a/cli/src/cli-rpc-ops.c
+++ b/cli/src/cli-rpc-ops.c
@@ -1557,10 +1557,12 @@ gf_cli_set_volume_cbk (struct rpc_req *req, struct iovec *iov,
                 goto out;
         }
 
-        if (rsp.op_ret &&  strcmp (rsp.op_errstr, ""))
-                cli_err ("volume set: failed: %s", rsp.op_errstr);
-
-        if (!rsp.op_ret) {
+        if (rsp.op_ret) {
+                if (strcmp (rsp.op_errstr, ""))
+                        cli_err ("volume set: failed: %s", rsp.op_errstr);
+                else
+                        cli_err ("volume set: failed");
+        } else {
                 if (help_str == NULL) {
                         if (debug_xlator == NULL)
                                 cli_out ("volume set: success");
@@ -1569,8 +1571,6 @@ gf_cli_set_volume_cbk (struct rpc_req *req, struct iovec *iov,
                 }else {
                         cli_out ("%s", help_str);
                 }
-        } else {
-                cli_err ("volume set: failed");
         }
 
         ret = rsp.op_ret;
diff --git a/tests/bugs/bug-782095.t b/tests/bugs/bug-782095.t
new file mode 100755
index 00000000000..a0cea14ee86
--- /dev/null
+++ b/tests/bugs/bug-782095.t
@@ -0,0 +1,48 @@
+#!/bin/bash
+
+. $(dirname $0)/../include.rc
+. $(dirname $0)/../volume.rc
+
+cleanup;
+
+## Start and create a volume
+TEST glusterd;
+TEST pidof glusterd;
+TEST $CLI volume create $V0 replica 2 stripe 2 $H0:$B0/${V0}{1,2,3,4,5,6,7,8};
+
+## Verify volume is is created
+EXPECT "$V0" volinfo_field $V0 'Volume Name';
+EXPECT 'Created' volinfo_field $V0 'Status';
+
+## Start volume and verify
+TEST $CLI volume start $V0;
+EXPECT 'Started' volinfo_field $V0 'Status';
+
+## Setting performance cache min size as 2MB
+TEST $CLI volume set $V0 performance.cache-min-file-size 2MB
+EXPECT '2MB' volinfo_field $V0 'performance.cache-min-file-size';
+
+## Setting performance cache max size as 20MB
+TEST $CLI volume set $V0 performance.cache-max-file-size 20MB
+EXPECT '20MB' volinfo_field $V0 'performance.cache-max-file-size';
+
+## Trying to set performance cache min size as 25MB
+TEST ! $CLI volume set $V0 performance.cache-min-file-size 25MB
+EXPECT '2MB' volinfo_field $V0 'performance.cache-min-file-size';
+
+## Able to set performance cache min size as long as its lesser than max size
+TEST $CLI volume set $V0 performance.cache-min-file-size 15MB
+EXPECT '15MB' volinfo_field $V0 'performance.cache-min-file-size';
+
+## Trying it out with only cache-max-file-size in CLI as 10MB
+TEST ! $CLI volume set $V0 cache-max-file-size 10MB
+EXPECT '20MB' volinfo_field $V0 'performance.cache-max-file-size';
+
+## Finish up
+TEST $CLI volume stop $V0;
+EXPECT 'Stopped' volinfo_field $V0 'Status';
+
+TEST $CLI volume delete $V0;
+TEST ! $CLI volume info $V0;
+
+cleanup;
diff --git a/xlators/mgmt/glusterd/src/glusterd-op-sm.c b/xlators/mgmt/glusterd/src/glusterd-op-sm.c
index 8f08052170a..e97b991b1be 100644
--- a/xlators/mgmt/glusterd/src/glusterd-op-sm.c
+++ b/xlators/mgmt/glusterd/src/glusterd-op-sm.c
@@ -105,6 +105,8 @@ static char *glusterd_op_sm_event_names[] = {
         "GD_OP_EVENT_INVALID"
 };
 
+extern struct volopt_map_entry glusterd_volopt_map[];
+
 char*
 glusterd_op_sm_state_name_get (int state)
 {
@@ -401,7 +403,8 @@ glusterd_op_stage_set_volume (dict_t *dict, char **op_errstr)
         uint32_t                        local_key_op_version    = 0;
         gf_boolean_t                    origin_glusterd         = _gf_true;
         gf_boolean_t                    check_op_version        = _gf_true;
-        gf_boolean_t                    all_vol        = _gf_false;
+        gf_boolean_t                    all_vol                 = _gf_false;
+        struct volopt_map_entry         *vme                    = NULL;
 
         GF_ASSERT (dict);
         this = THIS;
@@ -559,6 +562,18 @@ glusterd_op_stage_set_volume (dict_t *dict, char **op_errstr)
                 if (is_key_glusterd_hooks_friendly (key))
                         continue;
 
+                for (vme = &glusterd_volopt_map[0]; vme->key; vme++) {
+                        if ((vme->validate_fn) &&
+                            ((!strcmp (key, vme->key)) ||
+                             (!strcmp (key, strchr (vme->key, '.') + 1)))) {
+                                ret = vme->validate_fn (dict, key, value,
+                                                        op_errstr);
+                                if (ret)
+                                        goto out;
+                                break;
+                        }
+                }
+
                 exists = glusterd_check_option_exists (key, &key_fixed);
                 if (exists == -1) {
                         ret = -1;
diff --git a/xlators/mgmt/glusterd/src/glusterd-volgen.h b/xlators/mgmt/glusterd/src/glusterd-volgen.h
index 342183ccafc..0a9647bdf93 100644
--- a/xlators/mgmt/glusterd/src/glusterd-volgen.h
+++ b/xlators/mgmt/glusterd/src/glusterd-volgen.h
@@ -86,7 +86,7 @@ typedef enum {
 
 typedef enum  { DOC, NO_DOC, GLOBAL_DOC, GLOBAL_NO_DOC } option_type_t;
 
-typedef int (*vme_option_validation) (char *key, dict_t *dict,
+typedef int (*vme_option_validation) (dict_t *dict, char *key, char *value,
                                       char **op_errstr);
 
 struct volopt_map_entry {
diff --git a/xlators/mgmt/glusterd/src/glusterd-volume-set.c b/xlators/mgmt/glusterd/src/glusterd-volume-set.c
index 5e7f013efff..e66c1f9bbea 100644
--- a/xlators/mgmt/glusterd/src/glusterd-volume-set.c
+++ b/xlators/mgmt/glusterd/src/glusterd-volume-set.c
@@ -14,6 +14,153 @@
 #endif
 
 #include "glusterd-volgen.h"
+#include "glusterd-utils.h"
+
+static int
+check_dict_key_value (dict_t *dict, char *key, char *value)
+{
+        char                 errstr[2048]  = "";
+        glusterd_conf_t     *priv          = NULL;
+        int                  ret           = 0;
+        xlator_t            *this          = NULL;
+
+        this = THIS;
+        GF_ASSERT (this);
+        priv = this->private;
+        GF_ASSERT (priv);
+
+        if (!dict) {
+                snprintf (errstr, 2048, "Received Empty Dict.");
+                gf_log (this->name, GF_LOG_ERROR,
+                        "%s.", errstr);
+                ret = -1;
+                goto out;
+        }
+
+        if (!key) {
+                snprintf (errstr, 2048, "Received Empty Key.");
+                gf_log (this->name, GF_LOG_ERROR,
+                        "%s.", errstr);
+                ret = -1;
+                goto out;
+        }
+
+        if (!value) {
+                snprintf (errstr, 2048, "Received Empty Value.");
+                gf_log (this->name, GF_LOG_ERROR,
+                        "%s.", errstr);
+                ret = -1;
+                goto out;
+        }
+
+out:
+        gf_log (this->name, GF_LOG_DEBUG, "Returning %d", ret);
+
+        return ret;
+}
+
+static int
+get_volname_volinfo (dict_t *dict, char **volname, glusterd_volinfo_t **volinfo)
+{
+        char                 errstr[2048]  = "";
+        glusterd_conf_t     *priv          = NULL;
+        int                  ret           = 0;
+        xlator_t            *this          = NULL;
+
+        this = THIS;
+        GF_ASSERT (this);
+        priv = this->private;
+        GF_ASSERT (priv);
+
+        ret = dict_get_str (dict, "volname", volname);
+        if (ret) {
+                snprintf (errstr, 2048, "Unable to get volume name");
+                gf_log (this->name, GF_LOG_ERROR,
+                        "%s.", errstr);
+                goto out;
+        }
+
+        ret = glusterd_volinfo_find (*volname, volinfo);
+        if (ret) {
+                snprintf (errstr, 2048, "Unable to allocate memory");
+                gf_log (this->name, GF_LOG_ERROR,
+                        "%s.", errstr);
+                goto out;
+        }
+
+out:
+        gf_log (this->name, GF_LOG_DEBUG, "Returning %d", ret);
+
+        return ret;
+}
+
+int
+validate_cache_max_min_size (dict_t *dict, char *key, char *value,
+                             char **op_errstr)
+{
+        char                *current_max_value = NULL;
+        char                *current_min_value = NULL;
+        char                 errstr[2048]  = "";
+        char                *volname       = NULL;
+        glusterd_conf_t     *priv          = NULL;
+        glusterd_volinfo_t  *volinfo       = NULL;
+        int                  ret           = 0;
+        uint64_t             max_value     = 0;
+        uint64_t             min_value     = 0;
+        xlator_t            *this          = NULL;
+
+        this = THIS;
+        GF_ASSERT (this);
+        priv = this->private;
+        GF_ASSERT (priv);
+
+        ret = check_dict_key_value (dict, key, value);
+        if (ret)
+                goto out;
+
+        ret = get_volname_volinfo (dict, &volname, &volinfo);
+        if (ret)
+                goto out;
+
+        if ((!strcmp (key, "performance.cache-min-file-size")) ||
+            (!strcmp (key, "cache-min-file-size"))) {
+                glusterd_volinfo_get (volinfo,
+                                      "performance.cache-max-file-size",
+                                      &current_max_value);
+                if (current_max_value) {
+                        gf_string2bytesize (current_max_value, &max_value);
+                        gf_string2bytesize (value, &min_value);
+                        current_min_value = value;
+                }
+        } else  if ((!strcmp (key, "performance.cache-max-file-size")) ||
+                    (!strcmp (key, "cache-max-file-size"))) {
+                glusterd_volinfo_get (volinfo,
+                                      "performance.cache-min-file-size",
+                                      &current_min_value);
+                if (current_min_value) {
+                        gf_string2bytesize (current_min_value, &min_value);
+                        gf_string2bytesize (value, &max_value);
+                        current_max_value = value;
+                }
+        }
+
+        if (min_value > max_value) {
+                snprintf (errstr, sizeof (errstr),
+                          "cache-min-file-size (%s) is greater than "
+                          "cache-max-file-size (%s)",
+                          current_min_value, current_max_value);
+                gf_log (this->name, GF_LOG_ERROR, "%s", errstr);
+                *op_errstr = gf_strdup (errstr);
+                ret = -1;
+                goto out;
+        }
+
+out:
+        gf_log (this->name, GF_LOG_DEBUG, "Returning %d", ret);
+
+        return ret;
+}
+
 
 /* dispatch table for VOLUME SET
  * -----------------------------
@@ -275,12 +422,14 @@ struct volopt_map_entry glusterd_volopt_map[] = {
         { .key         = "performance.cache-max-file-size",
           .voltype     = "performance/io-cache",
           .option      = "max-file-size",
-          .op_version  = 1
+          .op_version  = 1,
+          .validate_fn = validate_cache_max_min_size
         },
         { .key         = "performance.cache-min-file-size",
           .voltype     = "performance/io-cache",
           .option      = "min-file-size",
-          .op_version  = 1
+          .op_version  = 1,
+          .validate_fn = validate_cache_max_min_size
         },
         { .key         = "performance.cache-refresh-timeout",
           .voltype     = "performance/io-cache",