|
|
d1681e |
From b1a9673560ec20df2c7944fce55a64f7b5913f77 Mon Sep 17 00:00:00 2001
|
|
|
d1681e |
From: Raghavendra G <rgowdapp@redhat.com>
|
|
|
d1681e |
Date: Tue, 7 Nov 2017 16:09:37 +0530
|
|
|
d1681e |
Subject: [PATCH 108/128] mount/fuse: use fstat in getattr implementation if
|
|
|
d1681e |
any opened fd is available
|
|
|
d1681e |
|
|
|
d1681e |
The restriction of using fds opened by the same Pid means fds cannot
|
|
|
d1681e |
be shared across threads of multithreaded application. Note that fops
|
|
|
d1681e |
from kernel have different Pid for different threads. Imagine
|
|
|
d1681e |
following sequence of operations:
|
|
|
d1681e |
|
|
|
d1681e |
* Turn off performance.open-behind
|
|
|
d1681e |
* Thread t1 opens an fd - fd1 - on file "file". Let's assume nodeid of
|
|
|
d1681e |
"file" is "nodeid-file".
|
|
|
d1681e |
* Thread t2 does RENAME ("newfile", "file"). Let's assume nodeid of
|
|
|
d1681e |
"newfile" as "nodeid-newfile".
|
|
|
d1681e |
* t2 proceeds to do fstat (fd1)
|
|
|
d1681e |
|
|
|
d1681e |
The above set of operations can sometimes result in ESTALE/ENOENT
|
|
|
d1681e |
errors. RENAME overwrites "file" with "newfile" changing its nodeid
|
|
|
d1681e |
from "nodeid-file" to "nodeid-newfile" and post RENAME, "nodeid-file" is
|
|
|
d1681e |
removed from the backend. If fstat carries nodeid-file as argument,
|
|
|
d1681e |
which can happen if lookup has not refreshed the nodeid of "file" and
|
|
|
d1681e |
since t2 doesn't have an fd opened, fuse_getattr_resume uses STAT
|
|
|
d1681e |
which will fail as "nodeid-file" no longer exists.
|
|
|
d1681e |
|
|
|
d1681e |
Since the above set of operations and sharing of fds across
|
|
|
d1681e |
multiple threads are valid, this is a bug.
|
|
|
d1681e |
|
|
|
d1681e |
The fix is to use any fd opened on the inode. In this specific example
|
|
|
d1681e |
fuse_getattr_resume will find fd1 and winds down the call as fstat
|
|
|
d1681e |
(fd1) which won't fail.
|
|
|
d1681e |
|
|
|
d1681e |
Cross-checked with "Miklos Szeredi" <mszeredi.at.redhat.dot.com> for
|
|
|
d1681e |
any security issues with this solution and he approves the solution.
|
|
|
d1681e |
|
|
|
d1681e |
Thanks to "Miklos Szeredi" <mszeredi.at.redhat.dot.com> for all the
|
|
|
d1681e |
pointers and discussions.
|
|
|
d1681e |
|
|
|
d1681e |
Change-Id: I88dd29b3607cd2594eee9d72a1637b5346c8d49c
|
|
|
d1681e |
BUG: 1492591
|
|
|
d1681e |
Signed-off-by: Raghavendra G <rgowdapp@redhat.com>
|
|
|
d1681e |
Upstream patch: https://review.gluster.org/#/c/18681/
|
|
|
d1681e |
Reviewed-on: https://code.engineering.redhat.com/gerrit/126511
|
|
|
d1681e |
Tested-by: RHGS Build Bot <nigelb@redhat.com>
|
|
|
d1681e |
Reviewed-by: Sunil Kumar Heggodu Gopala Acharya <sheggodu@redhat.com>
|
|
|
d1681e |
---
|
|
|
d1681e |
tests/bugs/replicate/bug-913051.t | 11 -----------
|
|
|
d1681e |
xlators/mount/fuse/src/fuse-bridge.c | 2 +-
|
|
|
d1681e |
2 files changed, 1 insertion(+), 12 deletions(-)
|
|
|
d1681e |
|
|
|
d1681e |
diff --git a/tests/bugs/replicate/bug-913051.t b/tests/bugs/replicate/bug-913051.t
|
|
|
d1681e |
index 43d1330..6794995 100644
|
|
|
d1681e |
--- a/tests/bugs/replicate/bug-913051.t
|
|
|
d1681e |
+++ b/tests/bugs/replicate/bug-913051.t
|
|
|
d1681e |
@@ -37,17 +37,6 @@ TEST fd_open $rfd "r" $M0/dir/b
|
|
|
d1681e |
TEST $CLI volume start $V0 force
|
|
|
d1681e |
EXPECT_WITHIN $CHILD_UP_TIMEOUT "1" afr_child_up_status $V0 0
|
|
|
d1681e |
|
|
|
d1681e |
-#check that the files are not opned on brick-0
|
|
|
d1681e |
-TEST stat $M0/dir/a
|
|
|
d1681e |
-realpatha=$(gf_get_gfid_backend_file_path $B0/${V0}0 "dir/a")
|
|
|
d1681e |
-EXPECT "N" gf_check_file_opened_in_brick $V0 $H0 $B0/${V0}0 "$realpatha"
|
|
|
d1681e |
-EXPECT "N" gf_check_file_opened_in_brick $V0 $H0 $B0/${V0}0 $B0/${V0}0/dir/a
|
|
|
d1681e |
-
|
|
|
d1681e |
-TEST stat $M0/dir/b
|
|
|
d1681e |
-realpathb=$(gf_get_gfid_backend_file_path $B0/${V0}0 "dir/b")
|
|
|
d1681e |
-EXPECT "N" gf_check_file_opened_in_brick $V0 $H0 $B0/${V0}0 "$realpathb"
|
|
|
d1681e |
-EXPECT "N" gf_check_file_opened_in_brick $V0 $H0 $B0/${V0}0 $B0/${V0}0/dir/b
|
|
|
d1681e |
-
|
|
|
d1681e |
#attempt self-heal so that the files are created on brick-0
|
|
|
d1681e |
|
|
|
d1681e |
TEST dd if=$M0/dir/a of=/dev/null bs=1024k
|
|
|
d1681e |
diff --git a/xlators/mount/fuse/src/fuse-bridge.c b/xlators/mount/fuse/src/fuse-bridge.c
|
|
|
d1681e |
index 34a0dbb..74b59b8 100644
|
|
|
d1681e |
--- a/xlators/mount/fuse/src/fuse-bridge.c
|
|
|
d1681e |
+++ b/xlators/mount/fuse/src/fuse-bridge.c
|
|
|
d1681e |
@@ -908,7 +908,7 @@ fuse_getattr_resume (fuse_state_t *state)
|
|
|
d1681e |
}
|
|
|
d1681e |
|
|
|
d1681e |
if (!IA_ISDIR (state->loc.inode->ia_type)) {
|
|
|
d1681e |
- state->fd = fd_lookup (state->loc.inode, state->finh->pid);
|
|
|
d1681e |
+ state->fd = fd_lookup (state->loc.inode, 0);
|
|
|
d1681e |
}
|
|
|
d1681e |
|
|
|
d1681e |
if (!state->fd) {
|
|
|
d1681e |
--
|
|
|
d1681e |
1.8.3.1
|
|
|
d1681e |
|