From: Patrick Donnelly Date: Tue, 10 Sep 2024 20:47:39 +0000 (-0400) Subject: client: hide alternate_name from API X-Git-Tag: v20.0.0^2~39 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=1e34963af11210ebd940e20219728b91031e720f;p=ceph.git client: hide alternate_name from API The dentry alternate_name was exposed primarily to facilitate testing. Instead, put these methods in the TestClient scaffolding to allow reading/manipulating alternate_name. Because we will be using alternate_name to handle encrypted names and the normalized / case folded name, we do not want the application to change the meaning of the metadata out-of-band. Signed-off-by: Patrick Donnelly Fixes: https://tracker.ceph.com/issues/66373 --- diff --git a/src/client/Client.cc b/src/client/Client.cc index 17ade3a88285d..983ba70f414f9 100644 --- a/src/client/Client.cc +++ b/src/client/Client.cc @@ -7603,7 +7603,7 @@ int Client::path_walk(const filepath& origpath, walk_dentry_result* result, cons // namespace ops -int Client::link(const char *relexisting, const char *relpath, const UserPerm& perm, std::string alternate_name) +int Client::do_link(const char *relexisting, const char *relpath, const UserPerm& perm, std::string alternate_name) { RWRef_t mref_reader(mount_state, CLIENT_MOUNTING); if (!mref_reader.is_state_satisfied()) @@ -7700,7 +7700,7 @@ int Client::unlinkat(int dirfd, const char *relpath, int flags, const UserPerm& return r; } -int Client::rename(const char *relfrom, const char *relto, const UserPerm& perm, std::string alternate_name) +int Client::do_rename(const char *relfrom, const char *relto, const UserPerm& perm, std::string alternate_name) { RWRef_t mref_reader(mount_state, CLIENT_MOUNTING); if (!mref_reader.is_state_satisfied()) @@ -7745,12 +7745,7 @@ out: // dirs -int Client::mkdir(const char *relpath, mode_t mode, const UserPerm& perm, std::string alternate_name) -{ - return mkdirat(CEPHFS_AT_FDCWD, relpath, mode, perm, alternate_name); -} - -int Client::mkdirat(int dirfd, const char *relpath, mode_t mode, const UserPerm& perm, +int Client::do_mkdirat(int dirfd, const char *relpath, mode_t mode, const UserPerm& perm, std::string alternate_name) { RWRef_t mref_reader(mount_state, CLIENT_MOUNTING); @@ -7888,12 +7883,7 @@ int Client::mknod(const char *relpath, mode_t mode, const UserPerm& perms, dev_t // symlinks -int Client::symlink(const char *target, const char *relpath, const UserPerm& perms, std::string alternate_name) -{ - return symlinkat(target, CEPHFS_AT_FDCWD, relpath, perms, alternate_name); -} - -int Client::symlinkat(const char *target, int dirfd, const char *relpath, const UserPerm& perms, +int Client::do_symlinkat(const char *target, int dirfd, const char *relpath, const UserPerm& perms, std::string alternate_name) { RWRef_t mref_reader(mount_state, CLIENT_MOUNTING); @@ -10114,15 +10104,7 @@ int Client::create_and_open(int dirfd, const char *relpath, int flags, return r; } -int Client::open(const char *relpath, int flags, const UserPerm& perms, - mode_t mode, int stripe_unit, int stripe_count, - int object_size, const char *data_pool, std::string alternate_name) -{ - return openat(CEPHFS_AT_FDCWD, relpath, flags, perms, mode, stripe_unit, - stripe_count, object_size, data_pool, std::move(alternate_name)); -} - -int Client::openat(int dirfd, const char *relpath, int flags, const UserPerm& perms, +int Client::do_openat(int dirfd, const char *relpath, int flags, const UserPerm& perms, mode_t mode, int stripe_unit, int stripe_count, int object_size, const char *data_pool, std::string alternate_name) { RWRef_t mref_reader(mount_state, CLIENT_MOUNTING); diff --git a/src/client/Client.h b/src/client/Client.h index c731def9b61a9..84c09e9250e80 100644 --- a/src/client/Client.h +++ b/src/client/Client.h @@ -394,15 +394,22 @@ public: void seekdir(dir_result_t *dirp, loff_t offset); int may_delete(const char *relpath, const UserPerm& perms); - int link(const char *existing, const char *newname, const UserPerm& perm, std::string alternate_name=""); + int link(const char *oldpath, const char *newpath, const UserPerm& perm) { + return do_link(oldpath, newpath, perm); + } int unlink(const char *path, const UserPerm& perm); int unlinkat(int dirfd, const char *relpath, int flags, const UserPerm& perm); - int rename(const char *from, const char *to, const UserPerm& perm, std::string alternate_name=""); + int rename(const char *from, const char *to, const UserPerm& perm) { + return do_rename(from, to, perm); + } // dirs - int mkdir(const char *path, mode_t mode, const UserPerm& perm, std::string alternate_name=""); - int mkdirat(int dirfd, const char *relpath, mode_t mode, const UserPerm& perm, - std::string alternate_name=""); + int mkdir(const char *path, mode_t mode, const UserPerm& perm) { + return mkdirat(CEPHFS_AT_FDCWD, path, mode, perm); + } + int mkdirat(int dirfd, const char *path, mode_t mode, const UserPerm& perm) { + return do_mkdirat(dirfd, path, mode, perm); + } int mkdirs(const char *path, mode_t mode, const UserPerm& perms); int rmdir(const char *path, const UserPerm& perms); @@ -410,12 +417,12 @@ public: int readlink(const char *path, char *buf, loff_t size, const UserPerm& perms); int readlinkat(int dirfd, const char *relpath, char *buf, loff_t size, const UserPerm& perms); - int symlink(const char *existing, const char *newname, const UserPerm& perms, std::string alternate_name=""); - int symlinkat(const char *target, int dirfd, const char *relpath, const UserPerm& perms, - std::string alternate_name=""); - - // path traversal for high-level interface - int walk(std::string_view path, struct walk_dentry_result* result, const UserPerm& perms, bool followsym=true); + int symlink(const char *target, const char *linkpath, const UserPerm& perms) { + return symlinkat(target, CEPHFS_AT_FDCWD, linkpath, perms); + } + int symlinkat(const char *target, int dirfd, const char *linkpath, const UserPerm& perms) { + return do_symlinkat(target, dirfd, linkpath, perms); + } // inode stuff unsigned statx_to_mask(unsigned int flags, unsigned int want); @@ -459,21 +466,21 @@ public: // file ops int mknod(const char *path, mode_t mode, const UserPerm& perms, dev_t rdev=0); - int create_and_open(int dirfd, const char *relpath, int flags, const UserPerm& perms, - mode_t mode, int stripe_unit, int stripe_count, int object_size, - const char *data_pool, std::string alternate_name); - int open(const char *path, int flags, const UserPerm& perms, mode_t mode=0, std::string alternate_name="") { - return open(path, flags, perms, mode, 0, 0, 0, NULL, alternate_name); + int open(const char *path, int flags, const UserPerm& perms, mode_t mode=0) { + return open(path, flags, perms, mode, 0, 0, 0, NULL); } int open(const char *path, int flags, const UserPerm& perms, mode_t mode, int stripe_unit, int stripe_count, int object_size, - const char *data_pool, std::string alternate_name=""); - int openat(int dirfd, const char *relpath, int flags, const UserPerm& perms, + const char *data_pool) { + return openat(CEPHFS_AT_FDCWD, path, flags, perms, mode, stripe_unit, stripe_count, object_size, data_pool); + } + int openat(int dirfd, const char *path, int flags, const UserPerm& perms, mode_t mode=0) { + return openat(dirfd, path, flags, perms, mode, 0, 0, 0, NULL); + } + int openat(int dirfd, const char *path, int flags, const UserPerm& perms, mode_t mode, int stripe_unit, int stripe_count, - int object_size, const char *data_pool, std::string alternate_name); - int openat(int dirfd, const char *path, int flags, const UserPerm& perms, mode_t mode=0, - std::string alternate_name="") { - return openat(dirfd, path, flags, perms, mode, 0, 0, 0, NULL, alternate_name); + int object_size, const char *data_pool) { + return do_openat(dirfd, path, flags, perms, mode, stripe_unit, stripe_count, object_size, data_pool); } int lookup_hash(inodeno_t ino, inodeno_t dirino, const char *name, @@ -988,6 +995,9 @@ protected: void dump_mds_requests(Formatter *f); void dump_mds_sessions(Formatter *f, bool cap_dump=false); + // path traversal for testing, looking at walk_dentry_result + int walk(std::string_view path, struct walk_dentry_result* result, const UserPerm& perms, bool followsym=true); + int make_request(MetaRequest *req, const UserPerm& perms, InodeRef *ptarget = 0, bool *pcreated = 0, mds_rank_t use_mds=-1, bufferlist *pdirbl=0, @@ -1016,6 +1026,16 @@ protected: void handle_client_reply(const MConstRef& reply); bool is_dir_operation(MetaRequest *request); + int create_and_open(int dirfd, const char *relpath, int flags, const UserPerm& perms, + mode_t mode, int stripe_unit, int stripe_count, int object_size, + const char *data_pool, std::string alternate_name); + + int do_mkdirat(int dirfd, const char *relpath, mode_t mode, const UserPerm& perm, std::string alternate_name=""); + int do_rename(const char *from, const char *to, const UserPerm& perm, std::string alternate_name=""); + int do_link(const char *existing, const char *newname, const UserPerm& perm, std::string alternate_name=""); + int do_symlinkat(const char *target, int dirfd, const char *linkpath, const UserPerm& perms, std::string alternate_name=""); + int do_openat(int dirfd, const char *path, int flags, const UserPerm& perms, mode_t mode, int stripe_unit, int stripe_count, int object_size, const char *data_pool, std::string alternate_name=""); + int path_walk(const filepath& fp, struct walk_dentry_result* result, const UserPerm& perms, bool followsym=true, int mask=0, InodeRef dirinode=nullptr); int path_walk(const filepath& fp, InodeRef *end, const UserPerm& perms, diff --git a/src/test/client/TestClient.h b/src/test/client/TestClient.h index 67cadc7baafcb..7de90373d5d6a 100644 --- a/src/test/client/TestClient.h +++ b/src/test/client/TestClient.h @@ -34,6 +34,8 @@ namespace ca = ceph::async; class ClientScaffold : public Client { public: + using Client::walk_dentry_result; + ClientScaffold(Messenger *m, MonClient *mc, Objecter *objecter_) : Client(m, mc, objecter_) {} virtual ~ClientScaffold() { } @@ -87,6 +89,31 @@ public: wait_on_list(waiting_for_reclaim); return session->reclaim_state == MetaSession::RECLAIM_FAIL ? true : false; } + + /* Expose alternate_name for testing. There is no need to use virtual + * methods as we will call these only from the Derived class. + */ + int symlinkat(const char *target, int dirfd, const char *linkpath, const UserPerm& perms, std::string alternate_name) { + return do_symlinkat(target, dirfd, linkpath, perms, std::move(alternate_name)); + } + int mkdirat(int dirfd, const char *path, mode_t mode, const UserPerm& perm, std::string alternate_name) { + return do_mkdirat(CEPHFS_AT_FDCWD, path, mode, perm, std::move(alternate_name)); + } + int rename(const char *from, const char *to, const UserPerm& perm, std::string alternate_name) { + return do_rename(from, to, perm, std::move(alternate_name)); + } + int link(const char *oldpath, const char *newpath, const UserPerm& perm, std::string alternate_name) { + return do_link(oldpath, newpath, perm, std::move(alternate_name)); + } + int openat(int dirfd, const char *path, int flags, const UserPerm& perms, + mode_t mode, int stripe_unit, int stripe_count, + int object_size, const char *data_pool, std::string alternate_name) { + return do_openat(dirfd, path, flags, perms, mode, stripe_unit, stripe_count, object_size, data_pool, std::move(alternate_name)); + } + + int walk(std::string_view path, struct walk_dentry_result* result, const UserPerm& perms, bool followsym=true) { + return Client::walk(path, result, perms, followsym); + } }; class TestClient : public ::testing::Test { @@ -141,6 +168,7 @@ public: delete messenger; messenger = nullptr; } + // TODO expose altname versions protected: static inline ceph::async::io_context_pool icp; static inline UserPerm myperm{0,0}; diff --git a/src/test/client/alternate_name.cc b/src/test/client/alternate_name.cc index 43f4280120ca6..57c29a44c4837 100644 --- a/src/test/client/alternate_name.cc +++ b/src/test/client/alternate_name.cc @@ -24,7 +24,7 @@ TEST_F(TestClient, AlternateNameRemount) { auto altname = std::string("foo"); auto dir = fmt::format("{}_{}", ::testing::UnitTest::GetInstance()->current_test_info()->name(), getpid()); - ASSERT_EQ(0, client->mkdir(dir.c_str(), 0777, myperm, altname)); + ASSERT_EQ(0, client->mkdirat(CEPHFS_AT_FDCWD, dir.c_str(), 0777, myperm, altname)); client->unmount(); TearDown(); @@ -32,7 +32,7 @@ TEST_F(TestClient, AlternateNameRemount) { client->mount("/", myperm, true); { - Client::walk_dentry_result wdr; + ClientScaffold::walk_dentry_result wdr; ASSERT_EQ(0, client->walk(dir.c_str(), &wdr, myperm)); ASSERT_EQ(wdr.alternate_name, altname); } @@ -43,10 +43,10 @@ TEST_F(TestClient, AlternateNameRemount) { TEST_F(TestClient, AlternateNameMkdir) { auto dir = fmt::format("{}_{}", ::testing::UnitTest::GetInstance()->current_test_info()->name(), getpid()); - ASSERT_EQ(0, client->mkdir(dir.c_str(), 0777, myperm, "foo")); + ASSERT_EQ(0, client->mkdirat(CEPHFS_AT_FDCWD, dir.c_str(), 0777, myperm, "foo")); { - Client::walk_dentry_result wdr; + ClientScaffold::walk_dentry_result wdr; ASSERT_EQ(0, client->walk(dir.c_str(), &wdr, myperm)); ASSERT_EQ(wdr.alternate_name, "foo"); } @@ -57,10 +57,10 @@ TEST_F(TestClient, AlternateNameMkdir) { TEST_F(TestClient, AlternateNameLong) { auto altname = std::string(4096+1024, '-'); auto dir = fmt::format("{}_{}", ::testing::UnitTest::GetInstance()->current_test_info()->name(), getpid()); - ASSERT_EQ(0, client->mkdir(dir.c_str(), 0777, myperm, altname)); + ASSERT_EQ(0, client->mkdirat(CEPHFS_AT_FDCWD, dir.c_str(), 0777, myperm, altname)); { - Client::walk_dentry_result wdr; + ClientScaffold::walk_dentry_result wdr; ASSERT_EQ(0, client->walk(dir.c_str(), &wdr, myperm)); ASSERT_EQ(wdr.alternate_name, altname); } @@ -71,13 +71,13 @@ TEST_F(TestClient, AlternateNameLong) { TEST_F(TestClient, AlternateNameCreat) { auto altname = std::string("foo"); auto file = fmt::format("{}_{}", ::testing::UnitTest::GetInstance()->current_test_info()->name(), getpid()); - int fd = client->open(file.c_str(), O_CREAT|O_WRONLY, myperm, 0777, altname); + int fd = client->openat(CEPHFS_AT_FDCWD, file.c_str(), O_CREAT|O_WRONLY, myperm, 0777, 0, 0, 0, nullptr, altname); ASSERT_LE(0, fd); ASSERT_EQ(3, client->write(fd, "baz", 3, 0)); ASSERT_EQ(0, client->close(fd)); { - Client::walk_dentry_result wdr; + ClientScaffold::walk_dentry_result wdr; ASSERT_EQ(0, client->walk(file, &wdr, myperm)); ASSERT_EQ(wdr.alternate_name, altname); } @@ -86,17 +86,17 @@ TEST_F(TestClient, AlternateNameCreat) { TEST_F(TestClient, AlternateNameSymlink) { auto altname = std::string("foo"); auto file = fmt::format("{}_{}", ::testing::UnitTest::GetInstance()->current_test_info()->name(), getpid()); - int fd = client->open(file.c_str(), O_CREAT|O_WRONLY, myperm, 0777, altname); + int fd = client->openat(CEPHFS_AT_FDCWD, file.c_str(), O_CREAT|O_WRONLY, myperm, 0777, 0, 0, 0, nullptr, altname); ASSERT_LE(0, fd); ASSERT_EQ(3, client->write(fd, "baz", 3, 0)); ASSERT_EQ(0, client->close(fd)); auto file2 = file+"2"; auto altname2 = altname+"2"; - ASSERT_EQ(0, client->symlink(file.c_str(), file2.c_str(), myperm, altname2)); + ASSERT_EQ(0, client->symlinkat(file.c_str(), CEPHFS_AT_FDCWD, file2.c_str(), myperm, altname2)); { - Client::walk_dentry_result wdr; + ClientScaffold::walk_dentry_result wdr; ASSERT_EQ(0, client->walk(file2, &wdr, myperm, false)); ASSERT_EQ(wdr.alternate_name, altname2); ASSERT_EQ(0, client->walk(file, &wdr, myperm)); @@ -107,7 +107,7 @@ TEST_F(TestClient, AlternateNameSymlink) { TEST_F(TestClient, AlternateNameRename) { auto altname = std::string("foo"); auto file = fmt::format("{}_{}", ::testing::UnitTest::GetInstance()->current_test_info()->name(), getpid()); - int fd = client->open(file.c_str(), O_CREAT|O_WRONLY, myperm, 0777, altname); + int fd = client->openat(CEPHFS_AT_FDCWD, file.c_str(), O_CREAT|O_WRONLY, myperm, 0777, 0, 0, 0, nullptr, altname); ASSERT_LE(0, fd); ASSERT_EQ(3, client->write(fd, "baz", 3, 0)); ASSERT_EQ(0, client->close(fd)); @@ -118,7 +118,7 @@ TEST_F(TestClient, AlternateNameRename) { ASSERT_EQ(0, client->rename(file.c_str(), file2.c_str(), myperm, altname2)); { - Client::walk_dentry_result wdr; + ClientScaffold::walk_dentry_result wdr; ASSERT_EQ(0, client->walk(file2, &wdr, myperm)); ASSERT_EQ(wdr.alternate_name, altname2); } @@ -127,7 +127,7 @@ TEST_F(TestClient, AlternateNameRename) { TEST_F(TestClient, AlternateNameRenameExistMatch) { auto altname = std::string("foo"); auto file = fmt::format("{}_{}", ::testing::UnitTest::GetInstance()->current_test_info()->name(), getpid()); - int fd = client->open(file.c_str(), O_CREAT|O_WRONLY, myperm, 0777, altname); + int fd = client->openat(CEPHFS_AT_FDCWD, file.c_str(), O_CREAT|O_WRONLY, myperm, 0777, 0, 0, 0, nullptr, altname); ASSERT_LE(0, fd); ASSERT_EQ(3, client->write(fd, "baz", 3, 0)); ASSERT_EQ(0, client->close(fd)); @@ -135,7 +135,7 @@ TEST_F(TestClient, AlternateNameRenameExistMatch) { auto file2 = file+"2"; auto altname2 = altname+"2"; - fd = client->open(file2.c_str(), O_CREAT|O_WRONLY, myperm, 0777, altname2); + fd = client->openat(CEPHFS_AT_FDCWD, file2.c_str(), O_CREAT|O_WRONLY, myperm, 0777, 0, 0, 0, nullptr, altname2); ASSERT_LE(0, fd); ASSERT_EQ(3, client->write(fd, "baz", 3, 0)); ASSERT_EQ(0, client->close(fd)); @@ -143,7 +143,7 @@ TEST_F(TestClient, AlternateNameRenameExistMatch) { ASSERT_EQ(0, client->rename(file.c_str(), file2.c_str(), myperm, altname2)); { - Client::walk_dentry_result wdr; + ClientScaffold::walk_dentry_result wdr; ASSERT_EQ(0, client->walk(file2, &wdr, myperm)); ASSERT_EQ(wdr.alternate_name, altname2); } @@ -152,7 +152,7 @@ TEST_F(TestClient, AlternateNameRenameExistMatch) { TEST_F(TestClient, AlternateNameRenameExistMisMatch) { auto altname = std::string("foo"); auto file = fmt::format("{}_{}", ::testing::UnitTest::GetInstance()->current_test_info()->name(), getpid()); - int fd = client->open(file.c_str(), O_CREAT|O_WRONLY, myperm, 0777, altname); + int fd = client->openat(CEPHFS_AT_FDCWD, file.c_str(), O_CREAT|O_WRONLY, myperm, 0777, 0, 0, 0, nullptr, altname); ASSERT_LE(0, fd); ASSERT_EQ(3, client->write(fd, "baz", 3, 0)); ASSERT_EQ(0, client->close(fd)); @@ -160,7 +160,7 @@ TEST_F(TestClient, AlternateNameRenameExistMisMatch) { auto file2 = file+"2"; auto altname2 = altname+"2"; - fd = client->open(file2.c_str(), O_CREAT|O_WRONLY, myperm, 0777, altname+"mismatch"); + fd = client->openat(CEPHFS_AT_FDCWD, file2.c_str(), O_CREAT|O_WRONLY, myperm, 0777, 0, 0, 0, nullptr, altname+"mismatch"); ASSERT_LE(0, fd); ASSERT_EQ(3, client->write(fd, "baz", 3, 0)); ASSERT_EQ(0, client->close(fd)); @@ -168,7 +168,7 @@ TEST_F(TestClient, AlternateNameRenameExistMisMatch) { ASSERT_EQ(-EINVAL, client->rename(file.c_str(), file2.c_str(), myperm, altname2)); { - Client::walk_dentry_result wdr; + ClientScaffold::walk_dentry_result wdr; ASSERT_EQ(0, client->walk(file2, &wdr, myperm)); ASSERT_EQ(wdr.alternate_name, altname+"mismatch"); } @@ -177,7 +177,7 @@ TEST_F(TestClient, AlternateNameRenameExistMisMatch) { TEST_F(TestClient, AlternateNameLink) { auto altname = std::string("foo"); auto file = fmt::format("{}_{}", ::testing::UnitTest::GetInstance()->current_test_info()->name(), getpid()); - int fd = client->open(file.c_str(), O_CREAT|O_WRONLY, myperm, 0777, altname); + int fd = client->openat(CEPHFS_AT_FDCWD, file.c_str(), O_CREAT|O_WRONLY, myperm, 0777, 0, 0, 0, nullptr, altname); ASSERT_LE(0, fd); ASSERT_EQ(3, client->write(fd, "baz", 3, 0)); ASSERT_EQ(0, client->close(fd)); @@ -188,7 +188,7 @@ TEST_F(TestClient, AlternateNameLink) { ASSERT_EQ(0, client->link(file.c_str(), file2.c_str(), myperm, altname2)); { - Client::walk_dentry_result wdr; + ClientScaffold::walk_dentry_result wdr; ASSERT_EQ(0, client->walk(file2, &wdr, myperm)); ASSERT_EQ(wdr.alternate_name, altname2); ASSERT_EQ(0, client->walk(file, &wdr, myperm));