glusterd: log changes in volume create and delete codepath

Making log changes involving two commands as they both share sections
of code (like the part where the volume metadata is cleaned up in vol
delete in case of success; and in vol create in case of failure).

* Most of the changes are of the 's/THIS/this' kind.

* Changed some of the log messages to give as much information as
  available in case of failure.

* Changed log levels in some of the log messages.

Change-Id: I10242511fe9400a07ab04717464d748d9172dd85
BUG: 812356
Signed-off-by: Krutika Dhananjay <kdhananj@redhat.com>
Reviewed-on: http://review.gluster.org/4462
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Anand Avati <avati@redhat.com>

4 files changed, 215 insertions, 152 deletions

diff --git a/xlators/mgmt/glusterd/src/glusterd-store.c b/xlators/mgmt/glusterd/src/glusterd-store.c
index 6f67a1bfc80..863b70c7ccb 100644
--- a/xlators/mgmt/glusterd/src/glusterd-store.c
+++ b/xlators/mgmt/glusterd/src/glusterd-store.c
@@ -52,7 +52,7 @@ glusterd_store_mkdir (char *path)
         ret = mkdir (path, 0777);
 
         if ((-1 == ret) && (EEXIST != errno)) {
-                gf_log ("", GF_LOG_ERROR, "mkdir() failed on path %s,"
+                gf_log (THIS->name, GF_LOG_ERROR, "mkdir() failed on path %s,"
                         "errno: %s", path, strerror (errno));
         } else {
                 ret = 0;
@@ -72,8 +72,8 @@ glusterd_store_handle_create_on_absence (glusterd_store_handle_t **shandle,
                 ret = glusterd_store_handle_new (path, shandle);
 
                 if (ret) {
-                        gf_log ("", GF_LOG_ERROR, "Unable to create store"
-                                " handle for path: %s", path);
+                        gf_log (THIS->name, GF_LOG_ERROR, "Unable to create "
+                                "store handle for path: %s", path);
                 }
         }
         return ret;
@@ -91,7 +91,7 @@ glusterd_store_mkstemp (glusterd_store_handle_t *shandle)
         snprintf (tmppath, sizeof (tmppath), "%s.tmp", shandle->path);
         fd = open (tmppath, O_RDWR | O_CREAT | O_TRUNC | O_SYNC, 0600);
         if (fd <= 0) {
-                gf_log ("glusterd", GF_LOG_ERROR, "Failed to open %s, "
+                gf_log (THIS->name, GF_LOG_ERROR, "Failed to open %s, "
                         "error: %s", tmppath, strerror (errno));
         }
 
@@ -178,7 +178,7 @@ glusterd_store_unlink_tmppath (glusterd_store_handle_t *shandle)
         snprintf (tmppath, sizeof (tmppath), "%s.tmp", shandle->path);
         ret = unlink (tmppath);
         if (ret && (errno != ENOENT)) {
-                gf_log ("glusterd", GF_LOG_ERROR, "Failed to mv %s to %s, "
+                gf_log (THIS->name, GF_LOG_ERROR, "Failed to mv %s to %s, "
                         "error: %s", tmppath, shandle->path, strerror (errno));
         } else {
                 ret = 0;
@@ -275,21 +275,26 @@ glusterd_store_is_valid_brickpath (char *volname, char *brick)
         glusterd_volinfo_t      *volinfo = NULL;
         int32_t                 ret = 0;
         size_t                  volname_len = strlen (volname);
+        xlator_t                *this = NULL;
+
+        this = THIS;
+        GF_ASSERT (this);
 
         ret = glusterd_brickinfo_new_from_brick (brick, &brickinfo);
         if (ret) {
-                gf_log ("", GF_LOG_WARNING, "brick path validation failed");
+                gf_log (this->name, GF_LOG_WARNING, "Failed to create brick "
+                        "info for brick %s", brick);
                 ret = 0;
                 goto out;
         }
         ret = glusterd_volinfo_new (&volinfo);
         if (ret) {
-                gf_log ("", GF_LOG_WARNING, "brick path validation failed");
+                gf_log (this->name, GF_LOG_WARNING, "Failed to create volinfo");
                 ret = 0;
                 goto out;
         }
         if (volname_len >= sizeof (volinfo->volname)) {
-                gf_log ("", GF_LOG_WARNING, "volume name too long");
+                gf_log (this->name, GF_LOG_WARNING, "volume name too long");
                 ret = 0;
                 goto out;
         }
@@ -380,7 +385,7 @@ glusterd_store_brickinfo_write (int fd, glusterd_brickinfo_t *brickinfo)
                 goto out;
 
 out:
-        gf_log ("", GF_LOG_DEBUG, "Returning %d", ret);
+        gf_log (THIS->name, GF_LOG_DEBUG, "Returning %d", ret);
         return ret;
 }
 
@@ -406,7 +411,7 @@ out:
                 glusterd_store_unlink_tmppath (brickinfo->shandle);
         if (fd > 0)
                 close (fd);
-        gf_log ("", GF_LOG_DEBUG, "Returning %d", ret);
+        gf_log (THIS->name, GF_LOG_DEBUG, "Returning %d", ret);
         return ret;
 }
 
@@ -436,7 +441,7 @@ glusterd_store_brickinfo (glusterd_volinfo_t *volinfo,
 
         ret = glusterd_store_perform_brick_store (brickinfo);
 out:
-        gf_log ("", GF_LOG_DEBUG, "Returning with %d", ret);
+        gf_log (THIS->name, GF_LOG_DEBUG, "Returning with %d", ret);
         return ret;
 }
 
@@ -507,6 +512,10 @@ glusterd_store_remove_bricks (glusterd_volinfo_t *volinfo)
         DIR                     *dir = NULL;
         struct dirent           *entry = NULL;
         char                    path[PATH_MAX] = {0,};
+        xlator_t                *this = NULL;
+
+        this = THIS;
+        GF_ASSERT (this);
 
         GF_ASSERT (volinfo);
 
@@ -516,7 +525,7 @@ glusterd_store_remove_bricks (glusterd_volinfo_t *volinfo)
                         goto out;
         }
 
-        priv = THIS->private;
+        priv = this->private;
         GF_ASSERT (priv);
 
         GLUSTERD_GET_BRICK_DIR (brickdir, volinfo, priv);
@@ -530,7 +539,7 @@ glusterd_store_remove_bricks (glusterd_volinfo_t *volinfo)
                           brickdir, entry->d_name);
                 ret = unlink (path);
                 if (ret && errno != ENOENT) {
-                        gf_log ("", GF_LOG_ERROR, "Unable to unlink %s, "
+                        gf_log (this->name, GF_LOG_ERROR, "Unable to unlink %s, "
                                 "reason: %s", path, strerror(errno));
                 }
                 glusterd_for_each_entry (entry, dir);
@@ -541,7 +550,7 @@ glusterd_store_remove_bricks (glusterd_volinfo_t *volinfo)
         ret = rmdir (brickdir);
 
 out:
-        gf_log ("", GF_LOG_DEBUG, "Returning with %d", ret);
+        gf_log (this->name, GF_LOG_DEBUG, "Returning with %d", ret);
         return ret;
 }
 
@@ -550,6 +559,10 @@ _storeslaves (dict_t *this, char *key, data_t *value, void *data)
 {
         int32_t                      ret = 0;
         glusterd_store_handle_t  *shandle = NULL;
+        xlator_t                 *xl = NULL;
+
+        xl = THIS;
+        GF_ASSERT (xl);
 
         shandle = (glusterd_store_handle_t*)data;
 
@@ -567,12 +580,12 @@ _storeslaves (dict_t *this, char *key, data_t *value, void *data)
         if (!value || !value->data)
                 return -1;
 
-        gf_log ("", GF_LOG_DEBUG, "Storing in volinfo:key= %s, val=%s",
+        gf_log (xl->name, GF_LOG_DEBUG, "Storing in volinfo:key= %s, val=%s",
                 key, value->data);
 
         ret = glusterd_store_save_value (shandle->fd, key, (char*)value->data);
         if (ret) {
-                gf_log ("", GF_LOG_ERROR, "Unable to write into store"
+                gf_log (xl->name, GF_LOG_ERROR, "Unable to write into store"
                                 " handle for path: %s", shandle->path);
                 return -1;
         }
@@ -585,6 +598,10 @@ int _storeopts (dict_t *this, char *key, data_t *value, void *data)
         int32_t                      ret = 0;
         int32_t                      exists = 0;
         glusterd_store_handle_t  *shandle = NULL;
+        xlator_t                 *xl = NULL;
+
+        xl = THIS;
+        GF_ASSERT (xl);
 
         shandle = (glusterd_store_handle_t*)data;
 
@@ -610,19 +627,19 @@ int _storeopts (dict_t *this, char *key, data_t *value, void *data)
         }
 
         if (1 == exists) {
-                gf_log ("", GF_LOG_DEBUG, "Storing in volinfo:key= %s, val=%s",
-                        key, value->data);
+                gf_log (xl->name, GF_LOG_DEBUG, "Storing in volinfo:key= %s, "
+                        "val=%s", key, value->data);
 
         } else {
-                gf_log ("", GF_LOG_DEBUG, "Discarding:key= %s, val=%s",
+                gf_log (xl->name, GF_LOG_DEBUG, "Discarding:key= %s, val=%s",
                         key, value->data);
                 return 0;
         }
 
         ret = glusterd_store_save_value (shandle->fd, key, (char*)value->data);
         if (ret) {
-                gf_log ("", GF_LOG_ERROR, "Unable to write into store"
-                                " handle for path: %s", shandle->path);
+                gf_log (xl->name, GF_LOG_ERROR, "Unable to write into store"
+                        " handle for path: %s", shandle->path);
                 return -1;
         }
         return 0;
@@ -717,8 +734,8 @@ glusterd_volume_exclude_options_write (int fd, glusterd_volinfo_t *volinfo)
 
 out:
         if (ret)
-                gf_log ("", GF_LOG_ERROR, "Unable to write volume values"
-                        " for %s", volinfo->volname);
+                gf_log (THIS->name, GF_LOG_ERROR, "Unable to write volume "
+                        "values for %s", volinfo->volname);
         return ret;
 }
 
@@ -747,7 +764,7 @@ glusterd_store_create_volume_dir (glusterd_volinfo_t *volinfo)
         glusterd_store_voldirpath_set (volinfo, voldirpath,
                                        sizeof (voldirpath));
         ret = glusterd_store_mkdir (voldirpath);
-        gf_log ("", GF_LOG_DEBUG, "Returning with %d", ret);
+        gf_log (THIS->name, GF_LOG_DEBUG, "Returning with %d", ret);
         return ret;
 }
 
@@ -771,7 +788,7 @@ glusterd_store_volinfo_write (int fd, glusterd_volinfo_t *volinfo)
         dict_foreach (volinfo->gsync_slaves, _storeslaves, shandle);
         shandle->fd = 0;
 out:
-        gf_log ("", GF_LOG_DEBUG, "Returning %d", ret);
+        gf_log (THIS->name, GF_LOG_DEBUG, "Returning %d", ret);
         return ret;
 }
 
@@ -881,7 +898,7 @@ glusterd_store_brickinfos (glusterd_volinfo_t *volinfo, int vol_fd)
                 brick_count++;
         }
 out:
