From d42f248c58b2ca73fb56a3e091c8e967e2435546 Mon Sep 17 00:00:00 2001 From: Pranith Kumar K Date: Wed, 22 Sep 2010 04:21:02 +0000 Subject: mgmt/glusterd: replace-brick validations Signed-off-by: Pranith Kumar K Signed-off-by: Vijay Bellur BUG: 1657 (validations for replace-brick while stage op) URL: http://bugs.gluster.com/cgi-bin/bugzilla3/show_bug.cgi?id=1657 --- cli/src/cli-cmd-parser.c | 59 +++++++------- cli/src/cli-cmd-volume.c | 2 +- cli/src/cli3_1-cops.c | 3 + rpc/xdr/src/cli1-xdr.c | 4 +- rpc/xdr/src/cli1-xdr.h | 1 + rpc/xdr/src/cli1.x | 1 + xlators/mgmt/glusterd/src/glusterd-handler.c | 56 ++++---------- xlators/mgmt/glusterd/src/glusterd-op-sm.c | 112 ++++++++++++++++++++++++--- xlators/mgmt/glusterd/src/glusterd-utils.c | 34 ++++++++ xlators/mgmt/glusterd/src/glusterd-utils.h | 2 + 10 files changed, 193 insertions(+), 81 deletions(-) diff --git a/cli/src/cli-cmd-parser.c b/cli/src/cli-cmd-parser.c index 2b7cdd9f612..9de13e56ce7 100644 --- a/cli/src/cli-cmd-parser.c +++ b/cli/src/cli-cmd-parser.c @@ -729,42 +729,45 @@ cli_cmd_volume_replace_brick_parse (const char **words, int wordcount, } delimiter = strchr ((char *)words[3], ':'); - if (delimiter && delimiter != words[3] - && *(delimiter+1) == '/') { + if (!delimiter || delimiter == words[3] + || *(delimiter+1) != '/') { + cli_out ("wrong brick type: %s, use " + ":", words[3]); + ret = -1; + goto out; + } else { cli_path_strip_trailing_slashes (delimiter + 1); - ret = dict_set_str (dict, "src-brick", (char *)words[3]); - - if (ret) - goto out; + } + ret = dict_set_str (dict, "src-brick", (char *)words[3]); - if (wordcount < 5) { - ret = -1; - goto out; - } + if (ret) + goto out; - delimiter = strchr ((char *)words[4], ':'); - if (!delimiter || delimiter == words[4] - || *(delimiter+1) != '/') { - cli_out ("wrong brick type: %s, use " - ":", words[4]); - ret = -1; - goto out; - } else { - cli_path_strip_trailing_slashes (delimiter + 1); - } + if (wordcount < 5) { + ret = -1; + goto out; + } + delimiter = strchr ((char *)words[4], ':'); + if (!delimiter || delimiter == words[4] + || *(delimiter+1) != '/') { + cli_out ("wrong brick type: %s, use " + ":", words[4]); + ret = -1; + goto out; + } else { + cli_path_strip_trailing_slashes (delimiter + 1); + } - ret = dict_set_str (dict, "dst-brick", (char *)words[4]); - if (ret) - goto out; + ret = dict_set_str (dict, "dst-brick", (char *)words[4]); - op_index = 5; - } else { - op_index = 3; - } + if (ret) + goto out; - if (wordcount < (op_index + 1)) { + op_index = 5; + if ((wordcount < (op_index + 1)) || + (wordcount > (op_index + 1))) { ret = -1; goto out; } diff --git a/cli/src/cli-cmd-volume.c b/cli/src/cli-cmd-volume.c index 28104c18704..c8345177823 100644 --- a/cli/src/cli-cmd-volume.c +++ b/cli/src/cli-cmd-volume.c @@ -611,7 +611,7 @@ void cli_cmd_volume_replace_brick_usage () { cli_out("Usage: volume replace-brick " - "( ) start|pause|abort|status"); + " start|pause|abort|status"); } diff --git a/cli/src/cli3_1-cops.c b/cli/src/cli3_1-cops.c index 53b80c55667..78683ccab0e 100644 --- a/cli/src/cli3_1-cops.c +++ b/cli/src/cli3_1-cops.c @@ -937,6 +937,9 @@ gf_cli3_1_replace_brick_cbk (struct rpc_req *req, struct iovec *iov, break; } + if (rsp.op_ret && (strcmp (rsp.op_errstr, ""))) { + rb_operation_str = rsp.op_errstr; + } gf_log ("cli", GF_LOG_NORMAL, "Received resp to replace brick"); cli_out ("%s", diff --git a/rpc/xdr/src/cli1-xdr.c b/rpc/xdr/src/cli1-xdr.c index 93f7e76818a..ca2bc09f3a4 100644 --- a/rpc/xdr/src/cli1-xdr.c +++ b/rpc/xdr/src/cli1-xdr.c @@ -94,7 +94,7 @@ bool_t xdr_gf1_cli_probe_rsp (XDR *xdrs, gf1_cli_probe_rsp *objp) { - register int32_t *buf; + register int32_t *buf; if (xdrs->x_op == XDR_ENCODE) { buf = XDR_INLINE (xdrs, 3 * BYTES_PER_XDR_UNIT); @@ -452,6 +452,8 @@ xdr_gf1_cli_replace_brick_rsp (XDR *xdrs, gf1_cli_replace_brick_rsp *objp) return FALSE; if (!xdr_int (xdrs, &objp->op_errno)) return FALSE; + if (!xdr_string (xdrs, &objp->op_errstr, ~0)) + return FALSE; if (!xdr_string (xdrs, &objp->volname, ~0)) return FALSE; if (!xdr_string (xdrs, &objp->status, ~0)) diff --git a/rpc/xdr/src/cli1-xdr.h b/rpc/xdr/src/cli1-xdr.h index c6d8e8bf523..0225acefe2f 100644 --- a/rpc/xdr/src/cli1-xdr.h +++ b/rpc/xdr/src/cli1-xdr.h @@ -275,6 +275,7 @@ typedef struct gf1_cli_replace_brick_req gf1_cli_replace_brick_req; struct gf1_cli_replace_brick_rsp { int op_ret; int op_errno; + char *op_errstr; char *volname; char *status; }; diff --git a/rpc/xdr/src/cli1.x b/rpc/xdr/src/cli1.x index 3f37f6d75cf..ba4288be535 100644 --- a/rpc/xdr/src/cli1.x +++ b/rpc/xdr/src/cli1.x @@ -188,6 +188,7 @@ struct gf1_cli_get_vol_rsp { struct gf1_cli_replace_brick_rsp { int op_ret; int op_errno; + string op_errstr<>; string volname<>; string status<>; } ; diff --git a/xlators/mgmt/glusterd/src/glusterd-handler.c b/xlators/mgmt/glusterd/src/glusterd-handler.c index 0c33b6590a3..4d9a662b36a 100644 --- a/xlators/mgmt/glusterd/src/glusterd-handler.c +++ b/xlators/mgmt/glusterd/src/glusterd-handler.c @@ -1126,8 +1126,6 @@ glusterd_handle_create_volume (rpcsvc_request_t *req) gf1_cli_create_vol_rsp rsp = {0,}; glusterd_conf_t *priv = NULL; int err_ret = 0; - glusterd_brickinfo_t *tmpbrkinfo = NULL; - glusterd_volinfo_t *volinfo = NULL; xlator_t *this = NULL; char *free_ptr = NULL; char *trans_type = NULL; @@ -1137,10 +1135,10 @@ glusterd_handle_create_volume (rpcsvc_request_t *req) GF_ASSERT (req); this = THIS; - priv = this->private; - GF_ASSERT(this); + priv = this->private; + if (!gf_xdr_to_cli_create_vol_req (req->msg[0], &cli_req)) { //failed to decode msg; req->rpc_err = GARBAGE_ARGS; @@ -1250,22 +1248,12 @@ glusterd_handle_create_volume (rpcsvc_request_t *req) goto out; } brick_validation: - list_for_each_entry (volinfo, &priv->volumes, vol_list) { - - list_for_each_entry (tmpbrkinfo, &volinfo->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; - } - } + err_ret = glusterd_is_exisiting_brick (brickinfo->hostname, + brickinfo->path); + if (err_ret) { + snprintf(err_str, 1048, "Brick: %s already in use", + brick); + goto out; } } ret = glusterd_create_volume (req, dict); @@ -1406,18 +1394,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; char *free_ptr = NULL; this = THIS; - priv = this->private; - GF_ASSERT(this); + priv = this->private; + GF_ASSERT (req); if (!gf_xdr_to_cli_add_brick_req (req->msg[0], &cli_req)) { @@ -1544,22 +1530,12 @@ brick_val: 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; - } - } + err_ret = glusterd_is_exisiting_brick (brickinfo->hostname, + brickinfo->path); + if (err_ret) { + snprintf(err_str, 1048, "Brick: %s already in use", + brick); + goto out; } } diff --git a/xlators/mgmt/glusterd/src/glusterd-op-sm.c b/xlators/mgmt/glusterd/src/glusterd-op-sm.c index 4506b3a6885..b8d2778e39a 100644 --- a/xlators/mgmt/glusterd/src/glusterd-op-sm.c +++ b/xlators/mgmt/glusterd/src/glusterd-op-sm.c @@ -753,16 +753,22 @@ out: } static int -glusterd_op_stage_replace_brick (gd1_mgmt_stage_op_req *req) +glusterd_op_stage_replace_brick (gd1_mgmt_stage_op_req *req, char **op_errstr) { int ret = 0; dict_t *dict = NULL; char *src_brick = NULL; char *dst_brick = NULL; char *volname = NULL; + gf1_cli_replace_op replace_op = 0; glusterd_volinfo_t *volinfo = NULL; glusterd_brickinfo_t *src_brickinfo = NULL; - gf_boolean_t exists = _gf_false; + char *host = NULL; + char *path = NULL; + char msg[2048] = {0}; + char *dup_dstbrick = NULL; + glusterd_peerinfo_t *peerinfo = NULL; + struct stat st_buf = {0,}; GF_ASSERT (req); @@ -804,36 +810,116 @@ glusterd_op_stage_replace_brick (gd1_mgmt_stage_op_req *req) goto out; } + ret = dict_get_int32 (dict, "operation", (int32_t *)&replace_op); + if (ret) { + gf_log ("", GF_LOG_DEBUG, + "dict get on replace-brick operation failed"); + goto out; + } + ret = glusterd_volinfo_find (volname, &volinfo); if (ret) { - gf_log ("", GF_LOG_ERROR, "Unable to allocate memory"); + snprintf (msg, sizeof (msg), "volume: %s does not exist", + volname); + *op_errstr = gf_strdup (msg); + goto out; + } + + if (GLUSTERD_STATUS_STARTED != volinfo->status) { + ret = -1; + snprintf (msg, sizeof (msg), "volume: %s is not started", + volname); + *op_errstr = gf_strdup (msg); goto out; } ret = glusterd_brickinfo_get (src_brick, volinfo, &src_brickinfo); if (ret) { - gf_log ("", GF_LOG_DEBUG, "Unable to get src-brickinfo"); + snprintf (msg, sizeof (msg), "brick: %s does not exist in " + "volume: %s", src_brick, volname); + *op_errstr = gf_strdup (msg); goto out; } if (!glusterd_is_local_addr (src_brickinfo->hostname)) { gf_log ("", GF_LOG_DEBUG, "I AM THE SOURCE HOST"); - exists = glusterd_check_volume_exists (volname); + } - if (!exists) { - gf_log ("", GF_LOG_ERROR, "Volume with name: %s " - "does not exist", - volname); + dup_dstbrick = gf_strdup (dst_brick); + if (!dup_dstbrick) { + ret = -1; + gf_log ("", GF_LOG_ERROR, "Memory allocation failed"); + goto out; + } + host = strtok (dup_dstbrick, ":"); + path = strtok (NULL, ":"); + + if (!host || !path) { + gf_log ("", GF_LOG_ERROR, + "dst brick %s is not of form :", + dst_brick); + ret = -1; + goto out; + } + if (glusterd_is_exisiting_brick (host, path)) { + snprintf(msg, sizeof(msg), "Brick: %s:%s already in use", + host, path); + *op_errstr = gf_strdup (msg); + ret = -1; + goto out; + } + + if (!glusterd_is_local_addr (host)) { + ret = stat (path, &st_buf); + if (ret == -1) { + snprintf (msg, sizeof (msg) ,"path: %s for brick: %s" + " does not exist", path, dst_brick); + gf_log ("glusterd", GF_LOG_ERROR, "%s", msg); + *op_errstr = gf_strdup (msg); + goto out; + } + if (!S_ISDIR (st_buf.st_mode)) { + snprintf (msg, sizeof (msg), "Volume name %s, brick" + ": %s, path %s is not a directory", volname, + dst_brick, path); + gf_log ("glusterd", GF_LOG_ERROR, + "%s", msg); + *op_errstr = gf_strdup (msg); + ret = -1; + goto out; + } + } else { + ret = glusterd_friend_find (NULL, host, &peerinfo); + if (ret) { + snprintf (msg, sizeof (msg), "%s, is not a friend", + host); + *op_errstr = gf_strdup (msg); + goto out; + } + + if (!peerinfo->connected) { + snprintf (msg, sizeof (msg), "%s, is not connected at " + "the moment", host); + *op_errstr = gf_strdup (msg); ret = -1; goto out; } - } + if (GD_FRIEND_STATE_BEFRIENDED != peerinfo->state.state) { + snprintf (msg, sizeof (msg), "%s, is not befriended " + "at the moment", host); + *op_errstr = gf_strdup (msg); + ret = -1; + goto out; + } + } ret = 0; out: + if (dup_dstbrick) + GF_FREE (dup_dstbrick); if (dict) dict_unref (dict); gf_log ("", GF_LOG_DEBUG, "Returning %d", ret); @@ -3657,6 +3743,10 @@ glusterd_op_send_cli_response (int32_t op, int32_t op_ret, rsp.status = ""; rsp.op_ret = op_ret; rsp.op_errno = op_errno; + if (op_errstr) + rsp.op_errstr = op_errstr; + else + rsp.op_errstr = ""; rsp.volname = ""; cli_rsp = &rsp; sfunc = gf_xdr_serialize_cli_replace_brick_rsp; @@ -3949,7 +4039,7 @@ glusterd_op_stage_validate (gd1_mgmt_stage_op_req *req, char **op_errstr) break; case GD_OP_REPLACE_BRICK: - ret = glusterd_op_stage_replace_brick (req); + ret = glusterd_op_stage_replace_brick (req, op_errstr); break; case GD_OP_SET_VOLUME: diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c index 358d6ffe46f..e3592891b18 100644 --- a/xlators/mgmt/glusterd/src/glusterd-utils.c +++ b/xlators/mgmt/glusterd/src/glusterd-utils.c @@ -1716,3 +1716,37 @@ glusterd_volume_count_get (void) return ret; } + +int +glusterd_is_exisiting_brick (char *hostname, char *path) +{ + glusterd_brickinfo_t *tmpbrkinfo = NULL; + glusterd_volinfo_t *volinfo = NULL; + glusterd_conf_t *priv = NULL; + xlator_t *this = NULL; + int ret = 0; + + GF_ASSERT (hostname); + GF_ASSERT (path); + + this = THIS; + GF_ASSERT (this); + + priv = this->private; + list_for_each_entry (volinfo, &priv->volumes, vol_list) { + + list_for_each_entry (tmpbrkinfo, &volinfo->bricks, + brick_list) { + + if ((!strcmp(hostname, tmpbrkinfo-> hostname) + && !strcmp(path, tmpbrkinfo->path))) { + gf_log ("glusterd", GF_LOG_ERROR, "Brick %s:%s" + " already in use", hostname, path); + ret = 1; + goto out; + } + } + } +out: + return ret; +} diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.h b/xlators/mgmt/glusterd/src/glusterd-utils.h index e03968dfeeb..72715a0f5df 100644 --- a/xlators/mgmt/glusterd/src/glusterd-utils.h +++ b/xlators/mgmt/glusterd/src/glusterd-utils.h @@ -158,4 +158,6 @@ glusterd_volume_count_get (void); int32_t glusterd_add_volume_to_dict (glusterd_volinfo_t *volinfo, dict_t *dict, int32_t count); +int +glusterd_is_exisiting_brick (char *hostname, char *path); #endif -- cgit