From 33fc26f81fca00646e827cae73365303082fd2e7 Mon Sep 17 00:00:00 2001 From: trapexit Date: Sun, 27 Jul 2025 16:23:45 -0500 Subject: [PATCH] Cleanup copy functions (#1500) --- src/fs_copydata.cpp | 13 ++-- src/fs_copydata.hpp | 10 ++-- src/fs_copydata_copy_file_range.cpp | 29 +++++---- src/fs_copydata_readwrite.cpp | 93 +++++++++++------------------ src/fs_copyfile.cpp | 74 +++++++++++++---------- src/fs_copyfile.hpp | 21 +++++-- src/fs_cow.cpp | 2 +- src/fs_movefile_and_open.cpp | 2 +- src/fs_pread.hpp | 13 ++-- src/fs_preadn.hpp | 44 ++++++++------ src/fs_pwriten.hpp | 16 +++-- src/mergerfs_fsck.cpp | 2 +- 12 files changed, 166 insertions(+), 153 deletions(-) diff --git a/src/fs_copydata.cpp b/src/fs_copydata.cpp index 7db437f5..8f35aec4 100644 --- a/src/fs_copydata.cpp +++ b/src/fs_copydata.cpp @@ -25,17 +25,16 @@ #include - -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_); diff --git a/src/fs_copydata.hpp b/src/fs_copydata.hpp index 4347bcb6..44100353 100644 --- a/src/fs_copydata.hpp +++ b/src/fs_copydata.hpp @@ -18,13 +18,13 @@ #pragma once -#include +#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); } diff --git a/src/fs_copydata_copy_file_range.cpp b/src/fs_copydata_copy_file_range.cpp index 04b163a6..e0d829d7 100644 --- a/src/fs_copydata_copy_file_range.cpp +++ b/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 diff --git a/src/fs_copydata_readwrite.cpp b/src/fs_copydata_readwrite.cpp index 19f346b8..7cf5031b 100644 --- a/src/fs_copydata_readwrite.cpp +++ b/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 -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 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); } diff --git a/src/fs_copyfile.cpp b/src/fs_copyfile.cpp index 83046827..fc36388c 100644 --- a/src/fs_copyfile.cpp +++ b/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_); +} diff --git a/src/fs_copyfile.hpp b/src/fs_copyfile.hpp index 5f7be3a3..75bdd168 100644 --- a/src/fs_copyfile.hpp +++ b/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); } diff --git a/src/fs_cow.cpp b/src/fs_cow.cpp index 26f20e6a..62f14cb8 100644 --- a/src/fs_cow.cpp +++ b/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 }); } diff --git a/src/fs_movefile_and_open.cpp b/src/fs_movefile_and_open.cpp index aabac173..bdc93046 100644 --- a/src/fs_movefile_and_open.cpp +++ b/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; diff --git a/src/fs_pread.hpp b/src/fs_pread.hpp index 7f7e64a5..a6407893 100644 --- a/src/fs_pread.hpp +++ b/src/fs_pread.hpp @@ -18,6 +18,7 @@ #pragma once +#include "int_types.h" #include "to_neg_errno.hpp" #include @@ -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_); diff --git a/src/fs_preadn.hpp b/src/fs_preadn.hpp index 1e964a3f..f9a7dd4e 100644 --- a/src/fs_preadn.hpp +++ b/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; } diff --git a/src/fs_pwriten.hpp b/src/fs_pwriten.hpp index 0e00b8e8..faaa606f 100644 --- a/src/fs_pwriten.hpp +++ b/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; diff --git a/src/mergerfs_fsck.cpp b/src/mergerfs_fsck.cpp index e2558dc7..e081d4ea 100644 --- a/src/mergerfs_fsck.cpp +++ b/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 {} - {}",