diff options
| author | Csaba Henk <csaba@redhat.com> | 2017-04-25 17:01:09 +0200 | 
|---|---|---|
| committer | Jeff Darcy <jeff@pl.atyp.us> | 2017-05-08 13:37:54 +0000 | 
| commit | 0d8923d6d70af702730a43a536a5d0b25b4061dc (patch) | |
| tree | 27e433b78f11918b9d4ee1f201e6583d178f4ce2 /libglusterfs | |
| parent | 52c28c0c04722a9ffaa7c39c49ffebdf0a5c75e1 (diff) | |
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>
Diffstat (limited to 'libglusterfs')
| -rw-r--r-- | libglusterfs/src/options.c | 23 | 
1 files changed, 7 insertions, 16 deletions
diff --git a/libglusterfs/src/options.c b/libglusterfs/src/options.c index a28f3b7ea4f..3b1e21b5d0f 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:  | 
