From 90092b0c276f599bbe415ecc3051ca12d99461b6 Mon Sep 17 00:00:00 2001 From: Raghavendra G Date: Sat, 17 Mar 2018 13:16:59 +0530 Subject: [PATCH 263/271] server/resolver: don't trust inode-table for RESOLVE_NOT There have been known races between fops which add a dentry (like lookup, create, mknod etc) and fops that remove a dentry (like rename, unlink, rmdir etc) due to which stale dentries are left out in inode table even though the dentry doesn't exist on backend. For eg., consider a lookup (parent/bname) and unlink (parent/bname) racing in the following order: * lookup hits storage/posix and finds that dentry exists * unlink removes the dentry on storage/posix * unlink reaches protocol/server where the dentry (parent/bname) is unlinked from the inode * lookup reaches protocol/server and creates a dentry (parent/bname) on the inode Now we've a stale dentry (parent/bname) associated with the inode in itable. This situation is bad for fops like link, create etc which invoke resolver with type RESOLVE_NOT. These fops fail with EEXIST even though there is no such dentry on backend fs. This issue can be solved in two ways: * Enable "dentry fop serializer" xlator [1]. # gluster volume set features.sdfs on * Make sure resolver does a lookup on backend when it finds a dentry in itable and validates the state of itable. - If a dentry is not found, unlink those stale dentries from itable and continue with fop - If dentry is found, fail the fop with EEXIST This patch implements second solution as sdfs is not enabled by default in brick xlator stack. Once sdfs is enabled by default, this patch can be reverted. [1] https://github.com/gluster/glusterfs/issues/397 >Change-Id: Ia8bb0cf97f97cb0e72639bce8aadb0f6d3f4a34a >updates: bz#1543279 >BUG: 1543279 >Signed-off-by: Raghavendra G upstream patch: https://review.gluster.org/19732 BUG: 1488120 Change-Id: I78df4f6751e5db1fc659ee15d92b76d1486455f0 Signed-off-by: Raghavendra G Reviewed-on: https://code.engineering.redhat.com/gerrit/138152 Tested-by: RHGS Build Bot Reviewed-by: Nithya Balachandran --- xlators/protocol/server/src/server-resolve.c | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/xlators/protocol/server/src/server-resolve.c b/xlators/protocol/server/src/server-resolve.c index d0126aa..6ffb909 100644 --- a/xlators/protocol/server/src/server-resolve.c +++ b/xlators/protocol/server/src/server-resolve.c @@ -58,10 +58,27 @@ resolve_gfid_entry_cbk (call_frame_t *frame, void *cookie, xlator_t *this, if (op_ret == -1) { if (op_errno == ENOENT) { - gf_msg_debug (this->name, 0, "%s/%s: failed to resolve" - " (%s)", + gf_msg_debug (this->name, 0, + "%s/%s: failed to resolve (%s)", uuid_utoa (resolve_loc->pargfid), resolve_loc->name, strerror (op_errno)); + if (resolve->type == RESOLVE_NOT) { + do { + inode = inode_grep (state->itable, + resolve_loc->parent, + resolve->bname); + + if (inode) { + gf_msg_debug (this->name, 0, "%s/%s: " + "removing stale dentry", + uuid_utoa (resolve_loc->pargfid), + resolve->bname); + inode_unlink (inode, + resolve_loc->parent, + resolve->bname); + } + } while (inode); + } } else { gf_msg (this->name, GF_LOG_WARNING, op_errno, PS_MSG_GFID_RESOLVE_FAILED, "%s/%s: failed to " @@ -318,11 +335,13 @@ resolve_entry_simple (call_frame_t *frame) if (resolve->type == RESOLVE_NOT) { gf_msg_debug (this->name, 0, "inode (pointer: %p gfid:%s found" - " for path (%s) while type is RESOLVE_NOT", + " for path (%s) while type is RESOLVE_NOT. " + "Performing lookup on backend to rule out any " + "possible stale dentries in inode table", inode, uuid_utoa (inode->gfid), resolve->path); resolve->op_ret = -1; resolve->op_errno = EEXIST; - ret = -1; + ret = 1; goto out; } -- 1.8.3.1