cluster/afr: Handle errors in build_parent_loc

BUG: 787671
Change-Id: I0b01b0f9e14a26d757748413dd71909e915c7573
Signed-off-by: Pranith Kumar K <pranithk@gluster.com>
Reviewed-on: http://review.gluster.com/2826
Tested-by: Gluster Build System <jenkins@build.gluster.com>
Reviewed-by: Vijay Bellur <vijay@gluster.com>

5 files changed, 81 insertions, 50 deletions

diff --git a/xlators/cluster/afr/src/afr-dir-write.c b/xlators/cluster/afr/src/afr-dir-write.c
index ef0025c8172..e3a8c51ad5d 100644
--- a/xlators/cluster/afr/src/afr-dir-write.c
+++ b/xlators/cluster/afr/src/afr-dir-write.c
@@ -47,31 +47,29 @@
 #include "afr.h"
 #include "afr-transaction.h"
 
-void
-afr_build_parent_loc (loc_t *parent, loc_t *child)
+int
+afr_build_parent_loc (loc_t *parent, loc_t *child, int32_t *op_errno)
 {
-        char *tmp = NULL;
+        int     ret = -1;
+        char    *child_path = NULL;
 
         if (!child->parent) {
-                //this should never be called with root as the child
-                GF_ASSERT (0);
-                loc_copy (parent, child);
-                return;
+                if (op_errno)
+                        *op_errno = EINVAL;
+                goto out;
         }
 
-        tmp = gf_strdup (child->path);
-        parent->path   = gf_strdup (dirname (tmp));
-        GF_FREE (tmp);
-
-        parent->name   = strrchr (parent->path, '/');
-        if (parent->name)
-                parent->name++;
-
+        child_path = gf_strdup (child->path);
+        if (!child_path) {
+                if (op_errno)
+                        *op_errno = ENOMEM;
+                goto out;
+        }
+        parent->path = dirname (child_path);
         parent->inode  = inode_ref (child->parent);
-        parent->parent = inode_parent (parent->inode, 0, NULL);
-
-        if (!uuid_is_null (child->pargfid))
-                uuid_copy (parent->gfid, child->pargfid);
+        ret = 0;
+out:
+        return ret;
 }
 
 /* {{{ create */
