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