DHT: cluster.min-free-disk option should validate correctly

PROBLEM:
Previously gluster accepting input value as a percentage which is out of range
[0-100] and accepting input value as a size (unit is byte) which is fractional
for option cluster.min-free-disk.

FIX:
Now with this change it will refer to correct validation function
and it will accept value that is in range [0-100] for input value as a
percentage and unsigned integer value for input as a size (unit in byte)
for option cluster.min-free-disk.

Change-Id: Iee1962a100542e146276cfc8a4068abddee2bf2d
BUG: 1163108
Signed-off-by: Gaurav Kumar Garg <ggarg@redhat.com>
Reviewed-on: http://review.gluster.org/9104
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Krishnan Parthasarathi <kparthas@redhat.com>
Reviewed-by: Raghavendra G <rgowdapp@redhat.com>
Reviewed-by: Vijay Bellur <vbellur@redhat.com>

4 files changed, 83 insertions, 8 deletions

diff --git a/libglusterfs/src/common-utils.c b/libglusterfs/src/common-utils.c
index 6d03b09a943..39da27d83dd 100644
--- a/libglusterfs/src/common-utils.c
+++ b/libglusterfs/src/common-utils.c
@@ -1449,8 +1449,7 @@ gf_string2bytesize_uint64 (const char *str, uint64_t *n)
 }
 
 int
-gf_string2percent_or_bytesize (const char *str,
-			       uint64_t *n,
+gf_string2percent_or_bytesize (const char *str, double *n,
 			       gf_boolean_t *is_percent)
 {
         double value = 0ULL;
@@ -1485,6 +1484,7 @@ gf_string2percent_or_bytesize (const char *str,
         if (errno == 0)
                 errno = old_errno;
 
+        /*Maximum accepted value for 64 bit OS will be (2^14 -1)PB*/
         if (tail[0] != '\0') {
                 if (strcasecmp (tail, GF_UNIT_KB_STRING) == 0)
                         value *= GF_UNIT_KB;
@@ -1508,7 +1508,7 @@ gf_string2percent_or_bytesize (const char *str,
                 return -1;
         }
 
-        *n = (uint64_t) value;
+        *n = value;
 
         return 0;
 }
diff --git a/libglusterfs/src/common-utils.h b/libglusterfs/src/common-utils.h
index eaede81a5c8..69b1e277861 100644
--- a/libglusterfs/src/common-utils.h
+++ b/libglusterfs/src/common-utils.h
@@ -580,7 +580,7 @@ int gf_string2uint64_base10 (const char *str, uint64_t *n);
 int gf_string2bytesize (const char *str, uint64_t *n);
 int gf_string2bytesize_size (const char *str, size_t *n);
 int gf_string2bytesize_uint64 (const char *str, uint64_t *n);
-int gf_string2percent_or_bytesize (const char *str, uint64_t *n,
+int gf_string2percent_or_bytesize (const char *str, double *n,
 				   gf_boolean_t *is_percent);
 
 int gf_string2boolean (const char *str, gf_boolean_t *b);
diff --git a/libglusterfs/src/options.c b/libglusterfs/src/options.c
index 9edb7c03df2..44f74a498c5 100644
--- a/libglusterfs/src/options.c
+++ b/libglusterfs/src/options.c
@@ -367,20 +367,58 @@ out:
 }
 
 static int
+xlator_option_validate_fractional_value (const char *value)
+{
+        const char *s   = NULL;
+        int        ret  = 0;
+
+        s = strchr (value, '.');
+        if (s) {
+                for (s = s+1; *s != '\0'; s++) {
+                        if (*s != '0') {
+                                return -1;
+                        }
+                }
+        }
+
+        return ret;
+}
+
+static int
 xlator_option_validate_percent_or_sizet (xlator_t *xl, const char *key,
                                          const char *value,
                                          volume_option_t *opt, char **op_errstr)
 {
-        int          ret = -1;
-        char         errstr[256];
-        uint64_t     size = 0;
+        int               ret = -1;
+        char              errstr[256];
+        double            size = 0;
 	gf_boolean_t is_percent = _gf_false;
 
 	if (gf_string2percent_or_bytesize (value, &size, &is_percent) == 0) {
 		if (is_percent) {
+                        if ((size < 0.0) || (size > 100.0)) {
+                                snprintf (errstr, sizeof (errstr),
+                                          "'%lf' in 'option %s %s' is out"
+                                          " of range [0 - 100]", size, key,
+                                          value);
+                                gf_log (xl->name, GF_LOG_ERROR, "%s", errstr);
+                                goto out;
+                        }
 			ret = 0;
 			goto out;
 		}
+
+                /*Input value of size(in byte) should not be fractional*/
+                ret = xlator_option_validate_fractional_value (value);
+                if (ret) {
+                        snprintf (errstr, sizeof (errstr), "'%lf' in 'option %s"
+                                  " %s' should not be fractional value. Use "
+                                  "valid unsigned integer value.", size, key,
+                                  value);
+                        gf_log (xl->name, GF_LOG_ERROR, "%s", errstr);
+                        goto out;
+                }
+
 		/* Check the range */
 		if ((opt->min == 0) && (opt->max == 0)) {
 			gf_log (xl->name, GF_LOG_TRACE,
@@ -392,7 +430,7 @@ xlator_option_validate_percent_or_sizet (xlator_t *xl, const char *key,
 		}
 		if ((size < opt->min) || (size > opt->max)) {
 			snprintf (errstr, 256,
-				  "'%"PRId64"' in 'option %s %s'"
+				  "'%lf' in 'option %s %s'"
 				  " is out of range [%.0f - %.0f]",
 				  size, key, value, opt->min, opt->max);
 			gf_log (xl->name, GF_LOG_ERROR, "%s", errstr);
diff --git a/tests/bugs/glusterd/bug-1163108-min-free-disk-option-validation.t b/tests/bugs/glusterd/bug-1163108-min-free-disk-option-validation.t
new file mode 100644
index 00000000000..9fc7ac3b845
--- /dev/null
+++ b/tests/bugs/glusterd/bug-1163108-min-free-disk-option-validation.t
@@ -0,0 +1,37 @@
+#!/bin/bash
+
+## Test case for cluster.min-free-disk option validation.
+
+
+. $(dirname $0)/../../include.rc
+. $(dirname $0)/../../volume.rc
+
+cleanup;
+
+## Start glusterd
+TEST glusterd
+TEST pidof glusterd
+
+## Lets create and start volume
+TEST $CLI volume create $V0 $H0:$B0/brick1 $H0:$B0/brick2
+TEST $CLI volume start $V0
+
+## Setting invalid value for option cluster.min-free-disk should fail
+TEST ! $CLI volume set $V0 min-free-disk ""
+TEST ! $CLI volume set $V0 min-free-disk 143.!/12
+TEST ! $CLI volume set $V0 min-free-disk 123%
+TEST ! $CLI volume set $V0 min-free-disk 194.34%
+
+## Setting fractional value as a size (unit is byte) for option
+## cluster.min-free-disk should fail
+TEST ! $CLI volume set $V0 min-free-disk 199.051
+TEST ! $CLI volume set $V0 min-free-disk 111.999
+
+## Setting valid value for option cluster.min-free-disk should pass
+TEST  $CLI volume set $V0 min-free-disk 12%
+TEST  $CLI volume set $V0 min-free-disk 56.7%
+TEST  $CLI volume set $V0 min-free-disk 120
+TEST  $CLI volume set $V0 min-free-disk 369.0000
+
+
+cleanup;