From e34557b686ae1c51ba90be80c7d47b14122bbd38 Mon Sep 17 00:00:00 2001
From: Csaba Henk <chenk@redhat.com>
Date: Tue, 9 May 2017 15:52:43 +0200
Subject: [PATCH 425/426] libglusterfs: stop special casing "cache-size" in
size_t validation
The original situation was as follows:
The function that validates xlator options indicating a size,
xlator_option_validate_sizet(), handles the case when the name
of the option is "cache-size" in a special way.
- Xlator options (things of type volume_option_t) has a
min and max attribute of type double.
- An xlator option is endowed with a gluster specific type (not
C type). An instance of an xlator option goes through a validation
process by a type specific validator function (which are collected
in option.c).
- Validators of numeric types - size being one of them - make use the
min and max attributes to perform a range check, except in one case:
if an option is defined with min = max = 0, then this option will be
exempt of range checking. (Note: the volume_option_t definition
features the following comments along the min, max fields:
double min; /* 0 means no range */
double max; /* 0 means no range */
which is slightly misleading as it lets one to conclude that
zeroing min or max buys exemption from low or high boundary check,
which is not true -- only *both* being zero buys exemption.)
- Besides this, the validator for options of size type,
xlator_option_validate_sizet() special cases options
named "cache-size" so that only min is enforced. (The only consequence
of a value exceeding max is that glusterd logs a warning about it, but
the cli user who makes such a setting gets no feedback on it.)
- This was introduced because a hard coded limit is not useful for
io-cache and quick-read. They rather use a runtime calculated
upper limit. (See changes
I7dd4d8c53051b89a293696abf1ee8dc237e39a20
I9c744b5ace10604d5a814e6218ca0d83c796db80
about the last two points.)
- As an unintended consequence, the upper limit check of
cache-size of write-behind, for which a conventional hard coded limit
is specified, is defeated.
What we do about it:
- Remove the special casing clause for cache-size in
xlator_option_validate_sizet. Thus the general range
check policy (as described above) will apply to
cache-size too.
- To implement a lower bound only check by the validator
for cache-size of io-cache and quick-read, change the
max attribute of these options to INFINITY.
The only behavioral difference is the omission of the warnings
about cache-size of io-cache and quick-read exceeding the former max
values. (They were rather heuristic anyway.)
> BUG: 1445609
> Change-Id: I0bd8bd391fa7d926f76e214a2178833fe4673b4a
> Signed-off-by: Csaba Henk <csaba@redhat.com>
> Reviewed-on: https://review.gluster.org/17125
> Smoke: Gluster Build System <jenkins@build.gluster.org>
> Reviewed-by: Amar Tumballi <amarts@redhat.com>
> Tested-by: Raghavendra G <rgowdapp@redhat.com>
> NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>
> CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
BUG: 1297743
Change-Id: Idd758c3ed47f0b50706d3becce281593005841ce
Signed-off-by: Csaba Henk <chenk@redhat.com>
Reviewed-on: https://code.engineering.redhat.com/gerrit/105675
Reviewed-by: Atin Mukherjee <amukherj@redhat.com>
---
libglusterfs/src/options.c | 23 +++++++----------------
xlators/performance/io-cache/src/io-cache.c | 3 ++-
xlators/performance/quick-read/src/quick-read.c | 3 ++-
3 files changed, 11 insertions(+), 18 deletions(-)
diff --git a/libglusterfs/src/options.c b/libglusterfs/src/options.c
index 53bd779..02928a9 100644
--- a/libglusterfs/src/options.c
+++ b/libglusterfs/src/options.c
@@ -152,22 +152,13 @@ xlator_option_validate_sizet (xlator_t *xl, const char *key, const char *value,
}
if ((size < opt->min) || (size > opt->max)) {
- if ((strncmp (key, "cache-size", 10) == 0) &&
- (size > opt->max)) {
- snprintf (errstr, 256, "Cache size %" GF_PRI_SIZET " is out of "
- "range [%.0f - %.0f]",
- size, opt->min, opt->max);
- gf_msg (xl->name, GF_LOG_WARNING, 0,
- LG_MSG_OUT_OF_RANGE, "%s", errstr);
- } else {
- snprintf (errstr, 256,
- "'%" GF_PRI_SIZET "' in 'option %s %s' "
- "is out of range [%.0f - %.0f]",
- size, key, value, opt->min, opt->max);
- gf_msg (xl->name, GF_LOG_ERROR, 0,
- LG_MSG_OUT_OF_RANGE, "%s", errstr);
- ret = -1;
- }
+ snprintf (errstr, 256,
+ "'%" GF_PRI_SIZET "' in 'option %s %s' "
+ "is out of range [%.0f - %.0f]",
+ size, key, value, opt->min, opt->max);
+ gf_msg (xl->name, GF_LOG_ERROR, 0,
+ LG_MSG_OUT_OF_RANGE, "%s", errstr);
+ ret = -1;
}
out:
diff --git a/xlators/performance/io-cache/src/io-cache.c b/xlators/performance/io-cache/src/io-cache.c
index 98c3774..9de6ee1 100644
--- a/xlators/performance/io-cache/src/io-cache.c
+++ b/xlators/performance/io-cache/src/io-cache.c
@@ -8,6 +8,7 @@
cases as published by the Free Software Foundation.
*/
+#include <math.h>
#include "glusterfs.h"
#include "logging.h"
#include "dict.h"
@@ -2155,7 +2156,7 @@ struct volume_options options[] = {
{ .key = {"cache-size"},
.type = GF_OPTION_TYPE_SIZET,
.min = 4 * GF_UNIT_MB,
- .max = 32 * GF_UNIT_GB,
+ .max = INFINITY,
.default_value = "32MB",
.description = "Size of the read cache."
},
diff --git a/xlators/performance/quick-read/src/quick-read.c b/xlators/performance/quick-read/src/quick-read.c
index 4e9c6dc..8edf495 100644
--- a/xlators/performance/quick-read/src/quick-read.c
+++ b/xlators/performance/quick-read/src/quick-read.c
@@ -8,6 +8,7 @@
cases as published by the Free Software Foundation.
*/
+#include <math.h>
#include "quick-read.h"
#include "statedump.h"
#include "quick-read-messages.h"
@@ -1144,7 +1145,7 @@ struct volume_options options[] = {
{ .key = {"cache-size"},
.type = GF_OPTION_TYPE_SIZET,
.min = 0,
- .max = 32 * GF_UNIT_GB,
+ .max = INFINITY,
.default_value = "128MB",
.description = "Size of the read cache."
},
--
1.8.3.1