[Gluster-devel] [PATCH: glusterfs] fixed? alu

[Amar Tumballi]

 This patch includes two smaller things:
 1. support to define an option of type PERCENT_OR_SIZET, (modification in
libglusterfs/src/*)

 2. support in alu to provide min-free-disk option with type
PERCENT_OR_SIZET


Review replies inline:

On Wed, Apr 22, 2009 at 4:10 PM, Paul Rawson <plrca2 at gmail.com> wrote:

> Sorry, I think gmail is screwing up the formatting. Let me know if
> this still isn't right.


formatting is fine now.

> +       case GF_OPTION_TYPE_PERCENTORSIZET:
> +       {
> +               uint32_t percent = 0;
> +               uint32_t input_size = 0;
> +
> +               /* Check if the value is valid percentage */
> +               if (gf_string2percent (pair->value->data,
> +                                      &percent) == 0) {
> +                       if ((percent < 0) || (percent > 100)) {
> +                               gf_log (xl->name, GF_LOG_ERROR,
> +                               "'%d' in 'option %s %s' is out of "
> +                               "range [0 - 100]",
> +                               percent, pair->key,
> +                               pair->value->data);
> +                       }
> +                       ret = 0;
> +                       goto out;


assume a user gives "option min-free-disk 131072" (with the intention of
disk size in size_t), gf_string2percent returns 0, but the check to validate
the range fails. but it won't pass the check for 'gf_string2bytesize in this
case. So, even logging 'error' in that case won't be valid.


> +               } else {
> +                       /* Check the range */
> +                       if (gf_string2bytesize (pair->value->data,
> +                                               &input_size) == 0) {
> +
> +                              if ((opt->min == 0) && (opt->max == 0)) {
> +                                       gf_log (xl->name, GF_LOG_DEBUG,
> +                                               "no range check required
> for "
> +                                               "'option %s %s'",
> +                                               pair->key,
> pair->value->data);
> +                                       ret = 0;
> +                                       break;
> +                               }
> +                               if ((input_size < opt->min) ||
> +                                   (input_size > opt->max)) {
> +                                       gf_log (xl->name, GF_LOG_ERROR,
> +                                               "'%"PRId64"' in 'option %s
> %s' is "
> +                                               "out of range [%"PRId64" -
> %"PRId64"]",
> +                                               input_size, pair->key,
> +                                               pair->value->data,
> +                                               opt->min, opt->max);
> +                               }
> +                               ret = 0;
> +                               goto out;
> +                       } else {
> +                               // It's not a percent or size
> +                               gf_log (xl->name, GF_LOG_ERROR,
> +                               "invalid number format \"%s\" "
> +                               "in \"option %s\"",
> +                               pair->value->data, pair->key);
> +
> +                       }
> +               }
> +       }
> +       break;
>

-- 
Amar Tumballi
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://supercolony.gluster.org/pipermail/gluster-devel/attachments/20090423/69316cfc/attachment-0003.html>