Remove brick validation

Added checks for duplicate bricks in cli arguments, valid bricks
for the volume, valid volume name, and prevent removing of incorrect
 pairs for replica.

Signed-off-by: shishir gowda <shishirng@gluster.com>
Signed-off-by: Vijay Bellur <vijay@dev.gluster.com>

BUG: 1486 ()
URL: http://bugs.gluster.com/cgi-bin/bugzilla3/show_bug.cgi?id=1486

7 files changed, 159 insertions, 7 deletions

diff --git a/cli/src/cli-cmd-parser.c b/cli/src/cli-cmd-parser.c
index 48c4a4fd9..f43349416 100644
--- a/cli/src/cli-cmd-parser.c
+++ b/cli/src/cli-cmd-parser.c
@@ -439,6 +439,10 @@ cli_cmd_volume_remove_brick_parse (const char **words, int wordcount,
         int     count = 0;
         char    key[50];
         int     brick_count = 0, brick_index = 0;
+        int32_t tmp_index = 0;
+        int32_t j = 0;
+        char    *tmp_brick = NULL;
+        char    *tmp_brick1 = NULL;
 
         GF_ASSERT (words);
         GF_ASSERT (options);
@@ -495,6 +499,25 @@ cli_cmd_volume_remove_brick_parse (const char **words, int wordcount,
         if (ret)
                 goto out;
 
+        tmp_index = brick_index;
+        tmp_brick = GF_MALLOC(2048 * sizeof(*tmp_brick), gf_common_mt_char);
+
+        if (!tmp_brick) {
+                gf_log ("",GF_LOG_ERROR,"cli_cmd_volume_remove_brick_parse: "
+                        "Unable to get memory");
+                ret = -1;
+                goto out;
+        }
+ 
+        tmp_brick1 = GF_MALLOC(2048 * sizeof(*tmp_brick1), gf_common_mt_char);
+
+        if (!tmp_brick1) {
+                gf_log ("",GF_LOG_ERROR,"cli_cmd_volume_remove_brick_parse: "
+                        "Unable to get memory");
+                ret = -1;
+                goto out;
+        }
+
         while (brick_index < wordcount) {
                 delimiter = strchr(words[brick_index], ':');
                 if (!delimiter || delimiter == words[brick_index]
@@ -504,7 +527,20 @@ cli_cmd_volume_remove_brick_parse (const char **words, int wordcount,
                         ret = -1;
                         goto out;
                 }
-
+                j = tmp_index;
+                strcpy(tmp_brick, words[brick_index]);
+                while ( j < brick_index) {
+                        strcpy(tmp_brick1, words[j]);
+                        if (!(strcmp (tmp_brick, tmp_brick1))) { 
+                                gf_log("",GF_LOG_ERROR, "Duplicate bricks"
+                                       " found %s", words[brick_index]);
+                                cli_out("Duplicate bricks found %s",
+                                        words[brick_index]);
+                                ret = -1;
+                                goto out;
+                        }
+                        j++;
+                }
                 snprintf (key, 50, "brick%d", ++brick_count);
                 ret = dict_set_str (dict, key, (char *)words[brick_index++]);
 
@@ -526,6 +562,11 @@ out:
                         dict_destroy (dict);
         }
 
+        if (tmp_brick)
+                GF_FREE (tmp_brick);
+        if (tmp_brick1)
+                GF_FREE (tmp_brick1);
+
         return ret;
 }
 
diff --git a/cli/src/cli3_1-cops.c b/cli/src/cli3_1-cops.c
index 6971a242d..e4b41c4db 100644
--- a/cli/src/cli3_1-cops.c
+++ b/cli/src/cli3_1-cops.c
@@ -704,6 +704,8 @@ gf_cli3_1_remove_brick_cbk (struct rpc_req *req, struct iovec *iov,
         gf_log ("cli", GF_LOG_NORMAL, "Received resp to remove brick");
         cli_out ("Remove Brick %s", (rsp.op_ret) ? "unsuccessful":
                                         "successful");
+        if (rsp.op_ret && rsp.op_errstr)
+                cli_out ("%s", rsp.op_errstr);
 
         ret = rsp.op_ret;
 
diff --git a/rpc/xdr/src/cli1-xdr.c b/rpc/xdr/src/cli1-xdr.c
index c0d488e7f..22c00f2f7 100644
--- a/rpc/xdr/src/cli1-xdr.c
+++ b/rpc/xdr/src/cli1-xdr.c
@@ -415,6 +415,8 @@ xdr_gf1_cli_remove_brick_rsp (XDR *xdrs, gf1_cli_remove_brick_rsp *objp)
 		 return FALSE;
 	 if (!xdr_string (xdrs, &objp->volname, ~0))
 		 return FALSE;
+	 if (!xdr_string (xdrs, &objp->op_errstr, ~0))
+		 return FALSE;
 	return TRUE;
 }
 
diff --git a/rpc/xdr/src/cli1-xdr.h b/rpc/xdr/src/cli1-xdr.h
index 64bae26f4..6f2f8d3e1 100644
--- a/rpc/xdr/src/cli1-xdr.h
+++ b/rpc/xdr/src/cli1-xdr.h
@@ -250,6 +250,7 @@ struct gf1_cli_remove_brick_rsp {
 	int op_ret;
 	int op_errno;
 	char *volname;
+	char *op_errstr;
 };
 typedef struct gf1_cli_remove_brick_rsp gf1_cli_remove_brick_rsp;
 
diff --git a/rpc/xdr/src/cli1.x b/rpc/xdr/src/cli1.x
index 0c4cbf833..9912f750b 100644
--- a/rpc/xdr/src/cli1.x
+++ b/rpc/xdr/src/cli1.x
@@ -169,6 +169,7 @@ struct gf1_cli_get_vol_rsp {
         int     op_ret;
         int     op_errno;
         string  volname<>;
+        string  op_errstr<>;
 }  ;
 
  struct gf1_cli_replace_brick_req {
diff --git a/xlators/mgmt/glusterd/src/glusterd-handler.c b/xlators/mgmt/glusterd/src/glusterd-handler.c
index 7ecebd051..fd90161f8 100644
--- a/xlators/mgmt/glusterd/src/glusterd-handler.c
+++ b/xlators/mgmt/glusterd/src/glusterd-handler.c
@@ -1525,6 +1525,17 @@ glusterd_handle_remove_brick (rpcsvc_request_t *req)
         char                            key[256] = {0,};
         char                            *brick_list = NULL;
         int                             i = 1;
+        glusterd_volinfo_t              *volinfo = NULL;
+        glusterd_brickinfo_t            *brickinfo = NULL;
+        int32_t                         pos = 0;
+        int32_t                         sub_volume = 0;
+        int32_t                         sub_volume_start = 0;
+        int32_t                         sub_volume_end = 0;
+        glusterd_brickinfo_t            *tmp = NULL;
+        int32_t                         err_ret = 0;
+        char                            *err_str = NULL;
+        gf1_cli_remove_brick_rsp        rsp = {0,};
+        void                            *cli_rsp = NULL;
 
         GF_ASSERT (req);
 
@@ -1557,6 +1568,39 @@ glusterd_handle_remove_brick (rpcsvc_request_t *req)
                 gf_log ("", GF_LOG_ERROR, "Unable to get count");
                 goto out;
         }
+
+        err_str = GF_MALLOC (2048 * sizeof(*err_str),gf_common_mt_char);
+
+        if (!err_str) {
+                gf_log ("",GF_LOG_ERROR,"glusterd_handle_remove_brick: "
+                        "Unable to get memory");
+                ret = -1;
+                goto out;
+        }
+
+        ret = glusterd_volinfo_find (cli_req.volname, &volinfo);
+        if (ret) {
+                 snprintf (err_str, 2048, "volname %s not found", 
+                          cli_req.volname);
+                 gf_log ("", GF_LOG_ERROR, "%s", err_str);
+                 err_ret = 1;
+                 goto out;
+        }
+
+        if ((volinfo->type == GF_CLUSTER_TYPE_REPLICATE) &&
+            !(volinfo->brick_count <= volinfo->sub_count)) {
+                if (volinfo->sub_count && (count % volinfo->sub_count != 0)) {
+                        snprintf (err_str, 2048, "Remove brick incorrect"
+                                  " brick count of %d for replica %d",
+                                   count, volinfo->sub_count);
+                        gf_log ("", GF_LOG_ERROR, "%s", err_str);
+                        err_ret = 1;
+                        ret = -1;
+                        goto out;
+                } 
+
+        }
+
         brick_list = GF_MALLOC (120000 * sizeof(brick_list),gf_common_mt_char);
 
         if (!brick_list) {
@@ -1565,7 +1609,8 @@ glusterd_handle_remove_brick (rpcsvc_request_t *req)
                 ret = -1;
                 goto out;
         }
-        strcpy(brick_list, " ");
+
+        strcpy (brick_list, " ");
         while ( i <= count) {
                 snprintf (key, 256, "brick%d", i);
                 ret = dict_get_str (dict, key, &brick);
@@ -1573,24 +1618,83 @@ glusterd_handle_remove_brick (rpcsvc_request_t *req)
                         gf_log ("", GF_LOG_ERROR, "Unable to get %s", key);
                         goto out;
                 }
-                gf_log ("", GF_LOG_DEBUG, "Remove brick count %i brick: %s",
+                gf_log ("", GF_LOG_DEBUG, "Remove brick count %d brick: %s",
                         i, brick);
 
-                strcat (brick_list, brick);
-                strcat (brick_list, " ");
+                ret = glusterd_brickinfo_get(brick, volinfo, &brickinfo); 
+                if (ret) {
+                        snprintf(err_str, 2048," Incorrect brick %s for volname"
+                                " %s", brick, cli_req.volname); 
+                        gf_log ("", GF_LOG_ERROR, "%s", err_str);
+                        err_ret = 1;
+                        goto out;
+                }
+                strcat(brick_list, brick);
+                strcat(brick_list, " ");
+
                 i++;
+                if ((volinfo->type != GF_CLUSTER_TYPE_REPLICATE) ||
+                    (volinfo->brick_count <= volinfo->sub_count))
+                        continue;
+ 
+                pos = 0;
+                list_for_each_entry (tmp, &volinfo->bricks, brick_list) {
+
+                        if ((!strcmp (tmp->hostname,brickinfo->hostname)) &&
+                            !strcmp (tmp->path, brickinfo->path)) {
+                                gf_log ("", GF_LOG_NORMAL, "Found brick");
+                                if (!sub_volume && volinfo->sub_count) {
+                                        sub_volume = (pos / volinfo->
+                                                      sub_count) + 1;
+                                        sub_volume_start = volinfo->sub_count *
+                                                           (sub_volume - 1);
+                                        sub_volume_end = (volinfo->sub_count *
+                                                          sub_volume) -1 ;
+                                } else {
+                                        if (pos < sub_volume_start ||
+                                            pos >sub_volume_end) {
+                                                ret = -1;
+                                                snprintf(err_str, 2048,"Bricks"
+                                                         " not from same subvol"
+                                                         " for replica");
+                                                gf_log ("",GF_LOG_ERROR,
+                                                        "%s", err_str);
+                                                err_ret = 1;         
+                                                goto out;
+                                        }
+                                }
+                                break;
+                        } 
+                        pos++;
+                }
         }
-//        strcat(brick_list,"\n");
         gf_cmd_log ("Volume remove-brick","volname:%s count:%d bricks:%s", 
                     cli_req.volname, count, brick_list);
 
         ret = glusterd_remove_brick (req, dict);
 
 out:
-        gf_cmd_log ("Volume remove-brick","on %s %s",cli_req.volname,
+        gf_cmd_log ("Volume remove-brick","on volname:%s %s",cli_req.volname,
                     (ret) ? "FAILED" : "SUCCESS");
+        if (err_ret) {
+                rsp.op_ret = -1;
+                rsp.op_errno = 0;
+                rsp.volname = "";
+                rsp.op_errstr = err_str;
+                cli_rsp = &rsp;
+                glusterd_submit_reply(req, cli_rsp, NULL, 0, NULL,
+                                      gf_xdr_serialize_cli_remove_brick_rsp);
+                if (!glusterd_opinfo_unlock())
+                        gf_log ("glusterd", GF_LOG_ERROR, "Unlock on "
+                               "opinfo failed");
+
+                ret = 0; //sent error to cli, prevent second reply
+                
+        }
         if (brick_list)
                 GF_FREE (brick_list);
+        if (err_str)
+                GF_FREE (err_str);
         return ret;
 }
 
diff --git a/xlators/mgmt/glusterd/src/glusterd-op-sm.c b/xlators/mgmt/glusterd/src/glusterd-op-sm.c
index ea3e59b68..59dae198c 100644
--- a/xlators/mgmt/glusterd/src/glusterd-op-sm.c
+++ b/xlators/mgmt/glusterd/src/glusterd-op-sm.c
@@ -2889,6 +2889,7 @@ glusterd_op_send_cli_response (int32_t op, int32_t op_ret,
                                 rsp.op_ret = op_ret;
                                 rsp.op_errno = op_errno;
                                 rsp.volname = "";
+                                rsp.op_errstr = "";
                                 cli_rsp = &rsp;
                                 sfunc = gf_xdr_serialize_cli_remove_brick_rsp;
                                 break;