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 <ravishankar@redhat.com>
Reviewed-on: http://review.gluster.org/5566
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Amar Tumballi <amarts@redhat.com>
Reviewed-by: Anand Avati <avati@redhat.com>

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 "