From 2baa37f5bd2d29c30e0154758449ac1980fc8db1 Mon Sep 17 00:00:00 2001
From: Poornima G <pgurusid@redhat.com>
Date: Thu, 6 Apr 2017 16:36:29 +0530
Subject: [PATCH 382/393] 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.
>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>
>Signed-off-by: Poornima G <pgurusid@redhat.com>
Change-Id: I902cd9ac9134c158ab6f8aea4b001254a03547bd
BUG: 1438245
Signed-off-by: Poornima G <pgurusid@redhat.com>
Reviewed-on: https://code.engineering.redhat.com/gerrit/103634
Reviewed-by: Atin Mukherjee <amukherj@redhat.com>
---
tests/bugs/readdir-ahead/bug-1439640.t | 30 ++++++++++++++++++
xlators/mgmt/glusterd/src/glusterd-volgen.c | 41 ++++++++++++++++++++-----
xlators/mgmt/glusterd/src/glusterd-volume-set.c | 1 +
3 files changed, 65 insertions(+), 7 deletions(-)
create mode 100755 tests/bugs/readdir-ahead/bug-1439640.t
diff --git a/tests/bugs/readdir-ahead/bug-1439640.t b/tests/bugs/readdir-ahead/bug-1439640.t
new file mode 100755
index 0000000..cfa5d1f
--- /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 c906306..6e52d44 100644
--- a/xlators/mgmt/glusterd/src/glusterd-volgen.c
+++ b/xlators/mgmt/glusterd/src/glusterd-volgen.c
@@ -3624,17 +3624,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 e619401..4a1c780 100644
--- a/xlators/mgmt/glusterd/src/glusterd-volume-set.c
+++ b/xlators/mgmt/glusterd/src/glusterd-volume-set.c
@@ -3024,6 +3024,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"
--
1.8.3.1