From 51c6fa7a354826744de98a1c5c747955ae65850c Mon Sep 17 00:00:00 2001 From: Ravishankar N Date: Fri, 19 Jul 2013 14:23:32 +0000 Subject: glusterd: add check in remove-brick start variant The 'start' variant of the remove-brick command only applies at the dht level wherein we can remove all the bricks of a sub-volume (and remove multiple such sub-volumes) but not select bricks of it. This patch disallows removing individual replica bricks of multiple sub-volumes (i.e. reducing the replcia count of the volume) using remove-brick 'start'. The preferred method for such an operation is to use commit force. This patch also reverts the check to prevent removal of bricks from a replicate volume (commit 0d415f7) BUG: 961669 Change-Id: I447ad27f73a0963b5e09fb317bf7267a7a5a6147 Signed-off-by: Ravishankar N Reviewed-on: http://review.gluster.org/5566 Tested-by: Gluster Build System Reviewed-by: Amar Tumballi Reviewed-by: Anand Avati --- tests/bugs/bug-961669.t | 27 +++++++++++++++----------- xlators/mgmt/glusterd/src/glusterd-brick-ops.c | 22 ++++++++++----------- 2 files changed, 27 insertions(+), 22 deletions(-) diff --git a/tests/bugs/bug-961669.t b/tests/bugs/bug-961669.t index 1b832622..751a63df 100644 --- a/tests/bugs/bug-961669.t +++ b/tests/bugs/bug-961669.t @@ -1,6 +1,6 @@ #!/bin/bash -#Test case: Create a replicate volume; mount and write to it; kill one brick; try to remove the other. +#Test case: Fail remove-brick 'start' variant when reducing the replica count of a volume. . $(dirname $0)/../include.rc . $(dirname $0)/../volume.rc @@ -12,8 +12,8 @@ TEST glusterd TEST pidof glusterd TEST $CLI volume info -#Create a 1x2 replicate volume -TEST $CLI volume create $V0 replica 2 $H0:$B0/${V0}{0,1}; +#Create a 3x3 dist-rep volume +TEST $CLI volume create $V0 replica 3 $H0:$B0/${V0}{0,1,2,3,4,5,6,7,8}; TEST $CLI volume start $V0 # Mount FUSE and create file/directory @@ -22,18 +22,23 @@ TEST touch $M0/zerobytefile.txt TEST mkdir $M0/test_dir TEST dd if=/dev/zero of=$M0/file bs=1024 count=1024 -#Kill one of the bricks. This step can be skipped without affecting the outcome of the test. -kill -9 `cat /var/lib/glusterd/vols/$V0/run/$H0-d-backends-${V0}1.pid`; +function remove_brick_start { + $CLI volume remove-brick $V0 replica 2 $H0:$B0/${V0}{1,4,7} start 2>&1|grep -oE 'success|failed' +} -function remove_brick_status { - $CLI volume remove-brick $V0 replica 1 $H0:$B0/${V0}1 start 2>&1 +function remove_brick { + $CLI volume remove-brick $V0 replica 2 $H0:$B0/${V0}{1,4,7} 2>&1|grep -oE 'success|failed' } -#Remove brick -EXPECT "volume remove-brick start: failed: Removing brick from a replicate volume is not allowed" remove_brick_status; +#remove-brick start variant +#Actual message displayed at cli is: +#"volume remove-brick start: failed: Rebalancing not needed when reducing replica count. Try without the 'start' option" +EXPECT "failed" remove_brick_start; -#Check the volume type -EXPECT "Replicate" echo `$CLI volume info |grep Type |awk '{print $2}'` +#remove-brick commit-force +#Actual message displayed at cli is: +#"volume remove-brick commit force: success" +EXPECT "success" remove_brick TEST umount $M0 TEST $CLI volume stop $V0 diff --git a/xlators/mgmt/glusterd/src/glusterd-brick-ops.c b/xlators/mgmt/glusterd/src/glusterd-brick-ops.c index efb0941e..7c8e55be 100644 --- a/xlators/mgmt/glusterd/src/glusterd-brick-ops.c +++ b/xlators/mgmt/glusterd/src/glusterd-brick-ops.c @@ -744,17 +744,6 @@ __glusterd_handle_remove_brick (rpcsvc_request_t *req) goto out; } - /*Do not allow remove-brick if the volume is a replicate volume*/ - if ((volinfo->type == GF_CLUSTER_TYPE_REPLICATE) && - (volinfo->brick_count == volinfo->replica_count)) { - snprintf (err_str, sizeof(err_str), - "Removing brick from a replicate volume " - "is not allowed"); - gf_log (this->name, GF_LOG_ERROR, "%s", err_str); - ret = -1; - goto out; - } - if (!replica_count && (volinfo->type == GF_CLUSTER_TYPE_STRIPE_REPLICATE) && (volinfo->brick_count == volinfo->dist_leaf_count)) { @@ -1371,6 +1360,7 @@ glusterd_op_stage_remove_brick (dict_t *dict, char **op_errstr) glusterd_volinfo_t *volinfo = NULL; char *errstr = NULL; int32_t brick_count = 0; + int32_t replica_cnt = 0; char msg[2048] = {0,}; int32_t flag = 0; gf1_op_commands cmd = GF_OP_CMD_NONE; @@ -1426,6 +1416,16 @@ glusterd_op_stage_remove_brick (dict_t *dict, char **op_errstr) case GF_OP_CMD_START: { + if ((volinfo->type == GF_CLUSTER_TYPE_REPLICATE) && + !dict_get_int32 (dict, "replica-count", &replica_cnt)) { + snprintf (msg, sizeof(msg), "Rebalancing not needed " + "when reducing replica count. Try without " + "the 'start' option"); + errstr = gf_strdup (msg); + gf_log (this->name, GF_LOG_ERROR, "%s", errstr); + goto out; + } + if (GLUSTERD_STATUS_STARTED != volinfo->status) { snprintf (msg, sizeof (msg), "Volume %s needs to be " "started before remove-brick (you can use " -- cgit