From baebaab45980e63f85622e70363fe898cdc572ad Mon Sep 17 00:00:00 2001 From: shishir gowda Date: Mon, 30 Aug 2010 01:36:33 +0000 Subject: Volume Add-brick validation for exports Added checks for export already in use, duplicate exports in command, and check whether exports are valid. Also, cleaned up error handling in glusterd_handle_add_bricks Signed-off-by: shishir gowda Signed-off-by: Vijay Bellur BUG: 1457 (volume add brick validation for export) URL: http://bugs.gluster.com/cgi-bin/bugzilla3/show_bug.cgi?id=1457 --- cli/src/cli-cmd-parser.c | 16 +++++ xlators/mgmt/glusterd/src/glusterd-handler.c | 102 +++++++++++++-------------- xlators/mgmt/glusterd/src/glusterd-op-sm.c | 17 ++++- 3 files changed, 83 insertions(+), 52 deletions(-) diff --git a/cli/src/cli-cmd-parser.c b/cli/src/cli-cmd-parser.c index 4b8520c676b..79f56e2d4b6 100644 --- a/cli/src/cli-cmd-parser.c +++ b/cli/src/cli-cmd-parser.c @@ -306,6 +306,9 @@ cli_cmd_volume_add_brick_parse (const char **words, int wordcount, int brick_count = 0, brick_index = 0; int brick_list_size = 1; char brick_list[120000] = {0,}; + int j = 0; + char *tmp_list = NULL; + char *tmpptr = NULL; GF_ASSERT (words); GF_ASSERT (options); @@ -374,6 +377,19 @@ cli_cmd_volume_add_brick_parse (const char **words, int wordcount, ret = -1; goto out; } + tmp_list = strdup(brick_list+1); + j = 0; + while(( brick_count != 0) && (j < brick_count)) { + strtok_r (tmp_list, " ", &tmpptr); + if (!(strcmp (tmp_list, words[brick_index]))) { + ret = -1; + cli_out ("Found duplicate" + " exports %s",words[brick_index]); + goto out; + } + tmp_list = tmpptr; + j++; + } strcat (brick_list, words[brick_index]); strcat (brick_list, " "); diff --git a/xlators/mgmt/glusterd/src/glusterd-handler.c b/xlators/mgmt/glusterd/src/glusterd-handler.c index 0306fdd18ac..42b8e126736 100644 --- a/xlators/mgmt/glusterd/src/glusterd-handler.c +++ b/xlators/mgmt/glusterd/src/glusterd-handler.c @@ -1190,6 +1190,16 @@ glusterd_handle_add_brick (rpcsvc_request_t *req) char err_str[1048]; gf1_cli_add_brick_rsp rsp = {0,}; glusterd_volinfo_t *volinfo = NULL; + glusterd_brickinfo_t *tmpbrkinfo = NULL; + int32_t err_ret = 0; + glusterd_volinfo_t *tmpvolinfo = NULL; + glusterd_conf_t *priv = NULL; + xlator_t *this = NULL; + + this = THIS; + priv = this->private; + + GF_ASSERT(this); GF_ASSERT (req); @@ -1224,22 +1234,10 @@ glusterd_handle_add_brick (rpcsvc_request_t *req) } if (!(ret = glusterd_check_volume_exists (volname))) { - gf_log ("", GF_LOG_ERROR, "Volname %s does not exist", - volname); - rsp.op_ret = -1; - rsp.op_errno = 0; - rsp.volname = ""; snprintf(err_str, 1048, "Volname %s does not exist", volname); - rsp.op_errstr = err_str; - cli_rsp = &rsp; - glusterd_submit_reply(req, cli_rsp, NULL, 0, NULL, - gf_xdr_serialize_cli_add_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 + gf_log ("glusterd", GF_LOG_ERROR, "%s", err_str); + err_ret = -1; goto out; } @@ -1255,22 +1253,12 @@ glusterd_handle_add_brick (rpcsvc_request_t *req) if (!brick_count || !volinfo->sub_count) goto brick_val; if ((brick_count % volinfo->sub_count) != 0) { - rsp.op_ret = -1; - rsp.op_errno = -1; - rsp.volname = ""; snprintf(err_str, 2048, "Incorrect number of bricks" " supplied %d for type %s with count %d", brick_count, (volinfo->type == 1)? "STRIPE": "REPLICATE", volinfo->sub_count); - rsp.op_errstr = err_str; - cli_rsp = &rsp; - glusterd_submit_reply(req, cli_rsp, NULL, 0, NULL, - gf_xdr_serialize_cli_add_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 + gf_log("glusterd", GF_LOG_ERROR, "%s", err_str); + err_ret = 1; goto out; } } else { @@ -1297,49 +1285,61 @@ brick_val: if (ret) goto out; if(!(ret = glusterd_is_local_addr(brickinfo->hostname))) - continue; //localhost, continue without validation + goto brick_validation; //localhost, continue without validation ret = glusterd_friend_find_by_hostname(brickinfo->hostname, &peerinfo); if (ret) { - rsp.op_ret = -1; - rsp.op_errno = 0; - rsp.volname = ""; snprintf(err_str, 1048, "Host %s not a friend", brickinfo->hostname); - rsp.op_errstr = err_str; - cli_rsp = &rsp; - glusterd_submit_reply(req, cli_rsp, NULL, 0, NULL, - gf_xdr_serialize_cli_add_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 + gf_log ("glusterd", GF_LOG_ERROR, "%s", err_str); + err_ret = 1; goto out; } if ((!peerinfo->connected) && (peerinfo->state.state != GD_FRIEND_STATE_BEFRIENDED)) { - rsp.op_ret = -1; - rsp.op_errno = 0; - rsp.volname = ""; snprintf(err_str, 1048, "Host %s not connected", brickinfo->hostname); - rsp.op_errstr = err_str; - cli_rsp = &rsp; - glusterd_submit_reply(req, cli_rsp, NULL, 0, NULL, - gf_xdr_serialize_cli_add_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 + gf_log ("glusterd", GF_LOG_ERROR, "%s", err_str); + err_ret = 1; goto out; } +brick_validation: + list_for_each_entry (tmpvolinfo, &priv->volumes, vol_list) { + + list_for_each_entry (tmpbrkinfo, &tmpvolinfo->bricks, + brick_list) { + + if ((!strcmp(brickinfo->hostname, tmpbrkinfo-> + hostname) && !strcmp(brickinfo->path, + tmpbrkinfo->path))) { + snprintf(err_str, 1048, "Brick %s already" + "in use", brick); + gf_log ("glusterd", GF_LOG_ERROR, "%s", + err_str); + err_ret = 1; + goto out; + } + } + } } ret = glusterd_add_brick (req, dict); out: + 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_add_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 + } return ret; } diff --git a/xlators/mgmt/glusterd/src/glusterd-op-sm.c b/xlators/mgmt/glusterd/src/glusterd-op-sm.c index 20a57660d0f..24cd1ab3310 100644 --- a/xlators/mgmt/glusterd/src/glusterd-op-sm.c +++ b/xlators/mgmt/glusterd/src/glusterd-op-sm.c @@ -535,6 +535,8 @@ glusterd_op_stage_add_brick (gd1_mgmt_stage_op_req *req) char *brick = NULL; glusterd_brickinfo_t *brickinfo = NULL; glusterd_volinfo_t *volinfo = NULL; + struct stat st_buf = {0,}; + char cmd_str[1024]; GF_ASSERT (req); @@ -588,7 +590,20 @@ glusterd_op_stage_add_brick (gd1_mgmt_stage_op_req *req) ret = -1; goto out; } else { - ret = 0; + ret = glusterd_brickinfo_from_brick(brick, &brickinfo); + if (ret) { + gf_log ("", GF_LOG_ERROR, "Add-brick: Unable" + " to get brickinfo"); + goto out; + } + } + snprintf (cmd_str, 1024, "%s", brickinfo->path); + ret = stat (cmd_str, &st_buf); + if (ret == -1) { + gf_log ("glusterd", GF_LOG_ERROR, "Volname %s, brick" + ":%s path %s not present", volname, + brick, brickinfo->path); + goto out; } brick = strtok_r (NULL, " \n", &saveptr); -- cgit