-        gf_log ("", GF_LOG_DEBUG, "Returning %d", ret);
+        gf_log (THIS->name, GF_LOG_DEBUG, "Returning %d", ret);
         return ret;
 }
 
@@ -972,7 +989,7 @@ out:
                 glusterd_store_unlink_tmppath (volinfo->rb_shandle);
         if (fd > 0)
                 close (fd);
-        gf_log ("", GF_LOG_DEBUG, "Returning %d", ret);
+        gf_log (THIS->name, GF_LOG_DEBUG, "Returning %d", ret);
         return ret;
 }
 
@@ -1039,7 +1056,7 @@ out:
                 glusterd_store_unlink_tmppath (volinfo->node_state_shandle);
         if (fd > 0)
                 close (fd);
-        gf_log ("", GF_LOG_DEBUG, "Returning %d", ret);
+        gf_log (THIS->name, GF_LOG_DEBUG, "Returning %d", ret);
         return ret;
 }
 
@@ -1069,7 +1086,7 @@ out:
                 glusterd_store_unlink_tmppath (volinfo->shandle);
         if (fd > 0)
                 close (fd);
-        gf_log ("", GF_LOG_DEBUG, "Returning %d", ret);
+        gf_log (THIS->name, GF_LOG_DEBUG, "Returning %d", ret);
         return ret;
 }
 
