Fixed a bug that regular files could not be created by mknod

.gitignore

@@ -85,6 +85,7 @@ test/chaos-http-proxy-*
 test/junk_data
 test/s3proxy-*
 test/write_multiblock
+test/mknod_test
 
 #
 # Windows ports

src/fdcache_auto.cpp

@@ -86,16 +86,16 @@ int AutoFdEntity::Detach()
     return fd;
 }
 
-bool AutoFdEntity::Attach(const char* path, int existfd)
+FdEntity* AutoFdEntity::Attach(const char* path, int existfd)
 {
     Close();
 
     if(NULL == (pFdEntity = FdManager::get()->GetFdEntity(path, existfd, false))){
         S3FS_PRN_DBG("Could not find fd entity object(file=%s, pseudo_fd=%d)", path, existfd);
-        return false;
+        return NULL;
     }
     pseudo_fd = existfd;
-    return true;
+    return pFdEntity;
 }
 
 FdEntity* AutoFdEntity::Open(const char* path, headers_t* pmeta, off_t size, time_t time, int flags, bool force_tmpfile, bool is_create, bool ignore_modify, AutoLock::Type type)

src/fdcache_auto.h

@@ -48,7 +48,7 @@ class AutoFdEntity
 
       bool Close();
       int Detach();
-      bool Attach(const char* path, int existfd);
+      FdEntity* Attach(const char* path, int existfd);
       int GetPseudoFd() const { return pseudo_fd; }
 
       FdEntity* Open(const char* path, headers_t* pmeta, off_t size, time_t time, int flags, bool force_tmpfile, bool is_create, bool ignore_modify, AutoLock::Type type);

src/fdcache_entity.cpp

@@ -96,7 +96,7 @@ ino_t FdEntity::GetInode(int fd)
 FdEntity::FdEntity(const char* tpath, const char* cpath) :
     is_lock_init(false), path(SAFESTRPTR(tpath)),
     physical_fd(-1), pfile(NULL), inode(0), size_orgmeta(0),
-    cachepath(SAFESTRPTR(cpath)), is_meta_pending(false)
+    cachepath(SAFESTRPTR(cpath)), pending_status(NO_UPDATE_PENDING)
 {
     holding_mtime.tv_sec = -1;
     holding_mtime.tv_nsec = 0;
@@ -1255,7 +1255,7 @@ int FdEntity::NoCachePreMultipartPost(PseudoFdInfo* pseudo_obj)
     s3fscurl.DestroyCurlHandle();
 
     // Clear the dirty flag, because the meta data is updated.
-    is_meta_pending = false;
+    pending_status = NO_UPDATE_PENDING;
 
     // reset upload_id
     if(!pseudo_obj->InitialUploadInfo(upload_id)){
@@ -1544,15 +1544,15 @@ int FdEntity::RowFlushMultipart(PseudoFdInfo* pseudo_obj, const char* tpath)
             // So the file has already been removed, skip error.
             S3FS_PRN_ERR("failed to truncate file(physical_fd=%d) to zero, but continue...", physical_fd);
         }
-        // put pending headers
+        // put pending headers or create new file
-        if(0 != (result = UploadPendingMeta())){
+        if(0 != (result = UploadPending())){
             return result;
         }
     }
 
     if(0 == result){
         pagelist.ClearAllModified();
-        is_meta_pending = false;
+        pending_status = NO_UPDATE_PENDING;
     }
     return result;
 }
@@ -1672,15 +1672,15 @@ int FdEntity::RowFlushMixMultipart(PseudoFdInfo* pseudo_obj, const char* tpath)
             // So the file has already been removed, skip error.
             S3FS_PRN_ERR("failed to truncate file(physical_fd=%d) to zero, but continue...", physical_fd);
         }
-        // put pending headers
+        // put pending headers or create new file
-        if(0 != (result = UploadPendingMeta())){
+        if(0 != (result = UploadPending())){
             return result;
         }
     }
 
     if(0 == result){
         pagelist.ClearAllModified();
-        is_meta_pending = false;
+        pending_status = NO_UPDATE_PENDING;
     }
     return result;
 }
