From d129d4eea33aae5db24dba17adcb04e9d4829817 Mon Sep 17 00:00:00 2001 From: Atin Mukherjee Date: Mon, 11 Apr 2016 16:07:40 +0530 Subject: glusterd: populate brickinfo->real_path conditionally glusterd_brickinfo_new_from_brick () is called from multiple places and one of them is glusterd_brick_rpc_notify where its very well possible that an underlying brick's file system has crashed and a disconnect event has been received. In this case glusterd tries to build the brickinfo from the brickid in the RPC request, however the same fails as glusterd_brickinfo_new_from_brick () fails from realpath. Fix is to skip populating real_path if its a disconnect event. Change-Id: I9d9149c64a9cf2247abb731f219c1b1eef037960 BUG: 1325841 Signed-off-by: Atin Mukherjee Reviewed-on: http://review.gluster.org/13965 Smoke: Gluster Build System CentOS-regression: Gluster Build System NetBSD-regression: NetBSD Build System Reviewed-by: Jeff Darcy --- xlators/mgmt/glusterd/src/glusterd-brick-ops.c | 18 ++++++---- xlators/mgmt/glusterd/src/glusterd-handler.c | 3 +- xlators/mgmt/glusterd/src/glusterd-log-ops.c | 6 ++-- xlators/mgmt/glusterd/src/glusterd-op-sm.c | 17 +++++---- xlators/mgmt/glusterd/src/glusterd-rebalance.c | 3 +- xlators/mgmt/glusterd/src/glusterd-replace-brick.c | 16 +++++---- xlators/mgmt/glusterd/src/glusterd-store.c | 2 +- xlators/mgmt/glusterd/src/glusterd-utils.c | 40 +++++++++++++--------- xlators/mgmt/glusterd/src/glusterd-utils.h | 7 ++-- xlators/mgmt/glusterd/src/glusterd-volgen.c | 2 +- xlators/mgmt/glusterd/src/glusterd-volume-ops.c | 6 ++-- 11 files changed, 76 insertions(+), 44 deletions(-) diff --git a/xlators/mgmt/glusterd/src/glusterd-brick-ops.c b/xlators/mgmt/glusterd/src/glusterd-brick-ops.c index 920f7f05623..b6d9703591c 100644 --- a/xlators/mgmt/glusterd/src/glusterd-brick-ops.c +++ b/xlators/mgmt/glusterd/src/glusterd-brick-ops.c @@ -1034,7 +1034,8 @@ __glusterd_handle_remove_brick (rpcsvc_request_t *req) " %s", i, brick); ret = glusterd_volume_brickinfo_get_by_brick(brick, volinfo, - &brickinfo); + &brickinfo, + _gf_true); if (ret) { snprintf (err_str, sizeof (err_str), "Incorrect brick " @@ -1274,7 +1275,8 @@ glusterd_op_perform_add_bricks (glusterd_volinfo_t *volinfo, int32_t count, if (brickid < 0) goto out; while ( i <= count) { - ret = glusterd_brickinfo_new_from_brick (brick, &brickinfo); + ret = glusterd_brickinfo_new_from_brick (brick, &brickinfo, + _gf_true); if (ret) goto out; @@ -1381,7 +1383,8 @@ glusterd_op_perform_add_bricks (glusterd_volinfo_t *volinfo, int32_t count, while (i <= count) { ret = glusterd_volume_brickinfo_get_by_brick (brick, volinfo, - &brickinfo); + &brickinfo, + _gf_true); if (ret) goto out; #ifdef HAVE_BD_XLATOR @@ -1484,7 +1487,8 @@ glusterd_op_perform_remove_brick (glusterd_volinfo_t *volinfo, char *brick, GF_ASSERT (priv); ret = glusterd_volume_brickinfo_get_by_brick (brick, volinfo, - &brickinfo); + &brickinfo, + _gf_true); if (ret) goto out; @@ -1699,7 +1703,8 @@ glusterd_op_stage_add_brick (dict_t *dict, char **op_errstr, dict_t *rsp_dict) } - ret = glusterd_brickinfo_new_from_brick (brick, &brickinfo); + ret = glusterd_brickinfo_new_from_brick (brick, &brickinfo, + _gf_true); if (ret) { gf_msg (THIS->name, GF_LOG_ERROR, 0, GD_MSG_BRICK_NOT_FOUND, @@ -1822,7 +1827,8 @@ glusterd_remove_brick_validate_bricks (gf1_op_commands cmd, int32_t brick_count, ret = glusterd_volume_brickinfo_get_by_brick(brick, volinfo, - &brickinfo); + &brickinfo, + _gf_true); if (ret) { snprintf (msg, sizeof (msg), "Incorrect brick " "%s for volume %s", brick, volinfo->volname); diff --git a/xlators/mgmt/glusterd/src/glusterd-handler.c b/xlators/mgmt/glusterd/src/glusterd-handler.c index 566383b15f9..f10d3cec4cd 100644 --- a/xlators/mgmt/glusterd/src/glusterd-handler.c +++ b/xlators/mgmt/glusterd/src/glusterd-handler.c @@ -4948,7 +4948,8 @@ get_brickinfo_from_brickid (char *brickid, glusterd_brickinfo_t **brickinfo) } ret = glusterd_volume_brickinfo_get_by_brick (brick, volinfo, - brickinfo); + brickinfo, + _gf_false); if (ret) goto out; diff --git a/xlators/mgmt/glusterd/src/glusterd-log-ops.c b/xlators/mgmt/glusterd/src/glusterd-log-ops.c index e1bb755fd3b..4d66df46132 100644 --- a/xlators/mgmt/glusterd/src/glusterd-log-ops.c +++ b/xlators/mgmt/glusterd/src/glusterd-log-ops.c @@ -146,7 +146,8 @@ glusterd_op_stage_log_rotate (dict_t *dict, char **op_errstr) goto out; } - ret = glusterd_volume_brickinfo_get_by_brick (brick, volinfo, NULL); + ret = glusterd_volume_brickinfo_get_by_brick (brick, volinfo, NULL, + _gf_true); if (ret) { snprintf (msg, sizeof (msg), "Incorrect brick %s " "for volume %s", brick, volname); @@ -205,7 +206,8 @@ glusterd_op_log_rotate (dict_t *dict) if (ret) goto cont; - ret = glusterd_brickinfo_new_from_brick (brick, &tmpbrkinfo); + ret = glusterd_brickinfo_new_from_brick (brick, &tmpbrkinfo, + _gf_true); if (ret) { gf_msg ("glusterd", GF_LOG_ERROR, 0, GD_MSG_BRICK_NOT_FOUND, diff --git a/xlators/mgmt/glusterd/src/glusterd-op-sm.c b/xlators/mgmt/glusterd/src/glusterd-op-sm.c index eda840a0483..951d06b2ff7 100644 --- a/xlators/mgmt/glusterd/src/glusterd-op-sm.c +++ b/xlators/mgmt/glusterd/src/glusterd-op-sm.c @@ -1724,7 +1724,8 @@ glusterd_op_stage_status_volume (dict_t *dict, char **op_errstr) goto out; ret = glusterd_volume_brickinfo_get_by_brick (brick, volinfo, - &brickinfo); + &brickinfo, + _gf_true); if (ret) { snprintf (msg, sizeof(msg), "No brick %s in" " volume %s", brick, volname); @@ -3241,7 +3242,8 @@ glusterd_op_status_volume (dict_t *dict, char **op_errstr, ret = glusterd_volume_brickinfo_get_by_brick (brick, volinfo, - &brickinfo); + &brickinfo, + _gf_true); if (ret) goto out; @@ -5951,7 +5953,8 @@ glusterd_bricks_select_remove_brick (dict_t *dict, char **op_errstr, } ret = glusterd_volume_brickinfo_get_by_brick (brick, volinfo, - &brickinfo); + &brickinfo, + _gf_true); if (ret) goto out; @@ -6103,8 +6106,9 @@ glusterd_bricks_select_profile_volume (dict_t *dict, char **op_errstr, } ret = dict_get_str (dict, "brick", &brick); if (!ret) { - ret = glusterd_volume_brickinfo_get_by_brick (brick, volinfo, - &brickinfo); + ret = glusterd_volume_brickinfo_get_by_brick + (brick, volinfo, &brickinfo, + _gf_true); if (ret) goto out; @@ -6809,7 +6813,8 @@ glusterd_bricks_select_status_volume (dict_t *dict, char **op_errstr, } ret = glusterd_volume_brickinfo_get_by_brick (brickname, volinfo, - &brickinfo); + &brickinfo, + _gf_true); if (ret) goto out; diff --git a/xlators/mgmt/glusterd/src/glusterd-rebalance.c b/xlators/mgmt/glusterd/src/glusterd-rebalance.c index 5bf456ba00b..60d1b204da5 100644 --- a/xlators/mgmt/glusterd/src/glusterd-rebalance.c +++ b/xlators/mgmt/glusterd/src/glusterd-rebalance.c @@ -602,7 +602,8 @@ glusterd_brick_validation (dict_t *dict, char *key, data_t *value, GF_ASSERT (this); ret = glusterd_volume_brickinfo_get_by_brick (value->data, volinfo, - &brickinfo); + &brickinfo, + _gf_true); if (ret) { gf_msg (this->name, GF_LOG_ERROR, EINVAL, GD_MSG_BRICK_NOT_FOUND, diff --git a/xlators/mgmt/glusterd/src/glusterd-replace-brick.c b/xlators/mgmt/glusterd/src/glusterd-replace-brick.c index 535f1256baf..30e6ee850e9 100644 --- a/xlators/mgmt/glusterd/src/glusterd-replace-brick.c +++ b/xlators/mgmt/glusterd/src/glusterd-replace-brick.c @@ -294,7 +294,8 @@ glusterd_op_stage_replace_brick (dict_t *dict, char **op_errstr, } ret = glusterd_volume_brickinfo_get_by_brick (src_brick, volinfo, - &src_brickinfo); + &src_brickinfo, + _gf_true); if (ret) { snprintf (msg, sizeof (msg), "brick: %s does not exist in " "volume: %s", src_brick, volname); @@ -363,7 +364,8 @@ glusterd_op_stage_replace_brick (dict_t *dict, char **op_errstr, goto out; } - ret = glusterd_brickinfo_new_from_brick (dst_brick, &dst_brickinfo); + ret = glusterd_brickinfo_new_from_brick (dst_brick, &dst_brickinfo, + _gf_true); if (ret) goto out; @@ -548,8 +550,8 @@ glusterd_op_perform_replace_brick (glusterd_volinfo_t *volinfo, conf = this->private; GF_ASSERT (conf); - ret = glusterd_brickinfo_new_from_brick (new_brick, - &new_brickinfo); + ret = glusterd_brickinfo_new_from_brick (new_brick, &new_brickinfo, + _gf_true); if (ret) goto out; @@ -559,7 +561,8 @@ glusterd_op_perform_replace_brick (glusterd_volinfo_t *volinfo, goto out; ret = glusterd_volume_brickinfo_get_by_brick (old_brick, - volinfo, &old_brickinfo); + volinfo, &old_brickinfo, + _gf_true); if (ret) goto out; @@ -678,7 +681,8 @@ glusterd_op_replace_brick (dict_t *dict, dict_t *rsp_dict) } ret = glusterd_volume_brickinfo_get_by_brick (src_brick, volinfo, - &src_brickinfo); + &src_brickinfo, + _gf_true); if (ret) { gf_msg_debug (this->name, 0, "Unable to get src-brickinfo"); diff --git a/xlators/mgmt/glusterd/src/glusterd-store.c b/xlators/mgmt/glusterd/src/glusterd-store.c index c776e4b5249..592a2952c4b 100644 --- a/xlators/mgmt/glusterd/src/glusterd-store.c +++ b/xlators/mgmt/glusterd/src/glusterd-store.c @@ -165,7 +165,7 @@ glusterd_store_is_valid_brickpath (char *volname, char *brick) this = THIS; GF_ASSERT (this); - ret = glusterd_brickinfo_new_from_brick (brick, &brickinfo); + ret = glusterd_brickinfo_new_from_brick (brick, &brickinfo, _gf_true); if (ret) { gf_msg (this->name, GF_LOG_WARNING, 0, GD_MSG_BRICK_CREATION_FAIL, "Failed to create brick " diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c index 22caee051a9..2aa6a06a4ef 100644 --- a/xlators/mgmt/glusterd/src/glusterd-utils.c +++ b/xlators/mgmt/glusterd/src/glusterd-utils.c @@ -1059,7 +1059,8 @@ out: int32_t glusterd_brickinfo_new_from_brick (char *brick, - glusterd_brickinfo_t **brickinfo) + glusterd_brickinfo_t **brickinfo, + gf_boolean_t construct_real_path) { char *hostname = NULL; char *path = NULL; @@ -1107,19 +1108,22 @@ glusterd_brickinfo_new_from_brick (char *brick, strncpy (new_brickinfo->hostname, hostname, 1024); strncpy (new_brickinfo->path, path, 1024); - if (!realpath (new_brickinfo->path, abspath)) { - /* ENOENT indicates that brick path has not been created which - * is a valid scenario */ - if (errno != ENOENT) { - gf_msg (this->name, GF_LOG_CRITICAL, errno, - GD_MSG_BRICKINFO_CREATE_FAIL, "realpath () failed for " - "brick %s. The underlying filesystem may be in bad " - "state", new_brickinfo->path); - ret = -1; - goto out; + if (construct_real_path) { + if (!realpath (new_brickinfo->path, abspath)) { + /* ENOENT indicates that brick path has not been created + * which is a valid scenario */ + if (errno != ENOENT) { + gf_msg (this->name, GF_LOG_CRITICAL, errno, + GD_MSG_BRICKINFO_CREATE_FAIL, "realpath" + " () failed for brick %s. The " + "underlying filesystem may be in bad " + "state", new_brickinfo->path); + ret = -1; + goto out; + } } + strncpy (new_brickinfo->real_path, abspath, strlen(abspath)); } - strncpy (new_brickinfo->real_path, abspath, strlen(abspath)); *brickinfo = new_brickinfo; @@ -1420,7 +1424,8 @@ out: int32_t glusterd_volume_brickinfo_get_by_brick (char *brick, glusterd_volinfo_t *volinfo, - glusterd_brickinfo_t **brickinfo) + glusterd_brickinfo_t **brickinfo, + gf_boolean_t construct_real_path) { int32_t ret = -1; glusterd_brickinfo_t *tmp_brickinfo = NULL; @@ -1428,7 +1433,8 @@ glusterd_volume_brickinfo_get_by_brick (char *brick, GF_ASSERT (brick); GF_ASSERT (volinfo); - ret = glusterd_brickinfo_new_from_brick (brick, &tmp_brickinfo); + ret = glusterd_brickinfo_new_from_brick (brick, &tmp_brickinfo, + construct_real_path); if (ret) goto out; @@ -5806,7 +5812,8 @@ glusterd_new_brick_validate (char *brick, glusterd_brickinfo_t *brickinfo, GF_ASSERT (op_errstr); if (!brickinfo) { - ret = glusterd_brickinfo_new_from_brick (brick, &newbrickinfo); + ret = glusterd_brickinfo_new_from_brick (brick, &newbrickinfo, + _gf_true); if (ret) goto out; is_allocated = _gf_true; @@ -10149,7 +10156,8 @@ gd_should_i_start_rebalance (glusterd_volinfo_t *volinfo) { goto out; ret = glusterd_volume_brickinfo_get_by_brick (brickname, volinfo, - &brick); + &brick, + _gf_true); if (ret) goto out; if (gf_uuid_compare (MY_UUID, brick->uuid) == 0) { diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.h b/xlators/mgmt/glusterd/src/glusterd-utils.h index 50d691acad5..730bc657199 100644 --- a/xlators/mgmt/glusterd/src/glusterd-utils.h +++ b/xlators/mgmt/glusterd/src/glusterd-utils.h @@ -113,7 +113,9 @@ int32_t glusterd_brickinfo_new (glusterd_brickinfo_t **brickinfo); int32_t -glusterd_brickinfo_new_from_brick (char *brick, glusterd_brickinfo_t **brickinfo); +glusterd_brickinfo_new_from_brick (char *brick, + glusterd_brickinfo_t **brickinfo, + gf_boolean_t construct_real_path); int32_t glusterd_volinfo_find (char *volname, glusterd_volinfo_t **volinfo); @@ -159,7 +161,8 @@ glusterd_is_cli_op_req (int32_t op); int32_t glusterd_volume_brickinfo_get_by_brick (char *brick, glusterd_volinfo_t *volinfo, - glusterd_brickinfo_t **brickinfo); + glusterd_brickinfo_t **brickinfo, + gf_boolean_t construct_real_path); int32_t glusterd_add_volumes_to_export_dict (dict_t **peer_data); diff --git a/xlators/mgmt/glusterd/src/glusterd-volgen.c b/xlators/mgmt/glusterd/src/glusterd-volgen.c index d3696b2a4e5..de8a795df91 100644 --- a/xlators/mgmt/glusterd/src/glusterd-volgen.c +++ b/xlators/mgmt/glusterd/src/glusterd-volgen.c @@ -5124,7 +5124,7 @@ glusterd_is_valid_volfpath (char *volname, char *brick) this = THIS; GF_ASSERT (this); - ret = glusterd_brickinfo_new_from_brick (brick, &brickinfo); + ret = glusterd_brickinfo_new_from_brick (brick, &brickinfo, _gf_true); if (ret) { gf_msg (this->name, GF_LOG_WARNING, 0, GD_MSG_BRICKINFO_CREATE_FAIL, diff --git a/xlators/mgmt/glusterd/src/glusterd-volume-ops.c b/xlators/mgmt/glusterd/src/glusterd-volume-ops.c index 2ecadce20fb..a01ef3a74fb 100644 --- a/xlators/mgmt/glusterd/src/glusterd-volume-ops.c +++ b/xlators/mgmt/glusterd/src/glusterd-volume-ops.c @@ -1247,7 +1247,8 @@ glusterd_op_stage_create_volume (dict_t *dict, char **op_errstr, goto out; } - ret = glusterd_brickinfo_new_from_brick (brick, &brick_info); + ret = glusterd_brickinfo_new_from_brick (brick, &brick_info, + _gf_true); if (ret) goto out; @@ -2333,7 +2334,8 @@ glusterd_op_create_volume (dict_t *dict, char **op_errstr) if (brickid < 0) goto out; while ( i <= count) { - ret = glusterd_brickinfo_new_from_brick (brick, &brickinfo); + ret = glusterd_brickinfo_new_from_brick (brick, &brickinfo, + _gf_true); if (ret) goto out; -- cgit