From 4bd4b0a9437189cef439833a0ed70005db9f8409 Mon Sep 17 00:00:00 2001 From: Atin Mukherjee Date: Wed, 3 Apr 2019 08:27:05 +0530 Subject: glusterd: remove redundant glusterd_check_volume_exists () calls A pattern of following was found in multiple places where both glusterd_check_volume_exists and glusterd_volinfo_find do the same job. We just need one of them not both. In a scaled environment having many volumes this is a bottleneck to iterate over the volume list to find a volume twice! exists = glusterd_check_volume_exists(volname); ret = glusterd_volinfo_find(volname, &volinfo); if ((ret) || (!exists)) { Credits: ykaul@redhat.com for finding this out Updates: bz#1193929 Change-Id: Ie116fe5c93e261a2bddd267c28ccb20a2884a36f Signed-off-by: Atin Mukherjee --- xlators/mgmt/glusterd/src/glusterd-brick-ops.c | 44 ++++++++----------------- xlators/mgmt/glusterd/src/glusterd-geo-rep.c | 24 +++----------- xlators/mgmt/glusterd/src/glusterd-log-ops.c | 5 +-- xlators/mgmt/glusterd/src/glusterd-op-sm.c | 28 ++-------------- xlators/mgmt/glusterd/src/glusterd-quota.c | 7 ---- xlators/mgmt/glusterd/src/glusterd-volume-ops.c | 32 +----------------- 6 files changed, 23 insertions(+), 117 deletions(-) (limited to 'xlators/mgmt') diff --git a/xlators/mgmt/glusterd/src/glusterd-brick-ops.c b/xlators/mgmt/glusterd/src/glusterd-brick-ops.c index e08ea7be215..4d18eb3cd96 100644 --- a/xlators/mgmt/glusterd/src/glusterd-brick-ops.c +++ b/xlators/mgmt/glusterd/src/glusterd-brick-ops.c @@ -351,10 +351,13 @@ __glusterd_handle_add_brick(rpcsvc_request_t *req) goto out; } - if (!(ret = glusterd_check_volume_exists(volname))) { - ret = -1; - snprintf(err_str, sizeof(err_str), "Volume %s does not exist", volname); - gf_msg(this->name, GF_LOG_ERROR, EINVAL, GD_MSG_VOL_NOT_FOUND, "%s", + ret = glusterd_volinfo_find(volname, &volinfo); + if (ret) { + snprintf(err_str, sizeof(err_str), + "Unable to get volinfo " + "for volume name %s", + volname); + gf_msg(this->name, GF_LOG_ERROR, 0, GD_MSG_VOLINFO_GET_FAIL, "%s", err_str); goto out; } @@ -396,17 +399,6 @@ __glusterd_handle_add_brick(rpcsvc_request_t *req) goto out; } - ret = glusterd_volinfo_find(volname, &volinfo); - if (ret) { - snprintf(err_str, sizeof(err_str), - "Unable to get volinfo " - "for volume name %s", - volname); - gf_msg(this->name, GF_LOG_ERROR, 0, GD_MSG_VOLINFO_GET_FAIL, "%s", - err_str); - goto out; - } - total_bricks = volinfo->brick_count + brick_count; if (dict_getn(dict, "attach-tier", SLEN("attach-tier"))) { @@ -3070,11 +3062,14 @@ __glusterd_handle_add_tier_brick(rpcsvc_request_t *req) goto out; } - if (!glusterd_check_volume_exists(volname)) { - snprintf(err_str, sizeof(err_str), "Volume %s does not exist", volname); - gf_msg(this->name, GF_LOG_ERROR, EINVAL, GD_MSG_VOL_NOT_FOUND, "%s", + ret = glusterd_volinfo_find(volname, &volinfo); + if (ret) { + snprintf(err_str, sizeof(err_str), + "Unable to get volinfo " + "for volume name %s", + volname); + gf_msg(this->name, GF_LOG_ERROR, 0, GD_MSG_VOLINFO_GET_FAIL, "%s", err_str); - ret = -1; goto out; } @@ -3109,17 +3104,6 @@ __glusterd_handle_add_tier_brick(rpcsvc_request_t *req) goto out; } - ret = glusterd_volinfo_find(volname, &volinfo); - if (ret) { - snprintf(err_str, sizeof(err_str), - "Unable to get volinfo " - "for volume name %s", - volname); - gf_msg(this->name, GF_LOG_ERROR, 0, GD_MSG_VOLINFO_GET_FAIL, "%s", - err_str); - goto out; - } - if (glusterd_is_tiering_supported(err_str) == _gf_false) { gf_msg(this->name, GF_LOG_ERROR, 0, GD_MSG_VERSION_UNSUPPORTED, "Tiering not supported at this version"); diff --git a/xlators/mgmt/glusterd/src/glusterd-geo-rep.c b/xlators/mgmt/glusterd/src/glusterd-geo-rep.c index 0f40beacb4a..c859bf4c5bc 100644 --- a/xlators/mgmt/glusterd/src/glusterd-geo-rep.c +++ b/xlators/mgmt/glusterd/src/glusterd-geo-rep.c @@ -2306,7 +2306,6 @@ glusterd_verify_gsync_status_opts(dict_t *dict, char **op_errstr) char errmsg[PATH_MAX] = { 0, }; - gf_boolean_t exists = _gf_false; glusterd_volinfo_t *volinfo = NULL; int ret = 0; char *conf_path = NULL; @@ -2334,9 +2333,8 @@ glusterd_verify_gsync_status_opts(dict_t *dict, char **op_errstr) goto out; } - exists = glusterd_check_volume_exists(volname); ret = glusterd_volinfo_find(volname, &volinfo); - if ((ret) || (!exists)) { + if (ret) { gf_msg(this->name, GF_LOG_WARNING, 0, GD_MSG_VOL_NOT_FOUND, "volume name does not exist"); snprintf(errmsg, sizeof(errmsg), @@ -2344,7 +2342,6 @@ glusterd_verify_gsync_status_opts(dict_t *dict, char **op_errstr) " exist", volname); *op_errstr = gf_strdup(errmsg); - ret = -1; goto out; } @@ -3140,7 +3137,6 @@ glusterd_op_stage_gsync_create(dict_t *dict, char **op_errstr) gf_boolean_t is_force = -1; gf_boolean_t is_no_verify = -1; gf_boolean_t is_force_blocker = -1; - gf_boolean_t exists = _gf_false; gf_boolean_t is_template_in_use = _gf_false; glusterd_conf_t *conf = NULL; glusterd_volinfo_t *volinfo = NULL; @@ -3190,18 +3186,15 @@ glusterd_op_stage_gsync_create(dict_t *dict, char **op_errstr) goto out; } - exists = glusterd_check_volume_exists(volname); ret = glusterd_volinfo_find(volname, &volinfo); - if ((ret) || (!exists)) { + if (ret) { gf_msg(this->name, GF_LOG_WARNING, 0, GD_MSG_VOL_NOT_FOUND, "volume name does not exist"); snprintf(errmsg, sizeof(errmsg), "Volume name %s does not" " exist", volname); - *op_errstr = gf_strdup(errmsg); - gf_msg_debug(this->name, 0, "Returning %d", ret); - return -1; + goto out; } ret = glusterd_get_slave_details_confpath(volinfo, dict, &slave_url, @@ -3536,7 +3529,6 @@ out: if (slave_url_buf) GF_FREE(slave_url_buf); - gf_msg_debug(this->name, 0, "Returning %d", ret); return ret; } @@ -3615,7 +3607,6 @@ glusterd_op_stage_gsync_set(dict_t *dict, char **op_errstr) char *statedir = NULL; char *path_list = NULL; char *conf_path = NULL; - gf_boolean_t exists = _gf_false; glusterd_volinfo_t *volinfo = NULL; char errmsg[PATH_MAX] = { 0, @@ -3666,14 +3657,12 @@ glusterd_op_stage_gsync_set(dict_t *dict, char **op_errstr) goto out; } - exists = glusterd_check_volume_exists(volname); ret = glusterd_volinfo_find(volname, &volinfo); - if ((ret) || (!exists)) { + if (ret) { snprintf(errmsg, sizeof(errmsg), "Volume name %s does not" " exist", volname); - ret = -1; goto out; } @@ -5118,7 +5107,6 @@ glusterd_get_gsync_status(dict_t *dict, char **op_errstr, dict_t *rsp_dict) char errmsg[PATH_MAX] = { 0, }; - gf_boolean_t exists = _gf_false; glusterd_volinfo_t *volinfo = NULL; int ret = 0; char my_hostname[256] = { @@ -5141,9 +5129,8 @@ glusterd_get_gsync_status(dict_t *dict, char **op_errstr, dict_t *rsp_dict) goto out; } - exists = glusterd_check_volume_exists(volname); ret = glusterd_volinfo_find(volname, &volinfo); - if ((ret) || (!exists)) { + if (ret) { gf_msg(this->name, GF_LOG_WARNING, 0, GD_MSG_VOL_NOT_FOUND, "volume name does not exist"); snprintf(errmsg, sizeof(errmsg), @@ -5151,7 +5138,6 @@ glusterd_get_gsync_status(dict_t *dict, char **op_errstr, dict_t *rsp_dict) " exist", volname); *op_errstr = gf_strdup(errmsg); - ret = -1; goto out; } diff --git a/xlators/mgmt/glusterd/src/glusterd-log-ops.c b/xlators/mgmt/glusterd/src/glusterd-log-ops.c index a202481d99a..5e0877e836c 100644 --- a/xlators/mgmt/glusterd/src/glusterd-log-ops.c +++ b/xlators/mgmt/glusterd/src/glusterd-log-ops.c @@ -105,7 +105,6 @@ glusterd_op_stage_log_rotate(dict_t *dict, char **op_errstr) int ret = -1; char *volname = NULL; glusterd_volinfo_t *volinfo = NULL; - gf_boolean_t exists = _gf_false; char msg[2048] = {0}; char *brick = NULL; @@ -116,13 +115,11 @@ glusterd_op_stage_log_rotate(dict_t *dict, char **op_errstr) goto out; } - exists = glusterd_check_volume_exists(volname); ret = glusterd_volinfo_find(volname, &volinfo); - if (!exists) { + if (ret) { snprintf(msg, sizeof(msg), "Volume %s does not exist", volname); gf_msg("glusterd", GF_LOG_ERROR, 0, GD_MSG_VOL_NOT_FOUND, "%s", msg); *op_errstr = gf_strdup(msg); - ret = -1; goto out; } diff --git a/xlators/mgmt/glusterd/src/glusterd-op-sm.c b/xlators/mgmt/glusterd/src/glusterd-op-sm.c index 115622d35c6..df761e9f948 100644 --- a/xlators/mgmt/glusterd/src/glusterd-op-sm.c +++ b/xlators/mgmt/glusterd/src/glusterd-op-sm.c @@ -1104,17 +1104,9 @@ glusterd_op_stage_set_volume(dict_t *dict, char **op_errstr) } if (strcasecmp(volname, "all") != 0) { - exists = glusterd_check_volume_exists(volname); - if (!exists) { - snprintf(errstr, sizeof(errstr), FMTSTR_CHECK_VOL_EXISTS, volname); - gf_msg(this->name, GF_LOG_ERROR, 0, GD_MSG_VOL_NOT_FOUND, "%s", - errstr); - ret = -1; - goto out; - } - ret = glusterd_volinfo_find(volname, &volinfo); if (ret) { + snprintf(errstr, sizeof(errstr), FMTSTR_CHECK_VOL_EXISTS, volname); gf_msg(this->name, GF_LOG_ERROR, 0, GD_MSG_VOL_NOT_FOUND, FMTSTR_CHECK_VOL_EXISTS, volname); goto out; @@ -1654,12 +1646,6 @@ glusterd_op_stage_reset_volume(dict_t *dict, char **op_errstr) } if (strcasecmp(volname, "all") != 0) { - exists = glusterd_check_volume_exists(volname); - if (!exists) { - snprintf(msg, sizeof(msg), FMTSTR_CHECK_VOL_EXISTS, volname); - ret = -1; - goto out; - } ret = glusterd_volinfo_find(volname, &volinfo); if (ret) { snprintf(msg, sizeof(msg), FMTSTR_CHECK_VOL_EXISTS, volname); @@ -1753,7 +1739,6 @@ glusterd_op_stage_sync_volume(dict_t *dict, char **op_errstr) char msg[2048] = { 0, }; - glusterd_volinfo_t *volinfo = NULL; ret = dict_get_strn(dict, "hostname", SLEN("hostname"), &hostname); if (ret) { @@ -1778,12 +1763,6 @@ glusterd_op_stage_sync_volume(dict_t *dict, char **op_errstr) ret = -1; goto out; } - ret = glusterd_volinfo_find(volname, &volinfo); - if (ret) - goto out; - - } else { - ret = 0; } } else { RCU_READ_LOCK; @@ -2008,7 +1987,6 @@ glusterd_op_stage_stats_volume(dict_t *dict, char **op_errstr) { int ret = -1; char *volname = NULL; - gf_boolean_t exists = _gf_false; char msg[2048] = { 0, }; @@ -2021,14 +1999,12 @@ glusterd_op_stage_stats_volume(dict_t *dict, char **op_errstr) goto out; } - exists = glusterd_check_volume_exists(volname); ret = glusterd_volinfo_find(volname, &volinfo); - if ((!exists) || (ret < 0)) { + if (ret) { snprintf(msg, sizeof(msg), "Volume %s, " "doesn't exist", volname); - ret = -1; goto out; } diff --git a/xlators/mgmt/glusterd/src/glusterd-quota.c b/xlators/mgmt/glusterd/src/glusterd-quota.c index 010e6beec86..a0848cdf0c5 100644 --- a/xlators/mgmt/glusterd/src/glusterd-quota.c +++ b/xlators/mgmt/glusterd/src/glusterd-quota.c @@ -2050,7 +2050,6 @@ glusterd_op_stage_quota(dict_t *dict, char **op_errstr, dict_t *rsp_dict) { int ret = 0; char *volname = NULL; - gf_boolean_t exists = _gf_false; int type = 0; xlator_t *this = NULL; glusterd_conf_t *priv = NULL; @@ -2074,12 +2073,6 @@ glusterd_op_stage_quota(dict_t *dict, char **op_errstr, dict_t *rsp_dict) goto out; } - exists = glusterd_check_volume_exists(volname); - if (!exists) { - gf_asprintf(op_errstr, FMTSTR_CHECK_VOL_EXISTS, volname); - ret = -1; - goto out; - } ret = glusterd_volinfo_find(volname, &volinfo); if (ret) { gf_asprintf(op_errstr, FMTSTR_CHECK_VOL_EXISTS, volname); diff --git a/xlators/mgmt/glusterd/src/glusterd-volume-ops.c b/xlators/mgmt/glusterd/src/glusterd-volume-ops.c index f9c7a2d70e9..5e8673640b6 100644 --- a/xlators/mgmt/glusterd/src/glusterd-volume-ops.c +++ b/xlators/mgmt/glusterd/src/glusterd-volume-ops.c @@ -1173,8 +1173,6 @@ glusterd_op_stage_create_volume(dict_t *dict, char **op_errstr, snprintf(msg, sizeof(msg), "Volume %s already exists", volname); ret = -1; goto out; - } else { - ret = 0; } ret = dict_get_int32n(dict, "count", SLEN("count"), &brick_count); @@ -1428,7 +1426,6 @@ glusterd_op_stage_start_volume(dict_t *dict, char **op_errstr, dict_t *rsp_dict) int flags = 0; int32_t brick_count = 0; int32_t local_brick_count = 0; - gf_boolean_t exists = _gf_false; glusterd_volinfo_t *volinfo = NULL; glusterd_brickinfo_t *brickinfo = NULL; char msg[2048] = { @@ -1458,16 +1455,9 @@ glusterd_op_stage_start_volume(dict_t *dict, char **op_errstr, dict_t *rsp_dict) if (ret) goto out; - exists = glusterd_check_volume_exists(volname); - - if (!exists) { - snprintf(msg, sizeof(msg), FMTSTR_CHECK_VOL_EXISTS, volname); - ret = -1; - goto out; - } - ret = glusterd_volinfo_find(volname, &volinfo); if (ret) { + snprintf(msg, sizeof(msg), FMTSTR_CHECK_VOL_EXISTS, volname); gf_msg(this->name, GF_LOG_ERROR, 0, GD_MSG_VOLINFO_GET_FAIL, FMTSTR_CHECK_VOL_EXISTS, volname); goto out; @@ -1637,7 +1627,6 @@ glusterd_op_stage_stop_volume(dict_t *dict, char **op_errstr) int ret = -1; char *volname = NULL; int flags = 0; - gf_boolean_t exists = _gf_false; glusterd_volinfo_t *volinfo = NULL; char msg[2048] = {0}; xlator_t *this = NULL; @@ -1652,15 +1641,6 @@ glusterd_op_stage_stop_volume(dict_t *dict, char **op_errstr) if (ret) goto out; - exists = glusterd_check_volume_exists(volname); - - if (!exists) { - snprintf(msg, sizeof(msg), FMTSTR_CHECK_VOL_EXISTS, volname); - gf_msg(this->name, GF_LOG_ERROR, 0, GD_MSG_VOL_NOT_FOUND, "%s", msg); - ret = -1; - goto out; - } - ret = glusterd_volinfo_find(volname, &volinfo); if (ret) { snprintf(msg, sizeof(msg), FMTSTR_CHECK_VOL_EXISTS, volname); @@ -1717,7 +1697,6 @@ glusterd_op_stage_delete_volume(dict_t *dict, char **op_errstr) { int ret = 0; char *volname = NULL; - gf_boolean_t exists = _gf_false; glusterd_volinfo_t *volinfo = NULL; char msg[2048] = {0}; xlator_t *this = NULL; @@ -1732,15 +1711,6 @@ glusterd_op_stage_delete_volume(dict_t *dict, char **op_errstr) goto out; } - exists = glusterd_check_volume_exists(volname); - if (!exists) { - snprintf(msg, sizeof(msg), FMTSTR_CHECK_VOL_EXISTS, volname); - ret = -1; - goto out; - } else { - ret = 0; - } - ret = glusterd_volinfo_find(volname, &volinfo); if (ret) { snprintf(msg, sizeof(msg), FMTSTR_CHECK_VOL_EXISTS, volname); -- cgit