Date: Thu, 9 May 2019 01:16:34 +0000 (UTC) From: Alan Somers <asomers@FreeBSD.org> To: src-committers@freebsd.org, svn-src-projects@freebsd.org Subject: svn commit: r347377 - in projects/fuse2: sys/fs/fuse tests/sys/fs/fusefs Message-ID: <201905090116.x491GZRZ020925@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: asomers Date: Thu May 9 01:16:34 2019 New Revision: 347377 URL: https://svnweb.freebsd.org/changeset/base/347377 Log: fusefs: clear a dir's attr cache when its contents change Any change to a directory's contents should cause its mtime and ctime to be updated by the FUSE daemon. Clear its attribute cache so we'll get the new attributs the next time that they're needed. This affects the following VOPs: VOP_CREATE, VOP_LINK, VOP_MKDIR, VOP_MKNOD, VOP_REMOVE, VOP_RMDIR, and VOP_SYMLINK Reported by: pjdfstest Sponsored by: The FreeBSD Foundation Modified: projects/fuse2/sys/fs/fuse/fuse_internal.c projects/fuse2/sys/fs/fuse/fuse_node.h projects/fuse2/sys/fs/fuse/fuse_vnops.c projects/fuse2/tests/sys/fs/fusefs/create.cc projects/fuse2/tests/sys/fs/fusefs/link.cc projects/fuse2/tests/sys/fs/fusefs/mockfs.cc projects/fuse2/tests/sys/fs/fusefs/rmdir.cc projects/fuse2/tests/sys/fs/fusefs/symlink.cc projects/fuse2/tests/sys/fs/fusefs/unlink.cc Modified: projects/fuse2/sys/fs/fuse/fuse_internal.c ============================================================================== --- projects/fuse2/sys/fs/fuse/fuse_internal.c Thu May 9 01:16:03 2019 (r347376) +++ projects/fuse2/sys/fs/fuse/fuse_internal.c Thu May 9 01:16:34 2019 (r347377) @@ -535,6 +535,13 @@ fuse_internal_newentry_core(struct vnode *dvp, feo->nodeid, 1); return err; } + + /* + * Purge the parent's attribute cache because the daemon should've + * updated its mtime and ctime + */ + fuse_vnode_clear_attr_cache(dvp); + fuse_internal_cache_attrs(*vpp, &feo->attr, feo->attr_valid, feo->attr_valid_nsec, NULL); Modified: projects/fuse2/sys/fs/fuse/fuse_node.h ============================================================================== --- projects/fuse2/sys/fs/fuse/fuse_node.h Thu May 9 01:16:03 2019 (r347376) +++ projects/fuse2/sys/fs/fuse/fuse_node.h Thu May 9 01:16:34 2019 (r347377) @@ -110,6 +110,12 @@ VTOVA(struct vnode *vp) return NULL; } +static inline void +fuse_vnode_clear_attr_cache(struct vnode *vp) +{ + bintime_clear(&VTOFUD(vp)->attr_cache_timeout); +} + #define VTOILLU(vp) ((uint64_t)(VTOFUD(vp) ? VTOI(vp) : 0)) #define FUSE_NULL_ID 0 Modified: projects/fuse2/sys/fs/fuse/fuse_vnops.c ============================================================================== --- projects/fuse2/sys/fs/fuse/fuse_vnops.c Thu May 9 01:16:03 2019 (r347376) +++ projects/fuse2/sys/fs/fuse/fuse_vnops.c Thu May 9 01:16:34 2019 (r347377) @@ -625,6 +625,11 @@ fuse_vnop_create(struct vop_create_args *ap) fuse_filehandle_init(*vpp, FUFH_RDWR, NULL, td, cred, foo); fuse_vnode_open(*vpp, foo->open_flags, td); + /* + * Purge the parent's attribute cache because the daemon should've + * updated its mtime and ctime + */ + fuse_vnode_clear_attr_cache(dvp); cache_purge_negative(dvp); out: @@ -817,9 +822,15 @@ fuse_vnop_link(struct vop_link_args *ap) feo = fdi.answ; err = fuse_internal_checkentry(feo, vnode_vtype(vp)); - if (!err) + if (!err) { + /* + * Purge the parent's attribute cache because the daemon + * should've updated its mtime and ctime + */ + fuse_vnode_clear_attr_cache(tdvp); fuse_internal_cache_attrs(vp, &feo->attr, feo->attr_valid, feo->attr_valid_nsec, NULL); + } out: fdisp_destroy(&fdi); return err; @@ -1398,8 +1409,14 @@ fuse_vnop_remove(struct vop_remove_args *ap) err = fuse_internal_remove(dvp, vp, cnp, FUSE_UNLINK); - if (err == 0) + if (err == 0) { fuse_internal_vnode_disappear(vp); + /* + * Purge the parent's attribute cache because the daemon + * should've updated its mtime and ctime + */ + fuse_vnode_clear_attr_cache(dvp); + } return err; } @@ -1511,8 +1528,14 @@ fuse_vnop_rmdir(struct vop_rmdir_args *ap) } err = fuse_internal_remove(dvp, vp, ap->a_cnp, FUSE_RMDIR); - if (err == 0) + if (err == 0) { fuse_internal_vnode_disappear(vp); + /* + * Purge the parent's attribute cache because the daemon + * should've updated its mtime and ctime + */ + fuse_vnode_clear_attr_cache(dvp); + } return err; } Modified: projects/fuse2/tests/sys/fs/fusefs/create.cc ============================================================================== --- projects/fuse2/tests/sys/fs/fusefs/create.cc Thu May 9 01:16:03 2019 (r347376) +++ projects/fuse2/tests/sys/fs/fusefs/create.cc Thu May 9 01:16:34 2019 (r347377) @@ -59,7 +59,7 @@ void expect_create(const char *relpath, mode_t mode, P }; /* - * If FUSE_CREATE sets the attr_valid, then subsequent GETATTRs should use the + * If FUSE_CREATE sets attr_valid, then subsequent GETATTRs should use the * attribute cache */ TEST_F(Create, attr_cache) @@ -90,6 +90,48 @@ TEST_F(Create, attr_cache) fd = open(FULLPATH, O_CREAT | O_EXCL, mode); EXPECT_LE(0, fd) << strerror(errno); + /* Deliberately leak fd. close(2) will be tested in release.cc */ +} + +/* A successful CREATE operation should purge the parent dir's attr cache */ +TEST_F(Create, clear_attr_cache) +{ + const char FULLPATH[] = "mountpoint/src"; + const char RELPATH[] = "src"; + mode_t mode = S_IFREG | 0755; + uint64_t ino = 42; + int fd; + struct stat sb; + + EXPECT_LOOKUP(1, RELPATH).WillOnce(Invoke(ReturnErrno(ENOENT))); + EXPECT_CALL(*m_mock, process( + ResultOf([=](auto in) { + return (in->header.opcode == FUSE_GETATTR && + in->header.nodeid == 1); + }, Eq(true)), + _) + ).Times(2) + .WillRepeatedly(Invoke(ReturnImmediate([=](auto i __unused, auto out) { + SET_OUT_HEADER_LEN(out, attr); + out->body.attr.attr.ino = 1; + out->body.attr.attr.mode = S_IFDIR | 0755; + out->body.attr.attr_valid = UINT64_MAX; + }))); + + expect_create(RELPATH, mode, + ReturnImmediate([=](auto in __unused, auto out) { + SET_OUT_HEADER_LEN(out, create); + out->body.create.entry.attr.mode = mode; + out->body.create.entry.nodeid = ino; + out->body.create.entry.entry_valid = UINT64_MAX; + out->body.create.entry.attr_valid = UINT64_MAX; + })); + + EXPECT_EQ(0, stat("mountpoint", &sb)) << strerror(errno); + fd = open(FULLPATH, O_CREAT | O_EXCL, mode); + EXPECT_LE(0, fd) << strerror(errno); + EXPECT_EQ(0, stat("mountpoint", &sb)) << strerror(errno); + /* Deliberately leak fd. close(2) will be tested in release.cc */ } Modified: projects/fuse2/tests/sys/fs/fusefs/link.cc ============================================================================== --- projects/fuse2/tests/sys/fs/fusefs/link.cc Thu May 9 01:16:03 2019 (r347376) +++ projects/fuse2/tests/sys/fs/fusefs/link.cc Thu May 9 01:16:34 2019 (r347377) @@ -39,12 +39,78 @@ using namespace testing; class Link: public FuseTest { public: +void expect_link(uint64_t ino, const char *relpath, mode_t mode, uint32_t nlink) +{ + EXPECT_CALL(*m_mock, process( + ResultOf([=](auto in) { + const char *name = (const char*)in->body.bytes + + sizeof(struct fuse_link_in); + return (in->header.opcode == FUSE_LINK && + in->body.link.oldnodeid == ino && + (0 == strcmp(name, relpath))); + }, Eq(true)), + _) + ).WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto out) { + SET_OUT_HEADER_LEN(out, entry); + out->body.entry.nodeid = ino; + out->body.entry.attr.mode = mode; + out->body.entry.attr.nlink = nlink; + out->body.entry.attr_valid = UINT64_MAX; + out->body.entry.entry_valid = UINT64_MAX; + }))); +} + void expect_lookup(const char *relpath, uint64_t ino) { FuseTest::expect_lookup(relpath, ino, S_IFREG | 0644, 0, 1); } }; +/* + * A successful link should clear the parent directory's attribute cache, + * because the fuse daemon should update its mtime and ctime + */ +TEST_F(Link, clear_attr_cache) +{ + const char FULLPATH[] = "mountpoint/src"; + const char RELPATH[] = "src"; + const char FULLDST[] = "mountpoint/dst"; + const char RELDST[] = "dst"; + const uint64_t ino = 42; + mode_t mode = S_IFREG | 0644; + struct stat sb; + + EXPECT_LOOKUP(1, RELPATH).WillOnce(Invoke(ReturnErrno(ENOENT))); + EXPECT_CALL(*m_mock, process( + ResultOf([=](auto in) { + return (in->header.opcode == FUSE_GETATTR && + in->header.nodeid == 1); + }, Eq(true)), + _) + ).Times(2) + .WillRepeatedly(Invoke(ReturnImmediate([=](auto i __unused, auto out) { + SET_OUT_HEADER_LEN(out, attr); + out->body.attr.attr.ino = 1; + out->body.attr.attr.mode = S_IFDIR | 0755; + out->body.attr.attr_valid = UINT64_MAX; + }))); + + EXPECT_LOOKUP(1, RELDST) + .WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto out) { + SET_OUT_HEADER_LEN(out, entry); + out->body.entry.attr.mode = mode; + out->body.entry.nodeid = ino; + out->body.entry.attr.nlink = 1; + out->body.entry.attr_valid = UINT64_MAX; + out->body.entry.entry_valid = UINT64_MAX; + }))); + expect_link(ino, RELPATH, mode, 2); + + EXPECT_EQ(0, stat("mountpoint", &sb)) << strerror(errno); + EXPECT_EQ(0, link(FULLDST, FULLPATH)) << strerror(errno); + EXPECT_EQ(0, stat("mountpoint", &sb)) << strerror(errno); +} + TEST_F(Link, emlink) { const char FULLPATH[] = "mountpoint/lnk"; @@ -91,24 +157,7 @@ TEST_F(Link, ok) out->body.entry.attr_valid = UINT64_MAX; out->body.entry.entry_valid = UINT64_MAX; }))); - - EXPECT_CALL(*m_mock, process( - ResultOf([=](auto in) { - const char *name = (const char*)in->body.bytes - + sizeof(struct fuse_link_in); - return (in->header.opcode == FUSE_LINK && - in->body.link.oldnodeid == ino && - (0 == strcmp(name, RELPATH))); - }, Eq(true)), - _) - ).WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto out) { - SET_OUT_HEADER_LEN(out, entry); - out->body.entry.attr.mode = mode; - out->body.entry.nodeid = ino; - out->body.entry.attr.nlink = 2; - out->body.entry.attr_valid = UINT64_MAX; - out->body.entry.entry_valid = UINT64_MAX; - }))); + expect_link(ino, RELPATH, mode, 2); ASSERT_EQ(0, link(FULLDST, FULLPATH)) << strerror(errno); // Check that the original file's nlink count has increased. Modified: projects/fuse2/tests/sys/fs/fusefs/mockfs.cc ============================================================================== --- projects/fuse2/tests/sys/fs/fusefs/mockfs.cc Thu May 9 01:16:03 2019 (r347376) +++ projects/fuse2/tests/sys/fs/fusefs/mockfs.cc Thu May 9 01:16:34 2019 (r347377) @@ -187,6 +187,9 @@ void debug_fuseop(const mockfs_buf_in *in) case FUSE_INTERRUPT: printf(" unique=%lu", in->body.interrupt.unique); break; + case FUSE_LINK: + printf(" oldnodeid=%lu", in->body.link.oldnodeid); + break; case FUSE_LOOKUP: printf(" %s", in->body.lookup); break; Modified: projects/fuse2/tests/sys/fs/fusefs/rmdir.cc ============================================================================== --- projects/fuse2/tests/sys/fs/fusefs/rmdir.cc Thu May 9 01:16:03 2019 (r347376) +++ projects/fuse2/tests/sys/fs/fusefs/rmdir.cc Thu May 9 01:16:34 2019 (r347377) @@ -65,25 +65,61 @@ void expect_lookup(const char *relpath, uint64_t ino) out->body.entry.attr.nlink = 2; }))); } + +void expect_rmdir(uint64_t parent, const char *relpath, int error) +{ + EXPECT_CALL(*m_mock, process( + ResultOf([=](auto in) { + return (in->header.opcode == FUSE_RMDIR && + 0 == strcmp(relpath, in->body.rmdir) && + in->header.nodeid == parent); + }, Eq(true)), + _) + ).WillOnce(Invoke(ReturnErrno(error))); +} }; -TEST_F(Rmdir, enotempty) +/* + * A successful rmdir should clear the parent directory's attribute cache, + * because the fuse daemon should update its mtime and ctime + */ +TEST_F(Rmdir, clear_attr_cache) { const char FULLPATH[] = "mountpoint/some_dir"; const char RELPATH[] = "some_dir"; + struct stat sb; uint64_t ino = 42; - expect_getattr(1, S_IFDIR | 0755); - expect_lookup(RELPATH, ino); EXPECT_CALL(*m_mock, process( ResultOf([=](auto in) { - return (in->header.opcode == FUSE_RMDIR && - 0 == strcmp(RELPATH, in->body.rmdir) && + return (in->header.opcode == FUSE_GETATTR && in->header.nodeid == 1); }, Eq(true)), _) - ).WillOnce(Invoke(ReturnErrno(ENOTEMPTY))); + ).Times(2) + .WillRepeatedly(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_IFDIR | 0755; + out->body.attr.attr_valid = UINT64_MAX; + }))); + expect_lookup(RELPATH, ino); + expect_rmdir(1, RELPATH, 0); + ASSERT_EQ(0, rmdir(FULLPATH)) << strerror(errno); + EXPECT_EQ(0, stat("mountpoint", &sb)) << strerror(errno); +} + +TEST_F(Rmdir, enotempty) +{ + const char FULLPATH[] = "mountpoint/some_dir"; + const char RELPATH[] = "some_dir"; + uint64_t ino = 42; + + expect_getattr(1, S_IFDIR | 0755); + expect_lookup(RELPATH, ino); + expect_rmdir(1, RELPATH, ENOTEMPTY); + ASSERT_NE(0, rmdir(FULLPATH)); ASSERT_EQ(ENOTEMPTY, errno); } @@ -96,14 +132,7 @@ TEST_F(Rmdir, ok) expect_getattr(1, S_IFDIR | 0755); expect_lookup(RELPATH, ino); - EXPECT_CALL(*m_mock, process( - ResultOf([=](auto in) { - return (in->header.opcode == FUSE_RMDIR && - 0 == strcmp(RELPATH, in->body.rmdir) && - in->header.nodeid == 1); - }, Eq(true)), - _) - ).WillOnce(Invoke(ReturnErrno(0))); + expect_rmdir(1, RELPATH, 0); ASSERT_EQ(0, rmdir(FULLPATH)) << strerror(errno); } Modified: projects/fuse2/tests/sys/fs/fusefs/symlink.cc ============================================================================== --- projects/fuse2/tests/sys/fs/fusefs/symlink.cc Thu May 9 01:16:03 2019 (r347376) +++ projects/fuse2/tests/sys/fs/fusefs/symlink.cc Thu May 9 01:16:34 2019 (r347377) @@ -37,8 +37,64 @@ extern "C" { using namespace testing; -class Symlink: public FuseTest {}; +class Symlink: public FuseTest { +public: +void expect_symlink(uint64_t ino, const char *target, const char *relpath) +{ + EXPECT_CALL(*m_mock, process( + ResultOf([=](auto in) { + const char *name = (const char*)in->body.bytes; + const char *linkname = name + strlen(name) + 1; + return (in->header.opcode == FUSE_SYMLINK && + (0 == strcmp(linkname, target)) && + (0 == strcmp(name, relpath))); + }, Eq(true)), + _) + ).WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto out) { + SET_OUT_HEADER_LEN(out, entry); + out->body.entry.attr.mode = S_IFLNK | 0777; + out->body.entry.nodeid = ino; + out->body.entry.entry_valid = UINT64_MAX; + out->body.entry.attr_valid = UINT64_MAX; + }))); +} + +}; + +/* + * A successful symlink should clear the parent directory's attribute cache, + * because the fuse daemon should update its mtime and ctime + */ +TEST_F(Symlink, clear_attr_cache) +{ + const char FULLPATH[] = "mountpoint/src"; + const char RELPATH[] = "src"; + const char dst[] = "dst"; + const uint64_t ino = 42; + struct stat sb; + + EXPECT_LOOKUP(1, RELPATH).WillOnce(Invoke(ReturnErrno(ENOENT))); + EXPECT_CALL(*m_mock, process( + ResultOf([=](auto in) { + return (in->header.opcode == FUSE_GETATTR && + in->header.nodeid == 1); + }, Eq(true)), + _) + ).Times(2) + .WillRepeatedly(Invoke(ReturnImmediate([=](auto i __unused, auto out) { + SET_OUT_HEADER_LEN(out, attr); + out->body.attr.attr.ino = 1; + out->body.attr.attr.mode = S_IFDIR | 0755; + out->body.attr.attr_valid = UINT64_MAX; + }))); + expect_symlink(ino, dst, RELPATH); + + EXPECT_EQ(0, stat("mountpoint", &sb)) << strerror(errno); + EXPECT_EQ(0, symlink(dst, FULLPATH)) << strerror(errno); + EXPECT_EQ(0, stat("mountpoint", &sb)) << strerror(errno); +} + TEST_F(Symlink, enospc) { const char FULLPATH[] = "mountpoint/lnk"; @@ -70,21 +126,7 @@ TEST_F(Symlink, ok) const uint64_t ino = 42; EXPECT_LOOKUP(1, RELPATH).WillOnce(Invoke(ReturnErrno(ENOENT))); - - EXPECT_CALL(*m_mock, process( - ResultOf([=](auto in) { - const char *name = (const char*)in->body.bytes; - const char *linkname = name + strlen(name) + 1; - return (in->header.opcode == FUSE_SYMLINK && - (0 == strcmp(linkname, dst)) && - (0 == strcmp(name, RELPATH))); - }, Eq(true)), - _) - ).WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto out) { - SET_OUT_HEADER_LEN(out, entry); - out->body.entry.attr.mode = S_IFLNK | 0777; - out->body.entry.nodeid = ino; - }))); + expect_symlink(ino, dst, RELPATH); EXPECT_EQ(0, symlink(dst, FULLPATH)) << strerror(errno); } Modified: projects/fuse2/tests/sys/fs/fusefs/unlink.cc ============================================================================== --- projects/fuse2/tests/sys/fs/fusefs/unlink.cc Thu May 9 01:16:03 2019 (r347376) +++ projects/fuse2/tests/sys/fs/fusefs/unlink.cc Thu May 9 01:16:34 2019 (r347377) @@ -61,6 +61,37 @@ void expect_lookup(const char *relpath, uint64_t ino, }; +/* + * A successful unlink should clear the parent directory's attribute cache, + * because the fuse daemon should update its mtime and ctime + */ +TEST_F(Unlink, clear_attr_cache) +{ + const char FULLPATH[] = "mountpoint/some_file.txt"; + const char RELPATH[] = "some_file.txt"; + struct stat sb; + uint64_t ino = 42; + + EXPECT_CALL(*m_mock, process( + ResultOf([=](auto in) { + return (in->header.opcode == FUSE_GETATTR && + in->header.nodeid == 1); + }, Eq(true)), + _) + ).Times(2) + .WillRepeatedly(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_IFDIR | 0755; + out->body.attr.attr_valid = UINT64_MAX; + }))); + expect_lookup(RELPATH, ino, 1); + expect_unlink(1, RELPATH, 0); + + ASSERT_EQ(0, unlink(FULLPATH)) << strerror(errno); + EXPECT_EQ(0, stat("mountpoint", &sb)) << strerror(errno); +} + TEST_F(Unlink, eperm) { const char FULLPATH[] = "mountpoint/some_file.txt";
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201905090116.x491GZRZ020925>