syncop: Change return value of syncop

Problem:
We found a day-1 bug when syncop_xxx() infra is used inside a synctask with
compilation optimization (CFLAGS -O2).

Detailed explanation of the Root cause:
We found the bug in 'gf_defrag_migrate_data' in rebalance operation:

Lets look at interesting parts of the function:

int
gf_defrag_migrate_data (xlator_t *this, gf_defrag_info_t *defrag, loc_t *loc,
                        dict_t *migrate_data)
{
.....
code section - [ Loop ]
        while ((ret = syncop_readdirp (this, fd, 131072, offset, NULL,
                                       &entries)) != 0) {
.....
code section - [ ERRNO-1 ] (errno of readdirp is stored in readdir_operrno by a
thread)
                /* Need to keep track of ENOENT errno, that means, there is no
                   need to send more readdirp() */
                readdir_operrno = errno;
.....
code section - [ SYNCOP-1 ] (syncop_getxattr is called by a thread)
                        ret = syncop_getxattr (this, &entry_loc, &dict,
                                               GF_XATTR_LINKINFO_KEY);
code section - [ ERRNO-2]   (checking for failures of syncop_getxattr(). This
may not always be executed in same thread which executed [SYNCOP-1])
                        if (ret < 0) {
                                if (errno != ENODATA) {
                                        loglevel = GF_LOG_ERROR;
                                        defrag->total_failures += 1;
.....
}

the function above could be executed by thread(t1) till [SYNCOP-1] and code
from [ERRNO-2] can be executed by a different thread(t2) because of the way
syncop-infra schedules the tasks.

when the code is compiled with -O2 optimization this is the assembly code that
is generated:
 [ERRNO-1]
1165                        readdir_operrno = errno; <<---- errno gets expanded
as *(__errno_location())
   0x00007fd149d48b60 <+496>:        callq  0x7fd149d410c0 <address@hidden>
   0x00007fd149d48b72 <+514>:        mov    %rax,0x50(%rsp) <<------ Address
returned by __errno_location() is stored in a special location in stack for
later use.
   0x00007fd149d48b77 <+519>:        mov    (%rax),%eax
   0x00007fd149d48b79 <+521>:        mov    %eax,0x78(%rsp)
....
 [ERRNO-2]
1281                                        if (errno != ENODATA) {
   0x00007fd149d492ae <+2366>:        mov    0x50(%rsp),%rax <<-----  Because
it already stored the address returned by __errno_location(), it just
dereferences the address to get the errno value. BUT THIS CODE NEED NOT BE
EXECUTED BY SAME THREAD!!!
   0x00007fd149d492b3 <+2371>:        mov    $0x9,%ebp
   0x00007fd149d492b8 <+2376>:        mov    (%rax),%edi
   0x00007fd149d492ba <+2378>:        cmp    $0x3d,%edi

The problem is that __errno_location() value of t1 and t2 are different. So
[ERRNO-2] ends up reading errno of t1 instead of errno of t2 even though t2 is
executing [ERRNO-2] code section.

When code is compiled without any optimization for [ERRNO-2]:
1281                                        if (errno != ENODATA) {
   0x00007fd58e7a326f <+2237>:        callq  0x7fd58e797300
<address@hidden><<--- As it is calling __errno_location() again it gets the
location from t2 so it works as intended.
   0x00007fd58e7a3274 <+2242>:        mov    (%rax),%eax
   0x00007fd58e7a3276 <+2244>:        cmp    $0x3d,%eax
   0x00007fd58e7a3279 <+2247>:        je     0x7fd58e7a32a1
<gf_defrag_migrate_data+2287>

Fix:
Make syncop_xxx() return (-errno) value as the return value in
case of errors and all the functions which make syncop_xxx() will need to use
(-ret) to figure out the reason for failure in case of syncop_xxx() failures.

Change-Id: I314d20dabe55d3e62ff66f3b4adb1cac2eaebb57
BUG: 1040356
Signed-off-by: Pranith Kumar K <pkarampu@redhat.com>
Reviewed-on: http://review.gluster.org/6475
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Anand Avati <avati@redhat.com>

5 files changed, 155 insertions, 100 deletions

diff --git a/xlators/cluster/afr/src/afr-self-heald.c b/xlators/cluster/afr/src/afr-self-heald.c
index 5f85c3047d4..9e5c1b3e79f 100644
--- a/xlators/cluster/afr/src/afr-self-heald.c
+++ b/xlators/cluster/afr/src/afr-self-heald.c
@@ -336,8 +336,9 @@ _get_path_from_gfid_loc (xlator_t *this, xlator_t *readdir_xl, loc_t *child,
 
         ret = syncop_getxattr (readdir_xl, child, &xattr, GFID_TO_PATH_KEY);
         if (ret < 0) {
-                if ((errno == ENOENT || errno == ESTALE) && missing)
+                if ((-ret == ENOENT || -ret == ESTALE) && missing)
                         *missing = _gf_true;
+                ret = -1;
                 goto out;
         }
         ret = dict_get_str (xattr, GFID_TO_PATH_KEY, &path);
@@ -437,10 +438,10 @@ _remove_stale_index (xlator_t *this, xlator_t *readdir_xl,
         gf_log (this->name, GF_LOG_DEBUG, "Removing stale index "
                 "for %s on %s", index_loc.name, readdir_xl->name);
         ret = syncop_unlink (readdir_xl, &index_loc);
-        if(ret && (errno != ENOENT)) {
+        if((ret < 0) && (-ret != ENOENT)) {
                 gf_log(this->name, GF_LOG_ERROR, "%s: Failed to remove index "
                        "on %s - %s",index_loc.name, readdir_xl->name,
-                       strerror (errno));
+                       strerror (-ret));
         }
         index_loc.path = NULL;
         loc_wipe (&index_loc);
@@ -467,8 +468,10 @@ _count_hard_links_under_base_indices_dir (xlator_t *this,
         child = crawl_data->child;
 
         ret = syncop_lookup (readdir_xl, childloc, NULL, iattr, NULL, &parent);
-        if (ret)
+        if (ret) {
+                ret = -1;
                 goto out;
+        }
 
         ret = dict_get_int32 (output, this->name, &xl_id);
         if (ret)
@@ -648,8 +651,10 @@ _self_heal_entry (xlator_t *this, afr_crawl_data_t *crawl_data, gf_dirent_t *ent
 
         ret = syncop_lookup (this, child, xattr_req,
                              iattr, &xattr_rsp, &parentbuf);
-        _crawl_post_sh_action (this, parent, child, ret, errno, xattr_rsp,
+        _crawl_post_sh_action (this, parent, child, ret, -ret, xattr_rsp,
                                crawl_data);
+        if (ret < 0)
+                ret = -1;
         if (xattr_rsp)
                 dict_unref (xattr_rsp);
         if (ret == 0)
@@ -1190,8 +1195,10 @@ afr_crawl_build_start_loc (xlator_t *this, afr_crawl_data_t *crawl_data,
                 afr_build_root_loc (this, &rootloc);
                 ret = syncop_getxattr (readdir_xl, &rootloc, &xattr,
                                        GF_XATTROP_INDEX_GFID);
-                if (ret < 0)
+                if (ret < 0) {
+                        ret = -1;
                         goto out;
+                }
                 ret = dict_get_ptr (xattr, GF_XATTROP_INDEX_GFID, &index_gfid);
                 if (ret < 0) {
                         gf_log (this->name, GF_LOG_ERROR, "failed to get index "
@@ -1210,11 +1217,12 @@ afr_crawl_build_start_loc (xlator_t *this, afr_crawl_data_t *crawl_data,
                 ret = syncop_lookup (readdir_xl, dirloc, NULL,
                                      &iattr, NULL, &parent);
                 if (ret < 0) {
-                        if (errno != ENOENT) {
+                        if (-ret != ENOENT) {
                                 gf_log (this->name, GF_LOG_ERROR, "lookup "
                                         "failed on index dir on %s - (%s)",
-                                        readdir_xl->name, strerror (errno));
+                                        readdir_xl->name, strerror (-ret));
                         }
+                        ret = -1;
                         goto out;
                 }
                 ret = _link_inode_update_loc (this, dirloc, &iattr);
@@ -1224,8 +1232,10 @@ afr_crawl_build_start_loc (xlator_t *this, afr_crawl_data_t *crawl_data,
                 afr_build_root_loc (this, &rootloc);
                 ret = syncop_getxattr (readdir_xl, &rootloc, &xattr,
                                        GF_BASE_INDICES_HOLDER_GFID);
-                if (ret < 0)
+                if (ret < 0) {
+                        ret = -1;
                         goto out;
+                }
                 ret = dict_get_ptr (xattr, GF_BASE_INDICES_HOLDER_GFID,
                                     &base_indices_holder_vgfid);
                 if (ret < 0) {
@@ -1246,16 +1256,17 @@ afr_crawl_build_start_loc (xlator_t *this, afr_crawl_data_t *crawl_data,
                 ret = syncop_lookup (readdir_xl, dirloc, NULL, &iattr, NULL,
                                      &parent);
                 if (ret < 0) {
-                        if (errno != ENOENT) {
+                        if (-ret != ENOENT) {
                                 gf_log (this->name, GF_LOG_ERROR, "lookup "
                                         "failed for base_indices_holder dir"
                                         " on %s - (%s)", readdir_xl->name,
-                                        strerror (errno));
+                                        strerror (-ret));
 
                         } else {
                                 gf_log (this->name, GF_LOG_ERROR, "base_indices"
                                         "_holder is not yet created.");
                         }
+                        ret = -1;
                         goto out;
                 }
                 ret = _link_inode_update_loc (this, dirloc, &iattr);
@@ -1290,6 +1301,7 @@ afr_crawl_opendir (xlator_t *this, afr_crawl_data_t *crawl_data, fd_t **dirfd,
                 if (ret < 0) {
                         gf_log (this->name, GF_LOG_ERROR,
                                 "opendir failed on %s", dirloc->path);
+                        ret = -1;
                         goto out;
                 }
         } else {
@@ -1442,8 +1454,13 @@ _crawl_directory (fd_t *fd, loc_t *loc, afr_crawl_data_t *crawl_data)
                 else
                         ret = syncop_readdir (readdir_xl, fd, 131072, offset,
                                               &entries);
-                if (ret <= 0)
+                if (ret < 0) {
+                        ret = -1;
+                        break;
+                } else if (ret == 0) {
                         break;
+                }
+
                 ret = 0;
                 free_entries = _gf_true;
 
@@ -1503,7 +1520,8 @@ afr_find_child_position (xlator_t *this, int child, afr_child_pos_t *pos)
                                GF_XATTR_NODE_UUID_KEY);
         if (ret < 0) {
                 gf_log (this->name, GF_LOG_ERROR, "getxattr failed on %s - "
-                        "(%s)", priv->children[child]->name, strerror (errno));
+                        "(%s)", priv->children[child]->name, strerror (-ret));
+                ret = -1;
                 goto out;
         }
 
diff --git a/xlators/cluster/afr/src/pump.c b/xlators/cluster/afr/src/pump.c
index a7f72fb30e9..9027e2a3318 100644
--- a/xlators/cluster/afr/src/pump.c
+++ b/xlators/cluster/afr/src/pump.c
@@ -504,9 +504,10 @@ pump_xattr_cleaner (call_frame_t *frame, void *cookie, xlator_t *this,
         for (i = 0; i < priv->child_count; i++) {
                 ret = syncop_removexattr (priv->children[i], &loc,
                                           PUMP_SOURCE_COMPLETE);
-                if (ret)
+                if (ret) {
                         gf_log (this->name, GF_LOG_DEBUG, "removexattr "
-                                "failed with %s", strerror (errno));
+                                "failed with %s", strerror (-ret));
+                }
         }
 
         loc_wipe (&loc);
@@ -598,6 +599,7 @@ pump_lookup_sink (loc_t *loc)
         if (ret) {
                 gf_log (this->name, GF_LOG_DEBUG,
                         "Lookup on sink child failed");
+                ret = -1;
                 goto out;
         }
 
diff --git a/xlators/cluster/dht/src/dht-helper.c b/xlators/cluster/dht/src/dht-helper.c
index 00a98f1cfa5..76bfbaedb77 100644
--- a/xlators/cluster/dht/src/dht-helper.c
+++ b/xlators/cluster/dht/src/dht-helper.c
@@ -769,17 +769,20 @@ dht_migration_complete_check_task (void *data)
                 dst_node = dht_linkfile_subvol (this, NULL, NULL, dict);
 
         if (ret) {
-                if (!dht_inode_missing(errno) || (!local->loc.inode)) {
+                if (!dht_inode_missing(-ret) || (!local->loc.inode)) {
                         gf_log (this->name, GF_LOG_ERROR,
                                 "%s: failed to get the 'linkto' xattr %s",
-                                local->loc.path, strerror (errno));
+                                local->loc.path, strerror (-ret));
+                        ret = -1;
                         goto out;
                 }
                 /* Need to do lookup on hashed subvol, then get the file */
                 ret = syncop_lookup (this, &local->loc, NULL, &stbuf, NULL,
                                      NULL);
-                if (ret)
+                if (ret) {
+                        ret = -1;
                         goto out;
+                }
                 dst_node = dht_subvol_get_cached (this, local->loc.inode);
         }
 
@@ -799,6 +802,7 @@ dht_migration_complete_check_task (void *data)
                         gf_log (this->name, GF_LOG_ERROR,
                                 "%s: failed to lookup the file on %s",
                                 local->loc.path, dst_node->name);
+                        ret = -1;
                         goto out;
                 }
 
@@ -869,10 +873,11 @@ dht_migration_complete_check_task (void *data)
 
                 ret = syncop_open (dst_node, &tmp_loc,
                                    iter_fd->flags, iter_fd);
-                if (ret == -1) {
+                if (ret < 0) {
                         gf_log (this->name, GF_LOG_ERROR, "failed to open "
                                 "the fd (%p, flags=0%o) on file %s @ %s",
                                 iter_fd, iter_fd->flags, path, dst_node->name);
+                        ret = -1;
                         open_failed = 1;
                 }
         }
@@ -955,11 +960,12 @@ dht_rebalance_inprogress_task (void *data)
                                         conf->link_xattr_name);
         }
 
-        if (ret) {
+        if (ret < 0) {
                 gf_log (this->name, GF_LOG_ERROR,
                         "%s: failed to get the 'linkto' xattr %s",
-                        local->loc.path, strerror (errno));
-                        goto out;
+                        local->loc.path, strerror (-ret));
+                ret = -1;
+                goto out;
         }
 
         dst_node = dht_linkfile_subvol (this, NULL, NULL, dict);
@@ -981,6 +987,7 @@ dht_rebalance_inprogress_task (void *data)
                         gf_log (this->name, GF_LOG_ERROR,
                                 "%s: failed to lookup the file on %s",
                                 local->loc.path, dst_node->name);
+                        ret = -1;
                         goto out;
                 }
 
@@ -1014,10 +1021,11 @@ dht_rebalance_inprogress_task (void *data)
 
                 ret = syncop_open (dst_node, &tmp_loc,
                                    iter_fd->flags, iter_fd);
-                if (ret == -1) {
+                if (ret < 0) {
                         gf_log (this->name, GF_LOG_ERROR, "failed to send open "
                                 "the fd (%p, flags=0%o) on file %s @ %s",
                                 iter_fd, iter_fd->flags, path, dst_node->name);
+                        ret = -1;
                         open_failed = 1;
                 }
         }
diff --git a/xlators/cluster/dht/src/dht-rebalance.c b/xlators/cluster/dht/src/dht-rebalance.c
index 9446dbe03ac..a5a4585f1ee 100644
--- a/xlators/cluster/dht/src/dht-rebalance.c
+++ b/xlators/cluster/dht/src/dht-rebalance.c
@@ -58,7 +58,8 @@ dht_write_with_holes (xlator_t *to, fd_t *fd, struct iovec *vec, int count,
                                 if (ret < 0) {
                                         gf_log (THIS->name, GF_LOG_WARNING,
                                                 "failed to write (%s)",
-                                                strerror (errno));
+                                                strerror (-ret));
+                                        ret = -1;
                                         goto out;
                                 }
 
@@ -76,7 +77,8 @@ dht_write_with_holes (xlator_t *to, fd_t *fd, struct iovec *vec, int count,
                                 /* 'path' will be logged in calling function */
                                 gf_log (THIS->name, GF_LOG_WARNING,
                                         "failed to write (%s)",
-                                        strerror (errno));
+                                        strerror (-ret));
+                                ret = -1;
                                 goto out;
                         }
                 }
@@ -158,7 +160,8 @@ gf_defrag_handle_hardlink (xlator_t *this, loc_t *loc, dict_t  *xattrs,
                 if (ret) {
                         gf_log (this->name, GF_LOG_ERROR, "Linkto setxattr "
                                 "failed %s -> %s (%s)", cached_subvol->name,
-                                loc->name, strerror (errno));
+                                loc->name, strerror (-ret));
+                        ret = -1;
                         goto out;
                 }
                 goto out;
@@ -173,7 +176,8 @@ gf_defrag_handle_hardlink (xlator_t *this, loc_t *loc, dict_t  *xattrs,
 
                 ret = syncop_link (hashed_subvol, loc, loc);
                 if  (ret) {
-                        op_errno = errno;
+                        op_errno = -ret;
+                        ret = -1;
                         gf_log (this->name, GF_LOG_ERROR, "link of %s -> %s"
                                 " failed on  subvol %s (%s)", loc->name,
                                 uuid_utoa(loc->gfid),
@@ -185,7 +189,8 @@ gf_defrag_handle_hardlink (xlator_t *this, loc_t *loc, dict_t  *xattrs,
         ret = syncop_lookup (hashed_subvol, loc, NULL, &iatt, NULL, NULL);
         if (ret) {
                 gf_log (this->name, GF_LOG_ERROR, "Failed lookup %s on %s (%s)"
-                        , loc->name, hashed_subvol->name, strerror (errno));
+                        , loc->name, hashed_subvol->name, strerror (-ret));
+                ret = -1;
                 goto out;
         }
 
@@ -289,11 +294,12 @@ __dht_rebalance_create_dst_file (xlator_t *to, xlator_t *from, loc_t *loc, struc
                         goto out;
                 }
         }
-        if ((ret == -1) && (errno != ENOENT)) {
+        if ((ret < 0) && (-ret != ENOENT)) {
                 /* File exists in destination, but not accessible */
                 gf_log (THIS->name, GF_LOG_WARNING,
                         "%s: failed to lookup file (%s)",
-                        loc->path, strerror (errno));
+                        loc->path, strerror (-ret));
+                ret = -1;
                 goto out;
         }
 
@@ -304,21 +310,22 @@ __dht_rebalance_create_dst_file (xlator_t *to, xlator_t *from, loc_t *loc, struc
         if (ret < 0) {
                 gf_log (this->name, GF_LOG_ERROR,
                         "failed to create %s on %s (%s)",
-                        loc->path, to->name, strerror (errno));
+                        loc->path, to->name, strerror (-ret));
+                ret = -1;
                 goto out;
         }
 
         ret = syncop_fsetxattr (to, fd, xattr, 0);
-        if (ret == -1)
+        if (ret < 0)
                 gf_log (this->name, GF_LOG_WARNING,
                         "%s: failed to set xattr on %s (%s)",
-                        loc->path, to->name, strerror (errno));
+                        loc->path, to->name, strerror (-ret));
 
         ret = syncop_ftruncate (to, fd, stbuf->ia_size);
         if (ret < 0)
                 gf_log (this->name, GF_LOG_ERROR,
                         "ftruncate failed for %s on %s (%s)",
-                        loc->path, to->name, strerror (errno));
+                        loc->path, to->name, strerror (-ret));
 
         ret = syncop_fsetattr (to, fd, stbuf,
                                (GF_SET_ATTR_UID | GF_SET_ATTR_GID),
@@ -326,7 +333,7 @@ __dht_rebalance_create_dst_file (xlator_t *to, xlator_t *from, loc_t *loc, struc
         if (ret < 0)
                 gf_log (this->name, GF_LOG_ERROR,
                         "chown failed for %s on %s (%s)",
-                        loc->path, to->name, strerror (errno));
+                        loc->path, to->name, strerror (-ret));
 
         if (dst_fd)
                 *dst_fd = fd;
@@ -356,7 +363,8 @@ __dht_check_free_space (xlator_t *to, xlator_t *from, loc_t *loc,
         if (ret) {
                 gf_log (this->name, GF_LOG_ERROR,
                         "failed to get statfs of %s on %s (%s)",
-                        loc->path, from->name, strerror (errno));
+                        loc->path, from->name, strerror (-ret));
+                ret = -1;
                 goto out;
         }
 
@@ -364,7 +372,8 @@ __dht_check_free_space (xlator_t *to, xlator_t *from, loc_t *loc,
         if (ret) {
                 gf_log (this->name, GF_LOG_ERROR,
                         "failed to get statfs of %s on %s (%s)",
-                        loc->path, to->name, strerror (errno));
+                        loc->path, to->name, strerror (-ret));
+                ret = -1;
                 goto out;
         }
 
@@ -463,6 +472,8 @@ __dht_rebalance_migrate_data (xlator_t *from, xlator_t *to, fd_t *src, fd_t *dst
 
         if (ret >= 0)
                 ret = 0;
+        else
+                ret = -1;
 
         return ret;
 }
@@ -491,10 +502,11 @@ __dht_rebalance_open_src_file (xlator_t *from, xlator_t *to, loc_t *loc,
         }
 
         ret = syncop_open (from, loc, O_RDWR, fd);
-        if (ret == -1) {
+        if (ret < 0) {
                 gf_log (this->name, GF_LOG_ERROR,
                         "failed to open file %s on %s (%s)",
-                        loc->path, from->name, strerror (errno));
+                        loc->path, from->name, strerror (-ret));
+                ret = -1;
                 goto out;
         }
 
@@ -517,7 +529,8 @@ __dht_rebalance_open_src_file (xlator_t *from, xlator_t *to, loc_t *loc,
         if (ret) {
                 gf_log (this->name, GF_LOG_ERROR,
                         "failed to set xattr on %s in %s (%s)",
-                        loc->path, from->name, strerror (errno));
+                        loc->path, from->name, strerror (-ret));
+                ret = -1;
                 goto out;
         }
 
@@ -531,7 +544,8 @@ __dht_rebalance_open_src_file (xlator_t *from, xlator_t *to, loc_t *loc,
         if (ret) {
                 gf_log (this->name, GF_LOG_ERROR,
                         "failed to set mode on %s in %s (%s)",
-                        loc->path, from->name, strerror (errno));
+                        loc->path, from->name, strerror (-ret));
+                ret = -1;
                 goto out;
         }
 
@@ -571,9 +585,10 @@ migrate_special_files (xlator_t *this, xlator_t *from, xlator_t *to, loc_t *loc,
 
         /* check in the destination if the file is link file */
         ret = syncop_lookup (to, loc, dict, &stbuf, &rsp_dict, NULL);
-        if ((ret == -1) && (errno != ENOENT)) {
+        if ((ret < 0) && (-ret != ENOENT)) {
                 gf_log (this->name, GF_LOG_WARNING, "%s: lookup failed (%s)",
-                        loc->path, strerror (errno));
+                        loc->path, strerror (-ret));
+                ret = -1;
                 goto out;
         }
 
@@ -596,7 +611,8 @@ migrate_special_files (xlator_t *this, xlator_t *from, xlator_t *to, loc_t *loc,
                 if (ret) {
                         gf_log (this->name, GF_LOG_WARNING,
                                 "%s: failed to delete the linkfile (%s)",
-                                loc->path, strerror (errno));
+                                loc->path, strerror (-ret));
+                        ret = -1;
                         goto out;
                 }
         }
@@ -616,7 +632,8 @@ migrate_special_files (xlator_t *this, xlator_t *from, xlator_t *to, loc_t *loc,
                 if (ret < 0) {
                         gf_log (this->name, GF_LOG_WARNING,
                                 "%s: readlink on symlink failed (%s)",
-                                loc->path, strerror (errno));
+                                loc->path, strerror (-ret));
+                        ret = -1;
                         goto out;
                 }
 
@@ -624,7 +641,8 @@ migrate_special_files (xlator_t *this, xlator_t *from, xlator_t *to, loc_t *loc,
                 if (ret) {
                         gf_log (this->name, GF_LOG_WARNING,
                                 "%s: creating symlink failed (%s)",
-                                loc->path, strerror (errno));
+                                loc->path, strerror (-ret));
+                        ret = -1;
                         goto out;
                 }
 
@@ -637,7 +655,8 @@ migrate_special_files (xlator_t *this, xlator_t *from, xlator_t *to, loc_t *loc,
                                      ia_minor (buf->ia_rdev)), dict, 0);
         if (ret) {
                 gf_log (this->name, GF_LOG_WARNING, "%s: mknod failed (%s)",
-                        loc->path, strerror (errno));
+                        loc->path, strerror (-ret));
+                ret = -1;
                 goto out;
         }
 
@@ -648,13 +667,16 @@ done:
         if (ret) {
                 gf_log (this->name, GF_LOG_WARNING,
                         "%s: failed to perform setattr on %s (%s)",
-                        loc->path, to->name, strerror (errno));
+                        loc->path, to->name, strerror (-ret));
+                ret = -1;
         }
 
         ret = syncop_unlink (from, loc);
-        if (ret)
+        if (ret) {
                 gf_log (this->name, GF_LOG_WARNING, "%s: unlink failed (%s)",
-                        loc->path, strerror (errno));
+                        loc->path, strerror (-ret));
+                ret = -1;
+        }
 
 out:
         if (dict)
@@ -708,7 +730,8 @@ dht_migrate_file (xlator_t *this, loc_t *loc, xlator_t *from, xlator_t *to,
         ret = syncop_lookup (from, loc, dict, &stbuf, &xattr_rsp, NULL);
         if (ret) {
                 gf_log (this->name, GF_LOG_ERROR, "%s: lookup failed on %s (%s)",
-                        loc->path, from->name, strerror (errno));
+                        loc->path, from->name, strerror (-ret));
+                ret = -1;
                 goto out;
         }
 
@@ -732,10 +755,12 @@ dht_migrate_file (xlator_t *this, loc_t *loc, xlator_t *from, xlator_t *to,
 
         /* TODO: move all xattr related operations to fd based operations */
         ret = syncop_listxattr (from, loc, &xattr);
-        if (ret == -1)
+        if (ret < 0) {
                 gf_log (this->name, GF_LOG_WARNING,
                         "%s: failed to get xattr from %s (%s)",
-                        loc->path, from->name, strerror (errno));
+                        loc->path, from->name, strerror (-ret));
+                ret = -1;
+        }
 
         /* create the destination, with required modes/xattr */
         ret = __dht_rebalance_create_dst_file (to, from, loc, &stbuf,
@@ -760,7 +785,8 @@ dht_migrate_file (xlator_t *this, loc_t *loc, xlator_t *from, xlator_t *to,
         ret = syncop_fstat (from, src_fd, &stbuf);
         if (ret) {
                 gf_log (this->name, GF_LOG_ERROR, "failed to lookup %s on %s (%s)",
-                        loc->path, from->name, strerror (errno));
+                        loc->path, from->name, strerror (-ret));
+                ret = -1;
                 goto out;
         }
 
@@ -779,7 +805,7 @@ dht_migrate_file (xlator_t *this, loc_t *loc, xlator_t *from, xlator_t *to,
                 if (ret) {
                         gf_log (this->name, GF_LOG_ERROR,
                                 "%s: failed to reset target size back to 0 (%s)",
-                                loc->path, strerror (errno));
+                                loc->path, strerror (-ret));
                 }
 
                 ret = -1;
@@ -789,10 +815,12 @@ dht_migrate_file (xlator_t *this, loc_t *loc, xlator_t *from, xlator_t *to,
         /* TODO: Sync the locks */
 
         ret = syncop_fsync (to, dst_fd, 0);
-        if (ret)
+        if (ret) {
                 gf_log (this->name, GF_LOG_WARNING,
                         "%s: failed to fsync on %s (%s)",
-                        loc->path, to->name, strerror (errno));
+                        loc->path, to->name, strerror (-ret));
+                ret = -1;
+        }
 
 
         /* Phase 2 - Data-Migration Complete, Housekeeping updates pending */
@@ -802,7 +830,8 @@ dht_migrate_file (xlator_t *this, loc_t *loc, xlator_t *from, xlator_t *to,
                 /* Failed to get the stat info */
                 gf_log (this->name, GF_LOG_ERROR,
                         "failed to fstat file %s on %s (%s)",
-                        loc->path, from->name, strerror (errno));
+                        loc->path, from->name, strerror (-ret));
+                ret = -1;
                 goto out;
         }
 
@@ -824,7 +853,8 @@ dht_migrate_file (xlator_t *this, loc_t *loc, xlator_t *from, xlator_t *to,
         if (ret) {
                 gf_log (this->name, GF_LOG_WARNING,
                         "%s: failed to perform setattr on %s (%s)",
-                        loc->path, to->name, strerror (errno));
+                        loc->path, to->name, strerror (-ret));
+                ret = -1;
                 goto out;
         }
 
@@ -835,7 +865,8 @@ dht_migrate_file (xlator_t *this, loc_t *loc, xlator_t *from, xlator_t *to,
         if (ret) {
                 gf_log (this->name, GF_LOG_WARNING,
                         "%s: failed to perform setattr on %s (%s)",
-                        loc->path, to->name, strerror (errno));
+                        loc->path, to->name, strerror (-ret));
+                ret = -1;
         }
 
         /* Make the source as a linkfile first before deleting it */
@@ -845,7 +876,8 @@ dht_migrate_file (xlator_t *this, loc_t *loc, xlator_t *from, xlator_t *to,
         if (ret) {
                 gf_log (this->name, GF_LOG_WARNING,             \
                         "%s: failed to perform setattr on %s (%s)",
-                        loc->path, from->name, strerror (errno));
+                        loc->path, from->name, strerror (-ret));
+                ret = -1;
                 goto out;
         }
 
@@ -855,7 +887,8 @@ dht_migrate_file (xlator_t *this, loc_t *loc, xlator_t *from, xlator_t *to,
         if (ret) {
                 gf_log (this->name, GF_LOG_WARNING,
                         "%s: failed to perform truncate on %s (%s)",
-                        loc->path, from->name, strerror (errno));
+                        loc->path, from->name, strerror (-ret));
+                ret = -1;
         }
 
         /* remove the 'linkto' xattr from the destination */
@@ -863,7 +896,8 @@ dht_migrate_file (xlator_t *this, loc_t *loc, xlator_t *from, xlator_t *to,
         if (ret) {
                 gf_log (this->name, GF_LOG_WARNING,
                         "%s: failed to perform removexattr on %s (%s)",
-                        loc->path, to->name, strerror (errno));
+                        loc->path, to->name, strerror (-ret));
+                ret = -1;
         }
 
         /* Do a stat and check the gfid before unlink */
@@ -871,7 +905,8 @@ dht_migrate_file (xlator_t *this, loc_t *loc, xlator_t *from, xlator_t *to,
         if (ret) {
                 gf_log (this->name, GF_LOG_WARNING,
                         "%s: failed to do a stat on %s (%s)",
-                        loc->path, from->name, strerror (errno));
+                        loc->path, from->name, strerror (-ret));
+                ret = -1;
                 goto out;
         }
 
@@ -881,7 +916,8 @@ dht_migrate_file (xlator_t *this, loc_t *loc, xlator_t *from, xlator_t *to,
                 if (ret) {
                         gf_log (this->name, GF_LOG_WARNING,
                                 "%s: failed to perform unlink on %s (%s)",
-                                loc->path, from->name, strerror (errno));
+                                loc->path, from->name, strerror (-ret));
+                        ret = -1;
                         goto out;
                 }
         }
@@ -890,7 +926,8 @@ dht_migrate_file (xlator_t *this, loc_t *loc, xlator_t *from, xlator_t *to,
         if (ret) {
                 gf_log (this->name, GF_LOG_DEBUG,
                         "%s: failed to lookup the file on subvolumes (%s)",
-                        loc->path, strerror (errno));
+                        loc->path, strerror (-ret));
+                ret = -1;
         }
 
         gf_log (this->name, GF_LOG_INFO,
@@ -1053,10 +1090,10 @@ gf_defrag_handle_migrate_error (int32_t op_errno, gf_defrag_info_t *defrag)
 {
         /* if errno is not ENOSPC or ENOTCONN, we can still continue
            with rebalance process */
-        if ((errno != ENOSPC) || (errno != ENOTCONN))
+        if ((op_errno != ENOSPC) || (op_errno != ENOTCONN))
                 return 1;
 
-        if (errno == ENOTCONN) {
+        if (op_errno == ENOTCONN) {
                 /* Most probably mount point went missing (mostly due
                    to a brick down), say rebalance failure to user,
                    let him restart it if everything is fine */
@@ -1064,7 +1101,7 @@ gf_defrag_handle_migrate_error (int32_t op_errno, gf_defrag_info_t *defrag)
                 return -1;
         }
 
-        if (errno == ENOSPC) {
+        if (op_errno == ENOSPC) {
                 /* rebalance process itself failed, may be
                    remote brick went down, or write failed due to
                    disk full etc etc.. */
@@ -1105,10 +1142,6 @@ gf_defrag_pattern_match (gf_defrag_info_t *defrag, char *name, uint64_t size)
  * have been fixed
  */
 
-#ifdef GF_LINUX_HOST_OS
-#pragma GCC push_options
-#pragma GCC optimize ("O0")
-#endif
 int
 gf_defrag_migrate_data (xlator_t *this, gf_defrag_info_t *defrag, loc_t *loc,
                         dict_t *migrate_data)
@@ -1126,7 +1159,6 @@ gf_defrag_migrate_data (xlator_t *this, gf_defrag_info_t *defrag, loc_t *loc,
         int32_t                  op_errno       = 0;
         char                    *uuid_str       = NULL;
         uuid_t                   node_uuid      = {0,};
-        int                      readdir_operrno = 0;
         struct timeval           dir_start      = {0,};
         struct timeval           end            = {0,};
         double                   elapsed        = {0,};
@@ -1148,6 +1180,7 @@ gf_defrag_migrate_data (xlator_t *this, gf_defrag_info_t *defrag, loc_t *loc,
         if (ret) {
                 gf_log (this->name, GF_LOG_ERROR, "Failed to open dir %s",
                         loc->path);
+                ret = -1;
                 goto out;
         }
 
@@ -1160,14 +1193,11 @@ gf_defrag_migrate_data (xlator_t *this, gf_defrag_info_t *defrag, loc_t *loc,
 
                         gf_log (this->name, GF_LOG_ERROR, "Readdir returned %s."
                                 " Aborting migrate-data",
-                                strerror(readdir_operrno));
+                                strerror(-ret));
+                        ret = -1;
                         goto out;
                 }
 
-                /* Need to keep track of ENOENT errno, that means, there is no
-                   need to send more readdirp() */
-                readdir_operrno = errno;
-
                 if (list_empty (&entries.list))
                         break;
 
@@ -1232,6 +1262,7 @@ gf_defrag_migrate_data (xlator_t *this, gf_defrag_info_t *defrag, loc_t *loc,
                         if (ret) {
                                 gf_log (this->name, GF_LOG_ERROR, "%s"
                                         " lookup failed", entry_loc.path);
+                                ret = -1;
                                 continue;
                         }
 
@@ -1240,6 +1271,7 @@ gf_defrag_migrate_data (xlator_t *this, gf_defrag_info_t *defrag, loc_t *loc,
                         if(ret < 0) {
                                 gf_log (this->name, GF_LOG_ERROR, "Failed to "
                                         "get node-uuid for %s", entry_loc.path);
+                                ret = -1;
                                 continue;
                         }
 
@@ -1249,6 +1281,7 @@ gf_defrag_migrate_data (xlator_t *this, gf_defrag_info_t *defrag, loc_t *loc,
                                 gf_log (this->name, GF_LOG_ERROR, "Failed to "
                                         "get node-uuid from dict for %s",
                                         entry_loc.path);
+                                ret = -1;
                                 continue;
                         }
 
@@ -1282,7 +1315,7 @@ gf_defrag_migrate_data (xlator_t *this, gf_defrag_info_t *defrag, loc_t *loc,
                         ret = syncop_getxattr (this, &entry_loc, &dict,
                                                GF_XATTR_LINKINFO_KEY);
                         if (ret < 0) {
-                                if (errno != ENODATA) {
+                                if (-ret != ENODATA) {
                                         loglevel = GF_LOG_ERROR;
                                         defrag->total_failures += 1;
                                 } else {
@@ -1290,7 +1323,8 @@ gf_defrag_migrate_data (xlator_t *this, gf_defrag_info_t *defrag, loc_t *loc,
                                 }
                                 gf_log (this->name, loglevel, "%s: failed to "
                                         "get "GF_XATTR_LINKINFO_KEY" key - %s",
-                                        entry_loc.path, strerror (errno));
+                                        entry_loc.path, strerror (-ret));
+                                ret = -1;
                                 continue;
                         }
 
@@ -1314,8 +1348,8 @@ gf_defrag_migrate_data (xlator_t *this, gf_defrag_info_t *defrag, loc_t *loc,
                                 }
                         }
 
-                        if (ret == -1) {
-                                op_errno = errno;
+                        if (ret < 0) {
+                                op_errno = -ret;
                                 ret = gf_defrag_handle_migrate_error (op_errno,
                                                                       defrag);
 
@@ -1350,9 +1384,6 @@ gf_defrag_migrate_data (xlator_t *this, gf_defrag_info_t *defrag, loc_t *loc,
                 gf_dirent_free (&entries);
                 free_entries = _gf_false;
                 INIT_LIST_HEAD (&entries.list);
-
-                if (readdir_operrno == ENOENT)
-                        break;
         }
 
         gettimeofday (&end, NULL);
@@ -1375,10 +1406,6 @@ out:
         return ret;
 
 }
-#ifdef GF_LINUX_HOST_OS
-#pragma GCC pop_options
-#endif
-
 
 int
 gf_defrag_fix_layout (xlator_t *this, gf_defrag_info_t *defrag, loc_t *loc,
@@ -1394,12 +1421,12 @@ gf_defrag_fix_layout (xlator_t *this, gf_defrag_info_t *defrag, loc_t *loc,
         dict_t                  *dict           = NULL;
         off_t                    offset         = 0;
         struct iatt              iatt           = {0,};
-        int                      readdirp_errno = 0;
 
         ret = syncop_lookup (this, loc, NULL, &iatt, NULL, NULL);
         if (ret) {
                 gf_log (this->name, GF_LOG_ERROR, "Lookup failed on %s",
                         loc->path);
+                ret = -1;
                 goto out;
         }
 
@@ -1433,14 +1460,11 @@ gf_defrag_fix_layout (xlator_t *this, gf_defrag_info_t *defrag, loc_t *loc,
 
                 if (ret < 0) {
                         gf_log (this->name, GF_LOG_ERROR, "Readdir returned %s"
-                                ". Aborting fix-layout",strerror(errno));
+                                ". Aborting fix-layout",strerror(-ret));
+                        ret = -1;
                         goto out;
                 }
 
-                /* Need to keep track of ENOENT errno, that means, there is no
-                   need to send more readdirp() */
-                readdirp_errno = errno;
-
                 if (list_empty (&entries.list))
                         break;
 
@@ -1494,6 +1518,7 @@ gf_defrag_fix_layout (xlator_t *this, gf_defrag_info_t *defrag, loc_t *loc,
                         if (ret) {
                                 gf_log (this->name, GF_LOG_ERROR, "%s"
                                         " lookup failed", entry_loc.path);
+                                ret = -1;
                                 continue;
                         }
 
@@ -1505,6 +1530,7 @@ gf_defrag_fix_layout (xlator_t *this, gf_defrag_info_t *defrag, loc_t *loc,
                                 defrag->defrag_status =
                                 GF_DEFRAG_STATUS_FAILED;
                                 defrag->total_failures ++;
+                                ret = -1;
                                 goto out;
                         }
                         ret = gf_defrag_fix_layout (this, defrag, &entry_loc,
@@ -1521,8 +1547,6 @@ gf_defrag_fix_layout (xlator_t *this, gf_defrag_info_t *defrag, loc_t *loc,
                 gf_dirent_free (&entries);
                 free_entries = _gf_false;
                 INIT_LIST_HEAD (&entries.list);
-                if (readdirp_errno == ENOENT)
-                        break;
         }
 
         ret = 0;
@@ -1587,6 +1611,7 @@ gf_defrag_start_crawl (void *data)
 
         if (ret) {
                 gf_log (this->name, GF_LOG_ERROR, "look up on / failed");
+                ret = -1;
                 goto out;
         }
 
@@ -1607,6 +1632,7 @@ gf_defrag_start_crawl (void *data)
                 gf_log (this->name, GF_LOG_ERROR, "fix layout on %s failed",
                         loc.path);
                 defrag->total_failures++;
+                ret = -1;
                 goto out;
         }
 
diff --git a/xlators/cluster/dht/src/dht-selfheal.c b/xlators/cluster/dht/src/dht-selfheal.c
index 3fe96b1c716..06fa1ed3a2e 100644
--- a/xlators/cluster/dht/src/dht-selfheal.c
+++ b/xlators/cluster/dht/src/dht-selfheal.c
@@ -1012,10 +1012,11 @@ dht_dir_attr_heal (void *data)
                 ret = syncop_setattr (subvol, &local->loc, &local->stbuf,
                                       (GF_SET_ATTR_UID | GF_SET_ATTR_GID),
                                       NULL, NULL);
-                if (ret)
+                if (ret) {
                         gf_log ("dht", GF_LOG_ERROR, "Failed to set uid/gid on"
                                 " %s on %s subvol (%s)", local->loc.path,
-                                subvol->name, strerror (errno));
+                                subvol->name, strerror (-ret));
+                }
         }
 out:
         return 0;