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 <amukherj@redhat.com>

6 files changed, 23 insertions, 117 deletions

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);