snapshot/config : Fix for bug which states gluster snapshot config

command should only accept the decimal numeric value.

Syntax : gluster snapshot config [volname]
                                 [snap-max-hard-limit <count>]
                                 [snap-max-soft-limit <percentage>]

Problem : Snapshot config used to consider the alphanumeric value
staring with digit as an integer (Example: "9abc" is converted to "9").

Solution : Refined the code to check if the entered value is numeric.

This patch also fixes some of the minor problems related to snapshot
config.
1) Output correction in gluster snapshot config snap-max-soft-limit.
2) setting the soft limit to greater than 100% displays that "Invalid
snap-max-soft-limit 0". The error message used to display "zero" in
the output, Changed this to display relevant value.
3) Setting greater than allowed snap-max-hard-limit output needs to
have space in between.

Change-Id: Ie7c7045722fe57b2b3c50c873664b67c28eb3853
BUG: 1087203
Signed-off-by: Sachin Pandit <spandit@redhat.com>
Reviewed-on: http://review.gluster.org/7457
Reviewed-by: Vijaikumar Mallikarjuna <vmallika@redhat.com>
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Vijay Bellur <vbellur@redhat.com>

3 files changed, 74 insertions, 11 deletions

diff --git a/cli/src/cli-cmd-parser.c b/cli/src/cli-cmd-parser.c
index b53b1ee648b..5fdb9d08a74 100644
--- a/cli/src/cli-cmd-parser.c
+++ b/cli/src/cli-cmd-parser.c
@@ -3418,13 +3418,18 @@ out:
 }
 
 
+/* return value:
+ *      -1  in case of failure.
+ *       0  in case of success.
+ */
 int32_t
 cli_snap_config_limit_parse (const char **words, dict_t *dict,
                              unsigned int wordcount, unsigned int index,
                              char *key)
 {
-        int             ret             =       -1;
-        int             limit           =       0;
+        int             ret             = -1;
+        unsigned int    limit           = 0;
+        char            *end_ptr        = NULL;
 
         GF_ASSERT (words);
         GF_ASSERT (dict);
@@ -3437,10 +3442,24 @@ cli_snap_config_limit_parse (const char **words, dict_t *dict,
                 goto out;
         }
 
-        limit = strtol (words[index], NULL, 0);
-        if (limit <= 0) {
+        limit = strtol (words[index], &end_ptr, 10);
+
+        if (limit <= 0 || strcmp (end_ptr, "") != 0) {
+                ret = -1;
+                cli_err("Please enter an integer value "
+                        "greater than zero for %s", key);
+                goto out;
+        }
+
+        if (strcmp (key, "snap-max-hard-limit") == 0 && limit > 256) {
+                ret = -1;
+                cli_err ("%s value cannot be more than 256", key);
+                goto out;
+        }
+
+        if (strcmp (key, "snap-max-soft-limit") == 0 && limit > 100) {
                 ret = -1;
-                cli_err ("%s should be greater than 0.", key);
+                cli_err ("%s value cannot be more than 100", key);
                 goto out;
         }
 
@@ -3461,7 +3480,8 @@ out:
  *                                         [snap-max-soft-limit <count>]
  *
    return value: <0  on failure
-                  1  if user cancels the operation
+                  1  if user cancels the operation, or limit value is out of
+                                                                      range
                   0  on success
 
   NOTE : snap-max-soft-limit can only be set for system.
diff --git a/tests/bugs/bug-1087203.t b/tests/bugs/bug-1087203.t
new file mode 100644
index 00000000000..0c4ee01cba5
--- /dev/null
+++ b/tests/bugs/bug-1087203.t
@@ -0,0 +1,44 @@
+#!/bin/bash
+
+. $(dirname $0)/../include.rc
+. $(dirname $0)/../volume.rc
+
+cleanup;
+
+TEST glusterd;
+TEST $CLI volume info;
+
+TEST $CLI volume create $V0 replica 2 $H0:$B0/${V0}-{0,1};
+EXPECT "$V0" volinfo_field $V0 'Volume Name';
+EXPECT 'Created' volinfo_field $V0 'Status';
+
+TEST $CLI volume start $V0
+EXPECT 'Started' volinfo_field $V0 'Status';
+
+
+# Setting system limit
+TEST $CLI snapshot config snap-max-hard-limit 100
+
+# Volume limit cannot exceed system limit, as limit is set to 100,
+# this should fail.
+TEST ! $CLI snapshot config $V0 snap-max-hard-limit 101
+
+# Following are the invalid cases
+TEST ! $CLI snapshot config $V0 snap-max-hard-limit a10
+TEST ! $CLI snapshot config snap-max-hard-limit 10a
+TEST ! $CLI snapshot config snap-max-hard-limit 10%
+TEST ! $CLI snapshot config snap-max-soft-limit 50%1
+TEST ! $CLI snapshot config snap-max-soft-limit 0111
+TEST ! $CLI snapshot config snap-max-hard-limit OXA
+TEST ! $CLI snapshot config snap-max-hard-limit 11.11
+TEST ! $CLI snapshot config snap-max-soft-limit 50%
+
+# Soft limit cannot be assigned to volume
+TEST ! $CLI snapshot config $V0 snap-max-soft-limit 10
+
+# Valid case
+TEST $CLI snapshot config snap-max-soft-limit 50
+TEST $CLI snapshot config $V0 snap-max-hard-limit 10
+
+cleanup;
+
diff --git a/xlators/mgmt/glusterd/src/glusterd-snapshot.c b/xlators/mgmt/glusterd/src/glusterd-snapshot.c
index 6e28efc3010..3c75f4ab31c 100644
--- a/xlators/mgmt/glusterd/src/glusterd-snapshot.c
+++ b/xlators/mgmt/glusterd/src/glusterd-snapshot.c
@@ -545,8 +545,8 @@ snap_max_hard_limits_validate (dict_t *dict, char *volname,
 
         if ((value < 0) || (value > max_limit)) {
                 ret = -1;
-                snprintf (err_str, PATH_MAX, "Invalid snap-max-hard-limit"
-                          "%"PRIu64 ". Expected range 0 - %"PRIu64,
+                snprintf (err_str, PATH_MAX, "Invalid snap-max-hard-limit "
+                          "%"PRIu64 ". Expected range 1 - %"PRIu64,
                           value, max_limit);
                 goto out;
         }
@@ -570,7 +570,6 @@ glusterd_snapshot_config_prevalidate (dict_t *dict, char **op_errstr)
         int                 config_command      = 0;
         char                err_str[PATH_MAX]   = {0,};
         glusterd_conf_t    *conf                = NULL;
-        uint64_t            value               = 0;
         uint64_t            hard_limit          = 0;
         uint64_t            soft_limit          = 0;
         gf_loglevel_t       loglevel            = GF_LOG_ERROR;
@@ -628,8 +627,8 @@ glusterd_snapshot_config_prevalidate (dict_t *dict, char **op_errstr)
                                 ret = -1;
                                 snprintf (err_str, PATH_MAX, "Invalid "
                                          "snap-max-soft-limit ""%"
-                                         PRIu64 ". Expected range 0 - %"PRIu64,
-                                         value, max_limit);
+                                         PRIu64 ". Expected range 1 - %"PRIu64,
+                                         soft_limit, max_limit);
                                 goto out;
                         }
                         break;