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