From e433e105baf29db49c750a860f6bbb6bf168e8bd Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Wed, 20 Mar 2019 20:07:12 +0000 Subject: [PATCH 01/11] googletest: Also build the new gtest_skip_test This is a follow-up to r345331 Reported by: ngie Sponsored by: The FreeBSD Foundation --- lib/googletest/gtest_main/tests/Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/googletest/gtest_main/tests/Makefile b/lib/googletest/gtest_main/tests/Makefile index 60ec30579b55..d7399ebaad4d 100644 --- a/lib/googletest/gtest_main/tests/Makefile +++ b/lib/googletest/gtest_main/tests/Makefile @@ -18,6 +18,7 @@ GTESTS+= gtest_prod_test GTESTS+= gtest_sole_header_test GTESTS+= googletest-test-part-test GTESTS+= gtest-typed-test_test +GTESTS+= gtest_skip_test GTESTS+= gtest_unittest CXXFLAGS+= -I${GOOGLETEST_SRCROOT}/include From 4f1543f359d6d2d67997cf32df63806e2b53d0ab Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Wed, 20 Mar 2019 20:36:46 +0000 Subject: [PATCH 02/11] fuse(4): use GTEST_SKIP in the tests Now the entire fuse test suite can "pass", or at least not fail. Skipped tests are reported to Kyua as passes, because googletest is still using Kyua's plain test adapter. Sponsored by: The FreeBSD Foundation --- tests/sys/fs/fuse/default_permissions.cc | 5 +- tests/sys/fs/fuse/mknod.cc | 8 ++-- tests/sys/fs/fuse/read.cc | 7 +-- tests/sys/fs/fuse/utils.cc | 59 +++++++++++++++--------- tests/sys/fs/fuse/write.cc | 27 ++++++----- 5 files changed, 63 insertions(+), 43 deletions(-) diff --git a/tests/sys/fs/fuse/default_permissions.cc b/tests/sys/fs/fuse/default_permissions.cc index ffc56bb6cdc3..9790f75601b0 100644 --- a/tests/sys/fs/fuse/default_permissions.cc +++ b/tests/sys/fs/fuse/default_permissions.cc @@ -46,11 +46,12 @@ using namespace testing; class DefaultPermissions: public FuseTest { virtual void SetUp() { + FuseTest::SetUp(); + if (geteuid() == 0) { - FAIL() << "This test requires an unprivileged user"; + GTEST_SKIP() << "This test requires an unprivileged user"; } m_default_permissions = true; - FuseTest::SetUp(); } public: diff --git a/tests/sys/fs/fuse/mknod.cc b/tests/sys/fs/fuse/mknod.cc index 4d456b48a0cb..f77a91bfe3cc 100644 --- a/tests/sys/fs/fuse/mknod.cc +++ b/tests/sys/fs/fuse/mknod.cc @@ -42,11 +42,11 @@ class Mknod: public FuseTest { public: virtual void SetUp() { - if (geteuid() != 0) { - // TODO: With GoogleTest 1.8.2, use SKIP instead - FAIL() << "Only root may use most mknod(2) variations"; - } FuseTest::SetUp(); + + if (geteuid() != 0) { + GTEST_SKIP() << "Only root may use most mknod(2) variations"; + } } /* Test an OK creation of a file with the given mode and device number */ diff --git a/tests/sys/fs/fuse/read.cc b/tests/sys/fs/fuse/read.cc index 15d73aa919ad..a1145c29a4ad 100644 --- a/tests/sys/fs/fuse/read.cc +++ b/tests/sys/fs/fuse/read.cc @@ -61,12 +61,13 @@ virtual void SetUp() { int val = 0; size_t size = sizeof(val); + FuseTest::SetUp(); + ASSERT_EQ(0, sysctlbyname(node, &val, &size, NULL, 0)) << strerror(errno); - // TODO: With GoogleTest 1.8.2, use SKIP instead if (!val) - FAIL() << "vfs.aio.enable_unsafe must be set for this test"; - FuseTest::SetUp(); + GTEST_SKIP() << + "vfs.aio.enable_unsafe must be set for this test"; } }; diff --git a/tests/sys/fs/fuse/utils.cc b/tests/sys/fs/fuse/utils.cc index a08daf36a8c1..b6776eafbf87 100644 --- a/tests/sys/fs/fuse/utils.cc +++ b/tests/sys/fs/fuse/utils.cc @@ -40,31 +40,36 @@ using namespace testing; +/* Check that fuse(4) is accessible and the current user can mount(2) */ +void check_environment() +{ + const char *mod_name = "fuse"; + const char *devnode = "/dev/fuse"; + const char *usermount_node = "vfs.usermount"; + int usermount_val = 0; + size_t usermount_size = sizeof(usermount_val); + if (modfind(mod_name) == -1) { + GTEST_SKIP() << "Module " << mod_name << + " could not be resolved"; + } + if (eaccess(devnode, R_OK | W_OK)) { + if (errno == ENOENT) { + GTEST_SKIP() << devnode << " does not exist"; + } else if (errno == EACCES) { + GTEST_SKIP() << devnode << + " is not accessible by the current user"; + } else { + GTEST_SKIP() << strerror(errno); + } + } + sysctlbyname(usermount_node, &usermount_val, &usermount_size, + NULL, 0); + if (geteuid() != 0 && !usermount_val) + GTEST_SKIP() << "current user is not allowed to mount"; +} + class FuseEnv: public Environment { virtual void SetUp() { - const char *mod_name = "fuse"; - const char *devnode = "/dev/fuse"; - const char *usermount_node = "vfs.usermount"; - int usermount_val = 0; - size_t usermount_size = sizeof(usermount_val); - if (modfind(mod_name) == -1) { - FAIL() << "Module " << mod_name << - " could not be resolved"; - } - if (eaccess(devnode, R_OK | W_OK)) { - if (errno == ENOENT) { - FAIL() << devnode << " does not exist"; - } else if (errno == EACCES) { - FAIL() << devnode << - " is not accessible by the current user"; - } else { - FAIL() << strerror(errno); - } - } - sysctlbyname(usermount_node, &usermount_val, &usermount_size, - NULL, 0); - if (geteuid() != 0 && !usermount_val) - FAIL() << "current user is not allowed to mount"; } }; @@ -73,6 +78,14 @@ void FuseTest::SetUp() { int val = 0; size_t size = sizeof(val); + /* + * XXX check_environment should be called from FuseEnv::SetUp, but + * can't due to https://github.com/google/googletest/issues/2189 + */ + check_environment(); + if (IsSkipped()) + return; + ASSERT_EQ(0, sysctlbyname(node, &val, &size, NULL, 0)) << strerror(errno); m_maxbcachebuf = val; diff --git a/tests/sys/fs/fuse/write.cc b/tests/sys/fs/fuse/write.cc index a57a7e322495..de20b03b8f6f 100644 --- a/tests/sys/fs/fuse/write.cc +++ b/tests/sys/fs/fuse/write.cc @@ -73,7 +73,8 @@ void require_sync_resize_0() { ASSERT_EQ(0, sysctlbyname(sync_resize_node, &val, &size, NULL, 0)) << strerror(errno); if (val != 0) - FAIL() << "vfs.fuse.sync_resize must be set to 0 for this test." + GTEST_SKIP() << + "vfs.fuse.sync_resize must be set to 0 for this test." " That sysctl will probably be removed soon."; } @@ -85,12 +86,13 @@ virtual void SetUp() { int val = 0; size_t size = sizeof(val); + FuseTest::SetUp(); + ASSERT_EQ(0, sysctlbyname(node, &val, &size, NULL, 0)) << strerror(errno); - // TODO: With GoogleTest 1.8.2, use SKIP instead if (!val) - FAIL() << "vfs.aio.enable_unsafe must be set for this test"; - FuseTest::SetUp(); + GTEST_SKIP() << + "vfs.aio.enable_unsafe must be set for this test"; } }; @@ -102,14 +104,15 @@ virtual void SetUp() { int val = 0; size_t size = sizeof(val); + FuseTest::SetUp(); + if (IsSkipped()) + return; + ASSERT_EQ(0, sysctlbyname(cache_mode_node, &val, &size, NULL, 0)) << strerror(errno); - // TODO: With GoogleTest 1.8.2, use SKIP instead if (val != 1) - FAIL() << "vfs.fuse.data_cache_mode must be set to 1 " + GTEST_SKIP() << "vfs.fuse.data_cache_mode must be set to 1 " "(writethrough) for this test"; - - FuseTest::SetUp(); } }; @@ -122,13 +125,15 @@ virtual void SetUp() { int val = 0; size_t size = sizeof(val); + FuseTest::SetUp(); + if (IsSkipped()) + return; + ASSERT_EQ(0, sysctlbyname(node, &val, &size, NULL, 0)) << strerror(errno); - // TODO: With GoogleTest 1.8.2, use SKIP instead if (val != 2) - FAIL() << "vfs.fuse.data_cache_mode must be set to 2 " + GTEST_SKIP() << "vfs.fuse.data_cache_mode must be set to 2 " "(writeback) for this test"; - FuseTest::SetUp(); } }; From 9821f1d3231d4dea3e1319358f416425be2266a6 Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Thu, 21 Mar 2019 00:11:43 +0000 Subject: [PATCH 03/11] fusefs: adapt the tests to the fuse => fusefs rename Sponsored by: The FreeBSD Foundation --- etc/mtree/BSD.tests.dist | 2 +- tests/sys/fs/Makefile | 2 +- tests/sys/fs/{fuse => fusefs}/Makefile | 2 +- tests/sys/fs/{fuse => fusefs}/access.cc | 0 tests/sys/fs/{fuse => fusefs}/create.cc | 0 .../sys/fs/{fuse => fusefs}/default_permissions.cc | 0 tests/sys/fs/{fuse => fusefs}/destroy.cc | 0 tests/sys/fs/{fuse => fusefs}/flush.cc | 0 tests/sys/fs/{fuse => fusefs}/fsync.cc | 0 tests/sys/fs/{fuse => fusefs}/fsyncdir.cc | 0 tests/sys/fs/{fuse => fusefs}/getattr.cc | 0 tests/sys/fs/{fuse => fusefs}/interrupt.cc | 0 tests/sys/fs/{fuse => fusefs}/link.cc | 0 tests/sys/fs/{fuse => fusefs}/locks.cc | 0 tests/sys/fs/{fuse => fusefs}/lookup.cc | 0 tests/sys/fs/{fuse => fusefs}/mkdir.cc | 0 tests/sys/fs/{fuse => fusefs}/mknod.cc | 0 tests/sys/fs/{fuse => fusefs}/mockfs.cc | 0 tests/sys/fs/{fuse => fusefs}/mockfs.hh | 2 +- tests/sys/fs/{fuse => fusefs}/open.cc | 0 tests/sys/fs/{fuse => fusefs}/opendir.cc | 0 tests/sys/fs/{fuse => fusefs}/read.cc | 0 tests/sys/fs/{fuse => fusefs}/readdir.cc | 0 tests/sys/fs/{fuse => fusefs}/readlink.cc | 0 tests/sys/fs/{fuse => fusefs}/release.cc | 0 tests/sys/fs/{fuse => fusefs}/releasedir.cc | 0 tests/sys/fs/{fuse => fusefs}/rename.cc | 0 tests/sys/fs/{fuse => fusefs}/rmdir.cc | 0 tests/sys/fs/{fuse => fusefs}/setattr.cc | 0 tests/sys/fs/{fuse => fusefs}/statfs.cc | 0 tests/sys/fs/{fuse => fusefs}/symlink.cc | 0 tests/sys/fs/{fuse => fusefs}/unlink.cc | 0 tests/sys/fs/{fuse => fusefs}/utils.cc | 4 ++-- tests/sys/fs/{fuse => fusefs}/utils.hh | 0 tests/sys/fs/{fuse => fusefs}/write.cc | 14 +++++++------- tests/sys/fs/{fuse => fusefs}/xattr.cc | 0 36 files changed, 13 insertions(+), 13 deletions(-) rename tests/sys/fs/{fuse => fusefs}/Makefile (99%) rename tests/sys/fs/{fuse => fusefs}/access.cc (100%) rename tests/sys/fs/{fuse => fusefs}/create.cc (100%) rename tests/sys/fs/{fuse => fusefs}/default_permissions.cc (100%) rename tests/sys/fs/{fuse => fusefs}/destroy.cc (100%) rename tests/sys/fs/{fuse => fusefs}/flush.cc (100%) rename tests/sys/fs/{fuse => fusefs}/fsync.cc (100%) rename tests/sys/fs/{fuse => fusefs}/fsyncdir.cc (100%) rename tests/sys/fs/{fuse => fusefs}/getattr.cc (100%) rename tests/sys/fs/{fuse => fusefs}/interrupt.cc (100%) rename tests/sys/fs/{fuse => fusefs}/link.cc (100%) rename tests/sys/fs/{fuse => fusefs}/locks.cc (100%) rename tests/sys/fs/{fuse => fusefs}/lookup.cc (100%) rename tests/sys/fs/{fuse => fusefs}/mkdir.cc (100%) rename tests/sys/fs/{fuse => fusefs}/mknod.cc (100%) rename tests/sys/fs/{fuse => fusefs}/mockfs.cc (100%) rename tests/sys/fs/{fuse => fusefs}/mockfs.hh (98%) rename tests/sys/fs/{fuse => fusefs}/open.cc (100%) rename tests/sys/fs/{fuse => fusefs}/opendir.cc (100%) rename tests/sys/fs/{fuse => fusefs}/read.cc (100%) rename tests/sys/fs/{fuse => fusefs}/readdir.cc (100%) rename tests/sys/fs/{fuse => fusefs}/readlink.cc (100%) rename tests/sys/fs/{fuse => fusefs}/release.cc (100%) rename tests/sys/fs/{fuse => fusefs}/releasedir.cc (100%) rename tests/sys/fs/{fuse => fusefs}/rename.cc (100%) rename tests/sys/fs/{fuse => fusefs}/rmdir.cc (100%) rename tests/sys/fs/{fuse => fusefs}/setattr.cc (100%) rename tests/sys/fs/{fuse => fusefs}/statfs.cc (100%) rename tests/sys/fs/{fuse => fusefs}/symlink.cc (100%) rename tests/sys/fs/{fuse => fusefs}/unlink.cc (100%) rename tests/sys/fs/{fuse => fusefs}/utils.cc (98%) rename tests/sys/fs/{fuse => fusefs}/utils.hh (100%) rename tests/sys/fs/{fuse => fusefs}/write.cc (97%) rename tests/sys/fs/{fuse => fusefs}/xattr.cc (100%) diff --git a/etc/mtree/BSD.tests.dist b/etc/mtree/BSD.tests.dist index 01efbf1d2a7b..71aec31abadd 100644 --- a/etc/mtree/BSD.tests.dist +++ b/etc/mtree/BSD.tests.dist @@ -713,7 +713,7 @@ file .. fs - fuse + fusefs .. tmpfs .. diff --git a/tests/sys/fs/Makefile b/tests/sys/fs/Makefile index ec7ebdfc449b..5b5e78068125 100644 --- a/tests/sys/fs/Makefile +++ b/tests/sys/fs/Makefile @@ -7,7 +7,7 @@ TESTSDIR= ${TESTSBASE}/sys/fs TESTSRC= ${SRCTOP}/contrib/netbsd-tests/fs #TESTS_SUBDIRS+= nullfs # XXX: needs rump -TESTS_SUBDIRS+= fuse +TESTS_SUBDIRS+= fusefs TESTS_SUBDIRS+= tmpfs ${PACKAGE}FILES+= h_funcs.subr diff --git a/tests/sys/fs/fuse/Makefile b/tests/sys/fs/fusefs/Makefile similarity index 99% rename from tests/sys/fs/fuse/Makefile rename to tests/sys/fs/fusefs/Makefile index c98020dc196d..d70e7a1a95a6 100644 --- a/tests/sys/fs/fuse/Makefile +++ b/tests/sys/fs/fusefs/Makefile @@ -2,7 +2,7 @@ PACKAGE= tests -TESTSDIR= ${TESTSBASE}/sys/fs/fuse +TESTSDIR= ${TESTSBASE}/sys/fs/fusefs # We could simply link all of these files into a single executable. But since # Kyua treats googletest programs as plain tests, it's better to separate them diff --git a/tests/sys/fs/fuse/access.cc b/tests/sys/fs/fusefs/access.cc similarity index 100% rename from tests/sys/fs/fuse/access.cc rename to tests/sys/fs/fusefs/access.cc diff --git a/tests/sys/fs/fuse/create.cc b/tests/sys/fs/fusefs/create.cc similarity index 100% rename from tests/sys/fs/fuse/create.cc rename to tests/sys/fs/fusefs/create.cc diff --git a/tests/sys/fs/fuse/default_permissions.cc b/tests/sys/fs/fusefs/default_permissions.cc similarity index 100% rename from tests/sys/fs/fuse/default_permissions.cc rename to tests/sys/fs/fusefs/default_permissions.cc diff --git a/tests/sys/fs/fuse/destroy.cc b/tests/sys/fs/fusefs/destroy.cc similarity index 100% rename from tests/sys/fs/fuse/destroy.cc rename to tests/sys/fs/fusefs/destroy.cc diff --git a/tests/sys/fs/fuse/flush.cc b/tests/sys/fs/fusefs/flush.cc similarity index 100% rename from tests/sys/fs/fuse/flush.cc rename to tests/sys/fs/fusefs/flush.cc diff --git a/tests/sys/fs/fuse/fsync.cc b/tests/sys/fs/fusefs/fsync.cc similarity index 100% rename from tests/sys/fs/fuse/fsync.cc rename to tests/sys/fs/fusefs/fsync.cc diff --git a/tests/sys/fs/fuse/fsyncdir.cc b/tests/sys/fs/fusefs/fsyncdir.cc similarity index 100% rename from tests/sys/fs/fuse/fsyncdir.cc rename to tests/sys/fs/fusefs/fsyncdir.cc diff --git a/tests/sys/fs/fuse/getattr.cc b/tests/sys/fs/fusefs/getattr.cc similarity index 100% rename from tests/sys/fs/fuse/getattr.cc rename to tests/sys/fs/fusefs/getattr.cc diff --git a/tests/sys/fs/fuse/interrupt.cc b/tests/sys/fs/fusefs/interrupt.cc similarity index 100% rename from tests/sys/fs/fuse/interrupt.cc rename to tests/sys/fs/fusefs/interrupt.cc diff --git a/tests/sys/fs/fuse/link.cc b/tests/sys/fs/fusefs/link.cc similarity index 100% rename from tests/sys/fs/fuse/link.cc rename to tests/sys/fs/fusefs/link.cc diff --git a/tests/sys/fs/fuse/locks.cc b/tests/sys/fs/fusefs/locks.cc similarity index 100% rename from tests/sys/fs/fuse/locks.cc rename to tests/sys/fs/fusefs/locks.cc diff --git a/tests/sys/fs/fuse/lookup.cc b/tests/sys/fs/fusefs/lookup.cc similarity index 100% rename from tests/sys/fs/fuse/lookup.cc rename to tests/sys/fs/fusefs/lookup.cc diff --git a/tests/sys/fs/fuse/mkdir.cc b/tests/sys/fs/fusefs/mkdir.cc similarity index 100% rename from tests/sys/fs/fuse/mkdir.cc rename to tests/sys/fs/fusefs/mkdir.cc diff --git a/tests/sys/fs/fuse/mknod.cc b/tests/sys/fs/fusefs/mknod.cc similarity index 100% rename from tests/sys/fs/fuse/mknod.cc rename to tests/sys/fs/fusefs/mknod.cc diff --git a/tests/sys/fs/fuse/mockfs.cc b/tests/sys/fs/fusefs/mockfs.cc similarity index 100% rename from tests/sys/fs/fuse/mockfs.cc rename to tests/sys/fs/fusefs/mockfs.cc diff --git a/tests/sys/fs/fuse/mockfs.hh b/tests/sys/fs/fusefs/mockfs.hh similarity index 98% rename from tests/sys/fs/fuse/mockfs.hh rename to tests/sys/fs/fusefs/mockfs.hh index 5061fd51524e..d9eb5888ae33 100644 --- a/tests/sys/fs/fuse/mockfs.hh +++ b/tests/sys/fs/fusefs/mockfs.hh @@ -171,7 +171,7 @@ ProcessMockerT ReturnImmediate( * "Mounts" a filesystem to a temporary directory and services requests * according to the programmed expectations. * - * Operates directly on the fuse(4) kernel API, not the libfuse(3) user api. + * Operates directly on the fusefs(4) kernel API, not the libfuse(3) user api. */ class MockFS { /* diff --git a/tests/sys/fs/fuse/open.cc b/tests/sys/fs/fusefs/open.cc similarity index 100% rename from tests/sys/fs/fuse/open.cc rename to tests/sys/fs/fusefs/open.cc diff --git a/tests/sys/fs/fuse/opendir.cc b/tests/sys/fs/fusefs/opendir.cc similarity index 100% rename from tests/sys/fs/fuse/opendir.cc rename to tests/sys/fs/fusefs/opendir.cc diff --git a/tests/sys/fs/fuse/read.cc b/tests/sys/fs/fusefs/read.cc similarity index 100% rename from tests/sys/fs/fuse/read.cc rename to tests/sys/fs/fusefs/read.cc diff --git a/tests/sys/fs/fuse/readdir.cc b/tests/sys/fs/fusefs/readdir.cc similarity index 100% rename from tests/sys/fs/fuse/readdir.cc rename to tests/sys/fs/fusefs/readdir.cc diff --git a/tests/sys/fs/fuse/readlink.cc b/tests/sys/fs/fusefs/readlink.cc similarity index 100% rename from tests/sys/fs/fuse/readlink.cc rename to tests/sys/fs/fusefs/readlink.cc diff --git a/tests/sys/fs/fuse/release.cc b/tests/sys/fs/fusefs/release.cc similarity index 100% rename from tests/sys/fs/fuse/release.cc rename to tests/sys/fs/fusefs/release.cc diff --git a/tests/sys/fs/fuse/releasedir.cc b/tests/sys/fs/fusefs/releasedir.cc similarity index 100% rename from tests/sys/fs/fuse/releasedir.cc rename to tests/sys/fs/fusefs/releasedir.cc diff --git a/tests/sys/fs/fuse/rename.cc b/tests/sys/fs/fusefs/rename.cc similarity index 100% rename from tests/sys/fs/fuse/rename.cc rename to tests/sys/fs/fusefs/rename.cc diff --git a/tests/sys/fs/fuse/rmdir.cc b/tests/sys/fs/fusefs/rmdir.cc similarity index 100% rename from tests/sys/fs/fuse/rmdir.cc rename to tests/sys/fs/fusefs/rmdir.cc diff --git a/tests/sys/fs/fuse/setattr.cc b/tests/sys/fs/fusefs/setattr.cc similarity index 100% rename from tests/sys/fs/fuse/setattr.cc rename to tests/sys/fs/fusefs/setattr.cc diff --git a/tests/sys/fs/fuse/statfs.cc b/tests/sys/fs/fusefs/statfs.cc similarity index 100% rename from tests/sys/fs/fuse/statfs.cc rename to tests/sys/fs/fusefs/statfs.cc diff --git a/tests/sys/fs/fuse/symlink.cc b/tests/sys/fs/fusefs/symlink.cc similarity index 100% rename from tests/sys/fs/fuse/symlink.cc rename to tests/sys/fs/fusefs/symlink.cc diff --git a/tests/sys/fs/fuse/unlink.cc b/tests/sys/fs/fusefs/unlink.cc similarity index 100% rename from tests/sys/fs/fuse/unlink.cc rename to tests/sys/fs/fusefs/unlink.cc diff --git a/tests/sys/fs/fuse/utils.cc b/tests/sys/fs/fusefs/utils.cc similarity index 98% rename from tests/sys/fs/fuse/utils.cc rename to tests/sys/fs/fusefs/utils.cc index b6776eafbf87..45756243d899 100644 --- a/tests/sys/fs/fuse/utils.cc +++ b/tests/sys/fs/fusefs/utils.cc @@ -40,10 +40,10 @@ using namespace testing; -/* Check that fuse(4) is accessible and the current user can mount(2) */ +/* Check that fusefs(4) is accessible and the current user can mount(2) */ void check_environment() { - const char *mod_name = "fuse"; + const char *mod_name = "fusefs"; const char *devnode = "/dev/fuse"; const char *usermount_node = "vfs.usermount"; int usermount_val = 0; diff --git a/tests/sys/fs/fuse/utils.hh b/tests/sys/fs/fusefs/utils.hh similarity index 100% rename from tests/sys/fs/fuse/utils.hh rename to tests/sys/fs/fusefs/utils.hh diff --git a/tests/sys/fs/fuse/write.cc b/tests/sys/fs/fusefs/write.cc similarity index 97% rename from tests/sys/fs/fuse/write.cc rename to tests/sys/fs/fusefs/write.cc index de20b03b8f6f..4a670b74f9c7 100644 --- a/tests/sys/fs/fuse/write.cc +++ b/tests/sys/fs/fusefs/write.cc @@ -66,7 +66,7 @@ void expect_release(uint64_t ino, ProcessMockerT r) } void require_sync_resize_0() { - const char *sync_resize_node = "vfs.fuse.sync_resize"; + const char *sync_resize_node = "vfs.fusefs.sync_resize"; int val = 0; size_t size = sizeof(val); @@ -74,7 +74,7 @@ void require_sync_resize_0() { << strerror(errno); if (val != 0) GTEST_SKIP() << - "vfs.fuse.sync_resize must be set to 0 for this test." + "vfs.fusefs.sync_resize must be set to 0 for this test." " That sysctl will probably be removed soon."; } @@ -100,7 +100,7 @@ virtual void SetUp() { class WriteThrough: public Write { virtual void SetUp() { - const char *cache_mode_node = "vfs.fuse.data_cache_mode"; + const char *cache_mode_node = "vfs.fusefs.data_cache_mode"; int val = 0; size_t size = sizeof(val); @@ -111,7 +111,7 @@ virtual void SetUp() { ASSERT_EQ(0, sysctlbyname(cache_mode_node, &val, &size, NULL, 0)) << strerror(errno); if (val != 1) - GTEST_SKIP() << "vfs.fuse.data_cache_mode must be set to 1 " + GTEST_SKIP() << "vfs.fusefs.data_cache_mode must be set to 1 " "(writethrough) for this test"; } @@ -121,7 +121,7 @@ virtual void SetUp() { class WriteBack: public Write { virtual void SetUp() { - const char *node = "vfs.fuse.data_cache_mode"; + const char *node = "vfs.fusefs.data_cache_mode"; int val = 0; size_t size = sizeof(val); @@ -132,7 +132,7 @@ virtual void SetUp() { ASSERT_EQ(0, sysctlbyname(node, &val, &size, NULL, 0)) << strerror(errno); if (val != 2) - GTEST_SKIP() << "vfs.fuse.data_cache_mode must be set to 2 " + GTEST_SKIP() << "vfs.fusefs.data_cache_mode must be set to 2 " "(writeback) for this test"; } @@ -343,7 +343,7 @@ TEST_F(Write, DISABLED_direct_io_short_write_iov) * write, then it must set the FUSE_WRITE_CACHE bit */ /* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236378 */ -// TODO: check vfs.fuse.mmap_enable +// TODO: check vfs.fusefs.mmap_enable TEST_F(Write, DISABLED_mmap) { const char FULLPATH[] = "mountpoint/some_file.txt"; diff --git a/tests/sys/fs/fuse/xattr.cc b/tests/sys/fs/fusefs/xattr.cc similarity index 100% rename from tests/sys/fs/fuse/xattr.cc rename to tests/sys/fs/fusefs/xattr.cc From 91ff3a0d3db23a3637f917bec6f7f943dd0116df Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Thu, 21 Mar 2019 19:56:33 +0000 Subject: [PATCH 04/11] fusefs: add a test case for the allow_other mount option Also, fix one of the default_permissions test cases. I forgot the expectation for FUSE_ACCESS, because that doesn't work right now. Sponsored by: The FreeBSD Foundation --- tests/sys/fs/fusefs/Makefile | 6 + tests/sys/fs/fusefs/access.cc | 12 -- tests/sys/fs/fusefs/allow_other.cc | 191 +++++++++++++++++++++ tests/sys/fs/fusefs/default_permissions.cc | 7 +- tests/sys/fs/fusefs/mockfs.cc | 22 ++- tests/sys/fs/fusefs/mockfs.hh | 8 +- tests/sys/fs/fusefs/utils.cc | 18 +- tests/sys/fs/fusefs/utils.hh | 8 + 8 files changed, 245 insertions(+), 27 deletions(-) create mode 100644 tests/sys/fs/fusefs/allow_other.cc diff --git a/tests/sys/fs/fusefs/Makefile b/tests/sys/fs/fusefs/Makefile index d70e7a1a95a6..cf258cd1b1f7 100644 --- a/tests/sys/fs/fusefs/Makefile +++ b/tests/sys/fs/fusefs/Makefile @@ -8,6 +8,7 @@ TESTSDIR= ${TESTSBASE}/sys/fs/fusefs # Kyua treats googletest programs as plain tests, it's better to separate them # out, so we get more granular reporting. GTESTS+= access +GTESTS+= allow_other GTESTS+= create GTESTS+= default_permissions GTESTS+= destroy @@ -42,6 +43,11 @@ SRCS.access+= getmntopts.c SRCS.access+= mockfs.cc SRCS.access+= utils.cc +SRCS.allow_other+= allow_other.cc +SRCS.allow_other+= getmntopts.c +SRCS.allow_other+= mockfs.cc +SRCS.allow_other+= utils.cc + SRCS.create+= create.cc SRCS.create+= getmntopts.c SRCS.create+= mockfs.cc diff --git a/tests/sys/fs/fusefs/access.cc b/tests/sys/fs/fusefs/access.cc index 094809be6462..016d9bcd8d8d 100644 --- a/tests/sys/fs/fusefs/access.cc +++ b/tests/sys/fs/fusefs/access.cc @@ -40,18 +40,6 @@ using namespace testing; class Access: public FuseTest { public: -void expect_access(uint64_t ino, mode_t access_mode, int error) -{ - EXPECT_CALL(*m_mock, process( - ResultOf([=](auto in) { - return (in->header.opcode == FUSE_ACCESS && - in->header.nodeid == ino && - in->body.access.mask == access_mode); - }, Eq(true)), - _) - ).WillOnce(Invoke(ReturnErrno(error))); -} - void expect_lookup(const char *relpath, uint64_t ino) { FuseTest::expect_lookup(relpath, ino, S_IFREG | 0644, 0, 1); diff --git a/tests/sys/fs/fusefs/allow_other.cc b/tests/sys/fs/fusefs/allow_other.cc new file mode 100644 index 000000000000..c39cc2e4cb8d --- /dev/null +++ b/tests/sys/fs/fusefs/allow_other.cc @@ -0,0 +1,191 @@ +/*- + * SPDX-License-Identifier: BSD-2-Clause-FreeBSD + * + * Copyright (c) 2019 The FreeBSD Foundation + * + * This software was developed by BFF Storage Systems, LLC under sponsorship + * from the FreeBSD Foundation. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF + * SUCH DAMAGE. + */ + +/* + * Tests for the "allow_other" mount option. They must be in their own + * file so they can be run as root + */ + +extern "C" { +#include +#include +#include +#include +#include +} + +#include "mockfs.hh" +#include "utils.hh" + +using namespace testing; + +void sighandler(int __unused sig) {} + +static void +get_unprivileged_uid(int *uid) +{ + struct passwd *pw; + + /* + * First try "tests", Kyua's default unprivileged user. XXX after + * GoogleTest gains a proper Kyua wrapper, get this with the Kyua API + */ + pw = getpwnam("tests"); + if (pw == NULL) { + /* Fall back to "nobody" */ + pw = getpwnam("nobody"); + } + if (pw == NULL) + GTEST_SKIP() << "Test requires an unprivileged user"; + *uid = pw->pw_uid; +} + +class NoAllowOther: public FuseTest { + +public: +/* Unprivileged user id */ +int m_uid; + +virtual void SetUp() { + if (geteuid() != 0) { + GTEST_SKIP() << "This test must be run as root"; + } + get_unprivileged_uid(&m_uid); + if (IsSkipped()) + return; + + FuseTest::SetUp(); +} +}; + +class AllowOther: public NoAllowOther { + +public: +virtual void SetUp() { + m_allow_other = true; + NoAllowOther::SetUp(); +} +}; + +TEST_F(AllowOther, allowed) +{ + const char FULLPATH[] = "mountpoint/some_file.txt"; + const char RELPATH[] = "some_file.txt"; + uint64_t ino = 42; + int fd; + pid_t child; + + signal(SIGUSR2, sighandler); + + if ((child = fork()) == 0) { + /* In child */ + pause(); + + /* Drop privileges before accessing */ + if (0 != setreuid(-1, m_uid)) { + perror("setreuid"); + _exit(1); + } + fd = open(FULLPATH, O_RDONLY); + if (fd < 0) { + perror("open"); + _exit(1); + } + _exit(0); + + /* Deliberately leak fd */ + } else if (child > 0) { + /* + * In parent. Cleanup must happen here, because it's still + * privileged. + */ + expect_lookup(RELPATH, ino, S_IFREG | 0644, 0, 1); + expect_open(ino, 0, 1); + expect_release(ino, 1, 0, 0); + /* Until the attr cache is working, we may send an additional + * GETATTR */ + expect_getattr(ino, 0); + m_mock->m_child_pid = child; + /* Signal the child process to go */ + kill(child, SIGUSR2); + int child_status; + + wait(&child_status); + ASSERT_EQ(0, WEXITSTATUS(child_status)); + } else { + FAIL() << strerror(errno); + } +} + +TEST_F(NoAllowOther, disallowed) +{ + const char FULLPATH[] = "mountpoint/some_file.txt"; + int fd; + pid_t child; + + signal(SIGUSR2, sighandler); + + if ((child = fork()) == 0) { + /* In child */ + pause(); + + /* Drop privileges before accessing */ + if (0 != setreuid(-1, m_uid)) { + perror("setreuid"); + _exit(1); + } + fd = open(FULLPATH, O_RDONLY); + if (fd >= 0) { + fprintf(stderr, "open should've failed\n"); + _exit(1); + } else if (errno != EPERM) { + fprintf(stderr, + "Unexpected error: %s\n", strerror(errno)); + _exit(1); + } + _exit(0); + + /* Deliberately leak fd */ + } else if (child > 0) { + /* + * In parent. Cleanup must happen here, because it's still + * privileged. + */ + m_mock->m_child_pid = child; + /* Signal the child process to go */ + kill(child, SIGUSR2); + int child_status; + + wait(&child_status); + ASSERT_EQ(0, WEXITSTATUS(child_status)); + } else { + FAIL() << strerror(errno); + } +} diff --git a/tests/sys/fs/fusefs/default_permissions.cc b/tests/sys/fs/fusefs/default_permissions.cc index 9790f75601b0..c5b69632da02 100644 --- a/tests/sys/fs/fusefs/default_permissions.cc +++ b/tests/sys/fs/fusefs/default_permissions.cc @@ -84,7 +84,8 @@ TEST_F(Access, DISABLED_eaccess) ASSERT_EQ(EACCES, errno); } -TEST_F(Access, ok) +/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236291 */ +TEST_F(Access, DISABLED_ok) { const char FULLPATH[] = "mountpoint/some_file.txt"; const char RELPATH[] = "some_file.txt"; @@ -92,10 +93,10 @@ TEST_F(Access, ok) mode_t access_mode = R_OK; expect_lookup(RELPATH, ino, S_IFREG | 0644); + expect_access(ino, access_mode, 0); /* * Once default_permissions is properly implemented, there might be - * another FUSE_GETATTR or something in here. But there should not be - * a FUSE_ACCESS + * another FUSE_GETATTR or something in here. */ ASSERT_EQ(0, access(FULLPATH, access_mode)) << strerror(errno); diff --git a/tests/sys/fs/fusefs/mockfs.cc b/tests/sys/fs/fusefs/mockfs.cc index 7812c6dd2250..d010f5a870b6 100644 --- a/tests/sys/fs/fusefs/mockfs.cc +++ b/tests/sys/fs/fusefs/mockfs.cc @@ -243,12 +243,13 @@ void debug_fuseop(const mockfs_buf_in *in) printf("\n"); } -MockFS::MockFS(int max_readahead, bool push_symlinks_in, - bool default_permissions, uint32_t flags) +MockFS::MockFS(int max_readahead, bool allow_other, bool default_permissions, + bool push_symlinks_in, uint32_t flags) { struct iovec *iov = NULL; int iovlen = 0; char fdstr[15]; + const bool trueval = true; m_daemon_id = NULL; m_maxreadahead = max_readahead; @@ -262,33 +263,36 @@ MockFS::MockFS(int max_readahead, bool push_symlinks_in, * googletest doesn't allow ASSERT_ in constructors, so we must throw * instead. */ - if (mkdir("mountpoint" , 0644) && errno != EEXIST) + if (mkdir("mountpoint" , 0755) && errno != EEXIST) throw(std::system_error(errno, std::system_category(), "Couldn't make mountpoint directory")); - m_fuse_fd = open("/dev/fuse", O_RDWR); + m_fuse_fd = open("/dev/fuse", O_CLOEXEC | O_RDWR); if (m_fuse_fd < 0) throw(std::system_error(errno, std::system_category(), "Couldn't open /dev/fuse")); sprintf(fdstr, "%d", m_fuse_fd); m_pid = getpid(); + m_child_pid = -1; build_iovec(&iov, &iovlen, "fstype", __DECONST(void *, "fusefs"), -1); build_iovec(&iov, &iovlen, "fspath", __DECONST(void *, "mountpoint"), -1); build_iovec(&iov, &iovlen, "from", __DECONST(void *, "/dev/fuse"), -1); build_iovec(&iov, &iovlen, "fd", fdstr, -1); - if (push_symlinks_in) { - const bool trueval = true; - build_iovec(&iov, &iovlen, "push_symlinks_in", + if (allow_other) { + build_iovec(&iov, &iovlen, "allow_other", __DECONST(void*, &trueval), sizeof(bool)); } if (default_permissions) { - const bool trueval = true; build_iovec(&iov, &iovlen, "default_permissions", __DECONST(void*, &trueval), sizeof(bool)); } + if (push_symlinks_in) { + build_iovec(&iov, &iovlen, "push_symlinks_in", + __DECONST(void*, &trueval), sizeof(bool)); + } if (nmount(iov, iovlen, 0)) throw(std::system_error(errno, std::system_category(), "Couldn't mount filesystem")); @@ -397,6 +401,8 @@ void MockFS::loop() { bool MockFS::pid_ok(pid_t pid) { if (pid == m_pid) { return (true); + } else if (pid == m_child_pid) { + return (true); } else { struct kinfo_proc *ki; bool ok = false; diff --git a/tests/sys/fs/fusefs/mockfs.hh b/tests/sys/fs/fusefs/mockfs.hh index d9eb5888ae33..ac4c9a030e3b 100644 --- a/tests/sys/fs/fusefs/mockfs.hh +++ b/tests/sys/fs/fusefs/mockfs.hh @@ -208,12 +208,16 @@ class MockFS { void read_request(mockfs_buf_in*); public: + /* pid of child process, for two-process test cases */ + pid_t m_child_pid; + /* Maximum size of a FUSE_WRITE write */ uint32_t m_max_write; /* Create a new mockfs and mount it to a tempdir */ - MockFS(int max_readahead, bool push_symlinks_in, - bool default_permissions, uint32_t flags); + MockFS(int max_readahead, bool allow_other, + bool default_permissions, bool push_symlinks_in, + uint32_t flags); virtual ~MockFS(); /* Kill the filesystem daemon without unmounting the filesystem */ diff --git a/tests/sys/fs/fusefs/utils.cc b/tests/sys/fs/fusefs/utils.cc index 45756243d899..3ca9bd7d6e04 100644 --- a/tests/sys/fs/fusefs/utils.cc +++ b/tests/sys/fs/fusefs/utils.cc @@ -91,13 +91,27 @@ void FuseTest::SetUp() { m_maxbcachebuf = val; try { - m_mock = new MockFS(m_maxreadahead, m_push_symlinks_in, - m_default_permissions, m_init_flags); + m_mock = new MockFS(m_maxreadahead, m_allow_other, + m_default_permissions, m_push_symlinks_in, + m_init_flags); } catch (std::system_error err) { FAIL() << err.what(); } } +void +FuseTest::expect_access(uint64_t ino, mode_t access_mode, int error) +{ + EXPECT_CALL(*m_mock, process( + ResultOf([=](auto in) { + return (in->header.opcode == FUSE_ACCESS && + in->header.nodeid == ino && + in->body.access.mask == access_mode); + }, Eq(true)), + _) + ).WillOnce(Invoke(ReturnErrno(error))); +} + void FuseTest::expect_getattr(uint64_t ino, uint64_t size) { /* Until the attr cache is working, we may send an additional GETATTR */ diff --git a/tests/sys/fs/fusefs/utils.hh b/tests/sys/fs/fusefs/utils.hh index 7791c4e0ef2c..452c063e1865 100644 --- a/tests/sys/fs/fusefs/utils.hh +++ b/tests/sys/fs/fusefs/utils.hh @@ -41,6 +41,7 @@ class FuseTest : public ::testing::Test { protected: uint32_t m_maxreadahead; uint32_t m_init_flags; + bool m_allow_other; bool m_default_permissions; bool m_push_symlinks_in; MockFS *m_mock = NULL; @@ -56,6 +57,7 @@ class FuseTest : public ::testing::Test { */ m_maxreadahead(UINT_MAX), m_init_flags(0), + m_allow_other(false), m_default_permissions(false), m_push_symlinks_in(false) {} @@ -67,6 +69,12 @@ class FuseTest : public ::testing::Test { delete m_mock; } + /* + * Create an expectation that FUSE_ACCESS will be called oncde 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_GETATTR will be called for the given * inode any number of times. It will respond with a few basic From 44dc9245e7fa98cab395019094dd18506f4cf1db Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Thu, 21 Mar 2019 21:41:07 +0000 Subject: [PATCH 05/11] fusefs: don't check for the fusefs module during the tests It's sufficient to check for /dev/fuse. And due to bug 236647, the module could be named either fuse or fusefs. PR: 236647 Sponsored by: The FreeBSD Foundation --- tests/sys/fs/fusefs/utils.cc | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tests/sys/fs/fusefs/utils.cc b/tests/sys/fs/fusefs/utils.cc index 3ca9bd7d6e04..f68d46ffbe51 100644 --- a/tests/sys/fs/fusefs/utils.cc +++ b/tests/sys/fs/fusefs/utils.cc @@ -43,15 +43,10 @@ using namespace testing; /* Check that fusefs(4) is accessible and the current user can mount(2) */ void check_environment() { - const char *mod_name = "fusefs"; const char *devnode = "/dev/fuse"; const char *usermount_node = "vfs.usermount"; int usermount_val = 0; size_t usermount_size = sizeof(usermount_val); - if (modfind(mod_name) == -1) { - GTEST_SKIP() << "Module " << mod_name << - " could not be resolved"; - } if (eaccess(devnode, R_OK | W_OK)) { if (errno == ENOENT) { GTEST_SKIP() << devnode << " does not exist"; From cc34f2f66a599fa3684e47a042809760c2e5ce03 Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Thu, 21 Mar 2019 21:53:55 +0000 Subject: [PATCH 06/11] fusefs: VOP_FSYNC should be synchronous returning asynchronously pretty much defeats the point of fsync PR: 236474 Sponsored by: The FreeBSD Foundation --- sys/fs/fuse/fuse_internal.c | 8 +++----- sys/fs/fuse/fuse_vnops.c | 4 ++-- tests/sys/fs/fusefs/fsync.cc | 32 ++------------------------------ 3 files changed, 7 insertions(+), 37 deletions(-) diff --git a/sys/fs/fuse/fuse_internal.c b/sys/fs/fuse/fuse_internal.c index 0c33c0cbe516..b5d54b8883dc 100644 --- a/sys/fs/fuse/fuse_internal.c +++ b/sys/fs/fuse/fuse_internal.c @@ -287,6 +287,7 @@ fuse_internal_fsync(struct vnode *vp, int op = FUSE_FSYNC; struct fuse_fsync_in *ffsi; struct fuse_dispatcher fdi; + int err; if (vnode_isdir(vp)) { op = FUSE_FSYNCDIR; @@ -298,13 +299,10 @@ fuse_internal_fsync(struct vnode *vp, ffsi->fsync_flags = 1; /* datasync */ - fuse_insert_callback(fdi.tick, fuse_internal_fsync_callback); - fuse_insert_message(fdi.tick); - + err = fdisp_wait_answ(&fdi); fdisp_destroy(&fdi); - return 0; - + return err; } /* readdir */ diff --git a/sys/fs/fuse/fuse_vnops.c b/sys/fs/fuse/fuse_vnops.c index 1b795af6e5d5..9f575eb8f0a2 100644 --- a/sys/fs/fuse/fuse_vnops.c +++ b/sys/fs/fuse/fuse_vnops.c @@ -454,12 +454,12 @@ fuse_vnop_fsync(struct vop_fsync_args *ap) for (type = 0; type < FUFH_MAXTYPE; type++) { fufh = &(fvdat->fufh[type]); if (FUFH_IS_VALID(fufh)) { - fuse_internal_fsync(vp, td, NULL, fufh); + err = fuse_internal_fsync(vp, td, NULL, fufh); } } out: - return 0; + return err; } /* diff --git a/tests/sys/fs/fusefs/fsync.cc b/tests/sys/fs/fusefs/fsync.cc index 9894a3d2888a..3198f064f491 100644 --- a/tests/sys/fs/fusefs/fsync.cc +++ b/tests/sys/fs/fusefs/fsync.cc @@ -77,7 +77,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 */ -/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236474 */ TEST_F(Fsync, DISABLED_aio_fsync) { const char FULLPATH[] = "mountpoint/some_file.txt"; @@ -149,8 +148,7 @@ TEST_F(Fsync, close) close(fd); } -/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236474 */ -TEST_F(Fsync, DISABLED_eio) +TEST_F(Fsync, eio) { const char FULLPATH[] = "mountpoint/some_file.txt"; const char RELPATH[] = "some_file.txt"; @@ -179,7 +177,6 @@ TEST_F(Fsync, DISABLED_eio) * subsequent calls to VOP_FSYNC will succeed automatically without being sent * to the filesystem daemon */ -/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236474 */ /* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236557 */ TEST_F(Fsync, DISABLED_enosys) { @@ -207,8 +204,7 @@ TEST_F(Fsync, DISABLED_enosys) } -/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236474 */ -TEST_F(Fsync, DISABLED_fdatasync) +TEST_F(Fsync, fdatasync) { const char FULLPATH[] = "mountpoint/some_file.txt"; const char RELPATH[] = "some_file.txt"; @@ -232,7 +228,6 @@ TEST_F(Fsync, DISABLED_fdatasync) } /* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236473 */ -/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236474 */ TEST_F(Fsync, DISABLED_fsync) { const char FULLPATH[] = "mountpoint/some_file.txt"; @@ -258,7 +253,6 @@ TEST_F(Fsync, DISABLED_fsync) /* Fsync should sync a file with dirty metadata but clean data */ /* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236473 */ -/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236474 */ TEST_F(Fsync, DISABLED_fsync_metadata_only) { const char FULLPATH[] = "mountpoint/some_file.txt"; @@ -289,25 +283,3 @@ TEST_F(Fsync, DISABLED_fsync_metadata_only) ASSERT_EQ(0, fsync(fd)) << strerror(errno); /* Deliberately leak fd. close(2) will be tested in release.cc */ } - -// fsync()ing a file that isn't dirty should be a no-op -TEST_F(Fsync, nop) -{ - const char FULLPATH[] = "mountpoint/some_file.txt"; - const char RELPATH[] = "some_file.txt"; - uint64_t ino = 42; - int fd; - - expect_lookup(RELPATH, ino); - expect_open(ino, 0, 1); - expect_getattr(ino, 0); - - fd = open(FULLPATH, O_WRONLY); - ASSERT_LE(0, fd) << strerror(errno); - - ASSERT_EQ(0, fsync(fd)) << strerror(errno); - - /* Deliberately leak fd. close(2) will be tested in release.cc */ -} - -// TODO: ENOSYS test From 90612f3c3834bafecfa6f8b6a8994d07cb6b31d3 Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Thu, 21 Mar 2019 22:17:10 +0000 Subject: [PATCH 07/11] fusefs: VOP_FSYNC should be synchronous -- sometimes I committed too hastily in r345390. There are cases, not directly reachable from userland, where VOP_FSYNC ought to be asynchronous. This commit fixes fusefs to handle VOP_FSYNC synchronously if and only if the VFS requests it. PR: 236474 X-MFC-With: 345390 Sponsored by: The FreeBSD Foundation --- sys/fs/fuse/fuse_internal.c | 12 +++++++++--- sys/fs/fuse/fuse_internal.h | 2 +- sys/fs/fuse/fuse_vnops.c | 3 ++- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/sys/fs/fuse/fuse_internal.c b/sys/fs/fuse/fuse_internal.c index b5d54b8883dc..8ebb1316cb9c 100644 --- a/sys/fs/fuse/fuse_internal.c +++ b/sys/fs/fuse/fuse_internal.c @@ -282,12 +282,13 @@ int fuse_internal_fsync(struct vnode *vp, struct thread *td, struct ucred *cred, - struct fuse_filehandle *fufh) + struct fuse_filehandle *fufh, + int waitfor) { int op = FUSE_FSYNC; struct fuse_fsync_in *ffsi; struct fuse_dispatcher fdi; - int err; + int err = 0; if (vnode_isdir(vp)) { op = FUSE_FSYNCDIR; @@ -299,7 +300,12 @@ fuse_internal_fsync(struct vnode *vp, ffsi->fsync_flags = 1; /* datasync */ - err = fdisp_wait_answ(&fdi); + 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); return err; diff --git a/sys/fs/fuse/fuse_internal.h b/sys/fs/fuse/fuse_internal.h index d809a5262587..b5a97dd3305c 100644 --- a/sys/fs/fuse/fuse_internal.h +++ b/sys/fs/fuse/fuse_internal.h @@ -199,7 +199,7 @@ 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); + struct ucred *cred, struct fuse_filehandle *fufh, int waitfor); int fuse_internal_fsync_callback(struct fuse_ticket *tick, struct uio *uio); /* readdir */ diff --git a/sys/fs/fuse/fuse_vnops.c b/sys/fs/fuse/fuse_vnops.c index 9f575eb8f0a2..969375e10434 100644 --- a/sys/fs/fuse/fuse_vnops.c +++ b/sys/fs/fuse/fuse_vnops.c @@ -435,6 +435,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); @@ -454,7 +455,7 @@ fuse_vnop_fsync(struct vop_fsync_args *ap) for (type = 0; type < FUFH_MAXTYPE; type++) { fufh = &(fvdat->fufh[type]); if (FUFH_IS_VALID(fufh)) { - err = fuse_internal_fsync(vp, td, NULL, fufh); + err = fuse_internal_fsync(vp, td, NULL, fufh, waitfor); } } From 915012e0d00f8413867796c40d54622894a09197 Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Thu, 21 Mar 2019 23:01:56 +0000 Subject: [PATCH 08/11] 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 --- sys/fs/fuse/fuse_internal.c | 47 ++++++++++++++++++----------- sys/fs/fuse/fuse_internal.h | 4 +-- sys/fs/fuse/fuse_vnops.c | 58 +++++++++++++++++------------------- tests/sys/fs/fusefs/fsync.cc | 37 +---------------------- 4 files changed, 61 insertions(+), 85 deletions(-) diff --git a/sys/fs/fuse/fuse_internal.c b/sys/fs/fuse/fuse_internal.c index 8ebb1316cb9c..a3631857882c 100644 --- a/sys/fs/fuse/fuse_internal.c +++ b/sys/fs/fuse/fuse_internal.c @@ -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; } diff --git a/sys/fs/fuse/fuse_internal.h b/sys/fs/fuse/fuse_internal.h index b5a97dd3305c..d5b06fd8475e 100644 --- a/sys/fs/fuse/fuse_internal.h +++ b/sys/fs/fuse/fuse_internal.h @@ -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 */ diff --git a/sys/fs/fuse/fuse_vnops.c b/sys/fs/fuse/fuse_vnops.c index 969375e10434..9d273399ac97 100644 --- a/sys/fs/fuse/fuse_vnops.c +++ b/sys/fs/fuse/fuse_vnops.c @@ -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); } /* diff --git a/tests/sys/fs/fusefs/fsync.cc b/tests/sys/fs/fusefs/fsync.cc index 3198f064f491..e0962425143e 100644 --- a/tests/sys/fs/fusefs/fsync.cc +++ b/tests/sys/fs/fusefs/fsync.cc @@ -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 */ -} From 6248288e97849eb184afde62db0ed9faaf5320cb Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Thu, 21 Mar 2019 23:31:10 +0000 Subject: [PATCH 09/11] fusefs: correctly handle cacheable negative LOOKUP responses The FUSE protocol allows for LOOKUP to return a cacheable negative response, which means that the file doesn't exist and the kernel can cache its nonexistence. As of this commit fusefs doesn't cache the nonexistence, but it does correctly handle such responses. Prior to this commit attempting to create a file, even with O_CREAT would fail with ENOENT if the daemon returned a cacheable negative response. PR: 236231 Sponsored by: The FreeBSD Foundation --- sys/fs/fuse/fuse_vnops.c | 1 + tests/sys/fs/fusefs/create.cc | 6 ++---- tests/sys/fs/fusefs/mkdir.cc | 6 ++---- tests/sys/fs/fusefs/rename.cc | 15 ++++----------- 4 files changed, 9 insertions(+), 19 deletions(-) diff --git a/sys/fs/fuse/fuse_vnops.c b/sys/fs/fuse/fuse_vnops.c index 9d273399ac97..60cbfe4c7f4e 100644 --- a/sys/fs/fuse/fuse_vnops.c +++ b/sys/fs/fuse/fuse_vnops.c @@ -747,6 +747,7 @@ fuse_vnop_lookup(struct vop_lookup_args *ap) * but it's also cacheable (which we keep * keep on doing not as of writing this) */ + fdi.answ_stat = ENOENT; lookup_err = ENOENT; } else if (nid == FUSE_ROOT_ID) { lookup_err = EINVAL; diff --git a/tests/sys/fs/fusefs/create.cc b/tests/sys/fs/fusefs/create.cc index 088569cb2e4e..02b287c1f9d9 100644 --- a/tests/sys/fs/fusefs/create.cc +++ b/tests/sys/fs/fusefs/create.cc @@ -185,8 +185,7 @@ TEST_F(Create, DISABLED_Enosys) /* * Creating a new file after FUSE_LOOKUP returned a negative cache entry */ -/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236231 */ -TEST_F(Create, DISABLED_entry_cache_negative) +TEST_F(Create, entry_cache_negative) { const char FULLPATH[] = "mountpoint/some_file.txt"; const char RELPATH[] = "some_file.txt"; @@ -240,8 +239,7 @@ TEST_F(Create, DISABLED_entry_cache_negative) /* * Creating a new file should purge any negative namecache entries */ -/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236231 */ -TEST_F(Create, DISABLED_entry_cache_negative_purge) +TEST_F(Create, entry_cache_negative_purge) { const char FULLPATH[] = "mountpoint/some_file.txt"; const char RELPATH[] = "some_file.txt"; diff --git a/tests/sys/fs/fusefs/mkdir.cc b/tests/sys/fs/fusefs/mkdir.cc index c6b0b0945a39..6b8a04386ae2 100644 --- a/tests/sys/fs/fusefs/mkdir.cc +++ b/tests/sys/fs/fusefs/mkdir.cc @@ -69,8 +69,7 @@ TEST_F(Mkdir, emlink) /* * Creating a new directory after FUSE_LOOKUP returned a negative cache entry */ -/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236231 */ -TEST_F(Mkdir, DISABLED_entry_cache_negative) +TEST_F(Mkdir, entry_cache_negative) { const char FULLPATH[] = "mountpoint/some_file.txt"; const char RELPATH[] = "some_file.txt"; @@ -109,8 +108,7 @@ TEST_F(Mkdir, DISABLED_entry_cache_negative) /* * Creating a new directory should purge any negative namecache entries */ -/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236231 */ -TEST_F(Mkdir, DISABLED_entry_cache_negative_purge) +TEST_F(Mkdir, entry_cache_negative_purge) { const char FULLPATH[] = "mountpoint/some_file.txt"; const char RELPATH[] = "some_file.txt"; diff --git a/tests/sys/fs/fusefs/rename.cc b/tests/sys/fs/fusefs/rename.cc index 81c039add5b4..52f3dc906c04 100644 --- a/tests/sys/fs/fusefs/rename.cc +++ b/tests/sys/fs/fusefs/rename.cc @@ -86,8 +86,7 @@ TEST_F(Rename, enoent) /* * Renaming a file after FUSE_LOOKUP returned a negative cache entry for dst */ -/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236231 */ -TEST_F(Rename, DISABLED_entry_cache_negative) +TEST_F(Rename, entry_cache_negative) { const char FULLDST[] = "mountpoint/dst"; const char RELDST[] = "dst"; @@ -126,8 +125,7 @@ TEST_F(Rename, DISABLED_entry_cache_negative) /* * Renaming a file should purge any negative namecache entries for the dst */ -/* https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=236231 */ -TEST_F(Rename, DISABLED_entry_cache_negative_purge) +TEST_F(Rename, entry_cache_negative_purge) { const char FULLDST[] = "mountpoint/dst"; const char RELDST[] = "dst"; @@ -136,12 +134,7 @@ TEST_F(Rename, DISABLED_entry_cache_negative_purge) // FUSE hardcodes the mountpoint to inode 1 uint64_t dst_dir_ino = 1; uint64_t ino = 42; - /* - * Set entry_valid = 0 because this test isn't concerned with whether - * or not we actually cache negative entries, only with whether we - * interpret negative cache responses correctly. - */ - struct timespec entry_valid = {.tv_sec = 0, .tv_nsec = 0}; + struct timespec entry_valid = {.tv_sec = TIME_T_MAX, .tv_nsec = 0}; expect_lookup(RELSRC, ino, S_IFREG | 0644, 0, 1); /* LOOKUP returns a negative cache entry for dst */ @@ -164,7 +157,7 @@ TEST_F(Rename, DISABLED_entry_cache_negative_purge) ASSERT_EQ(0, rename(FULLSRC, FULLDST)) << strerror(errno); /* Finally, a subsequent lookup should query the daemon */ - expect_lookup(RELSRC, ino, S_IFREG | 0644, 0, 1); + expect_lookup(RELDST, ino, S_IFREG | 0644, 0, 1); ASSERT_EQ(0, access(FULLDST, F_OK)) << strerror(errno); } From 8ba190efebbbe72ed980286ff0f50936a7cac7ea Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Fri, 22 Mar 2019 17:53:13 +0000 Subject: [PATCH 10/11] fusefs: fix a panic on mount Don't page fault if the file descriptor provided with "-o fd" is invalid. Sponsored by: The FreeBSD Foundation --- sys/fs/fuse/fuse_vfsops.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sys/fs/fuse/fuse_vfsops.c b/sys/fs/fuse/fuse_vfsops.c index 6fdc904beddb..c3870b44dd5b 100644 --- a/sys/fs/fuse/fuse_vfsops.c +++ b/sys/fs/fuse/fuse_vfsops.c @@ -225,7 +225,7 @@ fuse_vfsop_mount(struct mount *mp) size_t len; struct cdev *fdev; - struct fuse_data *data; + struct fuse_data *data = NULL; struct thread *td; struct file *fp, *fptmp; char *fspec, *subtype; @@ -361,7 +361,7 @@ fuse_vfsop_mount(struct mount *mp) out: if (err) { FUSE_LOCK(); - if (data->mp == mp) { + if (data && data->mp == mp) { /* * Destroy device only if we acquired reference to * it From f6a363a50d024e3805145de1e7c7b79f1886c5c8 Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Fri, 22 Mar 2019 18:42:34 +0000 Subject: [PATCH 11/11] Add man page for VOP_FDATASYNC(9) Reviewed by: kib MFC after: 2 weeks Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D19678 --- share/man/man9/Makefile | 1 + share/man/man9/VOP_FSYNC.9 | 27 +++++++++++++++++++++++---- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/share/man/man9/Makefile b/share/man/man9/Makefile index bd816bf519fa..6eb5ee9e7983 100644 --- a/share/man/man9/Makefile +++ b/share/man/man9/Makefile @@ -2233,6 +2233,7 @@ MLINKS+=VOP_ATTRIB.9 VOP_GETATTR.9 \ MLINKS+=VOP_CREATE.9 VOP_MKDIR.9 \ VOP_CREATE.9 VOP_MKNOD.9 \ VOP_CREATE.9 VOP_SYMLINK.9 +MLINKS+=VOP_FSYNC.9 VOP_FDATASYNC.9 MLINKS+=VOP_GETPAGES.9 VOP_PUTPAGES.9 MLINKS+=VOP_INACTIVE.9 VOP_RECLAIM.9 MLINKS+=VOP_LOCK.9 vn_lock.9 \ diff --git a/share/man/man9/VOP_FSYNC.9 b/share/man/man9/VOP_FSYNC.9 index 76e114ff6099..f59312a0d7cb 100644 --- a/share/man/man9/VOP_FSYNC.9 +++ b/share/man/man9/VOP_FSYNC.9 @@ -28,20 +28,27 @@ .\" .\" $FreeBSD$ .\" -.Dd July 24, 1996 +.Dd March 22, 2019 .Dt VOP_FSYNC 9 .Os .Sh NAME +.Nm VOP_FDATASYNC , .Nm VOP_FSYNC .Nd flush file system buffers for a file .Sh SYNOPSIS .In sys/param.h .In sys/vnode.h .Ft int +.Fn VOP_FDATASYNC "struct vnode *vp" "struct thread *td" +.Ft int .Fn VOP_FSYNC "struct vnode *vp" "int waitfor" "struct thread *td" .Sh DESCRIPTION -This call flushes any dirty file system buffers for the file. -It is used to implement the +.Fn VOP_FSYNC +ensures that a file can be recovered to its current state following a crash. +That typically requires flushing the file's dirty buffers, its inode, and +possibly other filesystem metadata to persistent media. +.Fn VOP_FSYNC +is used to implement the .Xr sync 2 and .Xr fsync 2 @@ -65,8 +72,20 @@ Push data not written by file system syncer. .It Fa td The calling thread. .El +.Pp +.Fn VOP_FDATASYNC +is similar, but it does not require that all of the file's metadata be flushed. +It only requires that the file's data be recoverable after a crash. +That implies that the data itself must be flushed to disk, as well as some +metadata such as the file's size but not necessarily its attributes. +.Fn VOP_FDATASYNC +should always wait for I/O to complete, as if called with +.Dv MNT_WAIT . +.Fn VOP_FDATASYNC +is used to implement +.Xr fdatasync 2 . .Sh LOCKS -The file should be locked on entry. +The vnode should be exclusively locked on entry, and stays locked on return. .Sh RETURN VALUES Zero is returned if the call is successful, otherwise an appropriate error code is returned.