Cleanup copy functions (#1500)

src/fs_copydata.cpp

@@ -25,17 +25,16 @@
 
 #include <stddef.h>
 
-
-int
-fs::copydata(const int    src_fd_,
-             const int    dst_fd_,
-             const size_t count_)
+s64
+fs::copydata(const int src_fd_,
+             const int dst_fd_,
+             const u64 count_)
 {
-  int rv;
+  s64 rv;
 
   rv = fs::ficlone(src_fd_,dst_fd_);
   if(rv >= 0)
-    return rv;
+    return count_;
 
   fs::fadvise_willneed(src_fd_,0,count_);
   fs::fadvise_sequential(src_fd_,0,count_);

src/fs_copydata.hpp

@@ -18,13 +18,13 @@
 
 #pragma once
 
-#include <stddef.h>
+#include "int_types.h"
 
 
 namespace fs
 {
-  int
-  copydata(const int    src_fd,
-           const int    dst_fd,
-           const size_t count);
+  s64
+  copydata(const int src_fd,
+           const int dst_fd,
+           const u64 count);
 }

src/fs_copydata_copy_file_range.cpp

@@ -27,7 +27,7 @@ static
 s64
 _copydata_copy_file_range(const int src_fd_,
                           const int dst_fd_,
-                          const u64 size_)
+                          const u64 count_)
 {
   s64 rv;
   u64 nleft;
@@ -36,24 +36,27 @@ _copydata_copy_file_range(const int src_fd_,
 
   src_off = 0;
   dst_off = 0;
-  nleft   = size_;
-  do
+  nleft   = count_;
+  while(nleft > 0)
     {
       rv = fs::copy_file_range(src_fd_,&src_off,dst_fd_,&dst_off,nleft,0);
-      if(rv == -EINTR)
-        continue;
-      if(rv == -EAGAIN)
-        continue;
-      if(rv == 0)
-        break;
-      if(rv < 0)
-        return rv;
+      switch(rv)
+        {
+        case -EINTR:
+        case -EAGAIN:
+          continue;
+        case 0:
+          break;
+        default:
+          if(rv < 0)
+            return rv;
+          break;
+        }
 
       nleft -= rv;
     }
-  while(nleft > 0);
 
-  return (size_ - nleft);
+  return (count_ - nleft);
 }
 
 s64

src/fs_copydata_readwrite.cpp

@@ -17,82 +17,58 @@
 #include "fs_copydata_readwrite.hpp"
 
 #include "errno.hpp"
-#include "fs_fstat.hpp"
-#include "fs_lseek.hpp"
-#include "fs_read.hpp"
-#include "fs_write.hpp"
+#include "fs_pread.hpp"
+#include "fs_pwriten.hpp"
 
 #include <vector>
 
 
-static
-s64
-_writen(const int   fd_,
-        const char *buf_,
-        const u64   size_)
-{
-  s64 rv;
-  u64 nleft;
-
-  nleft = size_;
-  do
-    {
-      rv = fs::write(fd_,buf_,nleft);
-      if(rv == -EINTR)
-        continue;
-      if(rv == -EAGAIN)
-        continue;
-      if(rv == 0)
-        break;
-      if(rv < 0)
-        return rv;
-
-      nleft -= rv;
-      buf_  += rv;
-    }
-  while(nleft > 0);
-
-  return (size_ - nleft);
-}
-
 static
 s64
 _copydata_readwrite(const int src_fd_,
                     const int dst_fd_,
-                    const u64 count_)
+                    const u64 count_,
+                    const u64 bufsize_)
 {
+  int err;
   s64 nr;
   s64 nw;
-  s64 bufsize;
-  u64 totalwritten;
+  s64 offset;
   std::vector<char> buf;
 
-  bufsize = (1024 * 1024);
-  buf.resize(bufsize);
+  if(bufsize_ == 0)
+    return -EINVAL;
 
-  totalwritten = 0;
-  while(totalwritten < count_)
-    {
-      nr = fs::read(src_fd_,&buf[0],bufsize);
-      if(nr == 0)
-        return totalwritten;
-      if(nr == -EINTR)
-        continue;
-      if(nr == -EAGAIN)
-        continue;
-      if(nr < 0)
-        return nr;
-
-      nw = ::_writen(dst_fd_,&buf[0],nr);
-      if(nw < 0)
-        return nw;
+  buf.resize(bufsize_);
 
-      totalwritten += nw;
+  offset = 0;
+  while(offset < (s64)count_)
+    {
+      nr = fs::pread(src_fd_,&buf[0],buf.size(),offset);
+      switch(nr)
+        {
+        case -EINTR:
+        case -EAGAIN:
+          continue;
+        case 0:
+          return offset;
+        default:
+          if(nr < 0)
+            return nr;
+          break;
+        }
+
+      nw = fs::pwriten(dst_fd_,&buf[0],nr,offset,&err);
+      if(err < 0)
+        return err;
+
+      offset += nw;
     }
 
-  return totalwritten;
+  return offset;
 }
 
+#define BUF_SIZE (256 * 1024)
 
 s64
 fs::copydata_readwrite(const int src_fd_,
@@ -101,5 +77,6 @@ fs::copydata_readwrite(const int src_fd_,
 {
   return ::_copydata_readwrite(src_fd_,
                                dst_fd_,
-                               count_);
+                               count_,
+                               BUF_SIZE);
 }

src/fs_copyfile.cpp

@@ -41,7 +41,7 @@
 
 static
 bool
-_ignorable_error(const int err_)
+_ignorable_error(const s64 err_)
 {
   switch(err_)
     {
@@ -56,12 +56,12 @@ _ignorable_error(const int err_)
   return false;
 }
 
-int
+s64
 fs::copyfile(const int          src_fd_,
              const struct stat &src_st_,
              const int          dst_fd_)
 {
-  int rv;
+  s64 rv;
 
   rv = fs::copydata(src_fd_,dst_fd_,src_st_.st_size);
   if(rv < 0)
@@ -90,21 +90,17 @@ fs::copyfile(const int          src_fd_,
   return 0;
 }
 
-
-
-
 // Limitations:
 // * Doesn't handle immutable files well. Will ignore the flag on attr
 // copy.
 // * Does not handle non-regular files.
 
-int
-fs::copyfile(const std::filesystem::path &src_,
-             const std::filesystem::path &dst_,
-             const u32                    flags_)
+s64
+fs::copyfile(const int                    src_fd_,
+             const std::filesystem::path &dst_filepath_,
+             const fs::CopyFileFlags     &flags_)
 {
-  int rv;
-  int src_fd;
+  s64 rv;
   int dst_fd;
   struct stat src_st = {0};
   std::string dst_tmppath;
@@ -118,14 +114,19 @@ fs::copyfile(const std::filesystem::path &src_,
   sigaction(SIGIO,&new_act,&old_act);
   DEFER { sigaction(SIGIO,&old_act,NULL); };
 
-  src_fd = fs::open(src_,O_RDONLY|O_NOFOLLOW);
-  if(src_fd < 0)
-    return src_fd;
-  DEFER { fs::close(src_fd); };
-
   while(true)
     {
-      std::tie(dst_fd,dst_tmppath) = fs::mktemp(dst_,O_RDWR);
+      // For comparison after the copy to see if the file was
+      // modified. This could be made more thorough by adding some
+      // hashing but probably overkill. Also used to provide size and
+      // other details for data copying.
+      rv = fs::fstat(src_fd_,&src_st);
+      if(rv < 0)
+        return rv;
+      if(!S_ISREG(src_st.st_mode))
+        return -EINVAL;
+
+      std::tie(dst_fd,dst_tmppath) = fs::mktemp(dst_filepath_,O_RDWR);
       if(dst_fd < 0)
         return dst_fd;
       DEFER { fs::close(dst_fd); };
@@ -135,37 +136,44 @@ fs::copyfile(const std::filesystem::path &src_,
       // copy happens. Opening read-only is fine but open for write or
       // truncate will block for others till this finishes or the
       // kernel wide timeout (/proc/sys/fs/lease-break-time).
-      fs::fcntl_setlease_rdlck(src_fd);
-      DEFER { fs::fcntl_setlease_unlck(src_fd); };
+      fs::fcntl_setlease_rdlck(src_fd_);
+      DEFER { fs::fcntl_setlease_unlck(src_fd_); };
 
-      // For comparison after the copy to see if the file was
-      // modified. This could be made more thorough by adding some
-      // hashing but probably overkill. Also used to provide size and
-      // other details for data copying.
-      rv = fs::fstat(src_fd,&src_st);
-      if(rv < 0)
-        return rv;
-
-      rv = fs::copyfile(src_fd,src_st,dst_fd);
+      rv = fs::copyfile(src_fd_,src_st,dst_fd);
       if(rv < 0)
         {
-          if(flags_ & FS_COPYFILE_CLEANUP_FAILURE)
+          if(flags_.cleanup_failure)
             fs::unlink(dst_tmppath);
           return rv;
         }
 
-      rv = fs::file_changed(src_fd,src_st);
+      rv = fs::file_changed(src_fd_,src_st);
       if(rv == FS_FILE_CHANGED)
         {
           fs::unlink(dst_tmppath);
           continue;
         }
 
-      rv = fs::rename(dst_tmppath,dst_);
-      if((rv < 0) && (flags_ & FS_COPYFILE_CLEANUP_FAILURE))
+      rv = fs::rename(dst_tmppath,dst_filepath_);
+      if((rv < 0) && (flags_.cleanup_failure))
         fs::unlink(dst_tmppath);
       break;
     }
 
   return rv;
 }
+
+s64
+fs::copyfile(const std::filesystem::path &src_filepath_,
+             const std::filesystem::path &dst_filepath_,
+             const fs::CopyFileFlags     &flags_)
+{
+  int src_fd;
+
+  src_fd = fs::open(src_filepath_,O_RDONLY|O_NOFOLLOW|O_NONBLOCK|O_NOATIME);
+  if(src_fd < 0)
+    return src_fd;
+  DEFER { fs::close(src_fd); };
+
+  return fs::copyfile(src_fd,dst_filepath_,flags_);
+}

src/fs_copyfile.hpp

@@ -9,16 +9,25 @@
 #define FS_COPYFILE_NONE (0)
 #define FS_COPYFILE_CLEANUP_FAILURE (1 << 0)
 
-
 namespace fs
 {
-  int
+  struct CopyFileFlags
+  {
+    int cleanup_failure:1;
+  };
+
+  s64
   copyfile(const int          src_fd,
            const struct stat &src_st,
            const int          dst_fd);
 
-  int
-  copyfile(const std::filesystem::path &src,
-           const std::filesystem::path &dst,
-           const u32                    flags);
+  s64
+  copyfile(const int                    src_fd,
+           const std::filesystem::path &dst_filepath,
+           const CopyFileFlags         &flags);
+
+  s64
+  copyfile(const std::filesystem::path &src_filepath,
+           const std::filesystem::path &dst_filepath,
+           const CopyFileFlags         &flags);
 }

src/fs_cow.cpp

@@ -74,5 +74,5 @@ fs::cow::break_link(const char *src_filepath_)
 {
   return fs::copyfile(src_filepath_,
                       src_filepath_,
-                      FS_COPYFILE_CLEANUP_FAILURE);
+                      { .cleanup_failure = true });
 }

src/fs_movefile_and_open.cpp

@@ -102,7 +102,7 @@ _movefile_and_open(const Policy::Create &createFunc_,
   src_filepath = fs::path::make(src_branch,fusepath_);
   dst_filepath = fs::path::make(dst_branch[0]->path,fusepath_);
 
-  rv = fs::copyfile(src_filepath,dst_filepath,FS_COPYFILE_CLEANUP_FAILURE);
+  rv = fs::copyfile(src_filepath,dst_filepath,{.cleanup_failure = true});
   if(rv < 0)
     return -ENOSPC;
 

src/fs_pread.hpp

@@ -18,6 +18,7 @@
 
 #pragma once
 
+#include "int_types.h"
 #include "to_neg_errno.hpp"
 
 #include <unistd.h>
@@ -25,13 +26,13 @@
 namespace fs
 {
   static
-  int
-  pread(const int     fd_,
-        void         *buf_,
-        const size_t  count_,
-        const off_t   offset_)
+  s64
+  pread(const int  fd_,
+        void      *buf_,
+        const u64  count_,
+        const s64  offset_)
   {
-    int rv;
+    s64 rv;
 
     rv = ::pread(fd_,buf_,count_,offset_);
 

src/fs_preadn.hpp

@@ -25,32 +25,40 @@ namespace fs
 {
   static
   inline
-  ssize_t
-  preadn(int const     fd_,
-         void         *buf_,
-         size_t const  count_,
-         off_t const   offset_,
-         int          *err_)
+  s64
+  preadn(const int  fd_,
+         void      *buf_,
+         const u64  count_,
+         const s64  offset_,
+         int       *err_)
   {
-    ssize_t rv;
-    ssize_t count = count_;
-    off_t   offset = offset_;
-    char const *buf = (char const *)buf_;
+    s64 rv;
+    s64 nleft = count_;
+    s64 offset = offset_;
+    const char *buf = (const char*)buf_;
 
     *err_ = 0;
-    while(count > 0)
+    while(nleft > 0)
       {
-        rv = fs::pread(fd_,buf,count,offset);
-        if(rv == 0)
-          return (count_ - count);
-        if(rv < 0)
+        rv = fs::pread(fd_,buf,nleft,offset);
+        switch(rv)
           {
-            *err_ = rv;
-            return (count_ - count);
+          case -EINTR:
+          case -EAGAIN:
+            continue;
+          case 0:
+            return (count_ - nleft);
+          default:
+            if(rv < 0)
+              {
+                *err_ = rv;
+                return (count_ - nleft);
+              }
+            break;
           }
 
         buf    += rv;
-        count  -= rv;
+        nleft  -= rv;
         offset += rv;
       }
 

src/fs_pwriten.hpp

@@ -41,12 +41,20 @@ namespace fs
     while(count > 0)
       {
         rv = fs::pwrite(fd_,buf,count,offset);
-        if(rv == 0)
-          return (count_ - count);
-        if(rv < 0)
+        switch(rv)
           {
-            *err_ = rv;
+          case -EINTR:
+          case -EAGAIN:
+            continue;
+          case 0:
             return (count_ - count);
+          default:
+            if(rv < 0)
+              {
+                *err_ = rv;
+                return (count_ - count);
+              }
+            break;
           }
 
         buf    += rv;

src/mergerfs_fsck.cpp

@@ -229,7 +229,7 @@ _copy_files(const std::string &src_,
       if(src_ == dst.path)
         continue;
 
-      rv = fs::copyfile(src_,dst.path,FS_COPYFILE_NONE);
+      rv = fs::copyfile(src_,dst.path,{.cleanup_failure=false});
       if(rv < 0)
         fmt::print(stderr,
                    "ERROR: failed copying to {} - {}",