14f8ab
From 71fc5b7949e00c4448f5ec1291e756b201a70082 Mon Sep 17 00:00:00 2001
14f8ab
From: Ravishankar N <ravishankar@redhat.com>
14f8ab
Date: Thu, 29 Apr 2021 18:34:57 +0530
14f8ab
Subject: [PATCH 543/543] glusterd: handle custom xlator failure cases
14f8ab
14f8ab
Problem-1:
14f8ab
custom xlator insertion was failing for those xlators in the brick graph
14f8ab
whose dbg_key was NULL in the server_graph_table. Looking at the git log,
14f8ab
the dbg_key was added in commit d1397dbd7d6cdbd2d81d5d36d608b6175d449db4
14f8ab
for inserting debug xlators.
14f8ab
14f8ab
Fix: I think it is fine to define it for all brick xlators below server.
14f8ab
14f8ab
Problem-2:
14f8ab
In the commit-op phase, glusterd_op_set_volume() updates the volinfo
14f8ab
dict with the key-value pairs and then proceeds to create the volfiles.
14f8ab
If any of the steps fail, the volinfo dict retains those key-values,
14f8ab
until glusterd is restarted or `gluster vol reset $VOLNAME` is issued.
14f8ab
14f8ab
Fix:
14f8ab
Make a copy of the volinfo dict and if there are any failures in
14f8ab
proceeding with the set volume logic, restore the dict to its original
14f8ab
state.
14f8ab
14f8ab
Backport of:
14f8ab
> Upstream-patch-link: https://github.com/gluster/glusterfs/pull/2371
14f8ab
> Change-Id: I9010dab33d0139b8e6d603308e331b6d220a4849
14f8ab
> Updates: #2370
14f8ab
> Signed-off-by: Ravishankar N <ravishankar@redhat.com>
14f8ab
14f8ab
Change-Id: I9010dab33d0139b8e6d603308e331b6d220a4849
14f8ab
BUG: 1953901
14f8ab
Signed-off-by: Ravishankar N <ravishankar@redhat.com>
14f8ab
Reviewed-on: https://code.engineering.redhat.com/gerrit/239889
14f8ab
Reviewed-by: Sunil Kumar Heggodu Gopala Acharya <sheggodu@redhat.com>
14f8ab
---
14f8ab
 tests/basic/user-xlator.t                   | 16 ++++++++++++++--
14f8ab
 xlators/mgmt/glusterd/src/glusterd-op-sm.c  | 16 ++++++++++++++++
14f8ab
 xlators/mgmt/glusterd/src/glusterd-volgen.c | 14 +++++++-------
14f8ab
 3 files changed, 37 insertions(+), 9 deletions(-)
14f8ab
14f8ab
diff --git a/tests/basic/user-xlator.t b/tests/basic/user-xlator.t
14f8ab
index a711f9f..ed2d831 100755
14f8ab
--- a/tests/basic/user-xlator.t
14f8ab
+++ b/tests/basic/user-xlator.t
14f8ab
@@ -35,8 +35,18 @@ EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" brick_up_status $V0 $H0 $B0/${V0}4
14f8ab
 EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" brick_up_status $V0 $H0 $B0/${V0}5
14f8ab
 EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" brick_up_status $V0 $H0 $B0/${V0}6
14f8ab
 
14f8ab
-TEST $CLI volume set $V0 user.xlator.hoge trash
14f8ab
-TEST grep -q 'user/hoge' ${SERVER_VOLFILE}
14f8ab
+# Test that the insertion at all positions between server and posix is successful.
14f8ab
+# It is not guaranteed that the brick process will start/work in all positions though.
14f8ab
+TESTS_EXPECTED_IN_LOOP=34
14f8ab
+declare -a brick_side_xlators=("decompounder" "io-stats" "quota" "index" "barrier"
14f8ab
+                               "marker" "selinux" "io-threads" "upcall" "leases"
14f8ab
+                               "read-only" "worm" "locks"  "access-control"
14f8ab
+                               "bitrot-stub" "changelog" "trash")
14f8ab
+for xlator in "${brick_side_xlators[@]}"
14f8ab
+  do
14f8ab
+    TEST_IN_LOOP $CLI volume set $V0 user.xlator.hoge $xlator
14f8ab
+    TEST_IN_LOOP grep -q 'user/hoge' ${SERVER_VOLFILE}
14f8ab
+  done
14f8ab
 
14f8ab
 TEST $CLI volume stop $V0
14f8ab
 TEST $CLI volume start $V0
14f8ab
@@ -49,6 +59,8 @@ EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" brick_up_status $V0 $H0 $B0/${V0}6
14f8ab
 
14f8ab
 TEST ! $CLI volume set $V0 user.xlator.hoge unknown
14f8ab
 TEST grep -q 'user/hoge' ${SERVER_VOLFILE} # When the CLI fails, the volfile is not modified.
14f8ab
+# User xlator insert failures must not prevent setting other volume options.
14f8ab
+TEST $CLI volume set $V0 storage.reserve 10%
14f8ab
 
14f8ab
 TEST $CLI volume stop $V0
14f8ab
 TEST $CLI volume start $V0
