glusterd: fix for remove-brick on just created/stopped volumes

in a fix to avoid race between stopping the brick and deleting
it in remove-brick (15396f490d23c665d51a64a049679cb40472ab05)
we moved delete of the brick inside stop volume, which was
totally inside the 'if (volinfo->status == STARTED)' section.
thus it made remove-brick of stopped/created volume as a failure.

Signed-off-by: Amar Tumballi <amarts@redhat.com>
BUG: 867252
Change-Id: Ie251e59a0b7ddb6965d8d48fb2a3bdb3bd11653d
Reviewed-on: http://review.gluster.org/4127
Reviewed-by: Jeff Darcy <jdarcy@redhat.com>
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Tested-by: Jeff Darcy <jdarcy@redhat.com>

3 files changed, 57 insertions, 16 deletions

diff --git a/tests/bugs/bug-867252.t b/tests/bugs/bug-867252.t
new file mode 100644
index 000000000..8309ed9b9
--- /dev/null
+++ b/tests/bugs/bug-867252.t
@@ -0,0 +1,41 @@
+#!/bin/bash
+
+. $(dirname $0)/../include.rc
+
+cleanup;
+
+TEST glusterd
+TEST pidof glusterd
+TEST $CLI volume info;
+
+TEST $CLI volume create $V0 $H0:$B0/${V0}1;
+
+
+function volinfo_field()
+{
+    local vol=$1;
+    local field=$2;
+
+    $CLI volume info $vol | grep "^$field: " | sed 's/.*: //';
+}
+
+
+function brick_count()
+{
+    local vol=$1;
+
+    $CLI volume info $vol | egrep "^Brick[0-9]+: " | wc -l;
+}
+
+
+EXPECT "$V0" volinfo_field $V0 'Volume Name';
+EXPECT 'Created' volinfo_field $V0 'Status';
+EXPECT '1' brick_count $V0
+
+TEST $CLI volume add-brick $V0 $H0:$B0/${V0}2;
+EXPECT '2' brick_count $V0
+
+TEST $CLI volume remove-brick $V0 $H0:$B0/${V0}2;
+EXPECT '1' brick_count $V0
+
+cleanup;
diff --git a/xlators/mgmt/glusterd/src/glusterd-brick-ops.c b/xlators/mgmt/glusterd/src/glusterd-brick-ops.c
index 5e93c061d..ade30fe95 100644
--- a/xlators/mgmt/glusterd/src/glusterd-brick-ops.c
+++ b/xlators/mgmt/glusterd/src/glusterd-brick-ops.c
@@ -981,14 +981,11 @@ glusterd_op_perform_remove_brick (glusterd_volinfo_t  *volinfo, char *brick,
         }
 
         if (force) {
-                if (GLUSTERD_STATUS_STARTED == volinfo->status) {
-                        ret = glusterd_brick_stop (volinfo, brickinfo,
-                                                   _gf_true);
-                        if (ret) {
-                                gf_log (THIS->name, GF_LOG_ERROR, "Unable to stop "
-                                        "glusterfs, ret: %d", ret);
-                                goto out;
-                        }
+                ret = glusterd_brick_stop (volinfo, brickinfo,
+                                           _gf_true);
+                if (ret) {
+                        gf_log (THIS->name, GF_LOG_ERROR, "Unable to stop "
+                                "glusterfs, ret: %d", ret);
                 }
                 goto out;
         }
diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c
index d9f589b20..e55fc3ad7 100644
--- a/xlators/mgmt/glusterd/src/glusterd-utils.c
+++ b/xlators/mgmt/glusterd/src/glusterd-utils.c
@@ -1400,20 +1400,23 @@ glusterd_volume_stop_glusterfs (glusterd_volinfo_t  *volinfo,
         if (del_brick)
                 list_del_init (&brickinfo->brick_list);
 
-        (void) glusterd_brick_disconnect (brickinfo);
+        if (GLUSTERD_STATUS_STARTED == volinfo->status) {
+                (void) glusterd_brick_disconnect (brickinfo);
 
-        GLUSTERD_GET_VOLUME_DIR (path, volinfo, priv);
-        GLUSTERD_GET_BRICK_PIDFILE (pidfile, path, brickinfo->hostname,
-                                    brickinfo->path);
+                GLUSTERD_GET_VOLUME_DIR (path, volinfo, priv);
+                GLUSTERD_GET_BRICK_PIDFILE (pidfile, path, brickinfo->hostname,
+                                            brickinfo->path);
 
-        ret = glusterd_service_stop ("brick", pidfile, SIGTERM, _gf_false);
-        if (ret == 0) {
-                glusterd_set_brick_status (brickinfo, GF_BRICK_STOPPED);
-                (void) glusterd_brick_unlink_socket_file (volinfo, brickinfo);
+                ret = glusterd_service_stop ("brick", pidfile, SIGTERM, _gf_false);
+                if (ret == 0) {
+                        glusterd_set_brick_status (brickinfo, GF_BRICK_STOPPED);
+                        (void) glusterd_brick_unlink_socket_file (volinfo, brickinfo);
+                }
         }
 
         if (del_brick)
                 glusterd_delete_brick (volinfo, brickinfo);
+
         return ret;
 }