diff options
author | Poornima G <pgurusid@redhat.com> | 2017-04-06 16:36:29 +0530 |
---|---|---|
committer | Atin Mukherjee <amukherj@redhat.com> | 2017-04-11 03:42:40 -0400 |
commit | c002b3d76486128ba78328b4a96ae0a1d8fe9cd0 (patch) | |
tree | 12ccab266ed43abbc5183544549a6ebc0beced1e | |
parent | b9b32f8363de1895a0cb144b6a641ed61eadade8 (diff) |
glusterd: Add validation for options rda-cache-limit rda-request-size
Currently when prarallel readdir is enabled, setting any junk value
to rda-cache-limit and rda-request-size succeeds. This is because of
bug in the special handling of these options.
Fixing the same in this patch
Change-Id: I902cd9ac9134c158ab6f8aea4b001254a03547bd
BUG: 1439640
Signed-off-by: Poornima G <pgurusid@redhat.com>
Reviewed-on: https://review.gluster.org/17008
NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>
Smoke: Gluster Build System <jenkins@build.gluster.org>
Reviewed-by: Atin Mukherjee <amukherj@redhat.com>
CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
-rwxr-xr-x | tests/bugs/readdir-ahead/bug-1439640.t | 30 | ||||
-rw-r--r-- | xlators/mgmt/glusterd/src/glusterd-volgen.c | 41 | ||||
-rw-r--r-- | xlators/mgmt/glusterd/src/glusterd-volume-set.c | 1 |
3 files changed, 65 insertions, 7 deletions
diff --git a/tests/bugs/readdir-ahead/bug-1439640.t b/tests/bugs/readdir-ahead/bug-1439640.t new file mode 100755 index 00000000000..cfa5d1ff659 --- /dev/null +++ b/tests/bugs/readdir-ahead/bug-1439640.t @@ -0,0 +1,30 @@ +#!/bin/bash + +. $(dirname $0)/../../include.rc +. $(dirname $0)/../../volume.rc + +cleanup; + +TEST glusterd + +TEST $CLI volume create $V0 $H0:$B{0..1}/$V0 +TEST $CLI volume start $V0 + +TEST ! $CLI volume set $V0 parallel-readdir sdf + +TEST $CLI volume set $V0 parallel-readdir off +TEST $CLI volume set $V0 parallel-readdir on + +TEST ! $CLI volume set $V0 rda-cache-limit 0 +TEST ! $CLI volume set $V0 rda-cache-limit -634 +TEST ! $CLI volume set $V0 rda-cache-limit 87adh +TEST ! $CLI volume set $V0 parallel-readdir sdf + +TEST ! $CLI volume set $V0 rda-request-size 0 +TEST ! $CLI volume set $V0 rda-request-size -634 +TEST ! $CLI volume set $V0 rda-request-size 87adh + +TEST $CLI volume set $V0 rda-cache-limit 10MB +TEST $CLI volume set $V0 rda-request-size 128KB + +#cleanup; diff --git a/xlators/mgmt/glusterd/src/glusterd-volgen.c b/xlators/mgmt/glusterd/src/glusterd-volgen.c index 59d1eceb182..30d22fadbf4 100644 --- a/xlators/mgmt/glusterd/src/glusterd-volgen.c +++ b/xlators/mgmt/glusterd/src/glusterd-volgen.c @@ -3787,17 +3787,44 @@ client_graph_set_rda_options (volgen_graph_t *graph, !glusterd_volinfo_get_boolean (volinfo, VKEY_READDIR_AHEAD)) goto out; - ret = glusterd_volinfo_get (volinfo, VKEY_RDA_CACHE_LIMIT, &rda_cache_s); - if (ret < 0) + /* glusterd_volinfo_get() will get the default value if nothing set + * explicitly. Hence it is important to check set_dict before checking + * glusterd_volinfo_get, so that we consider key value of the in + * progress volume set option. + */ + ret = dict_get_str (set_dict, VKEY_RDA_CACHE_LIMIT, &rda_cache_s); + if (ret < 0) { + ret = glusterd_volinfo_get (volinfo, VKEY_RDA_CACHE_LIMIT, + &rda_cache_s); + if (ret < 0) + goto out; + } + ret = gf_string2bytesize_uint64 (rda_cache_s, &rda_cache_size); + if (ret < 0) { + set_graph_errstr (graph, "invalid number format in option " + VKEY_RDA_CACHE_LIMIT); goto out; + } - gf_string2bytesize_uint64 (rda_cache_s, &rda_cache_size); - - ret = glusterd_volinfo_get (volinfo, VKEY_RDA_REQUEST_SIZE, &rda_req_s); - if (ret < 0) + ret = dict_get_str (set_dict, VKEY_RDA_REQUEST_SIZE, &rda_req_s); + if (ret < 0) { + ret = glusterd_volinfo_get (volinfo, VKEY_RDA_REQUEST_SIZE, + &rda_req_s); + if (ret < 0) + goto out; + } + gf_string2bytesize_uint64 (rda_req_s, &rda_req_size); + if (ret < 0) { + set_graph_errstr (graph, "invalid number format in option " + VKEY_RDA_REQUEST_SIZE); goto out; + } - gf_string2bytesize_uint64 (rda_req_s, &rda_req_size); + if (rda_cache_size == 0 || rda_req_size == 0) { + set_graph_errstr (graph, "Value cannot be 0"); + ret = -1; + goto out; + } new_cache_size = rda_cache_size / dist_count; if (new_cache_size < rda_req_size) { diff --git a/xlators/mgmt/glusterd/src/glusterd-volume-set.c b/xlators/mgmt/glusterd/src/glusterd-volume-set.c index 8fb5042b4d1..ca32552fb0d 100644 --- a/xlators/mgmt/glusterd/src/glusterd-volume-set.c +++ b/xlators/mgmt/glusterd/src/glusterd-volume-set.c @@ -3137,6 +3137,7 @@ struct volopt_map_entry glusterd_volopt_map[] = { .value = "off", .type = DOC, .op_version = GD_OP_VERSION_3_10_0, + .validate_fn = validate_boolean, .description = "If this option is enabled, the readdir operation is " "performed parallely on all the bricks, thus improving" " the performance of readdir. Note that the performance" |