@@ -312,7 +310,10 @@ afr_create (call_frame_t *frame, xlator_t *this,
         local->transaction.done   = afr_create_done;
         local->transaction.unwind = afr_create_unwind;
 
-        afr_build_parent_loc (&local->transaction.parent_loc, loc);
+        ret = afr_build_parent_loc (&local->transaction.parent_loc, loc,
+                                    &op_errno);
+        if (ret)
+                goto out;
 
         local->transaction.main_frame = frame;
         local->transaction.basename = AFR_BASENAME (loc->path);
@@ -533,7 +534,10 @@ afr_mknod (call_frame_t *frame, xlator_t *this,
         local->transaction.done   = afr_mknod_done;
         local->transaction.unwind = afr_mknod_unwind;
 
-        afr_build_parent_loc (&local->transaction.parent_loc, loc);
+        ret = afr_build_parent_loc (&local->transaction.parent_loc, loc,
+                                    &op_errno);
+        if (ret)
+                goto out;
 
         local->transaction.main_frame = frame;
         local->transaction.basename = AFR_BASENAME (loc->path);
@@ -755,7 +759,10 @@ afr_mkdir (call_frame_t *frame, xlator_t *this,
         local->transaction.done   = afr_mkdir_done;
         local->transaction.unwind = afr_mkdir_unwind;
 
-        afr_build_parent_loc (&local->transaction.parent_loc, loc);
+        ret = afr_build_parent_loc (&local->transaction.parent_loc, loc,
+                                    &op_errno);
+        if (ret)
+                goto out;
 
         local->transaction.main_frame = frame;
         local->transaction.basename = AFR_BASENAME (loc->path);
@@ -974,7 +981,10 @@ afr_link (call_frame_t *frame, xlator_t *this,
         local->transaction.done   = afr_link_done;
         local->transaction.unwind = afr_link_unwind;
 
-        afr_build_parent_loc (&local->transaction.parent_loc, newloc);
+        ret = afr_build_parent_loc (&local->transaction.parent_loc, newloc,
+                                    &op_errno);
+        if (ret)
+                goto out;
 
         local->transaction.main_frame   = frame;
         local->transaction.basename     = AFR_BASENAME (newloc->path);
@@ -1197,7 +1207,10 @@ afr_symlink (call_frame_t *frame, xlator_t *this,
         local->transaction.done   = afr_symlink_done;
         local->transaction.unwind = afr_symlink_unwind;
 
-        afr_build_parent_loc (&local->transaction.parent_loc, loc);
+        ret = afr_build_parent_loc (&local->transaction.parent_loc, loc,
+                                    &op_errno);
+        if (ret)
+                goto out;
 
         local->transaction.main_frame   = frame;
         local->transaction.basename     = AFR_BASENAME (loc->path);
@@ -1404,8 +1417,14 @@ afr_rename (call_frame_t *frame, xlator_t *this,
         local->transaction.done   = afr_rename_done;
         local->transaction.unwind = afr_rename_unwind;
 
-        afr_build_parent_loc (&local->transaction.parent_loc, oldloc);
-        afr_build_parent_loc (&local->transaction.new_parent_loc, newloc);
+        ret = afr_build_parent_loc (&local->transaction.parent_loc, oldloc,
+                                    &op_errno);
+        if (ret)
+                goto out;
+        ret = afr_build_parent_loc (&local->transaction.new_parent_loc, newloc,
+                                    &op_errno);
+        if (ret)
+                goto out;
 
         local->transaction.main_frame   = frame;
         local->transaction.basename     = AFR_BASENAME (oldloc->path);
@@ -1597,7 +1616,10 @@ afr_unlink (call_frame_t *frame, xlator_t *this,
         local->transaction.done   = afr_unlink_done;
         local->transaction.unwind = afr_unlink_unwind;
 
-        afr_build_parent_loc (&local->transaction.parent_loc, loc);
+        ret = afr_build_parent_loc (&local->transaction.parent_loc, loc,
+                                    &op_errno);
+        if (ret)
+                goto out;
 
         local->transaction.main_frame = frame;
         local->transaction.basename = AFR_BASENAME (loc->path);
@@ -1791,7 +1813,10 @@ afr_rmdir (call_frame_t *frame, xlator_t *this,
         local->transaction.done   = afr_rmdir_done;
         local->transaction.unwind = afr_rmdir_unwind;
 
-        afr_build_parent_loc (&local->transaction.parent_loc, loc);
+        ret = afr_build_parent_loc (&local->transaction.parent_loc, loc,
+                                    &op_errno);
+        if (ret)
+                goto out;
 
         local->transaction.main_frame = frame;
         local->transaction.basename = AFR_BASENAME (loc->path);
diff --git a/xlators/cluster/afr/src/afr-lk-common.c b/xlators/cluster/afr/src/afr-lk-common.c
index ebeb588e99c..3614aa128a2 100644
--- a/xlators/cluster/afr/src/afr-lk-common.c
+++ b/xlators/cluster/afr/src/afr-lk-common.c
@@ -550,7 +550,7 @@ lower_path (loc_t *l1, const char *b1, loc_t *l2, const char *b2)
 {
         int ret = 0;
 
-        ret = strcmp (l1->path, l2->path);
+        ret = uuid_compare (l1->inode->gfid, l2->inode->gfid);
 
         if (ret == 0)
                 ret = strcmp (b1, b2);
diff --git a/xlators/cluster/afr/src/afr-self-heal-common.c b/xlators/cluster/afr/src/afr-self-heal-common.c
index b7649b1271d..a70d2754b18 100644
--- a/xlators/cluster/afr/src/afr-self-heal-common.c
+++ b/xlators/cluster/afr/src/afr-self-heal-common.c
@@ -1086,6 +1086,7 @@ afr_sh_missing_entry_call_impunge_recreate (call_frame_t *frame, xlator_t *this,
         unsigned int    enoent_count = 0;
         afr_private_t   *priv = NULL;
         int             i = 0;
+        int32_t         op_errno = 0;
 
         local = frame->local;
         sh    = &local->self_heal;
@@ -1106,7 +1107,12 @@ afr_sh_missing_entry_call_impunge_recreate (call_frame_t *frame, xlator_t *this,
         impunge_local = impunge_frame->local;
         impunge_sh    = &impunge_local->self_heal;
         loc_copy (&impunge_local->loc, &local->loc);
-        afr_build_parent_loc (&impunge_sh->parent_loc, &impunge_local->loc);
+        ret = afr_build_parent_loc (&impunge_sh->parent_loc,
+                                    &impunge_local->loc, &op_errno);
+        if (ret) {
+                ret = -op_errno;
+                goto out;
+        }
         impunge_local->call_count = enoent_count;
         impunge_sh->entrybuf = sh->buf[sh->source];
         impunge_sh->parentbuf = sh->parentbufs[sh->source];
@@ -1655,25 +1661,17 @@ afr_sh_find_fresh_parents (call_frame_t *frame, xlator_t *this,
         sh = &local->self_heal;
         priv = this->private;
 
-        /* If We can't find a fresh parent directory here,
-         * we wont know which subvol is correct without finding a parent dir
-         * upwards which has correct xattrs, for that we may have to
-         * do lookups till root, we dont wanna do that,
-         * instead make sure that if there are conflicting gfid
-         * parent dirs, self-heal thus lookup is failed with EIO.
-         * if there are missing entries we dont know whether to delete or
-         * create so fail with EIO,
-         * If there are conflicting xattr fail with EIO.
-         */
         if (op_ret < 0)
                 goto out;
         enoent_count = afr_errno_count (NULL, sh->child_errno,
                                         priv->child_count, ENOENT);
         if (enoent_count > 0) {
-                gf_log (this->name, GF_LOG_ERROR, "Parent dir missing for %s,"
-                        " in missing entry self-heal, aborting self-heal",
+                gf_log (this->name, GF_LOG_INFO, "Parent dir missing for %s,"
+                        " in missing entry self-heal, aborting missing-entry "
+                        "self-heal",
                         local->loc.path);
-                goto out;
+                afr_sh_missing_entries_finish (frame, this);
+                return;
         }
 
         nsources = afr_build_sources (this, sh->xattr, sh->buf,
@@ -1883,6 +1881,9 @@ afr_self_heal_parent_entrylk (call_frame_t *frame, xlator_t *this,
 {
         afr_local_t         *local    = NULL;
         afr_self_heal_t     *sh       = NULL;
+        afr_internal_lock_t *int_lock = NULL;
+        int                 ret       = -1;
+        int32_t             op_errno  = 0;
 
         local    = frame->local;
         sh       = &local->self_heal;
@@ -1891,12 +1892,18 @@ afr_self_heal_parent_entrylk (call_frame_t *frame, xlator_t *this,
                 "attempting to recreate missing entries for path=%s",
                 local->loc.path);
 
-        GF_ASSERT (local->loc.parent);
-        afr_build_parent_loc (&sh->parent_loc, &local->loc);
+        ret = afr_build_parent_loc (&sh->parent_loc, &local->loc, &op_errno);
+        if (ret)
+                goto out;
 
         afr_sh_entrylk (frame, this, &sh->parent_loc, NULL,
                         lock_cbk);
         return 0;
+out:
+        int_lock = &local->internal_lock;
+        int_lock->lock_op_ret = -1;
+        lock_cbk (frame, this);
+        return 0;
 }
 
 static int
@@ -2133,8 +2140,7 @@ afr_self_heal (call_frame_t *frame, xlator_t *this, inode_t *inode)
                 UNLOCK (&priv->lock);
         }
 
-        if (!local->loc.name) {
-                /* nameless lookup */
+        if (!local->loc.parent) {
                 sh->do_missing_entry_self_heal = _gf_false;
                 sh->do_gfid_self_heal = _gf_false;
         }
diff --git a/xlators/cluster/afr/src/afr-self-heal-entry.c b/xlators/cluster/afr/src/afr-self-heal-entry.c
index c86d178b799..7cba96f06e1 100644
--- a/xlators/cluster/afr/src/afr-self-heal-entry.c
+++ b/xlators/cluster/afr/src/afr-self-heal-entry.c
@@ -412,7 +412,6 @@ afr_sh_entry_expunge_remove_cbk (call_frame_t *expunge_frame, void *cookie,
         }
 
         valid = GF_SET_ATTR_ATIME | GF_SET_ATTR_MTIME;
-        afr_build_parent_loc (&expunge_sh->parent_loc, &expunge_local->loc);
 
         STACK_WIND_COOKIE (expunge_frame, afr_sh_entry_expunge_parent_setattr_cbk,
                            (void *) (long) active_src,
@@ -734,6 +733,7 @@ afr_sh_entry_expunge_entry (call_frame_t *frame, xlator_t *this,
         expunge_sh->sh_frame = frame;
         expunge_sh->active_source = active_src;
         expunge_sh->entrybuf = entry->d_stat;
+        loc_copy (&expunge_sh->parent_loc, &local->loc);
 
         ret = afr_build_child_loc (this, &expunge_local->loc, &local->loc,
                                    name);
diff --git a/xlators/cluster/afr/src/afr.h b/xlators/cluster/afr/src/afr.h
index 87b557bcb1e..a9162b8adc0 100644
--- a/xlators/cluster/afr/src/afr.h
+++ b/xlators/cluster/afr/src/afr.h
@@ -820,8 +820,8 @@ void
 afr_inode_set_read_ctx (xlator_t *this, inode_t *inode, int32_t read_child,
                         int32_t *fresh_children);
 
-void
-afr_build_parent_loc (loc_t *parent, loc_t *child);
+int
+afr_build_parent_loc (loc_t *parent, loc_t *child, int32_t *op_errno);
 
 unsigned int
 afr_up_children_count (unsigned char *child_up, unsigned int child_count);