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