@@ -2089,29 +2089,51 @@ bool FdEntity::MergeOrgMeta(headers_t& updatemeta)
     if(0 <= atime.tv_sec){
         SetAtime(atime, true);
     }
-    is_meta_pending |= (IsUploading(true) || pagelist.IsModified());
 
-    return is_meta_pending;
+    if(NO_UPDATE_PENDING == pending_status && (IsUploading(true) || pagelist.IsModified())){
+        pending_status = UPDATE_META_PENDING;
+    }
+
+    return (NO_UPDATE_PENDING != pending_status);
 }
 
 // global function in s3fs.cpp
 int put_headers(const char* path, headers_t& meta, bool is_copy, bool use_st_size = true);
 
-int FdEntity::UploadPendingMeta()
+int FdEntity::UploadPending(int fd)
 {
-    if(!is_meta_pending) {
+    int result;
-       return 0;
-    }
 
+    if(NO_UPDATE_PENDING == pending_status){
+       // nothing to do
+       result = 0;
+
+    }else if(UPDATE_META_PENDING == pending_status){
         headers_t updatemeta = orgmeta;
         updatemeta["x-amz-copy-source"]        = urlEncode(service_path + S3fsCred::GetBucket() + get_realpath(path.c_str()));
         updatemeta["x-amz-metadata-directive"] = "REPLACE";
+
         // put headers, no need to update mtime to avoid dead lock
-    int result = put_headers(path.c_str(), updatemeta, true);
+        result = put_headers(path.c_str(), updatemeta, true);
         if(0 != result){
             S3FS_PRN_ERR("failed to put header after flushing file(%s) by(%d).", path.c_str(), result);
+        }else{
+            pending_status = NO_UPDATE_PENDING;
+        }
+
+    }else{  // CREATE_FILE_PENDING == pending_status
+        if(-1 == fd){
+            S3FS_PRN_ERR("could not create a new file(%s), because fd is not specified.", path.c_str());
+            result = -EBADF;
+        }else{
+            result = Flush(fd, true);
+            if(0 != result){
+                S3FS_PRN_ERR("failed to flush for file(%s) by(%d).", path.c_str(), result);
+            }else{
+                pending_status = NO_UPDATE_PENDING;
+            }
+        }
     }
-    is_meta_pending = false;
     return result;
 }
 
