From 1f1e48107533bd589cb2a1eebe3475636de6d1e5 Mon Sep 17 00:00:00 2001 From: Antonio SJ Musumeci Date: Sun, 30 Aug 2015 17:10:15 -0400 Subject: [PATCH] rework rename Return EXDEV if directories of tragets differ. If the old and new path exist in the same directory first rename each old found by the policy and then unlink/rmdir any new on a drive which didn't rename. The unlink/rmdir will occur only if there were no rename errors. Any failures of unlink/rmdir are ignored. The last rename error is returned. --- README.md | 4 +++ src/fs.cpp | 8 ++--- src/fs.hpp | 9 ++++++ src/rename.cpp | 85 ++++++++++++++++++++++++++++++-------------------- 4 files changed, 68 insertions(+), 38 deletions(-) diff --git a/README.md b/README.md index 7a61cc33..0711e949 100644 --- a/README.md +++ b/README.md @@ -83,6 +83,10 @@ Filesystem calls are broken up into 3 categories: **action**, **create**, **sear | create | epmfs | | search | ff | +#### rename #### + +[rename](http://man7.org/linux/man-pages/man2/rename.2.html) is a tricky function in a merged system. Normally if a rename can't be done atomically due to the from and to paths existing on different mount points it will return `-1` with `errno = EXDEV`. The atomic rename is most critical for replacing files in place atomically (such as securing writing to a temp file and then replacing a target). The problem is that by merging multiple paths you can have N instances of the source and destinations on different drives. Meaning that if you just renamed each source locally you could end up with the destination files not overwriten / replaced. To address this mergerfs works in the following way. If the source and destination exist in different directories it will immediately return `EXDEV`. Generally it's not expected for cross directory renames to work so it should be fine for most instances (mv,rsync,etc.). If they do belong to the same directory it then runs the `rename` policy to get the files to rename. It iterates through and renames each file while keeping track of those paths which have not been renamed. If all the renames succeed it will then `unlink` or `rmdir` the other paths to clean up any preexisting target files. This allows the new file to be found without the file itself ever disappearing. There may still be some issues with this behavior. Particularly on error. At the moment however this seems the best policy. + #### readdir #### [readdir](http://linux.die.net/man/3/readdir) is very different from most functions in this realm. It certainly could have it's own set of policies to tweak its behavior. At this time it provides a simple **first found** merging of directories and file found. That is: only the first file or directory found for a directory is returned. Given how FUSE works though the data representing the returned entry comes from **getattr**. diff --git a/src/fs.cpp b/src/fs.cpp index a6dab1d1..879276b0 100644 --- a/src/fs.cpp +++ b/src/fs.cpp @@ -133,7 +133,7 @@ namespace fs string path; struct stat st; - path = fs::path::make(paths[i],fusepath); + fs::path::make(paths[i],fusepath,path); rv = ::lstat(path.c_str(),&st); if(rv == 0) @@ -155,7 +155,7 @@ namespace fs string fullpath; struct stat st; - fullpath = fs::path::make(srcmounts[i],fusepath); + fs::path::make(srcmounts[i],fusepath,fullpath); rv = ::lstat(fullpath.c_str(),&st); if(rv == 0) @@ -456,14 +456,14 @@ namespace fs return -1; } - frompath = fs::path::make(fromsrc,relative); + fs::path::make(fromsrc,relative,frompath); rv = ::stat(frompath.c_str(),&st); if(rv == -1) return -1; else if(!S_ISDIR(st.st_mode)) return (errno = ENOTDIR,-1); - topath = fs::path::make(tosrc,relative); + fs::path::make(tosrc,relative,topath); rv = ::mkdir(topath.c_str(),st.st_mode); if(rv == -1) { diff --git a/src/fs.hpp b/src/fs.hpp index f6cfa915..6df000d2 100644 --- a/src/fs.hpp +++ b/src/fs.hpp @@ -59,6 +59,15 @@ namespace fs return base + suffix; } + inline + void + make(const string &base, + const string &suffix, + string &output) + { + output = base + suffix; + } + inline void append(string &base, diff --git a/src/rename.cpp b/src/rename.cpp index 7eee0f67..265189fe 100644 --- a/src/rename.cpp +++ b/src/rename.cpp @@ -30,6 +30,7 @@ #include #include +#include #include "ugid.hpp" #include "fs.hpp" @@ -38,40 +39,65 @@ using std::string; using std::vector; +using std::set; using mergerfs::Policy; static -int -_single_rename(Policy::Func::Search searchFunc, - const vector &srcmounts, - const size_t minfreespace, - const string &oldbasepath, - const string &oldfullpath, - const string &newpath) +bool +_different_dirname(const string &path0, + const string &path1) +{ + return (fs::path::dirname(path0) != fs::path::dirname(path1)); +} + +static +void +_unlink(const set &tounlink, + const string &newfusepath) { int rv; - const string newfullpath = fs::path::make(oldbasepath,newpath); + string fullpath; - rv = ::rename(oldfullpath.c_str(),newfullpath.c_str()); - if(rv == -1 && errno == ENOENT) + for(set::const_iterator i = tounlink.begin(), ei = tounlink.end(); i != ei; i++) { - string dirname; - vector newpathdir; + fs::path::make(*i,newfusepath,fullpath); - dirname = fs::path::dirname(newpath); - rv = searchFunc(srcmounts,dirname,minfreespace,newpathdir); - if(rv == -1) - return -1; + rv = ::unlink(fullpath.c_str()); + if(rv == -1 && errno == EISDIR) + ::rmdir(fullpath.c_str()); + } +} + +static +int +_rename(const vector &srcmounts, + const vector &basepaths, + const string &oldfusepath, + const string &newfusepath) +{ + int rv; + int error; + string oldfullpath; + string newfullpath; + set tounlink; - { - const mergerfs::ugid::SetResetGuard ugid(0,0); - fs::clonepath(newpathdir[0],oldbasepath,dirname); - } + error = 0; + tounlink.insert(srcmounts.begin(),srcmounts.end()); + for(size_t i = 0, ei = basepaths.size(); i != ei; i++) + { + fs::path::make(basepaths[i],oldfusepath,oldfullpath); + fs::path::make(basepaths[i],newfusepath,newfullpath); + tounlink.erase(basepaths[i]); rv = ::rename(oldfullpath.c_str(),newfullpath.c_str()); + if(rv == -1) + error = errno; } - return rv; + if(error == 0) + _unlink(tounlink,newfusepath); + + return -error; } static @@ -84,25 +110,16 @@ _rename(Policy::Func::Search searchFunc, const string &newfusepath) { int rv; - int error; vector oldbasepaths; + if(_different_dirname(oldfusepath,newfusepath)) + return -EXDEV; + rv = actionFunc(srcmounts,oldfusepath,minfreespace,oldbasepaths); if(rv == -1) return -errno; - error = 0; - for(size_t i = 0, ei = oldbasepaths.size(); i != ei; i++) - { - const string oldfullpath = fs::path::make(oldbasepaths[i],oldfusepath); - - rv = _single_rename(searchFunc,srcmounts,minfreespace, - oldbasepaths[i],oldfullpath,newfusepath); - if(rv == -1) - error = errno; - } - - return -error; + return _rename(srcmounts,oldbasepaths,oldfusepath,newfusepath); } namespace mergerfs