From c9800f0986a10bbde9121239590e3cb25b94c5f8 Mon Sep 17 00:00:00 2001 From: Krutika Dhananjay Date: Tue, 12 Feb 2013 17:26:35 +0530 Subject: glusterd: changes in 'volume create' behaviour This patch incorporates all the changes suggested on the behaviour of 'volume create' command in http://review.gluster.org/#change,4214 (comment #14, to be precise). Change-Id: Iaac524a59738b177415595b18aa8a136090d3d25 BUG: 948729 Signed-off-by: Krutika Dhananjay Reviewed-on: http://review.gluster.org/4740 Tested-by: Gluster Build System Reviewed-by: Anand Avati --- xlators/mgmt/glusterd/src/glusterd-brick-ops.c | 12 ++- xlators/mgmt/glusterd/src/glusterd-replace-brick.c | 11 ++- xlators/mgmt/glusterd/src/glusterd-utils.c | 109 +++++++++++++++++++-- xlators/mgmt/glusterd/src/glusterd-utils.h | 9 +- xlators/mgmt/glusterd/src/glusterd-volume-ops.c | 16 ++- 5 files changed, 137 insertions(+), 20 deletions(-) (limited to 'xlators/mgmt/glusterd') diff --git a/xlators/mgmt/glusterd/src/glusterd-brick-ops.c b/xlators/mgmt/glusterd/src/glusterd-brick-ops.c index 2ac2699cf..d21facb0a 100644 --- a/xlators/mgmt/glusterd/src/glusterd-brick-ops.c +++ b/xlators/mgmt/glusterd/src/glusterd-brick-ops.c @@ -423,6 +423,10 @@ glusterd_handle_add_brick (rpcsvc_request_t *req) stripe_count); } + if (!dict_get (dict, "force")) { + gf_log (this->name, GF_LOG_ERROR, "Failed to get flag"); + goto out; + } ret = glusterd_volinfo_find (volname, &volinfo); if (ret) { @@ -1034,6 +1038,7 @@ glusterd_op_stage_add_brick (dict_t *dict, char **op_errstr) gf_boolean_t brick_alloc = _gf_false; char *all_bricks = NULL; char *str_ret = NULL; + gf_boolean_t is_force = _gf_false; priv = THIS->private; if (!priv) @@ -1096,6 +1101,8 @@ glusterd_op_stage_add_brick (dict_t *dict, char **op_errstr) goto out; } + is_force = dict_get_str_boolean (dict, "force", _gf_false); + if (bricks) { brick_list = gf_strdup (bricks); all_bricks = gf_strdup (bricks); @@ -1137,10 +1144,9 @@ glusterd_op_stage_add_brick (dict_t *dict, char **op_errstr) } if (!uuid_compare (brickinfo->uuid, MY_UUID)) { - ret = glusterd_brick_create_path (brickinfo->hostname, - brickinfo->path, + ret = glusterd_validate_and_create_brickpath (brickinfo, volinfo->volume_id, - op_errstr); + op_errstr, is_force); if (ret) goto out; } diff --git a/xlators/mgmt/glusterd/src/glusterd-replace-brick.c b/xlators/mgmt/glusterd/src/glusterd-replace-brick.c index 36f0b10dc..0afe2203c 100644 --- a/xlators/mgmt/glusterd/src/glusterd-replace-brick.c +++ b/xlators/mgmt/glusterd/src/glusterd-replace-brick.c @@ -213,6 +213,7 @@ glusterd_op_stage_replace_brick (dict_t *dict, char **op_errstr, char pidfile[PATH_MAX] = {0}; char *task_id_str = NULL; xlator_t *this = NULL; + gf_boolean_t is_force = _gf_false; this = THIS; GF_ASSERT (this); @@ -349,6 +350,8 @@ glusterd_op_stage_replace_brick (dict_t *dict, char **op_errstr, ret = 0; } } + is_force = dict_get_str_boolean (dict, "force", _gf_false); + break; case GF_REPLACE_OP_PAUSE: @@ -383,7 +386,9 @@ glusterd_op_stage_replace_brick (dict_t *dict, char **op_errstr, } break; - case GF_REPLACE_OP_COMMIT_FORCE: break; + case GF_REPLACE_OP_COMMIT_FORCE: + is_force = _gf_true; + break; case GF_REPLACE_OP_STATUS: @@ -507,9 +512,9 @@ glusterd_op_stage_replace_brick (dict_t *dict, char **op_errstr, if (!glusterd_is_rb_ongoing (volinfo) && glusterd_is_local_addr (host)) { - ret = glusterd_brick_create_path (host, path, + ret = glusterd_validate_and_create_brickpath (dst_brickinfo, volinfo->volume_id, - op_errstr); + op_errstr, is_force); if (ret) goto out; } diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c index 09aeb7370..6c5d0e2f0 100644 --- a/xlators/mgmt/glusterd/src/glusterd-utils.c +++ b/xlators/mgmt/glusterd/src/glusterd-utils.c @@ -924,6 +924,105 @@ out: return available; } +int +glusterd_validate_and_create_brickpath (glusterd_brickinfo_t *brickinfo, + uuid_t volume_id, char **op_errstr, + gf_boolean_t is_force) +{ + int ret = -1; + char parentdir[PATH_MAX] = {0,}; + struct stat parent_st = {0,}; + struct stat brick_st = {0,}; + struct stat root_st = {0,}; + char msg[2048] = {0,}; + gf_boolean_t is_created = _gf_false; + + ret = mkdir (brickinfo->path, 0777); + if (ret) { + if (errno != EEXIST) { + snprintf (msg, sizeof (msg), "Failed to create brick " + "directory for brick %s:%s. Reason : %s ", + brickinfo->hostname, brickinfo->path, + strerror (errno)); + goto out; + } + } else { + is_created = _gf_true; + } + + ret = lstat (brickinfo->path, &brick_st); + if (ret) { + snprintf (msg, sizeof (msg), "lstat failed on %s. Reason : %s", + brickinfo->path, strerror (errno)); + goto out; + } + + if ((!is_created) && (!S_ISDIR (brick_st.st_mode))) { + snprintf (msg, sizeof (msg), "The provided path %s which is " + "already present, is not a directory", + brickinfo->path); + ret = -1; + goto out; + } + + snprintf (parentdir, sizeof (parentdir), "%s/..", brickinfo->path); + + ret = lstat ("/", &root_st); + if (ret) { + snprintf (msg, sizeof (msg), "lstat failed on /. Reason : %s", + strerror (errno)); + goto out; + } + + ret = lstat (parentdir, &parent_st); + if (ret) { + snprintf (msg, sizeof (msg), "lstat failed on %s. Reason : %s", + parentdir, strerror (errno)); + goto out; + } + + if (!is_force) { + if (brick_st.st_dev != parent_st.st_dev) { + snprintf (msg, sizeof (msg), "The brick %s:%s is a " + "mount point. Please create a sub-directory " + "under the mount point and use that as the " + "brick directory. Or use 'force' at the end " + "of the command if you want to override this " + "behavior.", brickinfo->hostname, + brickinfo->path); + ret = -1; + goto out; + } + else if (parent_st.st_dev == root_st.st_dev) { + snprintf (msg, sizeof (msg), "The brick %s:%s is " + "is being created in the root partition. It " + "is recommended that you don't use the " + "system's root partition for storage backend." + " Or use 'force' at the end of the command if" + " you want to override this behavior.", + brickinfo->hostname, brickinfo->path); + ret = -1; + goto out; + } + } + + ret = glusterd_check_and_set_brick_xattr (brickinfo->hostname, + brickinfo->path, volume_id, + op_errstr); + if (ret) + goto out; + + ret = 0; + +out: + if (ret && is_created) + rmdir (brickinfo->path); + if (ret && !*op_errstr && msg[0] != '\0') + *op_errstr = gf_strdup (msg); + + return ret; +} + int32_t glusterd_volume_brickinfo_get (uuid_t uuid, char *hostname, char *path, glusterd_volinfo_t *volinfo, @@ -5045,17 +5144,13 @@ out: } int -glusterd_brick_create_path (char *host, char *path, uuid_t uuid, - char **op_errstr) +glusterd_check_and_set_brick_xattr (char *host, char *path, uuid_t uuid, + char **op_errstr) { int ret = -1; char msg[2048] = {0,}; gf_boolean_t in_use = _gf_false; - ret = mkdir_p (path, 0777, _gf_true); - if (ret) - goto out; - /* Check for xattr support in backend fs */ ret = sys_lsetxattr (path, "trusted.glusterfs.test", "working", 8, 0); @@ -5068,7 +5163,6 @@ glusterd_brick_create_path (char *host, char *path, uuid_t uuid, } else { sys_lremovexattr (path, "trusted.glusterfs.test"); - } ret = glusterd_is_path_in_use (path, &in_use, op_errstr); @@ -5095,7 +5189,6 @@ out: *op_errstr = gf_strdup (msg); return ret; - } int diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.h b/xlators/mgmt/glusterd/src/glusterd-utils.h index c7fbe447e..28182d6ba 100644 --- a/xlators/mgmt/glusterd/src/glusterd-utils.h +++ b/xlators/mgmt/glusterd/src/glusterd-utils.h @@ -317,8 +317,13 @@ glusterd_rb_check_bricks (glusterd_volinfo_t *volinfo, glusterd_brickinfo_t *dst_brick); int -glusterd_brick_create_path (char *host, char *path, uuid_t uuid, - char **op_errstr); +glusterd_check_and_set_brick_xattr (char *host, char *path, uuid_t uuid, + char **op_errstr); + +int +glusterd_validate_and_create_brickpath (glusterd_brickinfo_t *brickinfo, + uuid_t volume_id, char **op_errstr, + gf_boolean_t is_force); int glusterd_sm_tr_log_transition_add (glusterd_sm_tr_log_t *log, int old_state, int new_state, diff --git a/xlators/mgmt/glusterd/src/glusterd-volume-ops.c b/xlators/mgmt/glusterd/src/glusterd-volume-ops.c index bc5f8f2a7..fd4e0268a 100644 --- a/xlators/mgmt/glusterd/src/glusterd-volume-ops.c +++ b/xlators/mgmt/glusterd/src/glusterd-volume-ops.c @@ -134,6 +134,11 @@ glusterd_handle_create_volume (rpcsvc_request_t *req) goto out; } + if (!dict_get (dict, "force")) { + gf_log (this->name, GF_LOG_ERROR, "Failed to get 'force' flag"); + goto out; + } + uuid_generate (volume_id); free_ptr = gf_strdup (uuid_utoa (volume_id)); ret = dict_set_dynstr (dict, "volume-id", free_ptr); @@ -610,6 +615,8 @@ glusterd_op_stage_create_volume (dict_t *dict, char **op_errstr) #ifdef HAVE_BD_XLATOR char *dev_type = NULL; #endif + gf_boolean_t is_force = _gf_false; + this = THIS; GF_ASSERT (this); priv = this->private; @@ -663,6 +670,8 @@ glusterd_op_stage_create_volume (dict_t *dict, char **op_errstr) goto out; } + is_force = dict_get_str_boolean (dict, "force", _gf_false); + if (bricks) { brick_list = gf_strdup (bricks); if (!brick_list) { @@ -715,10 +724,9 @@ glusterd_op_stage_create_volume (dict_t *dict, char **op_errstr) } else #endif if (!uuid_compare (brick_info->uuid, MY_UUID)) { - ret = glusterd_brick_create_path (brick_info->hostname, - brick_info->path, - volume_uuid, - op_errstr); + ret = glusterd_validate_and_create_brickpath (brick_info, + volume_uuid, op_errstr, + is_force); if (ret) goto out; brick_list = tmpptr; -- cgit