14f8ab
diff --git a/xlators/mgmt/glusterd/src/glusterd-op-sm.c b/xlators/mgmt/glusterd/src/glusterd-op-sm.c
14f8ab
index 1e84f5f..893af29 100644
14f8ab
--- a/xlators/mgmt/glusterd/src/glusterd-op-sm.c
14f8ab
+++ b/xlators/mgmt/glusterd/src/glusterd-op-sm.c
14f8ab
@@ -2911,6 +2911,7 @@ glusterd_op_set_volume(dict_t *dict, char **errstr)
14f8ab
     uint32_t new_op_version = 0;
14f8ab
     gf_boolean_t quorum_action = _gf_false;
14f8ab
     glusterd_svc_t *svc = NULL;
14f8ab
+    dict_t *volinfo_dict_orig = NULL;
14f8ab
 
14f8ab
     this = THIS;
14f8ab
     GF_ASSERT(this);
14f8ab
@@ -2918,6 +2919,10 @@ glusterd_op_set_volume(dict_t *dict, char **errstr)
14f8ab
     priv = this->private;
14f8ab
     GF_ASSERT(priv);
14f8ab
 
14f8ab
+    volinfo_dict_orig = dict_new();
14f8ab
+    if (!volinfo_dict_orig)
14f8ab
+        goto out;
14f8ab
+
14f8ab
     ret = dict_get_int32n(dict, "count", SLEN("count"), &dict_count);
14f8ab
     if (ret) {
14f8ab
         gf_msg(this->name, GF_LOG_ERROR, 0, GD_MSG_DICT_GET_FAILED,
14f8ab
@@ -2949,6 +2954,11 @@ glusterd_op_set_volume(dict_t *dict, char **errstr)
14f8ab
         goto out;
14f8ab
     }
14f8ab
 
14f8ab
+    if (dict_copy(volinfo->dict, volinfo_dict_orig) == NULL) {
14f8ab
+        ret = -ENOMEM;
14f8ab
+        goto out;
14f8ab
+    }
14f8ab
+
14f8ab
     /* TODO: Remove this once v3.3 compatibility is not required */
14f8ab
     check_op_version = dict_get_str_boolean(dict, "check-op-version",
14f8ab
                                             _gf_false);
14f8ab
@@ -3171,6 +3181,12 @@ out:
14f8ab
     gf_msg_debug(this->name, 0, "returning %d", ret);
14f8ab
     if (quorum_action)
14f8ab
         glusterd_do_quorum_action();
14f8ab
+    if (ret < 0 && count > 1) {
14f8ab
+        if (dict_reset(volinfo->dict) == 0)
14f8ab
+            dict_copy(volinfo_dict_orig, volinfo->dict);
14f8ab
+    }
14f8ab
+    if (volinfo_dict_orig)
14f8ab
+        dict_unref(volinfo_dict_orig);
14f8ab
     return ret;
14f8ab
 }
14f8ab
 
14f8ab
diff --git a/xlators/mgmt/glusterd/src/glusterd-volgen.c b/xlators/mgmt/glusterd/src/glusterd-volgen.c
14f8ab
index 71aed08..aa85bdb 100644
14f8ab
--- a/xlators/mgmt/glusterd/src/glusterd-volgen.c
14f8ab
+++ b/xlators/mgmt/glusterd/src/glusterd-volgen.c
14f8ab
@@ -2706,24 +2706,24 @@ out:
14f8ab
 static volgen_brick_xlator_t server_graph_table[] = {
14f8ab
     {brick_graph_add_server, NULL},
14f8ab
     {brick_graph_add_decompounder, "decompounder"},
14f8ab
-    {brick_graph_add_io_stats, "NULL"},
14f8ab
+    {brick_graph_add_io_stats, "io-stats"},
14f8ab
     {brick_graph_add_sdfs, "sdfs"},
14f8ab
     {brick_graph_add_namespace, "namespace"},
14f8ab
-    {brick_graph_add_cdc, NULL},
14f8ab
+    {brick_graph_add_cdc, "cdc" },
14f8ab
     {brick_graph_add_quota, "quota"},
14f8ab
     {brick_graph_add_index, "index"},
14f8ab
-    {brick_graph_add_barrier, NULL},
14f8ab
+    {brick_graph_add_barrier, "barrier" },
14f8ab
     {brick_graph_add_marker, "marker"},
14f8ab
     {brick_graph_add_selinux, "selinux"},
14f8ab
     {brick_graph_add_fdl, "fdl"},
14f8ab
     {brick_graph_add_iot, "io-threads"},
14f8ab
     {brick_graph_add_upcall, "upcall"},
14f8ab
     {brick_graph_add_leases, "leases"},
14f8ab
-    {brick_graph_add_pump, NULL},
14f8ab
-    {brick_graph_add_ro, NULL},
14f8ab
-    {brick_graph_add_worm, NULL},
14f8ab
+    {brick_graph_add_pump, "pump" },
14f8ab
+    {brick_graph_add_ro, "read-only" },
14f8ab
+    {brick_graph_add_worm, "worm" },
14f8ab
     {brick_graph_add_locks, "locks"},
14f8ab
-    {brick_graph_add_acl, "acl"},
14f8ab
+    {brick_graph_add_acl, "access-control"},
14f8ab
     {brick_graph_add_bitrot_stub, "bitrot-stub"},
14f8ab
     {brick_graph_add_changelog, "changelog"},
14f8ab
 #if USE_GFDB /* changetimerecorder depends on gfdb */
14f8ab
-- 
14f8ab
1.8.3.1
14f8ab