From 7f70d38b66ce755f848ff0197814457a28b321df Mon Sep 17 00:00:00 2001 From: Atin Mukherjee Date: Thu, 7 Sep 2017 19:14:23 +0530 Subject: glusterd: disallow replace brick for dist only volumes Allowing replace-brick on dist only volumes will lead to data loss. This patch blocks replace brick commit force to fail if a volume is dist only. Also removing tests/basic/pump.t as its of no use as per the discussion in http://lists.gluster.org/pipermail/gluster-devel/2017-September/053652.html Change-Id: Iabb0c16f865f3fc361b64a19bfcf0c0fbb5c2682 BUG: 1489432 Signed-off-by: Atin Mukherjee Reviewed-on: https://review.gluster.org/18226 Smoke: Gluster Build System Reviewed-by: N Balachandran CentOS-regression: Gluster Build System --- tests/basic/pump.t | 45 ---------------------- ...19-remove-replace-brick-support-from-glusterd.t | 2 +- .../bug-1483058-replace-brick-quorum-validation.t | 2 +- tests/bugs/glusterd/bug-857330/normal.t | 14 +++---- tests/bugs/glusterd/bug-857330/xml.t | 16 ++++---- xlators/mgmt/glusterd/src/glusterd-replace-brick.c | 12 +++++- 6 files changed, 28 insertions(+), 63 deletions(-) delete mode 100644 tests/basic/pump.t diff --git a/tests/basic/pump.t b/tests/basic/pump.t deleted file mode 100644 index ab62f77224f..00000000000 --- a/tests/basic/pump.t +++ /dev/null @@ -1,45 +0,0 @@ -#!/bin/bash - -. $(dirname $0)/../include.rc -. $(dirname $0)/../volume.rc - -cleanup; -START_TIMESTAMP=`date +%s` - -TEST glusterd -TEST pidof glusterd -TEST $CLI volume create $V0 $H0:$B0/${V0}0 -TEST $CLI volume start $V0 -TEST $GFS --volfile-id=/$V0 --volfile-server=$H0 $M0; -cd $M0 -for i in {1..3} -do - for j in {1..10} - do - dd if=/dev/urandom of=file$j bs=128K count=10 2>/dev/null 1>/dev/null - done - mkdir dir$i && cd dir$i -done -cd -EXPECT_WITHIN $UMOUNT_TIMEOUT "Y" force_umount $M0 -TEST $CLI volume replace-brick $V0 $H0:$B0/${V0}0 $H0:$B0/${V0}1 commit force -TEST $CLI volume stop $V0 - -files="" - -cd $B0/${V0}0 -for f in `find . -path ./.glusterfs -prune -o -print`; -do - if [ -d $f ]; then continue; fi - cmp $f $B0/${V0}1/$f - if [ $? -ne 0 ]; then - files="$files $f" - fi -done - -EXPECT "" echo $files - -# Check for non Linux systems that we did not mess with directory offsets -TEST ! log_newer $START_TIMESTAMP "offset reused from another DIR" - -cleanup diff --git a/tests/bugs/glusterd/bug-1094119-remove-replace-brick-support-from-glusterd.t b/tests/bugs/glusterd/bug-1094119-remove-replace-brick-support-from-glusterd.t index 43acfcf7289..30d375a1eb0 100644 --- a/tests/bugs/glusterd/bug-1094119-remove-replace-brick-support-from-glusterd.t +++ b/tests/bugs/glusterd/bug-1094119-remove-replace-brick-support-from-glusterd.t @@ -12,7 +12,7 @@ TEST glusterd TEST pidof glusterd ## Lets create and start volume -TEST $CLI volume create $V0 $H0:$B0/brick1 $H0:$B0/brick2 +TEST $CLI volume create $V0 replica 2 $H0:$B0/brick1 $H0:$B0/brick2 TEST $CLI volume start $V0 ## Now with this patch replace-brick only accept following commad diff --git a/tests/bugs/glusterd/bug-1483058-replace-brick-quorum-validation.t b/tests/bugs/glusterd/bug-1483058-replace-brick-quorum-validation.t index 28b2fbbb9cf..3dbe28a7917 100644 --- a/tests/bugs/glusterd/bug-1483058-replace-brick-quorum-validation.t +++ b/tests/bugs/glusterd/bug-1483058-replace-brick-quorum-validation.t @@ -18,7 +18,7 @@ TEST $CLI_1 peer probe $H3; EXPECT_WITHIN $PROBE_TIMEOUT 2 peer_count # Lets create the volume and set quorum type as a server -TEST $CLI_1 volume create $V0 $H1:$B1/${V0}0 $H2:$B2/${V0}1 $H3:$B3/${V0}2 +TEST $CLI_1 volume create $V0 replica 3 $H1:$B1/${V0}0 $H2:$B2/${V0}1 $H3:$B3/${V0}2 TEST $CLI_1 volume set $V0 cluster.server-quorum-type server # Start the volume diff --git a/tests/bugs/glusterd/bug-857330/normal.t b/tests/bugs/glusterd/bug-857330/normal.t index 70cb89dd462..ad0c8844fae 100755 --- a/tests/bugs/glusterd/bug-857330/normal.t +++ b/tests/bugs/glusterd/bug-857330/normal.t @@ -8,7 +8,7 @@ TEST glusterd TEST pidof glusterd TEST $CLI volume info; -TEST $CLI volume create $V0 $H0:$B0/${V0}1; +TEST $CLI volume create $V0 replica 2 $H0:$B0/${V0}1 $H0:$B0/${V0}2; TEST $CLI volume info $V0; TEST $CLI volume start $V0; @@ -22,7 +22,7 @@ EXPECT_WITHIN $UMOUNT_TIMEOUT "Y" force_umount $M0 ############### ## Rebalance ## ############### -TEST $CLI volume add-brick $V0 $H0:$B0/${V0}2; +TEST $CLI volume add-brick $V0 replica 2 $H0:$B0/${V0}3 $H0:$B0/${V0}4; COMMAND="volume rebalance $V0 start" PATTERN="ID:" @@ -39,16 +39,16 @@ EXPECT_WITHIN $REBALANCE_TIMEOUT "0" get-task-status $PATTERN ################### ## Replace-brick ## ################### -REP_BRICK_PAIR="$H0:$B0/${V0}2 $H0:$B0/${V0}3" +REP_BRICK_PAIR="$H0:$B0/${V0}2 $H0:$B0/${V0}5" TEST $CLI volume replace-brick $V0 $REP_BRICK_PAIR commit force; ################## ## Remove-brick ## ################## -EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" brick_up_status $V0 $H0 $B0/${V0}3 +EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" brick_up_status $V0 $H0 $B0/${V0}5 -COMMAND="volume remove-brick $V0 $H0:$B0/${V0}3 start" +COMMAND="volume remove-brick $V0 $H0:$B0/${V0}1 $H0:$B0/${V0}5 start" PATTERN="ID:" TEST check-and-store-task-id @@ -56,11 +56,11 @@ COMMAND="volume status $V0" PATTERN="ID" EXPECT $TASK_ID get-task-id -COMMAND="volume remove-brick $V0 $H0:$B0/${V0}3 status" +COMMAND="volume remove-brick $V0 $H0:$B0/${V0}1 $H0:$B0/${V0}5 status" PATTERN="completed" EXPECT_WITHIN $REBALANCE_TIMEOUT "0" get-task-status $PATTERN -TEST $CLI volume remove-brick $V0 $H0:$B0/${V0}3 commit +TEST $CLI volume remove-brick $V0 $H0:$B0/${V0}1 $H0:$B0/${V0}5 commit TEST $CLI volume stop $V0; TEST $CLI volume delete $V0; diff --git a/tests/bugs/glusterd/bug-857330/xml.t b/tests/bugs/glusterd/bug-857330/xml.t index 391d189e387..8383d2a0711 100755 --- a/tests/bugs/glusterd/bug-857330/xml.t +++ b/tests/bugs/glusterd/bug-857330/xml.t @@ -9,7 +9,7 @@ TEST glusterd TEST pidof glusterd TEST $CLI volume info; -TEST $CLI volume create $V0 $H0:$B0/${V0}1; +TEST $CLI volume create $V0 replica 2 $H0:$B0/${V0}1 $H0:$B0/${V0}2; TEST $CLI volume info $V0; TEST $CLI volume start $V0; @@ -24,7 +24,7 @@ EXPECT_WITHIN $UMOUNT_TIMEOUT "Y" force_umount $M0 ############### ## Rebalance ## ############### -TEST $CLI volume add-brick $V0 $H0:$B0/${V0}2; +TEST $CLI volume add-brick $V0 replica 2 $H0:$B0/${V0}3 $H0:$B0/${V0}4; COMMAND="volume rebalance $V0 start" PATTERN="task-id" @@ -47,14 +47,14 @@ EXPECT_WITHIN $REBALANCE_TIMEOUT "0" get-task-status $PATTERN ################### ## Replace-brick ## ################### -TEST $CLI volume replace-brick $V0 $H0:$B0/${V0}2 $H0:$B0/${V0}3 commit force +TEST $CLI volume replace-brick $V0 $H0:$B0/${V0}4 $H0:$B0/${V0}5 commit force ################## ## Remove-brick ## ################## -EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" brick_up_status $V0 $H0 $B0/${V0}3 +EXPECT_WITHIN $PROCESS_UP_TIMEOUT "1" brick_up_status $V0 $H0 $B0/${V0}5 -COMMAND="volume remove-brick $V0 $H0:$B0/${V0}3 start" +COMMAND="volume remove-brick $V0 $H0:$B0/${V0}3 $H0:$B0/${V0}5 start" PATTERN="task-id" TEST check-and-store-task-id-xml @@ -62,17 +62,17 @@ COMMAND="volume status $V0" PATTERN="id" EXPECT $TASK_ID get-task-id-xml -COMMAND="volume remove-brick $V0 $H0:$B0/${V0}3 status" +COMMAND="volume remove-brick $V0 $H0:$B0/${V0}3 $H0:$B0/${V0}5 status" PATTERN="task-id" EXPECT $TASK_ID get-task-id-xml -COMMAND="volume remove-brick $V0 $H0:$B0/${V0}3 status" +COMMAND="volume remove-brick $V0 $H0:$B0/${V0}3 $H0:$B0/${V0}5 status" PATTERN="completed" EXPECT_WITHIN $REBALANCE_TIMEOUT "0" get-task-status $PATTERN ## TODO: Add tests for remove-brick stop -COMMAND="volume remove-brick $V0 $H0:$B0/${V0}3 commit" +COMMAND="volume remove-brick $V0 $H0:$B0/${V0}3 $H0:$B0/${V0}5 commit" PATTERN="task-id" EXPECT $TASK_ID get-task-id-xml diff --git a/xlators/mgmt/glusterd/src/glusterd-replace-brick.c b/xlators/mgmt/glusterd/src/glusterd-replace-brick.c index ab38725ffb0..08a6df0235f 100644 --- a/xlators/mgmt/glusterd/src/glusterd-replace-brick.c +++ b/xlators/mgmt/glusterd/src/glusterd-replace-brick.c @@ -211,9 +211,19 @@ glusterd_op_stage_replace_brick (dict_t *dict, char **op_errstr, if (ret) goto out; + if (volinfo->type == GF_CLUSTER_TYPE_NONE) { + gf_msg (this->name, GF_LOG_ERROR, 0, GD_MSG_OP_NOT_PERMITTED, + "replace-brick is not permitted on distribute only " + "volumes"); + gf_asprintf (op_errstr, "replace-brick is not permitted on " + "distribute only volumes. Please use add-brick " + "and remove-brick operations instead."); + ret = -1; + goto out; + } ret = glusterd_validate_quorum (this, gd_op, dict, op_errstr); if (ret) { - gf_msg (this->name, GF_LOG_CRITICAL, 0, + gf_msg (this->name, GF_LOG_ERROR, 0, GD_MSG_SERVER_QUORUM_NOT_MET, "Server quorum not met. Rejecting operation."); goto out; -- cgit