From 4c5e364c36baa92374eb0eac60dafb8da3786286 Mon Sep 17 00:00:00 2001 From: shishir gowda Date: Wed, 1 Sep 2010 23:44:54 +0000 Subject: 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 Signed-off-by: Vijay Bellur BUG: 1486 () URL: http://bugs.gluster.com/cgi-bin/bugzilla3/show_bug.cgi?id=1486 --- cli/src/cli-cmd-parser.c | 43 +++++++++- cli/src/cli3_1-cops.c | 2 + rpc/xdr/src/cli1-xdr.c | 2 + rpc/xdr/src/cli1-xdr.h | 1 + rpc/xdr/src/cli1.x | 1 + xlators/mgmt/glusterd/src/glusterd-handler.c | 116 +++++++++++++++++++++++++-- xlators/mgmt/glusterd/src/glusterd-op-sm.c | 1 + 7 files changed, 159 insertions(+), 7 deletions(-) diff --git a/cli/src/cli-cmd-parser.c b/cli/src/cli-cmd-parser.c index 48c4a4fd9ef..f43349416be 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 6971a242d91..e4b41c4db3a 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 c0d488e7f9d..22c00f2f797 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 64bae26f4ef..6f2f8d3e116 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 0c4cbf83359..9912f750b3c 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 7ecebd051c0..fd90161f84c 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 ea3e59b68fb..59dae198c6a 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; -- cgit