fusefs: Don't treat fsync the same as fdatasync

For an unknown reason, fusefs was _always_ sending the fdatasync operation
instead of fsync.  Now it correctly sends one or the other.

Also, remove the Fsync.fsync_metadata_only test, along with the recently
removed Fsync.nop.  They should never have been added.  The kernel shouldn't
keep track of which files have dirty data; that's the daemon's job.

PR:		236473
Sponsored by:	The FreeBSD Foundation
This commit is contained in:
Alan Somers 2019-03-21 23:01:56 +00:00
parent 90612f3c38
commit 915012e0d0
4 changed files with 61 additions and 85 deletions

View File

@ -281,32 +281,45 @@ fuse_internal_fsync_callback(struct fuse_ticket *tick, struct uio *uio)
int
fuse_internal_fsync(struct vnode *vp,
struct thread *td,
struct ucred *cred,
struct fuse_filehandle *fufh,
int waitfor)
int waitfor,
bool datasync)
{
int op = FUSE_FSYNC;
struct fuse_fsync_in *ffsi;
struct fuse_dispatcher fdi;
struct fuse_filehandle *fufh;
struct fuse_vnode_data *fvdat = VTOFUD(vp);
int op = FUSE_FSYNC;
int type = 0;
int err = 0;
if (vnode_isdir(vp)) {
op = FUSE_FSYNCDIR;
if (!fsess_isimpl(vnode_mount(vp),
(vnode_vtype(vp) == VDIR ? FUSE_FSYNCDIR : FUSE_FSYNC))) {
return 0;
}
fdisp_init(&fdi, sizeof(*ffsi));
fdisp_make_vp(&fdi, op, vp, td, cred);
ffsi = fdi.indata;
ffsi->fh = fufh->fh_id;
for (type = 0; type < FUFH_MAXTYPE; type++) {
fufh = &(fvdat->fufh[type]);
if (FUFH_IS_VALID(fufh)) {
if (vnode_isdir(vp)) {
op = FUSE_FSYNCDIR;
}
fdisp_init(&fdi, sizeof(*ffsi));
fdisp_make_vp(&fdi, op, vp, td, NULL);
ffsi = fdi.indata;
ffsi->fh = fufh->fh_id;
ffsi->fsync_flags = 1; /* datasync */
if (datasync)
ffsi->fsync_flags = 1;
if (waitfor == MNT_WAIT) {
err = fdisp_wait_answ(&fdi);
} else {
fuse_insert_callback(fdi.tick, fuse_internal_fsync_callback);
fuse_insert_message(fdi.tick);
if (waitfor == MNT_WAIT) {
err = fdisp_wait_answ(&fdi);
} else {
fuse_insert_callback(fdi.tick,
fuse_internal_fsync_callback);
fuse_insert_message(fdi.tick);
}
fdisp_destroy(&fdi);
}
}
fdisp_destroy(&fdi);
return err;
}

View File

@ -198,8 +198,8 @@ void fuse_internal_cache_attrs(struct vnode *vp, struct fuse_attr *attr,
/* fsync */
int fuse_internal_fsync(struct vnode *vp, struct thread *td,
struct ucred *cred, struct fuse_filehandle *fufh, int waitfor);
int fuse_internal_fsync(struct vnode *vp, struct thread *td, int waitfor,
bool datasync);
int fuse_internal_fsync_callback(struct fuse_ticket *tick, struct uio *uio);
/* readdir */

View File

@ -120,6 +120,7 @@ static vop_access_t fuse_vnop_access;
static vop_close_t fuse_vnop_close;
static vop_create_t fuse_vnop_create;
static vop_deleteextattr_t fuse_vnop_deleteextattr;
static vop_fdatasync_t fuse_vnop_fdatasync;
static vop_fsync_t fuse_vnop_fsync;
static vop_getattr_t fuse_vnop_getattr;
static vop_getextattr_t fuse_vnop_getextattr;
@ -154,6 +155,7 @@ struct vop_vector fuse_vnops = {
.vop_create = fuse_vnop_create,
.vop_deleteextattr = fuse_vnop_deleteextattr,
.vop_fsync = fuse_vnop_fsync,
.vop_fdatasync = fuse_vnop_fdatasync,
.vop_getattr = fuse_vnop_getattr,
.vop_getextattr = fuse_vnop_getextattr,
.vop_inactive = fuse_vnop_inactive,
@ -410,22 +412,34 @@ fuse_vnop_create(struct vop_create_args *ap)
}
/*
* Our vnop_fsync roughly corresponds to the FUSE_FSYNC method. The Linux
* version of FUSE also has a FUSE_FLUSH method.
*
* On Linux, fsync() synchronizes a file's complete in-core state with that
* on disk. The call is not supposed to return until the system has completed
* that action or until an error is detected.
*
* Linux also has an fdatasync() call that is similar to fsync() but is not
* required to update the metadata such as access time and modification time.
*/
struct vnop_fdatasync_args {
struct vop_generic_args a_gen;
struct vnode * a_vp;
struct thread * a_td;
};
*/
static int
fuse_vnop_fdatasync(struct vop_fdatasync_args *ap)
{
struct vnode *vp = ap->a_vp;
struct thread *td = ap->a_td;
int waitfor = MNT_WAIT;
int err = 0;
if (fuse_isdeadfs(vp)) {
return 0;
}
if ((err = vop_stdfdatasync_buf(ap)))
return err;
return fuse_internal_fsync(vp, td, waitfor, true);
}
/*
struct vnop_fsync_args {
struct vnodeop_desc *a_desc;
struct vop_generic_args a_gen;
struct vnode * a_vp;
struct ucred * a_cred;
int a_waitfor;
struct thread * a_td;
};
@ -436,11 +450,7 @@ fuse_vnop_fsync(struct vop_fsync_args *ap)
struct vnode *vp = ap->a_vp;
struct thread *td = ap->a_td;
int waitfor = ap->a_waitfor;
struct fuse_filehandle *fufh;
struct fuse_vnode_data *fvdat = VTOFUD(vp);
int type, err = 0;
int err = 0;
if (fuse_isdeadfs(vp)) {
return 0;
@ -448,19 +458,7 @@ fuse_vnop_fsync(struct vop_fsync_args *ap)
if ((err = vop_stdfsync(ap)))
return err;
if (!fsess_isimpl(vnode_mount(vp),
(vnode_vtype(vp) == VDIR ? FUSE_FSYNCDIR : FUSE_FSYNC))) {
goto out;
}
for (type = 0; type < FUFH_MAXTYPE; type++) {
fufh = &(fvdat->fufh[type]);
if (FUFH_IS_VALID(fufh)) {
err = fuse_internal_fsync(vp, td, NULL, fufh, waitfor);
}
}
out:
return err;
return fuse_internal_fsync(vp, td, waitfor, false);
}
/*

View File

@ -76,7 +76,6 @@ void expect_write(uint64_t ino, uint64_t size, const void *contents)
};
/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236379 */
/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236473 */
TEST_F(Fsync, DISABLED_aio_fsync)
{
const char FULLPATH[] = "mountpoint/some_file.txt";
@ -227,8 +226,7 @@ TEST_F(Fsync, fdatasync)
/* Deliberately leak fd. close(2) will be tested in release.cc */
}
/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236473 */
TEST_F(Fsync, DISABLED_fsync)
TEST_F(Fsync, fsync)
{
const char FULLPATH[] = "mountpoint/some_file.txt";
const char RELPATH[] = "some_file.txt";
@ -250,36 +248,3 @@ TEST_F(Fsync, DISABLED_fsync)
/* Deliberately leak fd. close(2) will be tested in release.cc */
}
/* Fsync should sync a file with dirty metadata but clean data */
/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236473 */
TEST_F(Fsync, DISABLED_fsync_metadata_only)
{
const char FULLPATH[] = "mountpoint/some_file.txt";
const char RELPATH[] = "some_file.txt";
uint64_t ino = 42;
int fd;
mode_t mode = 0755;
expect_lookup(RELPATH, ino);
expect_open(ino, 0, 1);
expect_getattr(ino, 0);
EXPECT_CALL(*m_mock, process(
ResultOf([=](auto in) {
return (in->header.opcode == FUSE_SETATTR);
}, Eq(true)),
_)
).WillOnce(Invoke(ReturnImmediate([=](auto i __unused, auto out) {
SET_OUT_HEADER_LEN(out, attr);
out->body.attr.attr.ino = ino; // Must match nodeid
out->body.attr.attr.mode = S_IFREG | mode;
})));
expect_fsync(ino, 0, 0);
fd = open(FULLPATH, O_RDWR);
ASSERT_LE(0, fd) << strerror(errno);
ASSERT_EQ(0, fchmod(fd, mode)) << strerror(errno);
ASSERT_EQ(0, fsync(fd)) << strerror(errno);
/* Deliberately leak fd. close(2) will be tested in release.cc */
}