Patchwork [BUG:1457] Volume Add-brick validation for exports

login
register
Submitter shishir gowda
Date 2010-08-30 07:06:33
Message ID <20100830070633.GA30891@dev.gluster.com>
Download mbox | patch
Permalink /patch/4371/
State Accepted
Headers show

Comments

shishir gowda - 2010-08-30 07:06:33
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 <shishirng@gluster.com>
---
 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(-)

Patch

diff --git a/cli/src/cli-cmd-parser.c b/cli/src/cli-cmd-parser.c
index 4b8520c..79f56e2 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 0306fdd..42b8e12 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 cb8acbe..f74297f 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);