Merge branch 'rnou/215-unlink-gkfs_remove-can-delete-directories' into 'master'

Resolve "unlink (gkfs_remove) can delete directories"

Closes #215

See merge request !139

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
 

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;

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);
 

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)

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

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);
 }
 
 

tests/integration/harness/gkfs.io/unlink.cpp

+/*
+  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 <https://www.gnu.org/licenses/>.
+
+  SPDX-License-Identifier: GPL-3.0-or-later
+*/
+
+/* C++ includes */
+#include <CLI11/CLI11.hpp>
+#include <nlohmann/json.hpp>
+#include <memory>
+#include <fmt/format.h>
+#include <commands.hpp>
+#include <reflection.hpp>
+#include <serialize.hpp>
+#include <binary_buffer.hpp>
+
+/* C includes */
+#include <unistd.h>
+
+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<unlink_options>();
+    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); });
+}

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(),

tests/integration/operations/test_unlink_operations.py

+################################################################################
+# 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 <https://www.gnu.org/licenses/>.            #
+#                                                                              #
+# 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
+    
+
+    # 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