@@ -1219,7 +1236,7 @@ out:
         if (ret)
                 glusterd_store_volume_cleanup_tmp (volinfo);
 
-        gf_log ("", GF_LOG_DEBUG, "Returning %d", ret);
+        gf_log (THIS->name, GF_LOG_DEBUG, "Returning %d", ret);
 
         return ret;
 }
@@ -1235,21 +1252,28 @@ glusterd_store_delete_volume (glusterd_volinfo_t *volinfo)
         struct dirent *entry = NULL;
         char path[PATH_MAX] = {0,};
         struct stat     st = {0, };
+        xlator_t        *this = NULL;
+
+        this = THIS;
+        GF_ASSERT (this);
 
         GF_ASSERT (volinfo);
-        priv = THIS->private;
+        priv = this->private;
 
         GF_ASSERT (priv);
         snprintf (pathname, sizeof (pathname), "%s/vols/%s", priv->workdir,
                   volinfo->volname);
 
         dir = opendir (pathname);
-        if (!dir)
+        if (!dir) {
+                gf_log (this->name, GF_LOG_ERROR, "Failed to open directory %s."
+                        " Reason : %s", pathname, strerror (errno));
                 goto out;
+        }
         ret = glusterd_store_remove_bricks (volinfo);
 
         if (ret) {
-                gf_log ("", GF_LOG_ERROR, "Remove bricks failed for %s",
+                gf_log (this->name, GF_LOG_ERROR, "Remove bricks failed for %s",
                         volinfo->volname);
         }
 
@@ -1259,8 +1283,8 @@ glusterd_store_delete_volume (glusterd_volinfo_t *volinfo)
                 snprintf (path, PATH_MAX, "%s/%s", pathname, entry->d_name);
                 ret = stat (path, &st);
                 if (ret == -1) {
-                        gf_log ("", GF_LOG_ERROR, "Failed to stat entry: %s:%s",
-                                path, strerror (errno));
+                        gf_log (this->name, GF_LOG_ERROR, "Failed to stat "
+                                "entry %s : %s", path, strerror (errno));
                         goto stat_failed;
                 }
 
@@ -1270,11 +1294,11 @@ glusterd_store_delete_volume (glusterd_volinfo_t *volinfo)
                         ret = unlink (path);
 
                 if (ret)
-                        gf_log ("", GF_LOG_INFO, "errno:%d (%s)", errno,
-                                strerror (errno));
+                        gf_log (this->name, GF_LOG_ERROR, " Failed to remove "
+                                "%s. Reason : %s", path, strerror (errno));
 
-                gf_log ("", GF_LOG_INFO, "%s %s",
-                                ret?"Failed to remove":"Removed",
+                gf_log (this->name, ret ? GF_LOG_ERROR : GF_LOG_DEBUG, "%s %s",
+                                ret ? "Failed to remove":"Removed",
                                 entry->d_name);
 stat_failed:
                 memset (path, 0, sizeof(path));
@@ -1283,13 +1307,13 @@ stat_failed:
 
         ret = closedir (dir);
         if (ret) {
-                gf_log ("", GF_LOG_ERROR, "Failed to close dir, errno:%d",
-                        errno);
+                gf_log (this->name, GF_LOG_ERROR, "Failed to close dir %s. "
+                        "Reason : %s",pathname, strerror (errno));
         }
 
         ret = rmdir (pathname);
         if (ret) {
-                gf_log ("", GF_LOG_ERROR, "Failed to rmdir: %s, err: %s",
+                gf_log (this->name, GF_LOG_ERROR, "Failed to rmdir: %s, err: %s",
                         pathname, strerror (errno));
         }
 
@@ -1299,7 +1323,7 @@ out:
                 glusterd_store_handle_destroy (volinfo->shandle);
                 volinfo->shandle = NULL;
         }
-        gf_log ("", GF_LOG_DEBUG, "Returning %d", ret);
+        gf_log (this->name, GF_LOG_DEBUG, "Returning %d", ret);
 
         return ret;
 }
@@ -1429,21 +1453,24 @@ glusterd_store_save_value (int fd, char *key, char *value)
 {
         int32_t         ret = -1;
         FILE           *fp  = NULL;
+        xlator_t       *this = NULL;
 
+        this = THIS;
+        GF_ASSERT (this);
         GF_ASSERT (fd > 0);
         GF_ASSERT (key);
         GF_ASSERT (value);
 
         fp = fdopen (fd, "a+");
         if (fp == NULL) {
-                gf_log ("", GF_LOG_WARNING, "fdopen failed.");
+                gf_log (this->name, GF_LOG_WARNING, "fdopen failed.");
                 ret = -1;
                 goto out;
         }
 
         ret = fprintf (fp, "%s=%s\n", key, value);
         if (ret < 0) {
-                gf_log ("", GF_LOG_WARNING, "Unable to store key: %s,"
+                gf_log (this->name, GF_LOG_WARNING, "Unable to store key: %s,"
                         "value: %s, error: %s", key, value,
                         strerror (errno));
                 ret = -1;
@@ -1452,7 +1479,7 @@ glusterd_store_save_value (int fd, char *key, char *value)
 
         ret = fflush (fp);
         if (feof (fp)) {
-                gf_log ("", GF_LOG_WARNING,
+                gf_log (this->name, GF_LOG_WARNING,
                         "fflush failed, error: %s",
                         strerror (errno));
                 ret = -1;
@@ -1462,7 +1489,7 @@ glusterd_store_save_value (int fd, char *key, char *value)
         ret = 0;
 out:
 
-        gf_log ("", GF_LOG_DEBUG, "returning: %d", ret);
+        gf_log (this->name, GF_LOG_DEBUG, "returning: %d", ret);
         return ret;
 }
 
@@ -1473,6 +1500,10 @@ glusterd_store_handle_new (char *path, glusterd_store_handle_t **handle)
         glusterd_store_handle_t *shandle = NULL;
         int                     fd = -1;
         char                    *spath = NULL;
+        xlator_t                *this = NULL;
+
+        this = THIS;
+        GF_ASSERT (this);
 
         shandle = GF_CALLOC (1, sizeof (*shandle), gf_gld_mt_store_handle_t);
         if (!shandle)
@@ -1485,7 +1516,7 @@ glusterd_store_handle_new (char *path, glusterd_store_handle_t **handle)
 
         fd = open (path, O_RDWR | O_CREAT | O_APPEND, 0600);
         if (fd <= 0) {
-                gf_log ("glusterd", GF_LOG_ERROR, "Failed to open file: %s, "
+                gf_log (this->name, GF_LOG_ERROR, "Failed to open file: %s, "
                         "error: %s", path, strerror (errno));
                 goto out;
         }
@@ -1507,7 +1538,7 @@ out:
                 GF_FREE (shandle);
         }
 
-        gf_log ("", GF_LOG_DEBUG, "Returning %d", ret);
+        gf_log (this->name, GF_LOG_DEBUG, "Returning %d", ret);
         return ret;
 }
 
@@ -1546,7 +1577,7 @@ glusterd_store_handle_destroy (glusterd_store_handle_t *handle)
         ret = 0;
 
 out:
-        gf_log ("", GF_LOG_DEBUG, "Returning %d", ret);
+        gf_log (THIS->name, GF_LOG_DEBUG, "Returning %d", ret);
 
         return ret;
 }
diff --git a/xlators/mgmt/glusterd/src/glusterd-utils.c b/xlators/mgmt/glusterd/src/glusterd-utils.c
index f99758d5f51..285f8cc662f 100644
--- a/xlators/mgmt/glusterd/src/glusterd-utils.c
+++ b/xlators/mgmt/glusterd/src/glusterd-utils.c
@@ -588,7 +588,7 @@ glusterd_check_volume_exists (char *volname)
         ret = stat (pathname, &stbuf);
 
         if (ret) {
-                gf_log ("", GF_LOG_DEBUG, "Volume %s does not exist."
+                gf_log (THIS->name, GF_LOG_DEBUG, "Volume %s does not exist."
                         "stat failed with errno : %d on path: %s",
                         volname, errno, pathname);
                 return _gf_false;
@@ -635,7 +635,7 @@ glusterd_volinfo_new (glusterd_volinfo_t **volinfo)
         ret = 0;
 
 out:
-        gf_log ("", GF_LOG_DEBUG, "Returning %d", ret);
+        gf_log (THIS->name, GF_LOG_DEBUG, "Returning %d", ret);
         return ret;
 }
 
@@ -719,7 +719,7 @@ glusterd_volume_brickinfos_delete (glusterd_volinfo_t *volinfo)
         }
 
 out:
-        gf_log ("", GF_LOG_DEBUG, "Returning %d", ret);
+        gf_log (THIS->name, GF_LOG_DEBUG, "Returning %d", ret);
         return ret;
 }
 
@@ -747,7 +747,7 @@ glusterd_volinfo_delete (glusterd_volinfo_t *volinfo)
         ret = 0;
 
 out:
-        gf_log ("", GF_LOG_DEBUG, "Returning %d", ret);
+        gf_log (THIS->name, GF_LOG_DEBUG, "Returning %d", ret);
         return ret;
 }
 
@@ -773,7 +773,7 @@ glusterd_brickinfo_new (glusterd_brickinfo_t **brickinfo)
         ret = 0;
 
 out:
-        gf_log ("", GF_LOG_DEBUG, "Returning %d", ret);
+        gf_log (THIS->name, GF_LOG_DEBUG, "Returning %d", ret);
         return ret;
 }
 
