From e44d4e135747b04a5e2b7cfac21f9a3343e071db Mon Sep 17 00:00:00 2001 From: GauravKumarGarg Date: Wed, 12 Nov 2014 17:41:33 +0530 Subject: 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 Reviewed-on: http://review.gluster.org/9104 Tested-by: Gluster Build System Reviewed-by: Krishnan Parthasarathi Reviewed-by: Raghavendra G Reviewed-by: Vijay Bellur --- libglusterfs/src/common-utils.c | 6 +-- libglusterfs/src/common-utils.h | 2 +- libglusterfs/src/options.c | 46 ++++++++++++++++++++-- .../bug-1163108-min-free-disk-option-validation.t | 37 +++++++++++++++++ 4 files changed, 83 insertions(+), 8 deletions(-) create mode 100644 tests/bugs/glusterd/bug-1163108-min-free-disk-option-validation.t 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 @@ -366,21 +366,59 @@ out: return ret; } +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; -- cgit