d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fusefs: send FUSE_FLUSH during VOP_CLOSE

Browse Source 
The FUSE protocol says that FUSE_FLUSH should be send every time a file
descriptor is closed.  That's not quite possible in FreeBSD because multiple
file descriptors can share a single struct file, and closef doesn't call
fo_close until the last close.  However, we can still send FUSE_FLUSH on
every VOP_CLOSE, which is probably good enough.

There are two purposes for FUSE_FLUSH.  One is to allow file systems to
return EIO if they have an error when writing data that's cached
server-side.  The other is to release POSIX file locks (which fusefs(5) does
not yet support).

PR:		236405, 236327
Sponsored by:	The FreeBSD Foundation
This commit is contained in:
Alan Somers 2019-04-03 19:59:45 +00:00
parent e312493b37
commit 9f10f423a9
14 changed files with 133 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
sys/fs/fuse/fuse_file.c
View File

@ -273,12 +273,14 @@ fuse_filehandle_validrw(struct vnode *vp, int mode,
}
int
fuse_filehandle_get(struct vnode *vp, fufh_type_t fufh_type,
fuse_filehandle_get(struct vnode *vp, int fflag,
struct fuse_filehandle **fufhp, struct ucred *cred, pid_t pid)
{
struct fuse_vnode_data *fvdat = VTOFUD(vp);
struct fuse_filehandle *fufh;
fufh_type_t fufh_type;
fufh_type = fflags_2_fufh_type(fflag);
if (cred == NULL)
goto fallback;
@ -307,14 +309,14 @@ fuse_filehandle_get(struct vnode *vp, fufh_type_t fufh_type,
}
int
fuse_filehandle_getrw(struct vnode *vp, fufh_type_t fufh_type,
fuse_filehandle_getrw(struct vnode *vp, int fflag,
struct fuse_filehandle **fufhp, struct ucred *cred, pid_t pid)
{
int err;
err = fuse_filehandle_get(vp, fufh_type, fufhp, cred, pid);
err = fuse_filehandle_get(vp, fflag, fufhp, cred, pid);
if (err)
err = fuse_filehandle_get(vp, FUFH_RDWR, fufhp, cred, pid);
err = fuse_filehandle_get(vp, FREAD | FWRITE, fufhp, cred, pid);
return err;
}

4
sys/fs/fuse/fuse_file.h
View File

@ -150,10 +150,10 @@ struct fuse_filehandle {
bool fuse_filehandle_validrw(struct vnode *vp, int mode,
struct ucred *cred, pid_t pid);
int fuse_filehandle_get(struct vnode *vp, fufh_type_t fufh_type,
int fuse_filehandle_get(struct vnode *vp, int fflag,
struct fuse_filehandle **fufhp, struct ucred *cred,
pid_t pid);
int fuse_filehandle_getrw(struct vnode *vp, fufh_type_t fufh_type,
int fuse_filehandle_getrw(struct vnode *vp, int fflag,
struct fuse_filehandle **fufhp, struct ucred *cred,
pid_t pid);

15
sys/fs/fuse/fuse_io.c
View File

@ -127,12 +127,12 @@ fuse_io_dispatch(struct vnode *vp, struct uio *uio, int ioflag,
{
struct fuse_filehandle *fufh;
int err, directio;
fufh_type_t fufh_type;
int fflag;
MPASS(vp->v_type == VREG || vp->v_type == VDIR);
fufh_type = (uio->uio_rw == UIO_READ) ? FUFH_RDONLY : FUFH_WRONLY;
err = fuse_filehandle_getrw(vp, fufh_type, &fufh, cred, pid);
fflag = (uio->uio_rw == UIO_READ) ? FREAD : FWRITE;
err = fuse_filehandle_getrw(vp, fflag, &fufh, cred, pid);
if (err) {
printf("FUSE: io dispatch: filehandles are closed\n");
return err;
@ -643,7 +643,7 @@ fuse_io_strategy(struct vnode *vp, struct buf *bp)
struct uio uio;
struct iovec io;
int error = 0;
fufh_type_t fufh_type;
int fflag;
/* We don't know the true pid when we're dealing with the cache */
pid_t pid = 0;
@ -652,9 +652,9 @@ fuse_io_strategy(struct vnode *vp, struct buf *bp)
MPASS(vp->v_type == VREG || vp->v_type == VDIR);
MPASS(bp->b_iocmd == BIO_READ || bp->b_iocmd == BIO_WRITE);
fufh_type = bp->b_iocmd == BIO_READ ? FUFH_RDONLY : FUFH_WRONLY;
fflag = bp->b_iocmd == BIO_READ ? FREAD : FWRITE;
cred = bp->b_iocmd == BIO_READ ? bp->b_rcred : bp->b_wcred;
error = fuse_filehandle_getrw(vp, fufh_type, &fufh, cred, pid);
error = fuse_filehandle_getrw(vp, fflag, &fufh, cred, pid);
if (bp->b_iocmd == BIO_READ && error == EBADF) {
/*
* This may be a read-modify-write operation on a cached file
@ -662,8 +662,7 @@ fuse_io_strategy(struct vnode *vp, struct buf *bp)
*
* TODO: eliminate this hacky check once the FUFH table is gone
*/
fufh_type = FUFH_WRONLY;
error = fuse_filehandle_get(vp, fufh_type, &fufh, cred, pid);
error = fuse_filehandle_get(vp, FWRITE, &fufh, cred, pid);
}
if (error) {
printf("FUSE: strategy: filehandles are closed\n");

2
sys/fs/fuse/fuse_node.c
View File

@ -378,7 +378,7 @@ fuse_vnode_savesize(struct vnode *vp, struct ucred *cred, pid_t pid)
fsai->size = fvdat->filesize;
fsai->valid |= FATTR_SIZE;
fuse_filehandle_getrw(vp, FUFH_WRONLY, &fufh, cred, pid);
fuse_filehandle_getrw(vp, FWRITE, &fufh, cred, pid);
if (fufh) {
fsai->fh = fufh->fh_id;
fsai->valid |= FATTR_FH;

46
sys/fs/fuse/fuse_vnops.c
View File

@ -221,9 +221,41 @@ static int
fuse_filehandle_get_dir(struct vnode *vp, struct fuse_filehandle **fufhp,
struct ucred *cred, pid_t pid)
{
if (fuse_filehandle_get(vp, FUFH_RDONLY, fufhp, cred, pid) == 0)
if (fuse_filehandle_get(vp, FREAD, fufhp, cred, pid) == 0)
return 0;
return fuse_filehandle_get(vp, FUFH_EXEC, fufhp, cred, pid);
return fuse_filehandle_get(vp, FEXEC, fufhp, cred, pid);
}
/* Send FUSE_FLUSH for this vnode */
static int
fuse_flush(struct vnode *vp, struct ucred *cred, pid_t pid, int fflag)
{
struct fuse_flush_in *ffi;
struct fuse_filehandle *fufh;
struct fuse_dispatcher fdi;
struct thread *td = curthread;
struct mount *mp = vnode_mount(vp);
int err;
if (!fsess_isimpl(vnode_mount(vp), FUSE_FLUSH))
return 0;
err = fuse_filehandle_get(vp, fflag, &fufh, cred, pid);
if (err)
return err;
fdisp_init(&fdi, sizeof(*ffi));
fdisp_make_vp(&fdi, FUSE_FLUSH, vp, td, cred);
ffi = fdi.indata;
ffi->fh = fufh->fh_id;
err = fdisp_wait_answ(&fdi);
if (err == ENOSYS) {
fsess_set_notimpl(mp, FUSE_FLUSH);
err = 0;
}
fdisp_destroy(&fdi);
return err;
}
/*
@ -275,7 +307,7 @@ fuse_vnop_access(struct vop_access_args *ap)
}
/*
struct vnop_close_args {
struct vop_close_args {
struct vnode *a_vp;
int a_fflag;
struct ucred *a_cred;
@ -290,6 +322,7 @@ fuse_vnop_close(struct vop_close_args *ap)
int fflag = ap->a_fflag;
struct thread *td = ap->a_td;
pid_t pid = td->td_proc->p_pid;
int err = 0;
if (fuse_isdeadfs(vp)) {
return 0;
@ -297,6 +330,8 @@ fuse_vnop_close(struct vop_close_args *ap)
if (vnode_isdir(vp)) {
struct fuse_filehandle *fufh;
// XXX: what if two file descriptors have the same directory
// opened? We shouldn't close the file handle too soon.
if (fuse_filehandle_get_dir(vp, &fufh, cred, pid) == 0)
fuse_filehandle_close(vp, fufh, NULL, cred);
return 0;
@ -304,11 +339,12 @@ fuse_vnop_close(struct vop_close_args *ap)
if (fflag & IO_NDELAY) {
return 0;
}
err = fuse_flush(vp, cred, pid, fflag);
/* TODO: close the file handle, if we're sure it's no longer used */
if ((VTOFUD(vp)->flag & FN_SIZECHANGE) != 0) {
fuse_vnode_savesize(vp, cred, td->td_proc->p_pid);
}
return 0;
return err;
}
static void
@ -1611,7 +1647,7 @@ fuse_vnop_setattr(struct vop_setattr_args *ap)
newsize = vap->va_size;
fsai->valid |= FATTR_SIZE;
fuse_filehandle_getrw(vp, FUFH_WRONLY, &fufh, cred, pid);
fuse_filehandle_getrw(vp, FWRITE, &fufh, cred, pid);
if (fufh) {
fsai->fh = fufh->fh_id;
fsai->valid |= FATTR_FH;

1
tests/sys/fs/fusefs/allow_other.cc
View File

@ -78,6 +78,7 @@ TEST_F(AllowOther, allowed)
expect_lookup(RELPATH, ino, S_IFREG | 0644, 0, 1);
expect_open(ino, 0, 1);
expect_flush(ino, 1, ReturnErrno(0));
expect_release(ino, FH);
expect_getattr(ino, 0);
}, []() {

77
tests/sys/fs/fusefs/flush.cc
View File

@ -41,7 +41,8 @@ using namespace testing;
class Flush: public FuseTest {
public:
void expect_flush(uint64_t ino, int times, pid_t lo, ProcessMockerT r)
void
expect_flush(uint64_t ino, int times, pid_t lo, ProcessMockerT r)
{
EXPECT_CALL(*m_mock, process(
ResultOf([=](auto in) {
@ -55,9 +56,9 @@ void expect_flush(uint64_t ino, int times, pid_t lo, ProcessMockerT r)
.WillRepeatedly(Invoke(r));
}
void expect_lookup(const char *relpath, uint64_t ino)
void expect_lookup(const char *relpath, uint64_t ino, int times)
{
FuseTest::expect_lookup(relpath, ino, S_IFREG | 0644, 0, 1);
FuseTest::expect_lookup(relpath, ino, S_IFREG | 0644, 0, times);
}
/*
@ -82,16 +83,18 @@ class FlushWithLocks: public Flush {
}
};
/* If a file descriptor is duplicated, every close causes FLUSH */
/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236405 */
TEST_F(Flush, DISABLED_dup)
/*
* If multiple file descriptors refer to the same file handle, closing each
* should send FUSE_FLUSH
*/
TEST_F(Flush, open_twice)
{
const char FULLPATH[] = "mountpoint/some_file.txt";
const char RELPATH[] = "some_file.txt";
uint64_t ino = 42;
int fd, fd2;
expect_lookup(RELPATH, ino);
expect_lookup(RELPATH, ino, 2);
expect_open(ino, 0, 1);
expect_getattr(ino, 0);
expect_flush(ino, 2, 0, ReturnErrno(0));
@ -100,10 +103,11 @@ TEST_F(Flush, DISABLED_dup)
fd = open(FULLPATH, O_WRONLY);
EXPECT_LE(0, fd) << strerror(errno);
fd2 = dup(fd);
fd2 = open(FULLPATH, O_WRONLY);
EXPECT_LE(0, fd2) << strerror(errno);
ASSERT_EQ(0, close(fd2)) << strerror(errno);
ASSERT_EQ(0, close(fd)) << strerror(errno);
EXPECT_EQ(0, close(fd2)) << strerror(errno);
EXPECT_EQ(0, close(fd)) << strerror(errno);
}
/*
@ -114,15 +118,14 @@ TEST_F(Flush, DISABLED_dup)
* all.
*/
/* http://pubs.opengroup.org/onlinepubs/9699919799/functions/close.html */
/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236405 */
TEST_F(Flush, DISABLED_eio)
TEST_F(Flush, eio)
{
const char FULLPATH[] = "mountpoint/some_file.txt";
const char RELPATH[] = "some_file.txt";
uint64_t ino = 42;
int fd;
expect_lookup(RELPATH, ino);
expect_lookup(RELPATH, ino, 1);
expect_open(ino, 0, 1);
expect_getattr(ino, 0);
expect_flush(ino, 1, 0, ReturnErrno(EIO));
@ -138,41 +141,48 @@ TEST_F(Flush, DISABLED_eio)
* If the filesystem returns ENOSYS, it will be treated as success and
* no more FUSE_FLUSH operations will be sent to the daemon
*/
/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236405 */
/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236557 */
TEST_F(Flush, DISABLED_enosys)
TEST_F(Flush, enosys)
{
const char FULLPATH[] = "mountpoint/some_file.txt";
const char RELPATH[] = "some_file.txt";
uint64_t ino = 42;
int fd, fd2;
const char FULLPATH0[] = "mountpoint/some_file.txt";
const char RELPATH0[] = "some_file.txt";
const char FULLPATH1[] = "mountpoint/other_file.txt";
const char RELPATH1[] = "other_file.txt";
uint64_t ino0 = 42;
uint64_t ino1 = 43;
int fd0, fd1;
expect_lookup(RELPATH, ino);
expect_open(ino, 0, 1);
expect_getattr(ino, 0);
expect_lookup(RELPATH0, ino0, 1);
expect_open(ino0, 0, 1);
expect_getattr(ino0, 0);
/* On the 2nd close, FUSE_FLUSH won't be sent at all */
expect_flush(ino, 1, 0, ReturnErrno(ENOSYS));
expect_flush(ino0, 1, 0, ReturnErrno(ENOSYS));
expect_release();
fd = open(FULLPATH, O_WRONLY);
EXPECT_LE(0, fd) << strerror(errno);
expect_lookup(RELPATH1, ino1, 1);
expect_open(ino1, 0, 1);
expect_getattr(ino1, 0);
/* On the 2nd close, FUSE_FLUSH won't be sent at all */
expect_release();
fd2 = dup(fd);
fd0 = open(FULLPATH0, O_WRONLY);
ASSERT_LE(0, fd0) << strerror(errno);
EXPECT_EQ(0, close(fd2)) << strerror(errno);
EXPECT_EQ(0, close(fd)) << strerror(errno);
fd1 = open(FULLPATH1, O_WRONLY);
ASSERT_LE(0, fd1) << strerror(errno);
EXPECT_EQ(0, close(fd0)) << strerror(errno);
EXPECT_EQ(0, close(fd1)) << strerror(errno);
}
/* A FUSE_FLUSH should be sent on close(2) */
/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236405 */
TEST_F(Flush, DISABLED_flush)
TEST_F(Flush, flush)
{
const char FULLPATH[] = "mountpoint/some_file.txt";
const char RELPATH[] = "some_file.txt";
uint64_t ino = 42;
int fd;
expect_lookup(RELPATH, ino);
expect_lookup(RELPATH, ino, 1);
expect_open(ino, 0, 1);
expect_getattr(ino, 0);
expect_flush(ino, 1, 0, ReturnErrno(0));
@ -188,7 +198,6 @@ TEST_F(Flush, DISABLED_flush)
* When closing a file with a POSIX file lock, flush should release the lock,
* _even_if_ it's not the process's last file descriptor for this file.
*/
/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236405 */
/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=234581 */
TEST_F(FlushWithLocks, DISABLED_unlock_on_close)
{
@ -199,7 +208,7 @@ TEST_F(FlushWithLocks, DISABLED_unlock_on_close)
struct flock fl;
pid_t pid = getpid();
expect_lookup(RELPATH, ino);
expect_lookup(RELPATH, ino, 1);
expect_open(ino, 0, 1);
expect_getattr(ino, 0);
EXPECT_CALL(*m_mock, process(

1
tests/sys/fs/fusefs/fsync.cc
View File

@ -139,6 +139,7 @@ TEST_F(Fsync, close)
}, Eq(true)),
_)
).Times(0);
expect_flush(ino, 1, ReturnErrno(0));
expect_release(ino, FH);
fd = open(FULLPATH, O_RDWR);

3
tests/sys/fs/fusefs/mockfs.cc
View File

@ -169,7 +169,8 @@ void debug_fuseop(const mockfs_buf_in *in)
in->body.open.flags, name);
break;
case FUSE_FLUSH:
printf(" lock_owner=%lu", in->body.flush.lock_owner);
printf(" fh=%#lx lock_owner=%lu", in->body.flush.fh,
in->body.flush.lock_owner);
break;
case FUSE_FORGET:
printf(" nlookup=%lu", in->body.forget.nlookup);

1
tests/sys/fs/fusefs/open.cc
View File

@ -205,6 +205,7 @@ TEST_F(Open, multiple_creds)
SET_OUT_HEADER_LEN(out, open);
})));
expect_getattr(ino, 0);
expect_flush(ino, 2, ReturnErrno(0));
expect_release(ino, fh0);
expect_release(ino, fh1);

6
tests/sys/fs/fusefs/release.cc
View File

@ -82,6 +82,7 @@ TEST_F(Release, dup)
expect_lookup(RELPATH, ino, 1);
expect_open(ino, 0, 1);
expect_getattr(ino, 0);
expect_flush(ino, 1, ReturnErrno(0));
expect_release(ino, 0, O_RDONLY, 0);
fd = open(FULLPATH, O_RDONLY);
@ -111,6 +112,7 @@ TEST_F(Release, eio)
expect_lookup(RELPATH, ino, 1);
expect_open(ino, 0, 1);
expect_getattr(ino, 0);
expect_flush(ino, 1, ReturnErrno(0));
expect_release(ino, 0, O_WRONLY, EIO);
fd = open(FULLPATH, O_WRONLY);
@ -133,6 +135,7 @@ TEST_F(Release, DISABLED_flags)
expect_lookup(RELPATH, ino, 1);
expect_open(ino, 0, 1);
expect_getattr(ino, 0);
expect_flush(ino, 1, ReturnErrno(0));
expect_release(ino, 0, O_RDWR | O_APPEND, 0);
fd = open(FULLPATH, O_RDWR | O_APPEND);
@ -156,6 +159,7 @@ TEST_F(Release, multiple_opens)
expect_lookup(RELPATH, ino, 2);
expect_open(ino, 0, 2);
expect_getattr(ino, 0);
expect_flush(ino, 2, ReturnErrno(0));
expect_release(ino, 0, O_RDONLY, 0);
fd = open(FULLPATH, O_RDONLY);
@ -179,6 +183,7 @@ TEST_F(Release, ok)
expect_lookup(RELPATH, ino, 1);
expect_open(ino, 0, 1);
expect_getattr(ino, 0);
expect_flush(ino, 1, ReturnErrno(0));
expect_release(ino, 0, O_RDONLY, 0);
fd = open(FULLPATH, O_RDONLY);
@ -212,6 +217,7 @@ TEST_F(ReleaseWithLocks, DISABLED_unlock_on_close)
SET_OUT_HEADER_LEN(out, setlk);
out->body.setlk.lk = in->body.setlk.lk;
})));
expect_flush(ino, 1, ReturnErrno(0));
expect_release(ino, (uint64_t)pid, O_RDWR, 0);
fd = open(FULLPATH, O_RDWR);

13
tests/sys/fs/fusefs/utils.cc
View File

@ -114,6 +114,19 @@ FuseTest::expect_access(uint64_t ino, mode_t access_mode, int error)
).WillOnce(Invoke(ReturnErrno(error)));
}
void
FuseTest::expect_flush(uint64_t ino, int times, ProcessMockerT r)
{
EXPECT_CALL(*m_mock, process(
ResultOf([=](auto in) {
return (in->header.opcode == FUSE_FLUSH &&
in->header.nodeid == ino);
}, Eq(true)),
_)
).Times(times)
.WillRepeatedly(Invoke(r));
}
void FuseTest::expect_getattr(uint64_t ino, uint64_t size)
{
/* Until the attr cache is working, we may send an additional GETATTR */

8
tests/sys/fs/fusefs/utils.hh
View File

@ -70,11 +70,17 @@ class FuseTest : public ::testing::Test {
}
/*
* Create an expectation that FUSE_ACCESS will be called oncde for the
* Create an expectation that FUSE_ACCESS will be called once for the
* given inode with the given access_mode, returning the given errno
*/
void expect_access(uint64_t ino, mode_t access_mode, int error);
/*
* Create an expectation that FUSE_FLUSH will be called times times for
* the given inode
*/
void expect_flush(uint64_t ino, int times, ProcessMockerT r);
/*
* Create an expectation that FUSE_GETATTR will be called for the given
* inode any number of times. It will respond with a few basic

2
tests/sys/fs/fusefs/write.cc
View File

@ -374,6 +374,7 @@ TEST_F(Write, DISABLED_mmap)
* pid, so they must set FUSE_WRITE_CACHE
*/
expect_write(ino, 0, len, len, FUSE_WRITE_CACHE, expected);
expect_flush(ino, 1, ReturnErrno(0));
expect_release(ino, ReturnErrno(0));
fd = open(FULLPATH, O_RDWR);
@ -512,6 +513,7 @@ TEST_F(WriteBack, close)
SET_OUT_HEADER_LEN(out, attr);
out->body.attr.attr.ino = ino; // Must match nodeid
})));
expect_flush(ino, 1, ReturnErrno(0));
expect_release(ino, ReturnErrno(0));
fd = open(FULLPATH, O_RDWR);