From 0456375cd37538b4da63b29c47998454362fb57d Mon Sep 17 00:00:00 2001 From: asomers Date: Wed, 8 May 2019 18:12:38 +0000 Subject: [PATCH] fusefs: updated cached attributes during VOP_LINK. FUSE_LINK returns a new set of attributes. fusefs should cache them just like it does during other VOPs. This is not only a matter of performance but of correctness too; without caching the new attributes the vnode's nlink value would be out-of-date. Reported by: pjdfstest Sponsored by: The FreeBSD Foundation --- sys/fs/fuse/fuse_vnops.c | 3 ++ tests/sys/fs/fusefs/link.cc | 25 ++++++++++--- tests/sys/fs/fusefs/setattr.cc | 64 ++++++++++++++++++++++++++++++++++ 3 files changed, 87 insertions(+), 5 deletions(-) diff --git a/sys/fs/fuse/fuse_vnops.c b/sys/fs/fuse/fuse_vnops.c index 20a99cf5c6c..72265c8ca53 100644 --- a/sys/fs/fuse/fuse_vnops.c +++ b/sys/fs/fuse/fuse_vnops.c @@ -817,6 +817,9 @@ fuse_vnop_link(struct vop_link_args *ap) feo = fdi.answ; err = fuse_internal_checkentry(feo, vnode_vtype(vp)); + if (!err) + fuse_internal_cache_attrs(vp, &feo->attr, feo->attr_valid, + feo->attr_valid_nsec, NULL); out: fdisp_destroy(&fdi); return err; diff --git a/tests/sys/fs/fusefs/link.cc b/tests/sys/fs/fusefs/link.cc index 55a1785e342..790f37963da 100644 --- a/tests/sys/fs/fusefs/link.cc +++ b/tests/sys/fs/fusefs/link.cc @@ -77,26 +77,41 @@ TEST_F(Link, ok) const char RELPATH[] = "src"; const char FULLDST[] = "mountpoint/dst"; const char RELDST[] = "dst"; - uint64_t dst_ino = 42; const uint64_t ino = 42; + mode_t mode = S_IFREG | 0644; + struct stat sb; EXPECT_LOOKUP(1, RELPATH).WillOnce(Invoke(ReturnErrno(ENOENT))); - expect_lookup(RELDST, dst_ino); + 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_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 == dst_ino && + 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 = S_IFREG | 0644; + 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_EQ(0, link(FULLDST, FULLPATH)) << strerror(errno); + ASSERT_EQ(0, link(FULLDST, FULLPATH)) << strerror(errno); + // Check that the original file's nlink count has increased. + ASSERT_EQ(0, stat(FULLDST, &sb)) << strerror(errno); + EXPECT_EQ(2ul, sb.st_nlink); } diff --git a/tests/sys/fs/fusefs/setattr.cc b/tests/sys/fs/fusefs/setattr.cc index fea5848497c..51e4f93fee3 100644 --- a/tests/sys/fs/fusefs/setattr.cc +++ b/tests/sys/fs/fusefs/setattr.cc @@ -131,6 +131,70 @@ TEST_F(Setattr, chmod) EXPECT_EQ(0, chmod(FULLPATH, newmode)) << strerror(errno); } +/* + * Chmod a multiply-linked file with cached attributes. Check that both files' + * attributes have changed. + */ +TEST_F(Setattr, chmod_multiply_linked) +{ + 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"; + struct stat sb; + const uint64_t ino = 42; + const mode_t oldmode = 0777; + const mode_t newmode = 0666; + + EXPECT_LOOKUP(1, RELPATH0) + .WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto out) { + SET_OUT_HEADER_LEN(out, entry); + out->body.entry.attr.mode = S_IFREG | oldmode; + 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_LOOKUP(1, RELPATH1) + .WillOnce(Invoke(ReturnImmediate([=](auto in __unused, auto out) { + SET_OUT_HEADER_LEN(out, entry); + out->body.entry.attr.mode = S_IFREG | oldmode; + 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_CALL(*m_mock, process( + ResultOf([](auto in) { + uint32_t valid = FATTR_MODE; + return (in->header.opcode == FUSE_SETATTR && + in->header.nodeid == ino && + in->body.setattr.valid == valid && + in->body.setattr.mode == newmode); + }, Eq(true)), + _) + ).WillOnce(Invoke(ReturnImmediate([](auto in __unused, auto out) { + SET_OUT_HEADER_LEN(out, attr); + out->body.attr.attr.ino = ino; + out->body.attr.attr.mode = S_IFREG | newmode; + out->body.attr.attr.nlink = 2; + out->body.attr.attr_valid = UINT64_MAX; + }))); + + /* For a lookup of the 2nd file to get it into the cache*/ + ASSERT_EQ(0, stat(FULLPATH1, &sb)) << strerror(errno); + EXPECT_EQ(S_IFREG | oldmode, sb.st_mode); + + ASSERT_EQ(0, chmod(FULLPATH0, newmode)) << strerror(errno); + ASSERT_EQ(0, stat(FULLPATH0, &sb)) << strerror(errno); + EXPECT_EQ(S_IFREG | newmode, sb.st_mode); + ASSERT_EQ(0, stat(FULLPATH1, &sb)) << strerror(errno); + EXPECT_EQ(S_IFREG | newmode, sb.st_mode); +} + + /* Change the owner and group of a file */ TEST_F(Setattr, chown) { -- 2.45.0