From 0978b5a36d379839ff543fd54612fde476deede7 Mon Sep 17 00:00:00 2001 From: Pranith Kumar K Date: Mon, 27 Feb 2012 11:15:51 +0530 Subject: cluster/afr: Handle errors in build_parent_loc BUG: 787671 Change-Id: I0b01b0f9e14a26d757748413dd71909e915c7573 Signed-off-by: Pranith Kumar K Reviewed-on: http://review.gluster.com/2826 Tested-by: Gluster Build System Reviewed-by: Vijay Bellur --- xlators/cluster/afr/src/afr-dir-write.c | 81 +++++++++++++++++--------- xlators/cluster/afr/src/afr-lk-common.c | 2 +- xlators/cluster/afr/src/afr-self-heal-common.c | 42 +++++++------ xlators/cluster/afr/src/afr-self-heal-entry.c | 2 +- xlators/cluster/afr/src/afr.h | 4 +- 5 files changed, 81 insertions(+), 50 deletions(-) (limited to 'xlators/cluster') 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); -- cgit