@@ -835,7 +835,7 @@ out:
         GF_FREE (tmp_host);
         if (tmp_host)
                 GF_FREE (tmp_path);
-        gf_log ("", GF_LOG_DEBUG, "Returning %d", ret);
+        gf_log (THIS->name, GF_LOG_DEBUG, "Returning %d", ret);
         return ret;
 }
 
@@ -1682,8 +1682,8 @@ glusterd_volume_compute_cksum (glusterd_volinfo_t  *volinfo)
 
         sort_fd = mkstemp (sort_filepath);
         if (sort_fd < 0) {
-                gf_log (this->name, GF_LOG_ERROR, "Could not generate temp file, "
-                        "reason: %s for volume: %s", strerror (errno),
+                gf_log (this->name, GF_LOG_ERROR, "Could not generate temp "
+                        "file, reason: %s for volume: %s", strerror (errno),
                         volinfo->volname);
                 goto out;
         } else {
@@ -4649,11 +4649,14 @@ glusterd_friend_find_by_uuid (uuid_t uuid,
         int                     ret = -1;
         glusterd_conf_t         *priv = NULL;
         glusterd_peerinfo_t     *entry = NULL;
+        xlator_t                *this = NULL;
 
+        this = THIS;
+        GF_ASSERT (this);
         GF_ASSERT (peerinfo);
 
         *peerinfo = NULL;
-        priv    = THIS->private;
+        priv    = this->private;
 
         GF_ASSERT (priv);
 
@@ -4663,7 +4666,7 @@ glusterd_friend_find_by_uuid (uuid_t uuid,
         list_for_each_entry (entry, &priv->peers, uuid_list) {
                 if (!uuid_compare (entry->uuid, uuid)) {
 
-                        gf_log ("glusterd", GF_LOG_DEBUG,
+                        gf_log (this->name, GF_LOG_DEBUG,
                                  "Friend found... state: %s",
                         glusterd_friend_sm_state_name_get (entry->state.state));
                         *peerinfo = entry;
@@ -4671,7 +4674,7 @@ glusterd_friend_find_by_uuid (uuid_t uuid,
                 }
         }
 
-        gf_log ("glusterd", GF_LOG_DEBUG, "Friend with uuid: %s, not found",
+        gf_log (this->name, GF_LOG_DEBUG, "Friend with uuid: %s, not found",
                 uuid_utoa (uuid));
         return ret;
 }
@@ -4899,7 +4902,6 @@ glusterd_new_brick_validate (char *brick, glusterd_brickinfo_t *brickinfo,
         if (ret) {
                 snprintf(op_errstr, len, "Host %s is not in \'Peer "
                          "in Cluster\' state", newbrickinfo->hostname);
-                gf_log (this->name, GF_LOG_ERROR, "%s", op_errstr);
                 goto out;
         }
 
@@ -4910,7 +4912,6 @@ glusterd_new_brick_validate (char *brick, glusterd_brickinfo_t *brickinfo,
                         snprintf(op_errstr, len, "Brick: %s not available."
                                  " Brick may be containing or be contained "
                                  "by an existing brick", brick);
-                        gf_log (this->name, GF_LOG_ERROR, "%s", op_errstr);
                         ret = -1;
                         goto out;
                 }
@@ -4918,13 +4919,15 @@ glusterd_new_brick_validate (char *brick, glusterd_brickinfo_t *brickinfo,
         } else {
                 ret = glusterd_friend_find_by_uuid (newbrickinfo->uuid,
                                                     &peerinfo);
-                if (ret)
+                if (ret) {
+                        snprintf (op_errstr, len, "Failed to find host %s",
+                                  newbrickinfo->hostname);
                         goto out;
+                }
 
                 if ((!peerinfo->connected)) {
                         snprintf(op_errstr, len, "Host %s not connected",
                                  newbrickinfo->hostname);
-                        gf_log (this->name, GF_LOG_ERROR, "%s", op_errstr);
                         ret = -1;
                         goto out;
                 }
@@ -4933,7 +4936,6 @@ glusterd_new_brick_validate (char *brick, glusterd_brickinfo_t *brickinfo,
                         snprintf(op_errstr, len, "Host %s is not in \'Peer "
                                  "in Cluster\' state",
                                  newbrickinfo->hostname);
-                        gf_log (this->name, GF_LOG_ERROR, "%s", op_errstr);
                         ret = -1;
                         goto out;
                 }
@@ -4943,6 +4945,8 @@ glusterd_new_brick_validate (char *brick, glusterd_brickinfo_t *brickinfo,
 out:
         if (is_allocated)
                 glusterd_brickinfo_delete (newbrickinfo);
+        if (op_errstr[0] != '\0')
+                gf_log (this->name, GF_LOG_ERROR, "%s", op_errstr);
         gf_log (this->name, GF_LOG_DEBUG, "returning %d ", ret);
         return ret;
 }
@@ -5466,7 +5470,7 @@ glusterd_delete_volume (glusterd_volinfo_t *volinfo)
 
         ret = glusterd_volinfo_delete (volinfo);
 out:
-        gf_log ("", GF_LOG_DEBUG, "returning %d", ret);
+        gf_log (THIS->name, GF_LOG_DEBUG, "returning %d", ret);
         return ret;
 }
 
diff --git a/xlators/mgmt/glusterd/src/glusterd-volgen.c b/xlators/mgmt/glusterd/src/glusterd-volgen.c
index 811ff849428..01dbd8c79f2 100644
--- a/xlators/mgmt/glusterd/src/glusterd-volgen.c
+++ b/xlators/mgmt/glusterd/src/glusterd-volgen.c
@@ -3191,16 +3191,21 @@ glusterd_is_valid_volfpath (char *volname, char *brick)
         glusterd_brickinfo_t    *brickinfo = NULL;
         glusterd_volinfo_t      *volinfo = NULL;
         int32_t                 ret = 0;
+        xlator_t                *this = NULL;
+
+        this = THIS;
+        GF_ASSERT (this);
 
         ret = glusterd_brickinfo_new_from_brick (brick, &brickinfo);
         if (ret) {
-                gf_log ("", GF_LOG_WARNING, "brick path validation failed");
+                gf_log (this->name, GF_LOG_WARNING, "Failed to create brickinfo"
+                        " for brick %s", brick );
                 ret = 0;
                 goto out;
         }
         ret = glusterd_volinfo_new (&volinfo);
         if (ret) {
-                gf_log ("", GF_LOG_WARNING, "brick path validation failed");
+                gf_log (this->name, GF_LOG_WARNING, "Failed to create volinfo");
                 ret = 0;
                 goto out;
         }
diff --git a/xlators/mgmt/glusterd/src/glusterd-volume-ops.c b/xlators/mgmt/glusterd/src/glusterd-volume-ops.c
index 9aa8df61dd1..246a7a6f1a8 100644
--- a/xlators/mgmt/glusterd/src/glusterd-volume-ops.c
+++ b/xlators/mgmt/glusterd/src/glusterd-volume-ops.c
@@ -67,13 +67,14 @@ glusterd_handle_create_volume (rpcsvc_request_t *req)
         ret = -1;
         ret = xdr_to_generic (req->msg[0], &cli_req, (xdrproc_t)xdr_gf_cli_req);
         if (ret < 0) {
-                //failed to decode msg;
                 req->rpc_err = GARBAGE_ARGS;
-                snprintf (err_str, sizeof (err_str), "Garbage args received");
+                snprintf (err_str, sizeof (err_str), "Failed to decode request "
+                          "received from cli");
+                gf_log (this->name, GF_LOG_ERROR, "%s", err_str);
                 goto out;
         }
 
-        gf_log (this->name, GF_LOG_INFO, "Received create volume req");
+        gf_log (this->name, GF_LOG_DEBUG, "Received create volume req");
 
         if (cli_req.dict.dict_len) {
                 /* Unserialize the dictionary */
@@ -112,31 +113,31 @@ glusterd_handle_create_volume (rpcsvc_request_t *req)
 
         ret = dict_get_int32 (dict, "count", &brick_count);
         if (ret) {
-                snprintf (err_str, sizeof (err_str), "Unable to get volume "
-                          "brick count");
+                snprintf (err_str, sizeof (err_str), "Unable to get brick count"
+                          " for volume %s", volname);
                 gf_log (this->name, GF_LOG_ERROR, "%s", err_str);
                 goto out;
         }
 
         ret = dict_get_int32 (dict, "type", &type);
         if (ret) {
-                snprintf (err_str, sizeof (err_str), "Unable to get volume "
-                          "type");
+                snprintf (err_str, sizeof (err_str), "Unable to get type of "
+                          "volume %s", volname);
                 gf_log (this->name, GF_LOG_ERROR, "%s", err_str);
                 goto out;
         }
 
         ret = dict_get_str (dict, "transport", &trans_type);
         if (ret) {
-                snprintf (err_str, sizeof (err_str), "Unable to get volume "
-                          "transport-type");
+                snprintf (err_str, sizeof (err_str), "Unable to get "
+                          "transport-type of volume %s", volname);
                 gf_log (this->name, GF_LOG_ERROR, "%s", err_str);
                 goto out;
         }
         ret = dict_get_str (dict, "bricks", &bricks);
         if (ret) {
-                snprintf (err_str, sizeof (err_str), "Unable to get volume "
-                          "bricks");
+                snprintf (err_str, sizeof (err_str), "Unable to get bricks for "
+                          "volume %s", volname);
                 gf_log (this->name, GF_LOG_ERROR, "%s", err_str);
                 goto out;
         }
@@ -146,7 +147,7 @@ glusterd_handle_create_volume (rpcsvc_request_t *req)
         ret = dict_set_dynstr (dict, "volume-id", free_ptr);
         if (ret) {
                 snprintf (err_str, sizeof (err_str), "Unable to set volume "
-                          "id");
+                          "id of volume %s", volname);
                 gf_log (this->name, GF_LOG_ERROR, "%s", err_str);
                 goto out;
         }
@@ -163,8 +164,8 @@ glusterd_handle_create_volume (rpcsvc_request_t *req)
                 brick_list = tmpptr;
                 ret = glusterd_brickinfo_new_from_brick (brick, &brickinfo);
                 if (ret) {
-                        snprintf (err_str, sizeof (err_str), "Unable to get "
-                                  "brick info from brick %s", brick);
+                        snprintf (err_str, sizeof (err_str), "Failed to create "
+                                  "brickinfo for brick %s", brick);
                         goto out;
                 }
 
@@ -182,14 +183,20 @@ glusterd_handle_create_volume (rpcsvc_request_t *req)
         uuid_generate (tmp_uuid);
         username = gf_strdup (uuid_utoa (tmp_uuid));
         ret = dict_set_dynstr (dict, "internal-username", username);
-        if (ret)
+        if (ret) {
+                gf_log (this->name, GF_LOG_ERROR, "Failed to set username for "
+                        "volume %s", volname);
                 goto out;
+        }
 
         uuid_generate (tmp_uuid);
         password = gf_strdup (uuid_utoa (tmp_uuid));
         ret = dict_set_dynstr (dict, "internal-password", password);
-        if (ret)
+        if (ret) {
+                gf_log (this->name, GF_LOG_ERROR, "Failed to set password for "
+                        "volume %s", volname);
                 goto out;
+        }
 
         ret = glusterd_op_begin (req, GD_OP_CREATE_VOLUME, dict, err_str,
                                  sizeof (err_str));
@@ -377,7 +384,9 @@ glusterd_handle_cli_delete_volume (rpcsvc_request_t *req)
 
         ret = xdr_to_generic (req->msg[0], &cli_req, (xdrproc_t)xdr_gf_cli_req);
         if (ret < 0) {
-                //failed to decode msg;
+                snprintf (err_str, sizeof (err_str), "Failed to decode request "
+                          "received from cli");
+                gf_log (this->name, GF_LOG_ERROR, "%s", err_str);
                 req->rpc_err = GARBAGE_ARGS;
                 goto out;
         }
@@ -394,7 +403,7 @@ glusterd_handle_cli_delete_volume (rpcsvc_request_t *req)
                                 "failed to "
                                 "unserialize req-buffer to dictionary");
                         snprintf (err_str, sizeof (err_str), "Unable to decode "
-                                   "the command");
+                                  "the command");
                         goto out;
                 }
         }
@@ -408,7 +417,7 @@ glusterd_handle_cli_delete_volume (rpcsvc_request_t *req)
                 goto out;
         }
 
-        gf_log (this->name, GF_LOG_INFO, "Received delete vol req"
+        gf_log (this->name, GF_LOG_DEBUG, "Received delete vol req"
                 "for volume %s", volname);
 
         ret = glusterd_op_begin (req, GD_OP_DELETE_VOLUME, dict,
@@ -649,7 +658,6 @@ glusterd_op_stage_create_volume (dict_t *dict, char **op_errstr)
         int32_t                                 i = 0;
         char                                    *brick = NULL;
         char                                    *tmpptr = NULL;
-        char                                    cmd_str[1024];
         xlator_t                                *this = NULL;
         glusterd_conf_t                         *priv = NULL;
         char                                    msg[2048] = {0};
@@ -659,62 +667,55 @@ glusterd_op_stage_create_volume (dict_t *dict, char **op_errstr)
         char                                    *dev_type = NULL;
 #endif
         this = THIS;
-        if (!this) {
-                gf_log ("glusterd", GF_LOG_ERROR,
-                        "this is NULL");
-                goto out;
-        }
-
+        GF_ASSERT (this);
         priv = this->private;
-        if (!priv) {
-                gf_log ("glusterd", GF_LOG_ERROR,
-                        "priv is NULL");
-                goto out;
-        }
+        GF_ASSERT (priv);
 
         ret = dict_get_str (dict, "volname", &volname);
-
         if (ret) {
-                gf_log ("", GF_LOG_ERROR, "Unable to get volume name");
+                gf_log (this->name, GF_LOG_ERROR, "Unable to get volume name");
                 goto out;
         }
 
         exists = glusterd_check_volume_exists (volname);
-
         if (exists) {
                 snprintf (msg, sizeof (msg), "Volume %s already exists",
                           volname);
-                gf_log ("", GF_LOG_ERROR, "%s", msg);
-                *op_errstr = gf_strdup (msg);
                 ret = -1;
                 goto out;
         } else {
                 ret = 0;
         }
+
         ret = dict_get_int32 (dict, "count", &brick_count);
         if (ret) {
-                gf_log ("", GF_LOG_ERROR, "Unable to get count");
+                gf_log (this->name, GF_LOG_ERROR, "Unable to get brick count "
+                        "for volume %s", volname);
                 goto out;
         }
+
         ret = dict_get_str (dict, "volume-id", &volume_uuid_str);
         if (ret) {
-                gf_log ("", GF_LOG_ERROR, "Unable to get volume id");
+                gf_log (this->name, GF_LOG_ERROR, "Unable to get volume id of "
+                        "volume %s", volname);
                 goto out;
         }
+
         ret = uuid_parse (volume_uuid_str, volume_uuid);
         if (ret) {
-                gf_log ("", GF_LOG_ERROR, "Unable to parse volume id");
+                gf_log (this->name, GF_LOG_ERROR, "Unable to parse volume id of"
+                        " volume %s", volname);
                 goto out;
         }
 
-
 #ifdef HAVE_BD_XLATOR
         ret = dict_get_str (dict, "device", &dev_type);
 #endif
 
         ret = dict_get_str (dict, "bricks", &bricks);
         if (ret) {
-                gf_log ("", GF_LOG_ERROR, "Unable to get bricks");
+                gf_log (this->name, GF_LOG_ERROR, "Unable to get bricks for "
+                        "volume %s", volname);
                 goto out;
         }
 
@@ -722,7 +723,6 @@ glusterd_op_stage_create_volume (dict_t *dict, char **op_errstr)
                 brick_list = gf_strdup (bricks);
                 if (!brick_list) {
                         ret = -1;
-                        gf_log ("", GF_LOG_ERROR, "Out of memory");
                         goto out;
                 } else {
                         free_ptr = brick_list;
@@ -738,9 +738,6 @@ glusterd_op_stage_create_volume (dict_t *dict, char **op_errstr)
                         !glusterd_is_valid_volfpath (volname, brick)) {
                         snprintf (msg, sizeof (msg), "brick path %s is too "
                                   "long.", brick);
-                        gf_log ("", GF_LOG_ERROR, "%s", msg);
-                        *op_errstr = gf_strdup (msg);
-
                         ret = -1;
                         goto out;
                 }
@@ -748,12 +745,11 @@ glusterd_op_stage_create_volume (dict_t *dict, char **op_errstr)
                 ret = glusterd_brickinfo_new_from_brick (brick, &brick_info);
                 if (ret)
                         goto out;
-                snprintf (cmd_str, 1024, "%s", brick_info->path);
+
                 ret = glusterd_resolve_brick (brick_info);
                 if (ret) {
-                        gf_log ("glusterd", GF_LOG_ERROR, "cannot resolve "
-                                "brick: %s:%s", brick_info->hostname,
-                                brick_info->path);
+                        gf_log (this->name, GF_LOG_ERROR, FMTSTR_RESOLVE_BRICK,
+                                brick_info->hostname, brick_info->path);
                         goto out;
                 }
 
@@ -763,7 +759,6 @@ glusterd_op_stage_create_volume (dict_t *dict, char **op_errstr)
                         if (ret) {
                                 snprintf (msg, sizeof(msg), "invalid vg %s",
                                   brick_info->path);
-                                *op_errstr = gf_strdup (msg);
                                 goto out;
                         }
 
@@ -786,7 +781,12 @@ out:
         GF_FREE (free_ptr);
         if (brick_info)
                 glusterd_brickinfo_delete (brick_info);
-        gf_log ("", GF_LOG_DEBUG, "Returning %d", ret);
+
+        if (msg[0] != '\0') {
+                gf_log (this->name, GF_LOG_ERROR, "%s", msg);
+                *op_errstr = gf_strdup (msg);
+        }
+        gf_log (this->name, GF_LOG_DEBUG, "Returning %d", ret);
 
         return ret;
 }
@@ -1026,21 +1026,20 @@ glusterd_op_stage_delete_volume (dict_t *dict, char **op_errstr)
         gf_boolean_t                            exists = _gf_false;
         glusterd_volinfo_t                      *volinfo = NULL;
         char                                    msg[2048] = {0};
+        xlator_t                                *this = NULL;
 
-        ret = dict_get_str (dict, "volname", &volname);
+        this = THIS;
+        GF_ASSERT (this);
 
+        ret = dict_get_str (dict, "volname", &volname);
         if (ret) {
-                gf_log ("", GF_LOG_ERROR, "Unable to get volume name");
+                gf_log (this->name, GF_LOG_ERROR, "Unable to get volume name");
                 goto out;
         }
 
         exists = glusterd_check_volume_exists (volname);
-
         if (!exists) {
-                snprintf (msg, sizeof (msg), "Volume %s does not exist",
-                          volname);
-                gf_log ("", GF_LOG_ERROR, "%s", msg);
-                *op_errstr = gf_strdup (msg);
+                snprintf (msg, sizeof (msg), FMTSTR_CHECK_VOL_EXISTS, volname);
                 ret = -1;
                 goto out;
         } else {
@@ -1048,9 +1047,10 @@ glusterd_op_stage_delete_volume (dict_t *dict, char **op_errstr)
         }
 
         ret  = glusterd_volinfo_find (volname, &volinfo);
-
-        if (ret)
+        if (ret) {
+                snprintf (msg, sizeof (msg), FMTSTR_CHECK_VOL_EXISTS, volname);
                 goto out;
+        }
 
         ret = glusterd_validate_volume_id (dict, volinfo);
         if (ret)
@@ -1060,8 +1060,6 @@ glusterd_op_stage_delete_volume (dict_t *dict, char **op_errstr)
                 snprintf (msg, sizeof (msg), "Volume %s has been started."
                           "Volume needs to be stopped before deletion.",
                           volname);
-                gf_log ("", GF_LOG_ERROR, "%s", msg);
-                *op_errstr = gf_strdup (msg);
                 ret = -1;
                 goto out;
         }
@@ -1069,7 +1067,11 @@ glusterd_op_stage_delete_volume (dict_t *dict, char **op_errstr)
         ret = 0;
 
 out:
-        gf_log ("", GF_LOG_DEBUG, "Returning %d", ret);
+        if (msg[0] != '\0') {
+                gf_log (this->name, GF_LOG_ERROR, "%s", msg);
+                *op_errstr = gf_strdup (msg);
+        }
+        gf_log (this->name, GF_LOG_DEBUG, "Returning %d", ret);
 
         return ret;
 }
@@ -1428,15 +1430,14 @@ glusterd_op_create_volume (dict_t *dict, char **op_errstr)
 
         if (ret) {
                 gf_log (this->name, GF_LOG_ERROR,
-                        "Unable to allocate memory");
+                        "Unable to allocate memory for volinfo");
                 goto out;
         }
 
         ret = dict_get_str (dict, "volname", &volname);
 
         if (ret) {
-                gf_log (this->name, GF_LOG_ERROR,
-                        "Unable to get volume name");
+                gf_log (this->name, GF_LOG_ERROR, "Unable to get volume name");
                 goto out;
         }
 
