From c4cc5b556ce79e55fda1dd1e4a46bc23448306fb Mon Sep 17 00:00:00 2001 From: Dan Goodliffe Date: Mon, 23 Sep 2019 19:33:42 +0100 Subject: Fuse separation First swing at separating the fuse library interface with the fuse operations --- Jamroot.jam | 2 + netfs/daemon/daemonDirectory.cpp | 1 - netfs/fuse/fuseApp.cpp | 65 ++++++----------------- netfs/fuse/fuseApp.h | 21 +++----- netfs/fuse/fuseAppBase.cpp | 47 +++++------------ netfs/fuse/fuseAppBase.h | 47 ++++++++--------- netfs/fuse/netfs.cpp | 45 +++++++++++----- netfs/unittests/Jamfile.jam | 13 ++++- netfs/unittests/mockFuse.cpp | 41 ++++----------- netfs/unittests/mockFuse.h | 8 +-- netfs/unittests/testFuse.cpp | 108 +++++++++++++++++++++++++++++++++++++++ 11 files changed, 225 insertions(+), 173 deletions(-) create mode 100644 netfs/unittests/testFuse.cpp diff --git a/Jamroot.jam b/Jamroot.jam index cd93e3c..7c6a0bd 100644 --- a/Jamroot.jam +++ b/Jamroot.jam @@ -25,9 +25,11 @@ project tidy:clang-* tidy:misc-* tidy:modernize-* + tidy:modernize-use-trailing-return-type tidy:hicpp-* tidy:hicpp-vararg tidy:hicpp-signed-bitwise + tidy:hicpp-no-array-decay tidy:performance-* ; diff --git a/netfs/daemon/daemonDirectory.cpp b/netfs/daemon/daemonDirectory.cpp index 5e5c8ad..22c0e4f 100644 --- a/netfs/daemon/daemonDirectory.cpp +++ b/netfs/daemon/daemonDirectory.cpp @@ -54,7 +54,6 @@ DirectoryServer::listdir(const Ice::Current&) // LCOV_EXCL_STOP } struct stat s {}; - // NOLINTNEXTLINE(hicpp-no-array-decay) if (::fstatat(fd, d->d_name, &s, AT_SYMLINK_NOFOLLOW) != 0) { // LCOV_EXCL_START throw NetFS::SystemError(errno); diff --git a/netfs/fuse/fuseApp.cpp b/netfs/fuse/fuseApp.cpp index ef1ca7f..aaec9bd 100644 --- a/netfs/fuse/fuseApp.cpp +++ b/netfs/fuse/fuseApp.cpp @@ -14,12 +14,22 @@ namespace AdHoc { template class CallCacheable; } -NetFS::FuseApp::FuseApp(Ice::StringSeq a) : - iceArgs(std::move(a)), +NetFS::FuseApp::FuseApp(Ice::StringSeq && a) : + ic(Ice::initialize(a)), sessionOpened(false), openHandleId(0), converter(userLookup, groupLookup) { + if (!a.empty()) { + const auto & arg = a.front(); + if (arg.find("://") != std::string::npos) { + fcr = configureFromUri(arg); + } + else if (auto colon = arg.find(':'); colon != std::string::npos) { + fcr = configureFromFile(arg.substr(0, colon), arg.substr(colon + 1)); + } + } + BOOST_ASSERT(fcr); } NetFS::FuseApp::~FuseApp() @@ -65,25 +75,12 @@ NetFS::FuseApp::~FuseApp() } } -NetFS::Client::ConfigurationPtr -NetFS::FuseApp::ReadConfiguration(const std::filesystem::path & path) const -{ - auto s = Slicer::FileDeserializerFactory::createNew(path.extension().string(), path); - return Slicer::DeserializeAnyWith(s); -} - -void * -NetFS::FuseApp::init(struct fuse_conn_info *) -{ - ic = Ice::initialize(iceArgs); - fcr = configurator(); - return nullptr; -} - NetFS::Client::ResourcePtr -NetFS::FuseApp::configureFromFile(const std::string & configPath, const std::string & resourceName) const +NetFS::FuseApp::configureFromFile(const std::filesystem::path & path, const std::string & resourceName) const { - return AdHoc::safeMapLookup(ReadConfiguration(configPath)->Resources, resourceName); + auto s = Slicer::FileDeserializerFactory::createNew(path.extension().string(), path); + auto c = Slicer::DeserializeAnyWith(s); + return AdHoc::safeMapLookup(c->Resources, resourceName); } AdHocFormatter(IceEndpointFmt, "%? -h %? -p %?"); @@ -101,36 +98,6 @@ NetFS::FuseApp::configureFromUri(const std::string & uriString) const return r; } -int -NetFS::FuseApp::opt_parse(void *, const char * arg, int, struct fuse_args *) -{ - if (strncmp(arg, "--Ice.", 6) == 0) { - iceArgs.push_back(arg); - return 0; - } - else if (strncmp(arg, "_netdev", 7) == 0) { - return 0; - } - else if (arg[0] == '-') { - return 1; - } - else if (!configurator) { - if (strstr(arg, "://")) { - configurator = std::bind(&NetFS::FuseApp::configureFromUri, this, arg); - } - else { - const char * colon = strchr(arg, ':'); - configurator = std::bind(&NetFS::FuseApp::configureFromFile, this, std::string(arg, colon), colon + 1); - } - return 0; - } - else if (mountPoint.empty()) { - mountPoint = arg; - return 1; - } - return 1; -} - void NetFS::FuseApp::connectSession() { diff --git a/netfs/fuse/fuseApp.h b/netfs/fuse/fuseApp.h index df342b6..5b5a8fa 100644 --- a/netfs/fuse/fuseApp.h +++ b/netfs/fuse/fuseApp.h @@ -16,7 +16,7 @@ #include namespace NetFS { - class DLL_PUBLIC FuseApp : public FuseAppBase { + class DLL_PUBLIC FuseApp : public FuseAppBaseT { private: class OpenDir { public: @@ -49,15 +49,12 @@ namespace NetFS { typedef std::map OpenFiles; public: - FuseApp(Ice::StringSeq); + FuseApp(Ice::StringSeq &&); ~FuseApp(); - private: - void * init (struct fuse_conn_info * info) override; - int opt_parse(void *, const char * arg, int key, struct fuse_args *) override; - + protected: void connectSession(); - void connectToService(); + virtual void connectToService(); void connectToVolume(); void connectHandles(); void verifyConnection(); @@ -100,13 +97,10 @@ namespace NetFS { virtual struct fuse_context * fuse_get_context() = 0; protected: - typedef std::function Configurator; - Configurator configurator; - virtual NetFS::Client::ConfigurationPtr ReadConfiguration(const std::filesystem::path &) const; - virtual NetFS::Client::ResourcePtr configureFromFile(const std::string &, const std::string &) const; - virtual NetFS::Client::ResourcePtr configureFromUri(const std::string &) const; + NetFS::Client::ResourcePtr configureFromFile(const std::filesystem::path &, const std::string &) const; + NetFS::Client::ResourcePtr configureFromUri(const std::string &) const; - private: + protected: template void setProxy(uint64_t & fh, const Params & ...); template @@ -121,7 +115,6 @@ namespace NetFS { ReqEnv reqEnv(); - Ice::StringSeq iceArgs; Ice::CommunicatorPtr ic; Client::ResourcePtr fcr; mutable std::shared_mutex _proxymaplock; diff --git a/netfs/fuse/fuseAppBase.cpp b/netfs/fuse/fuseAppBase.cpp index 01ec8e1..0b6a90e 100644 --- a/netfs/fuse/fuseAppBase.cpp +++ b/netfs/fuse/fuseAppBase.cpp @@ -5,22 +5,28 @@ #include #include #include +#include FuseAppBase * FuseAppBase::fuseApp; +FuseAppBase::FuseAppBase() +{ + BOOST_ASSERT(!fuseApp); + BOOST_ASSERT(this); + fuseApp = this; +} + +FuseAppBase::~FuseAppBase() +{ + BOOST_ASSERT(fuseApp); + fuseApp = nullptr; +} + // LCOV_EXCL_START // These are all excluded from coverage because it is impossible // to call them in a realistic manner. They exist only as the default // implementation of the function which is never passed to libfuse // unless it is overridden. -void * FuseAppBase::init(fuse_conn_info*) -{ - return nullptr; -} -int FuseAppBase::opt_parse(void*, const char *, int, fuse_args*) -{ - return 1; -} int FuseAppBase::access(const char *, int) { return -ENOSYS; @@ -190,11 +196,8 @@ void FuseAppBase::log(int level, const char * message) const noexcept void FuseAppBase::logf(int level, const char * fmt, ...) const noexcept { va_list args; - // NOLINTNEXTLINE(hicpp-no-array-decay) va_start(args, fmt); - // NOLINTNEXTLINE(hicpp-no-array-decay) vlogf(level, fmt, args); - // NOLINTNEXTLINE(hicpp-no-array-decay) va_end(args); } @@ -208,25 +211,3 @@ void FuseAppBase::beforeOperation() { } -void * FuseAppBase::fuseInit (struct fuse_conn_info *conn) -{ - return fuseApp->init(conn); -} -void FuseAppBase::fuseDestroy(void *) -{ - delete fuseApp; -} - -struct fuse_args -FuseAppBase::runint(int argc, char ** argv) -{ - std::array fuse_opts {}; - fuseApp = this; - struct fuse_args args = FUSE_ARGS_INIT(argc, argv); - if (fuse_opt_parse(&args, fuseApp, fuse_opts.data(), - &internalHelper) == -1) { - exit(1); - } - return args; -} - diff --git a/netfs/fuse/fuseAppBase.h b/netfs/fuse/fuseAppBase.h index 2775c00..97810d2 100644 --- a/netfs/fuse/fuseAppBase.h +++ b/netfs/fuse/fuseAppBase.h @@ -15,9 +15,9 @@ class DLL_PUBLIC FuseAppBase { public: - virtual ~FuseAppBase() = default; - virtual void * init (struct fuse_conn_info * info); - virtual int opt_parse(void *, const char * arg, int key, struct fuse_args *); + FuseAppBase(); + virtual ~FuseAppBase(); + virtual int access(const char *, int); virtual int chmod(const char *, mode_t); virtual int chown(const char *, uid_t, gid_t); @@ -64,18 +64,19 @@ class DLL_PUBLIC FuseAppBase { void logf(int level, const char * fmt, ...) const throw() __attribute__ ((__format__ (__printf__, 3, 4))); virtual void vlogf(int level, const char * fmt, va_list) const throw() __attribute__ ((__format__ (__printf__, 3, 0))) = 0; - virtual int fuse_opt_parse(struct fuse_args *args, void *data, const struct fuse_opt opts[], fuse_opt_proc_t proc) = 0; - virtual int main(int, char **, const struct fuse_operations *) = 0; + mutable std::shared_mutex _lock; + protected: + static FuseAppBase * fuseApp; +}; +template +class FuseAppBaseT : public FuseAppBase { + public: #define GetHelper(func) getHelper(&FuseAppBase::func) - template - static int run(int argc, char ** argv, FuseApp * fa) - { - auto args = fa->runint(argc, argv); - struct fuse_operations operations = { + FuseAppBaseT() : operations({ GetHelper(getattr), GetHelper(readlink), - NULL, // getdir deprecated + nullptr, // getdir deprecated GetHelper(mknod), GetHelper(mkdir), GetHelper(unlink), @@ -86,7 +87,7 @@ class DLL_PUBLIC FuseAppBase { GetHelper(chmod), GetHelper(chown), GetHelper(truncate), - NULL, // utime deprecated + nullptr, // utime deprecated GetHelper(open), GetHelper(read), GetHelper(write), @@ -102,8 +103,8 @@ class DLL_PUBLIC FuseAppBase { GetHelper(readdir), GetHelper(releasedir), GetHelper(fsyncdir), - fuseInit, - fuseDestroy, + nullptr, //fuseInit + nullptr, //fuseDestroy GetHelper(access), GetHelper(create), GetHelper(ftruncate), @@ -129,15 +130,14 @@ class DLL_PUBLIC FuseAppBase { #endif #endif #endif - }; - return fa->main(args.argc, args.argv, &operations); + }) + { } - struct fuse_args runint(int, char **); +#undef GetHelper - private: - static void * fuseInit(struct fuse_conn_info *conn); - static void fuseDestroy(void *); + const struct fuse_operations operations; + private: template static int(*getHelper(int(FuseAppBase::*)(Args...)))(Args...) { @@ -169,16 +169,13 @@ class DLL_PUBLIC FuseAppBase { } return nullptr; } + + protected: template static int internalHelper(Args ... a) { return (fuseApp->*f)(a...); } - - static FuseAppBase * fuseApp; - - protected: - mutable std::shared_mutex _lock; }; #endif diff --git a/netfs/fuse/netfs.cpp b/netfs/fuse/netfs.cpp index c21711c..14c3c93 100644 --- a/netfs/fuse/netfs.cpp +++ b/netfs/fuse/netfs.cpp @@ -1,10 +1,37 @@ #include "fuseApp.h" #include -class FuseImpl : public NetFS::FuseApp { +class FuseImpl : public fuse_args, public NetFS::FuseApp { public: - explicit FuseImpl(const Ice::StringSeq & a) : - NetFS::FuseApp(a) + static int opt_parse(void * data, const char * arg, int, struct fuse_args *) + { + auto iceArgs = static_cast(data); + if (strncmp(arg, "--Ice.", 6) == 0) { + iceArgs->push_back(arg); + return 0; + } + else if (strncmp(arg, "_netdev", 7) == 0) { + return 0; + } + else if (arg[0] == '-') { + return 1; + } + else if (iceArgs->empty() || strncmp(iceArgs->front().c_str(), "--Ice.", 6) == 0) { + iceArgs->insert(iceArgs->begin(), arg); + return 0; + } + return 1; + } + + FuseImpl(int c, char ** v) : + fuse_args(FUSE_ARGS_INIT(c, v)), + NetFS::FuseApp([this](){ + Ice::StringSeq iceArgs; + if (fuse_opt_parse(this, &iceArgs, nullptr, opt_parse) == -1) { + exit(-1); + } + return iceArgs; + }()) { openlog("netfs", LOG_CONS | LOG_PID | LOG_NDELAY, LOG_USER); } @@ -24,15 +51,9 @@ class FuseImpl : public NetFS::FuseApp { return ::fuse_get_context(); } - // NOLINTNEXTLINE(modernize-avoid-c-arrays, hicpp-avoid-c-arrays) - int fuse_opt_parse(struct fuse_args * args, void * data, const struct fuse_opt opts[], fuse_opt_proc_t proc) override - { - return ::fuse_opt_parse(args, data, opts, proc); - } - - int main(int argc, char ** argv, const struct fuse_operations * ops) override + int run() { - return ::fuse_main(argc, argv, ops, this); + return ::fuse_main(argc, argv, &operations, this); } void vlogf(int priority, const char * fmt, va_list args) const noexcept override @@ -44,6 +65,6 @@ class FuseImpl : public NetFS::FuseApp { int main(int argc, char* argv[]) { - return FuseAppBase::run(argc, argv, new FuseImpl(Ice::argsToStringSeq(argc, argv))); + return FuseImpl(argc, argv).run(); } diff --git a/netfs/unittests/Jamfile.jam b/netfs/unittests/Jamfile.jam index 2d3fac3..2fbaf4a 100644 --- a/netfs/unittests/Jamfile.jam +++ b/netfs/unittests/Jamfile.jam @@ -50,7 +50,7 @@ run testCore.cpp run testGlacier.cpp : -- : defaultDaemon.xml - defaultFuse.xml + defaultFuse.xml : BOOST_TEST_DYN_LINK boost_utf @@ -60,10 +60,19 @@ run testGlacier.cpp run testEdgeCases.cpp : -- : defaultDaemon.xml - defaultFuse.xml + defaultFuse.xml : BOOST_TEST_DYN_LINK boost_utf testMocks : testEdgeCases ; +run testFuse.cpp + : : + defaultDaemon.xml + : + BOOST_TEST_DYN_LINK + boost_utf + testMocks + ; + diff --git a/netfs/unittests/mockFuse.cpp b/netfs/unittests/mockFuse.cpp index da6b07f..fd93388 100644 --- a/netfs/unittests/mockFuse.cpp +++ b/netfs/unittests/mockFuse.cpp @@ -3,7 +3,6 @@ FuseMock::FuseMock(std::string ep, Ice::StringSeq a) : NetFS::FuseApp(std::move(a)), - ops({}), testEndpoint(std::move(ep)), context({}) { @@ -19,34 +18,21 @@ FuseMock::fuse_get_context() return &context; } -int -// NOLINTNEXTLINE(modernize-avoid-c-arrays, hicpp-avoid-c-arrays) -FuseMock::fuse_opt_parse(struct fuse_args * args, void * data, const struct fuse_opt [], fuse_opt_proc_t proc) +void +FuseMock::connectToService() { - for (int n = 0; n < args->argc; n += 1) { - proc(data, args->argv[n], n, args); + if (testEndpoint.empty()) { + return FuseApp::connectToService(); } - return 0; -} -int -FuseMock::main(int, char **, const struct fuse_operations * o) -{ - o->init(nullptr); - ops = *o; - return 0; -} - -NetFS::Client::ConfigurationPtr -FuseMock::ReadConfiguration(const std::filesystem::path & path) const -{ - auto c = FuseApp::ReadConfiguration(path); - for(const auto & r : c->Resources) { - for(auto & e : r.second->Endpoints) { - e = testEndpoint; + if (!service) { + Lock(_lock); + std::string addr = fcr->ServiceIdentity + ":" + testEndpoint; + service = Ice::checkedCast(ic->stringToProxy(addr)); + if (!service) { + throw std::runtime_error("Invalid service proxy: " + testEndpoint); } } - return c; } void @@ -57,12 +43,7 @@ FuseMock::vlogf(int, const char * fmt, va_list args) const noexcept FuseMockHost::FuseMockHost(std::string ep, const Ice::StringSeq & a) : app(std::make_unique(std::move(ep), a)), - fuse(&app->ops) + fuse(&app->operations) { - std::vector argv; - for (auto & arg : a) { - argv.push_back(const_cast(arg.c_str())); - } - FuseAppBase::run(a.size(), &argv.front(), app.get()); } diff --git a/netfs/unittests/mockFuse.h b/netfs/unittests/mockFuse.h index 7e4f3ad..4461da6 100644 --- a/netfs/unittests/mockFuse.h +++ b/netfs/unittests/mockFuse.h @@ -9,15 +9,9 @@ class DLL_PUBLIC FuseMock : public NetFS::FuseApp { FuseMock(std::string, Ice::StringSeq); struct fuse_context * fuse_get_context() override; - int fuse_opt_parse(struct fuse_args * args, void * data, const struct fuse_opt [], fuse_opt_proc_t proc) override; - int main(int, char **, const struct fuse_operations * o) override; void vlogf(int, const char *, va_list) const throw() override; - fuse_operations ops; - - protected: - virtual NetFS::Client::ConfigurationPtr ReadConfiguration(const std::filesystem::path &) const override; - + void connectToService() override; private: const std::string testEndpoint; fuse_context context; diff --git a/netfs/unittests/testFuse.cpp b/netfs/unittests/testFuse.cpp new file mode 100644 index 0000000..c5f9bbf --- /dev/null +++ b/netfs/unittests/testFuse.cpp @@ -0,0 +1,108 @@ +#define BOOST_TEST_MODULE TestNetFSFuse +#define FUSE_USE_VERSION 26 +#include +#include +#include +#include +#include +#include +#include +#include "mockDaemon.h" + +static const std::filesystem::path mntpnt { binDir / "mnt" }; +const std::string testEndpoint("tcp -h localhost -p 12015"); +const std::string testUri("tcp://localhost:12015/testvol"); + +class FuseMountPoint : public MockDaemonHost, public NetFS::FuseApp { + public: + FuseMountPoint() : + MockDaemonHost(::testEndpoint, { + "--NetFSD.ConfigPath=" + (rootDir / "defaultDaemon.xml").string() + }), + NetFS::FuseApp({::testUri}) + { + std::filesystem::remove(mntpnt); + std::filesystem::create_directory(mntpnt); + struct fuse_args fargs { }; + ch = ::fuse_mount(mntpnt.c_str(), &fargs); + BOOST_REQUIRE(ch); + fs = ::fuse_new(ch, &fargs, &operations, sizeof(fuse_operations), this); + BOOST_REQUIRE(fs); + th = std::make_unique(::fuse_loop, fs); + } + + ~FuseMountPoint() override + { + if (ch) { + ::fuse_unmount(mntpnt.c_str(), ch); + } + if (th) { + th->join(); + } + std::filesystem::remove(mntpnt); + } + + FuseMountPoint(const FuseMountPoint &) = delete; + FuseMountPoint(FuseMountPoint &&) = delete; + + FuseMountPoint & operator=(const FuseMountPoint &) = delete; + FuseMountPoint & operator=(FuseMountPoint &&) = delete; + + struct fuse_context * fuse_get_context() override + { + return ::fuse_get_context(); + } + + static inline char * vstrdupf(const char * fmt, va_list args) + { + char * out {}; + BOOST_REQUIRE_GE(vasprintf(&out, fmt, args), 0); + BOOST_REQUIRE(out); + return out; + } + + void vlogf(int, const char * fmt, va_list args) const noexcept override + { + std::unique_ptr msg(vstrdupf(fmt, args), std::free); + BOOST_TEST_MESSAGE(msg.get()); + } + + struct fuse_chan * ch; + struct fuse * fs; + std::unique_ptr th; +}; + +struct pathrange { + const std::filesystem::path path; + + [[nodiscard]] std::filesystem::directory_iterator begin() const + { + return std::filesystem::directory_iterator(path); + } + + [[nodiscard]] std::filesystem::directory_iterator end() const + { + return {}; + } +}; + +BOOST_FIXTURE_TEST_SUITE(fmp, FuseMountPoint); + +BOOST_AUTO_TEST_CASE(fuse, * boost::unit_test::timeout(5)) +{ + BOOST_REQUIRE(std::filesystem::is_directory(mntpnt)); +} + +BOOST_AUTO_TEST_CASE(fuse_ls, * boost::unit_test::timeout(5)) +{ + BOOST_REQUIRE(std::filesystem::is_directory(mntpnt)); + std::filesystem::create_symlink(selfExe, mntpnt / "me"); + std::set paths(std::filesystem::directory_iterator(mntpnt), {}); + BOOST_REQUIRE_EQUAL(paths.size(), 1); + BOOST_CHECK_EQUAL(paths.begin()->filename(), "me"); + BOOST_CHECK(std::filesystem::is_symlink(mntpnt / "me")); + BOOST_CHECK(std::filesystem::is_symlink(mntpnt / "me")); +} + +BOOST_AUTO_TEST_SUITE_END(); + -- cgit v1.2.3