From 87a605405429062b20b20c083842db93c9586154 Mon Sep 17 00:00:00 2001 From: Andreas Hindborg Date: Thu, 17 Mar 2022 11:09:56 +0100 Subject: [PATCH] util,fs: Fix parameter handling in utility functions Closes #97. Signed-off-by: Andreas Hindborg --- fs/fs_zenfs.cc | 16 ++-- fs/fs_zenfs.h | 8 +- fs/util.cc | 207 ++++++++++++++++++++++++++----------------------- fs/util.h | 14 ++-- util/zenfs.cc | 111 ++++++++++++++++---------- 5 files changed, 199 insertions(+), 157 deletions(-) diff --git a/fs/fs_zenfs.cc b/fs/fs_zenfs.cc index 777f8653..154722af 100644 --- a/fs/fs_zenfs.cc +++ b/fs/fs_zenfs.cc @@ -236,9 +236,9 @@ IOStatus ZenMetaLog::ReadRecord(Slice* record, std::string* scratch) { return IOStatus::OK(); } -ZenFS::ZenFS(ZonedBlockDevice* zbd, std::shared_ptr aux_fs, - std::shared_ptr logger) - : FileSystemWrapper(aux_fs), zbd_(zbd), logger_(logger) { +ZenFS::ZenFS(std::unique_ptr zbd, + std::shared_ptr aux_fs, std::shared_ptr logger) + : FileSystemWrapper(aux_fs), zbd_(zbd.release()), logger_(logger) { Info(logger_, "ZenFS initializing"); Info(logger_, "ZenFS parameters: block device: %s, aux filesystem: %s", zbd_->GetFilename().c_str(), target()->Name()); @@ -1396,13 +1396,14 @@ Status ZenFS::Mount(bool readonly) { return Status::OK(); } -Status ZenFS::MkFS(std::string aux_fs_path, uint32_t finish_threshold) { +Status ZenFS::MkFS(std::filesystem::path const& aux_fs_path, + uint32_t finish_threshold) { std::vector metazones = zbd_->GetMetaZones(); std::unique_ptr log; Zone* meta_zone = nullptr; IOStatus s; - if (aux_fs_path.length() > 255) { + if (aux_fs_path.string().length() > 255) { return Status::InvalidArgument( "Aux filesystem path must be less than 256 bytes\n"); } @@ -1506,7 +1507,8 @@ Status NewZenFS(FileSystem** fs, const std::string& bdevname, } #endif - ZonedBlockDevice* zbd = new ZonedBlockDevice(bdevname, logger, metrics); + std::unique_ptr zbd{ + new ZonedBlockDevice(bdevname, logger, metrics)}; IOStatus zbd_status = zbd->Open(false, true); if (!zbd_status.ok()) { Error(logger, "mkfs: Failed to open zoned block device: %s", @@ -1514,7 +1516,7 @@ Status NewZenFS(FileSystem** fs, const std::string& bdevname, return Status::IOError(zbd_status.ToString()); } - ZenFS* zenFS = new ZenFS(zbd, FileSystem::Default(), logger); + ZenFS* zenFS = new ZenFS(std::move(zbd), FileSystem::Default(), logger); s = zenFS->Mount(false); if (!s.ok()) { delete zenFS; diff --git a/fs/fs_zenfs.h b/fs/fs_zenfs.h index 3fe62ef9..d2634ef9 100644 --- a/fs/fs_zenfs.h +++ b/fs/fs_zenfs.h @@ -6,13 +6,13 @@ #pragma once +#include #include #include "io_zenfs.h" #include "metrics.h" #include "rocksdb/env.h" #include "rocksdb/file_system.h" -#include "rocksdb/status.h" #include "snapshot.h" #include "version.h" #include "zbd_zenfs.h" @@ -257,12 +257,14 @@ class ZenFS : public FileSystemWrapper { IODebugContext* dbg, bool reopen); public: - explicit ZenFS(ZonedBlockDevice* zbd, std::shared_ptr aux_fs, + explicit ZenFS(std::unique_ptr zbd, + std::shared_ptr aux_fs, std::shared_ptr logger); virtual ~ZenFS(); Status Mount(bool readonly); - Status MkFS(std::string aux_fs_path, uint32_t finish_threshold); + Status MkFS(std::filesystem::path const& aux_fs_path, + uint32_t finish_threshold); std::map GetWriteLifeTimeHints(); const char* Name() const override { diff --git a/fs/util.cc b/fs/util.cc index 71b5aecb..7dc9b483 100644 --- a/fs/util.cc +++ b/fs/util.cc @@ -1,10 +1,7 @@ #include "util.h" -#include #include -#include -#include #include #include @@ -16,138 +13,152 @@ namespace ROCKSDB_NAMESPACE { -std::unique_ptr zbd_open(std::string const &zbd_path, - bool readonly, bool exclusive) { - std::unique_ptr zbd{ - new ZonedBlockDevice(zbd_path, nullptr)}; - IOStatus open_status = zbd->Open(readonly, exclusive); +IOStatus zenfs_zbd_open(std::filesystem::path const &path, bool readonly, + bool exclusive, + std::unique_ptr &out_zbd) { + out_zbd = + std::unique_ptr{new ZonedBlockDevice(path, nullptr)}; + IOStatus status = out_zbd->Open(readonly, exclusive); - if (!open_status.ok()) { + if (!status.ok()) { fprintf(stderr, "Failed to open zoned block device: %s, error: %s\n", - zbd_path.c_str(), open_status.ToString().c_str()); - zbd.reset(); + path.c_str(), status.ToString().c_str()); + out_zbd.reset(); } - return zbd; + return status; } -// Here we pass 'zbd' by non-const reference to be able to pass its ownership -// to 'zenFS' -Status zenfs_mount(std::unique_ptr &zbd, - std::unique_ptr *zenFS, bool readonly) { - Status s; - - std::unique_ptr localZenFS{ - new ZenFS(zbd.release(), FileSystem::Default(), nullptr)}; - s = localZenFS->Mount(readonly); - if (!s.ok()) { - localZenFS.reset(); +Status zenfs_mount(std::unique_ptr &zbd, bool readonly, + std::unique_ptr &out_zen_fs) { + Status status; + + out_zen_fs = std::unique_ptr{ + new ZenFS(std::move(zbd), FileSystem::Default(), nullptr)}; + status = out_zen_fs->Mount(readonly); + if (!status.ok()) { + out_zen_fs.reset(); } - *zenFS = std::move(localZenFS); - return s; + return status; } -static int is_dir(const char *path) { - struct stat st; - if (stat(path, &st) != 0) { - fprintf(stderr, "Failed to stat %s\n", path); - return 1; - } - return S_ISDIR(st.st_mode); +static Status zenfs_exists(std::filesystem::path const &zbd_path, + bool &out_exists) { + Status status; + std::unique_ptr zbd; + std::unique_ptr zen_fs; + + status = zenfs_zbd_open(zbd_path, false, true, zbd); + if (!status.ok()) return Status::IOError("Failed to open ZBD"); + + status = zenfs_mount(zbd, false, zen_fs); + out_exists = status.ok() || !status.IsNotFound(); + + return Status::OK(); } // Create or check pre-existing aux directory and fail if it is // inaccessible by current user and if it has previous data -static int create_aux_dir(const char *path) { - struct dirent *dent; - size_t nfiles = 0; - int ret = 0; - - errno = 0; - ret = mkdir(path, 0750); - if (ret < 0 && EEXIST != errno) { - fprintf(stderr, "Failed to create aux directory %s: %s\n", path, - strerror(errno)); - return 1; +static Status create_aux_dir(std::filesystem::path const &path) { + namespace fs = std::filesystem; + std::error_code ec; + + bool aux_exists = fs::exists(path, ec); + if (ec) { + return Status::Aborted("Failed to check if aux directory exists: " + + ec.message()); + } + + bool aux_is_dir = false; + if (aux_exists) { + aux_is_dir = fs::is_directory(path, ec); + if (ec) { + return Status::Aborted("Failed to check if aux_dir is directory" + + ec.message()); + } } - // The aux_path is now available, check if it is a directory infact - // and is empty and the user has access permission - if (!is_dir(path)) { - fprintf(stderr, "Aux path %s is not a directory\n", path); - return 1; + if (aux_exists && !aux_is_dir) { + return Status::Aborted("Aux path exists but is not a directory"); } - errno = 0; - - auto closedirDeleter = [](DIR *d) { - if (d != nullptr) closedir(d); - }; - std::unique_ptr aux_dir{ - opendir(path), std::move(closedirDeleter)}; - if (errno) { - fprintf(stderr, "Failed to open aux directory %s: %s\n", path, - strerror(errno)); - return 1; + if (!aux_exists) { + if (!fs::create_directory(path, ec) || ec) { + return Status::IOError("Failed to create aux path:" + ec.message()); + } + + fs::permissions( + path, + fs::perms::owner_all | fs::perms::group_read | fs::perms::group_exec, + ec); + if (ec) { + return Status::IOError("Failed to set permissions on aux path:" + + ec.message()); + } } - // Consider the directory as non-empty if any files/dir other - // than . and .. are found. - while ((dent = readdir(aux_dir.get())) != NULL && nfiles <= 2) ++nfiles; - if (nfiles > 2) { - fprintf(stderr, "Aux directory %s is not empty.\n", path); - return 1; + if (access(path.c_str(), R_OK | W_OK | X_OK) < 0) { + return Status::Aborted( + "User does not have access permissions on aux directory " + + path.string()); } - if (access(path, R_OK | W_OK | X_OK) < 0) { - fprintf(stderr, - "User does not have access permissions on " - "aux directory %s\n", - path); - return 1; + if (std::distance(fs::directory_iterator{path}, {}) > 2) { + return Status::Aborted("Aux directory " + path.string() + " is not empty"); } - return 0; + return Status::OK(); } -int zenfs_mkfs(std::string const &zbd_path, std::string const &aux_path, - int finish_threshold, bool force) { - Status s; +static Status zenfs_create(std::filesystem::path const &zbd_path, + std::filesystem::path const &aux_path, + uint32_t finish_threshold - if (create_aux_dir(aux_path.c_str())) return 1; +) { + Status status; + std::unique_ptr zbd; + std::unique_ptr zen_fs; - std::unique_ptr zbd = zbd_open(zbd_path, false, true); - if (!zbd) return 1; + status = create_aux_dir(aux_path); + if (!status.ok()) return status; - std::unique_ptr zenFS; - s = zenfs_mount(zbd, &zenFS, false); - if ((s.ok() || !s.IsNotFound()) && !force) { - fprintf( - stderr, - "Existing filesystem found, use --force if you want to replace it.\n"); - return 1; + status = zenfs_zbd_open(zbd_path, false, true, zbd); + if (!status.ok()) { + return status; } - zenFS.reset(); - - zbd = zbd_open(zbd_path, false, true); - ZonedBlockDevice *zbdRaw = zbd.get(); - zenFS.reset(new ZenFS(zbd.release(), FileSystem::Default(), nullptr)); + zen_fs.reset(new ZenFS(std::move(zbd), FileSystem::Default(), nullptr)); std::string aux_path_patched = aux_path; if (aux_path_patched.back() != '/') aux_path_patched.append("/"); - s = zenFS->MkFS(aux_path_patched, finish_threshold); - if (!s.ok()) { - fprintf(stderr, "Failed to create file system, error: %s\n", - s.ToString().c_str()); - return 1; + status = zen_fs->MkFS(aux_path_patched, finish_threshold); + if (!status.ok()) { + return status; } - fprintf(stdout, "ZenFS file system created. Free space: %lu MB\n", - zbdRaw->GetFreeSpace() / (1024 * 1024)); + return status; +} + +Status zenfs_mkfs(std::filesystem::path const &zbd_path, + std::filesystem::path const &aux_path, + uint32_t finish_threshold, bool force) { + Status status; + bool exists = false; + + status = zenfs_exists(zbd_path, exists); + if (!status.ok()) { + return status; + } - return 0; + if (exists && !force) { + return Status::Aborted( + "Existing filesystem found, use --force if you want to replace " + "it."); + } + + return zenfs_create(zbd_path, aux_path, finish_threshold); } + } // namespace ROCKSDB_NAMESPACE diff --git a/fs/util.h b/fs/util.h index a59378b5..a13e06d8 100644 --- a/fs/util.h +++ b/fs/util.h @@ -8,12 +8,14 @@ namespace ROCKSDB_NAMESPACE { -std::unique_ptr zbd_open(std::string const& zbd_path, - bool readonly, bool exclusive); +IOStatus zenfs_zbd_open(std::filesystem::path const &path, bool readonly, + bool exclusive, + std::unique_ptr &out_zbd); -Status zenfs_mount(std::unique_ptr& zbd, - std::unique_ptr* zenFS, bool readonly); +Status zenfs_mount(std::unique_ptr &zbd, bool readonly, + std::unique_ptr &out_zen_fs); -int zenfs_mkfs(std::string const& zbd_path, std::string const& aux_path, - int finish_threshold, bool force); +Status zenfs_mkfs(std::filesystem::path const &zbd_path, + std::filesystem::path const &aux_path, + uint32_t finish_threshold, bool force); } // namespace ROCKSDB_NAMESPACE diff --git a/util/zenfs.cc b/util/zenfs.cc index b6666c16..00008709 100644 --- a/util/zenfs.cc +++ b/util/zenfs.cc @@ -12,6 +12,7 @@ #include #include +#include #include #include #include @@ -87,9 +88,11 @@ int zenfs_tool_mkfs() { return 1; } - return zenfs_mkfs(std::filesystem::path(FLAGS_zbd), - std::filesystem::path(FLAGS_aux_path), - FLAGS_finish_threshold, FLAGS_force); + s = zenfs_mkfs(std::filesystem::path(FLAGS_zbd), + std::filesystem::path(FLAGS_aux_path), FLAGS_finish_threshold, + FLAGS_force); + + return !s.ok(); } void list_children(const std::unique_ptr &zenFS, @@ -133,11 +136,14 @@ void list_children(const std::unique_ptr &zenFS, int zenfs_tool_list() { Status s; - std::unique_ptr zbd = zbd_open(FLAGS_zbd, true, false); - if (!zbd) return 1; + std::unique_ptr zbd; + s = zenfs_zbd_open(FLAGS_zbd, true, false, zbd); + if (!s.ok() || !zbd) { + return 1; + } std::unique_ptr zenFS; - s = zenfs_mount(zbd, &zenFS, true); + s = zenfs_mount(zbd, true, zenFS); if (!s.ok()) { fprintf(stderr, "Failed to mount filesystem, error: %s\n", s.ToString().c_str()); @@ -155,20 +161,22 @@ int zenfs_tool_list() { int zenfs_tool_df() { Status s; - std::unique_ptr zbd = zbd_open(FLAGS_zbd, true, false); - if (!zbd) return 1; - ZonedBlockDevice *zbdRaw = zbd.get(); - + std::unique_ptr zbd; std::unique_ptr zenFS; - s = zenfs_mount(zbd, &zenFS, true); + + s = zenfs_zbd_open(FLAGS_zbd, true, false, zbd); + if (!s.ok() || !zbd) return 1; + + uint64_t used = zbd->GetUsedSpace(); + uint64_t free = zbd->GetFreeSpace(); + uint64_t reclaimable = zbd->GetReclaimableSpace(); + + s = zenfs_mount(zbd, true, zenFS); if (!s.ok()) { fprintf(stderr, "Failed to mount filesystem, error: %s\n", s.ToString().c_str()); return 1; } - uint64_t used = zbdRaw->GetUsedSpace(); - uint64_t free = zbdRaw->GetFreeSpace(); - uint64_t reclaimable = zbdRaw->GetReclaimableSpace(); /* Avoid divide by zero */ if (used == 0) used = 1; @@ -375,7 +383,12 @@ int zenfs_tool_backup() { IOStatus io_status; IOOptions opts; IODebugContext dbg; - std::unique_ptr zbd = zbd_open(FLAGS_zbd, true, true); + std::unique_ptr zbd; + + status = zenfs_zbd_open(FLAGS_zbd, true, true, zbd); + if (!status.ok()) { + return 1; + } if (!zenfs_format_path(FLAGS_backup_path, false)) { fprintf(stderr, "Error: invalid backup path \n"); @@ -387,14 +400,17 @@ int zenfs_tool_backup() { fprintf(stderr, "WARNING: attempting to back up a zoned block device in use! " "Expect data loss and corruption.\n"); - zbd = zbd_open(FLAGS_zbd, true, false); + status = zenfs_zbd_open(FLAGS_zbd, true, false, zbd); + if (!status.ok()) { + return 1; + } } } if (!zbd) return 1; std::unique_ptr zenFS; - status = zenfs_mount(zbd, &zenFS, true); + status = zenfs_mount(zbd, true, zenFS); if (!status.ok()) { fprintf(stderr, "Failed to mount filesystem, error: %s\n", status.ToString().c_str()); @@ -444,16 +460,17 @@ int zenfs_tool_link() { IOStatus io_s; IOOptions iopts; IODebugContext dbg; + std::unique_ptr zbd; + std::unique_ptr zenFS; if (FLAGS_src_file.empty() || FLAGS_dest_file.empty()) { fprintf(stderr, "Error: Specify --src_file and --dest_file to be linked\n"); return 1; } - std::unique_ptr zbd = zbd_open(false, true); - if (!zbd) return 1; + s = zenfs_zbd_open(FLAGS_zbd, false, true, zbd); + if (!s.ok() || !zbd) return 1; - std::unique_ptr zenFS; - s = zenfs_mount(zbd, &zenFS, false); + s = zenfs_mount(zbd, false, zenFS); if (!s.ok()) { fprintf(stderr, "Failed to mount filesystem, error: %s\n", s.ToString().c_str()); @@ -476,16 +493,18 @@ int zenfs_tool_delete_file() { IOStatus io_s; IOOptions iopts; IODebugContext dbg; + std::unique_ptr zbd; + std::unique_ptr zenFS; if (FLAGS_path.empty()) { fprintf(stderr, "Error: Specify --path of the file to be deleted.\n"); return 1; } - std::unique_ptr zbd = zbd_open(false, true); - if (!zbd) return 1; - std::unique_ptr zenFS; - s = zenfs_mount(zbd, &zenFS, false); + s = zenfs_zbd_open(FLAGS_zbd, false, true, zbd); + if (!s.ok() || !zbd) return 1; + + s = zenfs_mount(zbd, false, zenFS); if (!s.ok()) { fprintf(stderr, "Failed to mount filesystem, error: %s\n", s.ToString().c_str()); @@ -507,17 +526,19 @@ int zenfs_tool_rename_file() { IOStatus io_s; IOOptions iopts; IODebugContext dbg; + std::unique_ptr zbd; + std::unique_ptr zenFS; if (FLAGS_src_file.empty() || FLAGS_dest_file.empty()) { fprintf(stderr, "Error: Specify --src_file and --dest_file to be renamed\n"); return 1; } - std::unique_ptr zbd = zbd_open(false, true); - if (!zbd) return 1; - std::unique_ptr zenFS; - s = zenfs_mount(zbd, &zenFS, false); + s = zenfs_zbd_open(FLAGS_zbd, false, true, zbd); + if (!s.ok() || !zbd) return 1; + + s = zenfs_mount(zbd, false, zenFS); if (!s.ok()) { fprintf(stderr, "Failed to mount filesystem, error: %s\n", s.ToString().c_str()); @@ -589,11 +610,12 @@ int zenfs_tool_restore() { ReadWriteLifeTimeHints(); - std::unique_ptr zbd = zbd_open(FLAGS_zbd, false, true); - if (!zbd) return 1; + std::unique_ptr zbd; + status = zenfs_zbd_open(FLAGS_zbd, false, true, zbd); + if (!status.ok() || !zbd) return 1; std::unique_ptr zenFS; - status = zenfs_mount(zbd, &zenFS, false); + status = zenfs_mount(zbd, false, zenFS); if (!status.ok()) { fprintf(stderr, "Failed to mount filesystem, error: %s\n", status.ToString().c_str()); @@ -622,21 +644,23 @@ int zenfs_tool_restore() { int zenfs_tool_dump() { Status s; - std::unique_ptr zbd = zbd_open(FLAGS_zbd, true, false); - if (!zbd) return 1; - ZonedBlockDevice *zbdRaw = zbd.get(); - + std::unique_ptr zbd; std::unique_ptr zenFS; - s = zenfs_mount(zbd, &zenFS, true); + std::ostream &json_stream = std::cout; + + s = zenfs_zbd_open(FLAGS_zbd, true, false, zbd); + if (!s.ok() || !zbd) return 1; + + json_stream << "{\"zones\":"; + zbd->EncodeJson(json_stream); + + s = zenfs_mount(zbd, true, zenFS); if (!s.ok()) { fprintf(stderr, "Failed to mount filesystem, error: %s\n", s.ToString().c_str()); return 1; } - std::ostream &json_stream = std::cout; - json_stream << "{\"zones\":"; - zbdRaw->EncodeJson(json_stream); json_stream << ",\"files\":"; zenFS->EncodeJson(json_stream); json_stream << "}"; @@ -646,11 +670,12 @@ int zenfs_tool_dump() { int zenfs_tool_fsinfo() { Status s; - std::unique_ptr zbd = zbd_open(FLAGS_zbd, true, false); - if (!zbd) return 1; + std::unique_ptr zbd; + s = zenfs_zbd_open(FLAGS_zbd, true, false, zbd); + if (!s.ok() || !zbd) return 1; std::unique_ptr zenFS; - s = zenfs_mount(zbd, &zenFS, true); + s = zenfs_mount(zbd, true, zenFS); if (!s.ok()) { fprintf(stderr, "Failed to mount filesystem, error: %s\n", s.ToString().c_str());