@@ -2194,7 +2216,7 @@ bool FdEntity::PunchHole(off_t start, size_t size)
 void FdEntity::MarkDirtyNewFile()
 {
     pagelist.Init(0, false, true);
-    is_meta_pending = true;
+    pending_status = CREATE_FILE_PENDING;
 }
 
 /*

src/fdcache_entity.h

@@ -32,6 +32,17 @@
 class FdEntity
 {
     private:
+        // [NOTE]
+        // Distinguish between meta pending and new file creation pending,
+        // because the processing(request) at these updates is different.
+        // Therefore, the pending state is expressed by this enum type.
+        //
+        enum pending_status_t {
+            NO_UPDATE_PENDING = 0,
+            UPDATE_META_PENDING,        // pending meta header
+            CREATE_FILE_PENDING         // pending file creation and meta header
+        };
+
         static bool     mixmultipart;   // whether multipart uploading can use copy api.
 
         pthread_mutex_t fdent_lock;
@@ -49,7 +60,7 @@ class FdEntity
         std::string     cachepath;      // local cache file path
                                         // (if this is empty, does not load/save pagelist.)
         std::string     mirrorpath;     // mirror file path to local cache file path
-        bool            is_meta_pending;
+        pending_status_t pending_status;// status for new file creation and meta update
         struct timespec holding_mtime;  // if mtime is updated while the file is open, it is set time_t value
 
     private:
@@ -73,7 +84,6 @@ class FdEntity
         ssize_t WriteNoMultipart(PseudoFdInfo* pseudo_obj, const char* bytes, off_t start, size_t size);
         ssize_t WriteMultipart(PseudoFdInfo* pseudo_obj, const char* bytes, off_t start, size_t size);
         ssize_t WriteMixMultipart(PseudoFdInfo* pseudo_obj, const char* bytes, off_t start, size_t size);
-        int UploadPendingMeta();
 
     public:
         static bool GetNoMixMultipart() { return mixmultipart; }
@@ -95,6 +105,7 @@ class FdEntity
         int GetPhysicalFd() const { return physical_fd; }
         bool IsModified() const;
         bool MergeOrgMeta(headers_t& updatemeta);
+        int UploadPending(int fd = -1);
 
         bool GetStats(struct stat& st, bool lock_already_held = false);
         int SetCtime(struct timespec time, bool lock_already_held = false);

src/s3fs.cpp

@@ -2488,10 +2488,17 @@ static int s3fs_release(const char* _path, struct fuse_file_info* fi)
         // The pseudo fd stored in fi->fh is attached to AutoFdEntry so that it can be
         // destroyed here.
         //
-        if(!autoent.Attach(path, static_cast<int>(fi->fh))){
+        FdEntity* ent;
+        if(NULL == (ent = autoent.Attach(path, static_cast<int>(fi->fh)))){
             S3FS_PRN_ERR("could not find pseudo_fd(%llu) for path(%s)", (unsigned long long)(fi->fh), path);
             return -EIO;
         }
+
+        int result = ent->UploadPending(static_cast<int>(fi->fh));
+        if(0 != result){
+            S3FS_PRN_ERR("could not upload pending data(meta, etc) for pseudo_fd(%llu) / path(%s)", (unsigned long long)(fi->fh), path);
+            return result;
+        }
     }
 
     // check - for debug

test/Makefile.am

@@ -31,10 +31,12 @@ testdir = test
 
 noinst_PROGRAMS = \
     junk_data \
-    write_multiblock
+    write_multiblock \
+    mknod_test
 
 junk_data_SOURCES = junk_data.c
 write_multiblock_SOURCES = write_multiblock.cc
+mknod_test_SOURCES = mknod_test.c
 
 #
 # Local variables:

test/integration-test-main.sh

@@ -749,6 +749,16 @@ function test_hardlink {
     rm_test_file "${ALT_TEST_TEXT_FILE}"
 }
 
+function test_mknod {
+    describe "Testing mknod system call function ..."
+
+    local MKNOD_TEST_FILE_BASENAME="mknod_testfile"
+
+    rm -f "${MKNOD_TEST_FILE_BASENAME}*"
+
+    ../../mknod_test "${MKNOD_TEST_FILE_BASENAME}"
+}
+
 function test_symlink {
     describe "Testing symlinks ..."
 
@@ -1894,6 +1904,9 @@ function add_all_tests {
     add_tests test_special_characters
     add_tests test_hardlink
     add_tests test_symlink
+    if ! uname | grep -q Darwin; then
+        add_tests test_mknod
+    fi
     add_tests test_extended_attributes
     add_tests test_mtime_file
 

test/mknod_test.c

@@ -0,0 +1,177 @@
+/*
+ * s3fs - FUSE-based file system backed by Amazon S3
+ *
+ * Copyright(C) 2021 Andrew Gaul <andrew@gaul.org>
+ *
+ * This program 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 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program 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 this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <stdbool.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <string.h>
+#include <errno.h>
+#ifndef __APPLE__
+#include <sys/sysmacros.h>
+#endif
+
+//---------------------------------------------------------
+// Const
+//---------------------------------------------------------
+const char usage_string[]  = "Usage : \"mknod_test <base file path>\"";
+
+const char str_mode_reg[]  = "REGULAR";
+const char str_mode_chr[]  = "CHARACTER";
+const char str_mode_blk[]  = "BLOCK";
+const char str_mode_fifo[] = "FIFO";
+const char str_mode_sock[] = "SOCK";
+
+const char str_ext_reg[]   = "reg";
+const char str_ext_chr[]   = "chr";
+const char str_ext_blk[]   = "blk";
+const char str_ext_fifo[]  = "fifo";
+const char str_ext_sock[]  = "sock";
+
+// [NOTE]
+// It would be nice if PATH_MAX could be used as is, but since there are
+// issues using on Linux and we also must support for macos, this simple
+// test program defines a fixed value for simplicity.
+//
+#define S3FS_TEST_PATH_MAX   255
+int max_base_path_length   = S3FS_TEST_PATH_MAX - 5;
+
+//---------------------------------------------------------
+// Test function
+//---------------------------------------------------------
+bool TestMknod(const char* basepath, mode_t mode)
+{
+    if(!basepath){
+        fprintf(stderr, "[ERROR] Called function with wrong basepath argument.\n");
+        return false;
+    }
+
+    const char* str_mode;
+    dev_t       dev;
+    char        filepath[S3FS_TEST_PATH_MAX];
+    switch(mode){
+        case S_IFREG:
+            str_mode = str_mode_reg;
+            dev      = 0;
+            sprintf(filepath, "%s.%s", basepath, str_ext_reg);
+            break;
+        case S_IFCHR:
+            str_mode = str_mode_chr;
+            dev      = makedev(0, 0);
+            sprintf(filepath, "%s.%s", basepath, str_ext_chr);
+            break;
+        case S_IFBLK:
+            str_mode = str_mode_blk;
+            dev      = makedev((long long)(259), 0);     // temporary value
+            sprintf(filepath, "%s.%s", basepath, str_ext_blk);
+            break;
+        case S_IFIFO:
+            str_mode = str_mode_fifo;
+            dev      = 0;
+            sprintf(filepath, "%s.%s", basepath, str_ext_fifo);
+            break;
+        case S_IFSOCK:
+            str_mode = str_mode_sock;
+            dev      = 0;
+            sprintf(filepath, "%s.%s", basepath, str_ext_sock);
+            break;
+        default:
+            fprintf(stderr, "[ERROR] Called function with wrong mode argument.\n");
+            return false;
+    }
+
+    //
+    // Create
+    //
+    if(0 != mknod(filepath, mode | S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH, dev)){
+        fprintf(stderr, "[ERROR] Could not create %s file(%s) : errno = %d\n", str_mode, filepath, errno);
+        return false;
+    }
+
+    //
+    // Check
+    //
+    struct stat st;
+    if(0 != stat(filepath, &st)){
+        fprintf(stderr, "[ERROR] Could not get stat from %s file(%s) : errno = %d\n", str_mode, filepath, errno);
+        return false;
+    }
+    if(mode != (st.st_mode & S_IFMT)){
+        fprintf(stderr, "[ERROR] Created %s file(%s) does not have 0%o stat\n", str_mode, filepath, mode);
+        return false;
+    }
+
+    //
+    // Remove
+    //
+    if(0 != unlink(filepath)){
+        fprintf(stderr, "[WARNING] Could not remove %s file(%s) : errno = %d\n", str_mode, filepath, mode);
+    }
+    return true;
+}
+
+//---------------------------------------------------------
+// Main
+//---------------------------------------------------------
+int main(int argc, char *argv[])
+{
+    // Parse parameters
+    if(2 != argc){
+        fprintf(stderr, "[ERROR] No paraemter is specified.\n");
+        fprintf(stderr, "%s\n", usage_string);
+        exit(EXIT_FAILURE);
+    }
+    if(0 == strcasecmp("-h", argv[1]) || 0 == strcasecmp("--help", argv[1])){
+        fprintf(stdout, "%s\n", usage_string);
+        exit(EXIT_SUCCESS);
+    }
+    if(max_base_path_length < strlen(argv[1])){
+        fprintf(stderr, "[ERROR] Base file path is too long, it must be less than %d\n", max_base_path_length);
+        exit(EXIT_FAILURE);
+    }
+
+    // Test
+    //
+    // [NOTE]
+    // Privilege is required to execute S_IFBLK.
+    //
+    if(!TestMknod(argv[1], S_IFREG)  ||
+       !TestMknod(argv[1], S_IFCHR)  ||
+       !TestMknod(argv[1], S_IFIFO)  ||
+       !TestMknod(argv[1], S_IFSOCK) ||
+       (0 == geteuid() && !TestMknod(argv[1], S_IFBLK)))
+    {
+        exit(EXIT_FAILURE);
+    }
+
+    exit(EXIT_SUCCESS);
+}
+
+/*
+* Local variables:
+* tab-width: 4
+* c-basic-offset: 4
+* End:
+* vim600: expandtab sw=4 ts=4 fdm=marker
+* vim<600: expandtab sw=4 ts=4
+*/