@@ -1445,13 +1446,15 @@ glusterd_op_create_volume (dict_t *dict, char **op_errstr)
 
         ret = dict_get_int32 (dict, "type", &volinfo->type);
         if (ret) {
-                gf_log (this->name, GF_LOG_ERROR, "Unable to get type");
+                gf_log (this->name, GF_LOG_ERROR, "Unable to get type of volume"
+                        " %s", volname);
                 goto out;
         }
 
         ret = dict_get_int32 (dict, "count", &volinfo->brick_count);
         if (ret) {
-                gf_log (this->name, GF_LOG_ERROR, "Unable to get count");
+                gf_log (this->name, GF_LOG_ERROR, "Unable to get brick count of"
+                        " volume %s", volname);
                 goto out;
         }
 
@@ -1465,7 +1468,8 @@ glusterd_op_create_volume (dict_t *dict, char **op_errstr)
 
         ret = dict_get_str (dict, "bricks", &bricks);
         if (ret) {
-                gf_log (this->name, GF_LOG_ERROR, "Unable to get bricks");
+                gf_log (this->name, GF_LOG_ERROR, "Unable to get bricks for "
+                        "volume %s", volname);
                 goto out;
         }
 
@@ -1483,22 +1487,34 @@ glusterd_op_create_volume (dict_t *dict, char **op_errstr)
         if (GF_CLUSTER_TYPE_REPLICATE == volinfo->type) {
                 ret = dict_get_int32 (dict, "replica-count",
                                       &volinfo->replica_count);
-                if (ret)
+                if (ret) {
+                        gf_log (this->name, GF_LOG_ERROR, "Failed to get "
+                                 "replica count for volume %s", volname);
                         goto out;
+                }
         } else if (GF_CLUSTER_TYPE_STRIPE == volinfo->type) {
                 ret = dict_get_int32 (dict, "stripe-count",
                                       &volinfo->stripe_count);
-                if (ret)
+                if (ret) {
+                        gf_log (this->name, GF_LOG_ERROR, "Failed to get stripe"
+                                " count for volume %s", volname);
                         goto out;
+                }
         } else if (GF_CLUSTER_TYPE_STRIPE_REPLICATE == volinfo->type) {
                 ret = dict_get_int32 (dict, "stripe-count",
                                       &volinfo->stripe_count);
-                if (ret)
+                if (ret) {
+                        gf_log (this->name, GF_LOG_ERROR, "Failed to get stripe"
+                                " count for volume %s", volname);
                         goto out;
+                }
                 ret = dict_get_int32 (dict, "replica-count",
                                       &volinfo->replica_count);
-                if (ret)
+                if (ret) {
+                        gf_log (this->name, GF_LOG_ERROR, "Failed to get "
+                                "replica count for volume %s", volname);
                         goto out;
+                }
         }
 
         /* dist-leaf-count is the count of brick nodes for a given
@@ -1514,27 +1530,28 @@ glusterd_op_create_volume (dict_t *dict, char **op_errstr)
         ret = dict_get_str (dict, "transport", &trans_type);
         if (ret) {
                 gf_log (this->name, GF_LOG_ERROR,
-                        "Unable to get transport");
+                        "Unable to get transport type of volume %s", volname);
                 goto out;
         }
 
         ret = dict_get_str (dict, "volume-id", &str);
         if (ret) {
                 gf_log (this->name, GF_LOG_ERROR,
-                        "Unable to get volume-id");
+                        "Unable to get volume-id of volume %s", volname);
                 goto out;
         }
         ret = uuid_parse (str, volinfo->volume_id);
         if (ret) {
                 gf_log (this->name, GF_LOG_ERROR,
-                        "unable to parse uuid %s", str);
+                        "unable to parse uuid %s of volume %s", str, volname);
                 goto out;
         }
 
         ret = dict_get_str (dict, "internal-username", &username);
         if (ret) {
                 gf_log (this->name, GF_LOG_ERROR,
-                        "unable to get internal username");
+                        "unable to get internal username of volume %s",
+                        volname);
                 goto out;
         }
         glusterd_auth_set_username (volinfo, username);
@@ -1542,7 +1559,8 @@ glusterd_op_create_volume (dict_t *dict, char **op_errstr)
         ret = dict_get_str (dict, "internal-password", &password);
         if (ret) {
                 gf_log (this->name, GF_LOG_ERROR,
-                        "unable to get internal password");
+                        "unable to get internal password of volume %s",
+                        volname);
                 goto out;
         }
         glusterd_auth_set_password (volinfo, password);
@@ -1572,8 +1590,11 @@ glusterd_op_create_volume (dict_t *dict, char **op_errstr)
                         goto out;
 
                 ret = glusterd_resolve_brick (brickinfo);
-                if (ret)
+                if (ret) {
+                        gf_log (this->name, GF_LOG_ERROR, FMTSTR_RESOLVE_BRICK,
+                                brickinfo->hostname, brickinfo->path);
                         goto out;
+                }
                 list_add_tail (&brickinfo->brick_list, &volinfo->bricks);
                 brick = strtok_r (NULL, " \n", &saveptr);
                 i++;
@@ -1709,18 +1730,20 @@ glusterd_op_delete_volume (dict_t *dict)
 
         ret = dict_get_str (dict, "volname", &volname);
         if (ret) {
-                gf_log ("", GF_LOG_ERROR, "Unable to get volume name");
+                gf_log (this->name, GF_LOG_ERROR, "Unable to get volume name");
                 goto out;
         }
 
         ret  = glusterd_volinfo_find (volname, &volinfo);
-
-        if (ret)
+        if (ret) {
+                gf_log (this->name, GF_LOG_ERROR, FMTSTR_CHECK_VOL_EXISTS,
+                        volname);
                 goto out;
+        }
 
         ret = glusterd_delete_volume (volinfo);
 out:
-        gf_log ("", GF_LOG_DEBUG, "returning %d", ret);
+        gf_log (this->name, GF_LOG_DEBUG, "returning %d", ret);
         return ret;
 }