From 84f3bcea6848faf73cc6b21f1a7f5d5887cc3c70 Mon Sep 17 00:00:00 2001 From: Ramon Nou Date: Fri, 13 May 2022 09:41:28 +0200 Subject: [PATCH 1/4] Implemented unlink testing and ISDIR errno --- src/client/gkfs_functions.cpp | 7 ++ tests/integration/harness/CMakeLists.txt | 1 + .../integration/harness/gkfs.io/commands.hpp | 3 + tests/integration/harness/gkfs.io/main.cpp | 1 + tests/integration/harness/gkfs.io/unlink.cpp | 98 +++++++++++++++++++ tests/integration/harness/io.py | 10 ++ .../operations/test_unlink_operations.py | 87 ++++++++++++++++ 7 files changed, 207 insertions(+) create mode 100644 tests/integration/harness/gkfs.io/unlink.cpp create mode 100644 tests/integration/operations/test_unlink_operations.py diff --git a/src/client/gkfs_functions.cpp b/src/client/gkfs_functions.cpp index e505e270a..a9cb5069f 100644 --- a/src/client/gkfs_functions.cpp +++ b/src/client/gkfs_functions.cpp @@ -276,6 +276,13 @@ gkfs_remove(const std::string& path) { if(!md) { return -1; } + + if(S_ISDIR(md->mode())) { + LOG(ERROR, "Cannot remove directory '{}'", path); + errno = EISDIR; + return -1; + } + auto err = gkfs::rpc::forward_remove(path); if(err) { errno = err; diff --git a/tests/integration/harness/CMakeLists.txt b/tests/integration/harness/CMakeLists.txt index 20af1a029..30bae73e6 100644 --- a/tests/integration/harness/CMakeLists.txt +++ b/tests/integration/harness/CMakeLists.txt @@ -62,6 +62,7 @@ add_executable(gkfs.io gkfs.io/getcwd_validate.cpp gkfs.io/symlink.cpp gkfs.io/directory_validate.cpp + gkfs.io/unlink.cpp ) include(FetchContent) diff --git a/tests/integration/harness/gkfs.io/commands.hpp b/tests/integration/harness/gkfs.io/commands.hpp index 8cc92eadb..e489a44b6 100644 --- a/tests/integration/harness/gkfs.io/commands.hpp +++ b/tests/integration/harness/gkfs.io/commands.hpp @@ -108,4 +108,7 @@ getcwd_validate_init(CLI::App& app); void symlink_init(CLI::App& app); +void +unlink_init(CLI::App& app); + #endif // IO_COMMANDS_HPP diff --git a/tests/integration/harness/gkfs.io/main.cpp b/tests/integration/harness/gkfs.io/main.cpp index a7ce919b0..c737722a9 100644 --- a/tests/integration/harness/gkfs.io/main.cpp +++ b/tests/integration/harness/gkfs.io/main.cpp @@ -62,6 +62,7 @@ init_commands(CLI::App& app) { chdir_init(app); getcwd_validate_init(app); symlink_init(app); + unlink_init(app); } diff --git a/tests/integration/harness/gkfs.io/unlink.cpp b/tests/integration/harness/gkfs.io/unlink.cpp new file mode 100644 index 000000000..94118ce20 --- /dev/null +++ b/tests/integration/harness/gkfs.io/unlink.cpp @@ -0,0 +1,98 @@ +/* + Copyright 2018-2022, Barcelona Supercomputing Center (BSC), Spain + Copyright 2015-2022, Johannes Gutenberg Universitaet Mainz, Germany + + This software was partially supported by the + EC H2020 funded project NEXTGenIO (Project ID: 671951, www.nextgenio.eu). + + This software was partially supported by the + ADA-FS project under the SPPEXA project funded by the DFG. + + This file is part of GekkoFS. + + GekkoFS is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + GekkoFS is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with GekkoFS. If not, see . + + SPDX-License-Identifier: GPL-3.0-or-later +*/ + +/* C++ includes */ +#include +#include +#include +#include +#include +#include +#include +#include + +/* C includes */ +#include + +using json = nlohmann::json; + +struct unlink_options { + bool verbose{}; + std::string pathname; + + REFL_DECL_STRUCT(unlink_options, REFL_DECL_MEMBER(bool, verbose), + REFL_DECL_MEMBER(std::string, pathname)); +}; + +struct unlink_output { + int retval; + int errnum; + + REFL_DECL_STRUCT(unlink_output, REFL_DECL_MEMBER(int, retval), + REFL_DECL_MEMBER(int, errnum)); +}; + +void +to_json(json& record, const unlink_output& out) { + record = serialize(out); +} + +void +unlink_exec(const unlink_options& opts) { + + auto fd = ::unlink(opts.pathname.c_str()); + + if(opts.verbose) { + fmt::print("unlink(pathname=\"{}\") = {}, errno: {} [{}]\n", + opts.pathname, errno, ::strerror(errno)); + return; + } + + json out = unlink_output{fd, errno}; + fmt::print("{}\n", out.dump(2)); + + return; +} + +void +unlink_init(CLI::App& app) { + + // Create the option and subcommand objects + auto opts = std::make_shared(); + auto* cmd = app.add_subcommand("unlink", "Execute the unlink() system call"); + + // Add options to cmd, binding them to opts + cmd->add_flag("-v,--verbose", opts->verbose, + "Produce human readable output"); + + cmd->add_option("pathname", opts->pathname, "File name") + ->required() + ->type_name(""); + + cmd->callback([opts]() { unlink_exec(*opts); }); +} diff --git a/tests/integration/harness/io.py b/tests/integration/harness/io.py index 05423d857..7b65adc23 100644 --- a/tests/integration/harness/io.py +++ b/tests/integration/harness/io.py @@ -383,6 +383,15 @@ class SymlinkOutputSchema(Schema): def make_object(self, data, **kwargs): return namedtuple('SymlinkReturn', ['retval', 'errno'])(**data) +class UnlinkOutputSchema(Schema): + """Schema to deserialize the results of an unlink() execution""" + retval = fields.Integer(required=True) + errno = Errno(data_key='errnum', required=True) + + @post_load + def make_object(self, data, **kwargs): + return namedtuple('UnlinkReturn', ['retval', 'errno'])(**data) + # UTIL class FileCompareOutputSchema(Schema): @@ -419,6 +428,7 @@ class IOParser: 'write_validate' : WriteValidateOutputSchema(), 'truncate': TruncateOutputSchema(), 'directory_validate' : DirectoryValidateOutputSchema(), + 'unlink' : UnlinkOutputSchema(), # UTIL 'file_compare': FileCompareOutputSchema(), 'chdir' : ChdirOutputSchema(), diff --git a/tests/integration/operations/test_unlink_operations.py b/tests/integration/operations/test_unlink_operations.py new file mode 100644 index 000000000..967c6b791 --- /dev/null +++ b/tests/integration/operations/test_unlink_operations.py @@ -0,0 +1,87 @@ +################################################################################ +# Copyright 2018-2022, Barcelona Supercomputing Center (BSC), Spain # +# Copyright 2015-2022, Johannes Gutenberg Universitaet Mainz, Germany # +# # +# This software was partially supported by the # +# EC H2020 funded project NEXTGenIO (Project ID: 671951, www.nextgenio.eu). # +# # +# This software was partially supported by the # +# ADA-FS project under the SPPEXA project funded by the DFG. # +# # +# This file is part of GekkoFS. # +# # +# GekkoFS is free software: you can redistribute it and/or modify # +# it under the terms of the GNU General Public License as published by # +# the Free Software Foundation, either version 3 of the License, or # +# (at your option) any later version. # +# # +# GekkoFS is distributed in the hope that it will be useful, # +# but WITHOUT ANY WARRANTY; without even the implied warranty of # +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the # +# GNU General Public License for more details. # +# # +# You should have received a copy of the GNU General Public License # +# along with GekkoFS. If not, see . # +# # +# SPDX-License-Identifier: GPL-3.0-or-later # +################################################################################ + +import harness +from pathlib import Path +import errno +import stat +import os +import ctypes +import sys +import pytest +from harness.logger import logger + +nonexisting = "nonexisting" + + + +def test_unlink(gkfs_daemon, gkfs_client): + + file = gkfs_daemon.mountdir / "file" + dir = gkfs_daemon.mountdir / "dir" + + + # Delete an unexistent file + ret = gkfs_client.unlink(file) + assert ret.retval == -1 + assert ret.errno == errno.ENOENT + + + + + + # create a file in gekkofs + ret = gkfs_client.open(file, + os.O_CREAT | os.O_WRONLY, + stat.S_IRWXU | stat.S_IRWXG | stat.S_IRWXO) + + assert ret.retval == 10000 + + # write a buffer we know + buf = b'42' + ret = gkfs_client.write(file, buf, len(buf)) + assert ret.retval == len(buf) # Return the number of written bytes + + + + ret = gkfs_client.unlink(file) # Remove renamed file (success) + assert ret.retval == 0 + + ret = gkfs_client.stat(file) # file does not exist + assert ret.retval != 0 + assert ret.errno == errno.ENOENT + + ret = gkfs_client.mkdir(dir, stat.S_IRWXU | stat.S_IRWXG | stat.S_IRWXO) # Create a directory + assert ret.retval == 0 + + ret = gkfs_client.unlink(dir) + assert ret.retval == -1 + assert ret.errno == errno.EISDIR + + + -- GitLab From 3101bd5e21c53c8aa593da1164514bdec0eb2e8b Mon Sep 17 00:00:00 2001 From: Ramon Nou Date: Fri, 13 May 2022 10:56:09 +0200 Subject: [PATCH 2/4] Test remove of data chunks --- .../integration/operations/test_unlink_operations.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/integration/operations/test_unlink_operations.py b/tests/integration/operations/test_unlink_operations.py index 967c6b791..070c31475 100644 --- a/tests/integration/operations/test_unlink_operations.py +++ b/tests/integration/operations/test_unlink_operations.py @@ -84,4 +84,16 @@ def test_unlink(gkfs_daemon, gkfs_client): assert ret.errno == errno.EISDIR + # create a file in gekkofs + ret = gkfs_client.open(file, + os.O_CREAT | os.O_WRONLY, + stat.S_IRWXU | stat.S_IRWXG | stat.S_IRWXO) + assert ret.retval == 10000 + + # > 4 chunks + ret = gkfs_client.write_validate(file, 2097153) + assert ret.retval == 1 + + ret = gkfs_client.unlink(file) # Remove renamed file (extra chunks, success) + assert ret.retval == 0 -- GitLab From fd55f465fdebf56fd4e67047d61b0e908da6b7f3 Mon Sep 17 00:00:00 2001 From: Marc Vef Date: Tue, 17 May 2022 12:00:31 +0200 Subject: [PATCH 3/4] Fix wrong log verbosity in forward_get_dirents() --- src/client/rpc/forward_metadata.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/rpc/forward_metadata.cpp b/src/client/rpc/forward_metadata.cpp index b18115d32..dd76a638e 100644 --- a/src/client/rpc/forward_metadata.cpp +++ b/src/client/rpc/forward_metadata.cpp @@ -483,7 +483,7 @@ forward_get_dirents(const string& path) { } } - LOG(INFO, + LOG(DEBUG, "{}() path '{}' send rpc_srv_get_dirents() rpc to '{}' targets. per_host_buff_size '{}' Waiting on reply next and deserialize", __func__, path, targets.size(), per_host_buff_size); -- GitLab From cebc72a9a5880c5569ba06b32037537455afb562 Mon Sep 17 00:00:00 2001 From: Marc Vef Date: Tue, 17 May 2022 12:04:30 +0200 Subject: [PATCH 4/4] Adding changelog entry --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f09bff31d..6a7d164c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### New +### Changed +### Removed +### Fixed +- Using `unlink` now fails if it is a directory unless the `AT_REMOVEDIR` flag is used (POSIX compliance) ([!139](https://storage.bsc.es/gitlab/hpc/gekkofs/-/merge_requests/139)). ## [0.9.1] - 2022-04-29 -- GitLab