1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
|
From 71fc5b7949e00c4448f5ec1291e756b201a70082 Mon Sep 17 00:00:00 2001
From: Ravishankar N <ravishankar@redhat.com>
Date: Thu, 29 Apr 2021 18:34:57 +0530
Subject: [PATCH 543/543] glusterd: handle custom xlator failure cases
Problem-1:
custom xlator insertion was failing for those xlators in the brick graph
whose dbg_key was NULL in the server_graph_table. Looking at the git log,
the dbg_key was added in commit d1397dbd7d6cdbd2d81d5d36d608b6175d449db4
for inserting debug xlators.
Fix: I think it is fine to define it for all brick xlators below server.
Problem-2:
In the commit-op phase, glusterd_op_set_volume() updates the volinfo
dict with the key-value pairs and then proceeds to create the volfiles.
If any of the steps fail, the volinfo dict retains those key-values,
until glusterd is restarted or `gluster vol reset $VOLNAME` is issued.
Fix:
Make a copy of the volinfo dict and if there are any failures in
proceeding with the set volume logic, restore the dict to its original
state.
Backport of:
> Upstream-patch-link: https://github.com/gluster/glusterfs/pull/2371
> Change-Id: I9010dab33d0139b8e6d603308e331b6d220a4849
> Updates: #2370
> Signed-off-by: Ravishankar N <ravishankar@redhat.com>
Change-Id: I9010dab33d0139b8e6d603308e331b6d220a4849
BUG: 1953901
Signed-off-by: Ravishankar N <ravishankar@redhat.com>
Reviewed-on: https://code.engineering.redhat.com/gerrit/239889
Reviewed-by: Sunil Kumar Heggodu Gopala Acharya <sheggodu@redhat.com>
---
tests/basic/user-xlator.t | 16 ++++++++++++++--
xlators/mgmt/glusterd/src/glusterd-op-sm.c | 16 ++++++++++++++++
xlators/mgmt/glusterd/src/glusterd-volgen.c | 14 +++++++-------
3 files changed, 37 insertions(+), 9 deletions(-)
diff --git a/tests/basic/user-xlator.t b/tests/basic/user-xlator.t
index a711f9f..ed2d831 100755
--- a/tests/basic/user-xlator.t
+++ b/tests/basic/user-xlator.t
@@ -35,8 +35,18 @@ EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" brick_up_status $V0 $H0 $B0/${V0}4
EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" brick_up_status $V0 $H0 $B0/${V0}5
EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" brick_up_status $V0 $H0 $B0/${V0}6
-TEST $CLI volume set $V0 user.xlator.hoge trash
-TEST grep -q 'user/hoge' ${SERVER_VOLFILE}
+# Test that the insertion at all positions between server and posix is successful.
+# It is not guaranteed that the brick process will start/work in all positions though.
+TESTS_EXPECTED_IN_LOOP=34
+declare -a brick_side_xlators=("decompounder" "io-stats" "quota" "index" "barrier"
+ "marker" "selinux" "io-threads" "upcall" "leases"
+ "read-only" "worm" "locks" "access-control"
+ "bitrot-stub" "changelog" "trash")
+for xlator in "${brick_side_xlators[@]}"
+ do
+ TEST_IN_LOOP $CLI volume set $V0 user.xlator.hoge $xlator
+ TEST_IN_LOOP grep -q 'user/hoge' ${SERVER_VOLFILE}
+ done
TEST $CLI volume stop $V0
TEST $CLI volume start $V0
@@ -49,6 +59,8 @@ EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" brick_up_status $V0 $H0 $B0/${V0}6
TEST ! $CLI volume set $V0 user.xlator.hoge unknown
TEST grep -q 'user/hoge' ${SERVER_VOLFILE} # When the CLI fails, the volfile is not modified.
+# User xlator insert failures must not prevent setting other volume options.
+TEST $CLI volume set $V0 storage.reserve 10%
TEST $CLI volume stop $V0
TEST $CLI volume start $V0
diff --git a/xlators/mgmt/glusterd/src/glusterd-op-sm.c b/xlators/mgmt/glusterd/src/glusterd-op-sm.c
index 1e84f5f..893af29 100644
--- a/xlators/mgmt/glusterd/src/glusterd-op-sm.c
+++ b/xlators/mgmt/glusterd/src/glusterd-op-sm.c
@@ -2911,6 +2911,7 @@ glusterd_op_set_volume(dict_t *dict, char **errstr)
uint32_t new_op_version = 0;
gf_boolean_t quorum_action = _gf_false;
glusterd_svc_t *svc = NULL;
+ dict_t *volinfo_dict_orig = NULL;
this = THIS;
GF_ASSERT(this);
@@ -2918,6 +2919,10 @@ glusterd_op_set_volume(dict_t *dict, char **errstr)
priv = this->private;
GF_ASSERT(priv);
+ volinfo_dict_orig = dict_new();
+ if (!volinfo_dict_orig)
+ goto out;
+
ret = dict_get_int32n(dict, "count", SLEN("count"), &dict_count);
if (ret) {
gf_msg(this->name, GF_LOG_ERROR, 0, GD_MSG_DICT_GET_FAILED,
@@ -2949,6 +2954,11 @@ glusterd_op_set_volume(dict_t *dict, char **errstr)
goto out;
}
+ if (dict_copy(volinfo->dict, volinfo_dict_orig) == NULL) {
+ ret = -ENOMEM;
+ goto out;
+ }
+
/* TODO: Remove this once v3.3 compatibility is not required */
check_op_version = dict_get_str_boolean(dict, "check-op-version",
_gf_false);
@@ -3171,6 +3181,12 @@ out:
gf_msg_debug(this->name, 0, "returning %d", ret);
if (quorum_action)
glusterd_do_quorum_action();
+ if (ret < 0 && count > 1) {
+ if (dict_reset(volinfo->dict) == 0)
+ dict_copy(volinfo_dict_orig, volinfo->dict);
+ }
+ if (volinfo_dict_orig)
+ dict_unref(volinfo_dict_orig);
return ret;
}
diff --git a/xlators/mgmt/glusterd/src/glusterd-volgen.c b/xlators/mgmt/glusterd/src/glusterd-volgen.c
index 71aed08..aa85bdb 100644
--- a/xlators/mgmt/glusterd/src/glusterd-volgen.c
+++ b/xlators/mgmt/glusterd/src/glusterd-volgen.c
@@ -2706,24 +2706,24 @@ out:
static volgen_brick_xlator_t server_graph_table[] = {
{brick_graph_add_server, NULL},
{brick_graph_add_decompounder, "decompounder"},
- {brick_graph_add_io_stats, "NULL"},
+ {brick_graph_add_io_stats, "io-stats"},
{brick_graph_add_sdfs, "sdfs"},
{brick_graph_add_namespace, "namespace"},
- {brick_graph_add_cdc, NULL},
+ {brick_graph_add_cdc, "cdc" },
{brick_graph_add_quota, "quota"},
{brick_graph_add_index, "index"},
- {brick_graph_add_barrier, NULL},
+ {brick_graph_add_barrier, "barrier" },
{brick_graph_add_marker, "marker"},
{brick_graph_add_selinux, "selinux"},
{brick_graph_add_fdl, "fdl"},
{brick_graph_add_iot, "io-threads"},
{brick_graph_add_upcall, "upcall"},
{brick_graph_add_leases, "leases"},
- {brick_graph_add_pump, NULL},
- {brick_graph_add_ro, NULL},
- {brick_graph_add_worm, NULL},
+ {brick_graph_add_pump, "pump" },
+ {brick_graph_add_ro, "read-only" },
+ {brick_graph_add_worm, "worm" },
{brick_graph_add_locks, "locks"},
- {brick_graph_add_acl, "acl"},
+ {brick_graph_add_acl, "access-control"},
{brick_graph_add_bitrot_stub, "bitrot-stub"},
{brick_graph_add_changelog, "changelog"},
#if USE_GFDB /* changetimerecorder depends on gfdb */
--
1.8.3.1
|
