From a3e6a0352d7fc0388fde20358075fa2d405a5efc Mon Sep 17 00:00:00 2001 From: Antonio SJ Musumeci Date: Tue, 15 Dec 2015 23:52:09 -0500 Subject: [PATCH] rework rename algo to minimize likelihood of EXDEV being returned. closes #187 --- README.md | 36 +++++- src/rename.cpp | 303 ++++++++++++++++++++++++++++++++++++++----------- 2 files changed, 267 insertions(+), 72 deletions(-) diff --git a/README.md b/README.md index 83c5dd51..3e1a3a26 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ % mergerfs(1) mergerfs user manual % Antonio SJ Musumeci -% 2015-10-29 +% 2016-01-12 # NAME @@ -99,7 +99,37 @@ Filesystem calls are broken up into 3 categories: **action**, **create**, **sear #### 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. +[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 source and destination 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. This can lead to several undesirable situtations with or without errors and it's not entirely obvious what to do when an error occurs. + +Originally mergerfs would return EXDEV whenever a rename was requested which was cross directory in any way. This made the code simple and was technically complient with POSIX requirements. However, many applications fail to handle EXDEV at all and treat it as a normal error or they only partially support EXDEV (don't respond the same as `mv` would). Such apps include: gvfsd-fuse v1.20.3 and prior, Finder / CIFS/SMB client in Apple OSX 10.9+, NZBGet, Samba's recycling bin feature. + +* If using a policy which tries to preserve directories (epmfs) + * Using the `rename` policy get the list of files to rename + * For each file attempt rename: + * If failure with ENOENT run `create` policy + * If create policy returns the same drive as currently evaluating then clone the path + * Re-attempt rename + * If **any** of the renames succeed the higher level rename is considered a success + * If **no** renames succeed the first error encountered will be returned + * On success: + * Remove the target from all drives with no source file + * Remove the source from all drives which failed to rename +* If using a policy which does **not** try to preserve directories + * Using the `rename` policy get the list of files to rename + * Using the `getattr` policy get the target path + * For each file attempt rename: + * If the source drive != target drive: + * Clone target path from target drive to source drive + * Rename + * If **any** of the renames succeed the higher level rename is considered a success + * If **no** renames succeed the first error encountered will be returned + * On success: + * Remove the target from all drives with no source file + * Remove the source from all drives which failed to rename + +The the removals are subject to normal entitlement checks. + +The above behavior will help minimize the likelihood of EXDEV being returned but it will still be possible. To remove the possibility all together mergerfs would need to perform the as `mv` does when it receives EXDEV normally. #### readdir #### @@ -289,7 +319,7 @@ A B C # Known Issues / Bugs #### Samba -* Moving files or directories between directories on a SMB share fail with IO errors. +* Moving files or directories between some directories on a SMB share fail with IO errors. Workaround: Copy the file/directory and then remove the original rather than move. diff --git a/src/rename.cpp b/src/rename.cpp index 042e1446..ac8a9b5e 100644 --- a/src/rename.cpp +++ b/src/rename.cpp @@ -1,38 +1,38 @@ /* - The MIT License (MIT) - - Copyright (c) 2014 Antonio SJ Musumeci - - Permission is hereby granted, free of charge, to any person obtaining a copy - of this software and associated documentation files (the "Software"), to deal - in the Software without restriction, including without limitation the rights - to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - copies of the Software, and to permit persons to whom the Software is - furnished to do so, subject to the following conditions: - - The above copyright notice and this permission notice shall be included in - all copies or substantial portions of the Software. - - THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN - THE SOFTWARE. -*/ + The MIT License (MIT) -#include + Copyright (c) 2016 Antonio SJ Musumeci + + Permission is hereby granted, free of charge, to any person obtaining a copy + of this software and associated documentation files (the "Software"), to deal + in the Software without restriction, including without limitation the rights + to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + copies of the Software, and to permit persons to whom the Software is + furnished to do so, subject to the following conditions: + + The above copyright notice and this permission notice shall be included in + all copies or substantial portions of the Software. + + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + THE SOFTWARE. +*/ -#include #include +#include #include +#include +#include #include #include -#include #include "config.hpp" +#include "fs_clonepath.hpp" #include "fs_path.hpp" #include "rwlock.hpp" #include "ugid.hpp" @@ -41,85 +41,241 @@ using std::string; using std::vector; using std::set; using mergerfs::Policy; +using namespace mergerfs; static bool -_different_dirname(const string &path0, - const string &path1) +member(const vector &haystack, + const string &needle) { - return (fs::path::dirname(path0) != fs::path::dirname(path1)); + return (std::find(haystack.begin(),haystack.end(),needle) != haystack.end()); +} + +// a single success trumps any failure +static +int +_process_rv(const int rv, + const int preverror, + const int error) +{ + if(rv == -1) + { + if(preverror == 0) + return 0; + return error; + } + + return 0; } static void -_unlink(const set &tounlink, - const string &newfusepath) +_remove(const vector &toremove) +{ + for(size_t i = 0, ei = toremove.size(); i != ei; i++) + ::remove(toremove[i].c_str()); +} + +static +void +_rename_create_path_one(const vector &oldbasepaths, + const string &oldbasepath, + const string &newbasepath, + const string &oldfusepath, + const string &newfusepath, + const string &newfusedirpath, + int &error, + vector &tounlink) { int rv; - string fullpath; + bool ismember; + string oldfullpath; + string newfullpath; + + fs::path::make(oldbasepath,newfusepath,newfullpath); - for(set::const_iterator i = tounlink.begin(), ei = tounlink.end(); i != ei; i++) + ismember = member(oldbasepaths,oldbasepath); + if(ismember) { - fs::path::make(*i,newfusepath,fullpath); + if(oldbasepath != newbasepath) + { + const ugid::SetRootGuard ugidGuard; + fs::clonepath(newbasepath,oldbasepath,newfusedirpath); + } - rv = ::unlink(fullpath.c_str()); - if(rv == -1 && errno == EISDIR) - ::rmdir(fullpath.c_str()); + fs::path::make(oldbasepath,oldfusepath,oldfullpath); + + rv = ::rename(oldfullpath.c_str(),newfullpath.c_str()); + error = _process_rv(rv,error,errno); + if(rv == -1) + tounlink.push_back(oldfullpath); + } + else + { + tounlink.push_back(newfullpath); } } static int -_rename(const vector &srcmounts, - const vector &basepaths, - const string &oldfusepath, - const string &newfusepath) +_rename_create_path(Policy::Func::Search searchFunc, + Policy::Func::Action actionFunc, + const vector &srcmounts, + const size_t minfreespace, + const string &oldfusepath, + const string &newfusepath) { int rv; int error; - string oldfullpath; - string newfullpath; - set tounlink; + string newbasepath; + vector toremove; + vector oldbasepaths; + + rv = actionFunc(srcmounts,oldfusepath,minfreespace,oldbasepaths); + if(rv == -1) + return -errno; + + const string newfusedirpath = fs::path::dirname(newfusepath); + rv = searchFunc(srcmounts,newfusedirpath,minfreespace,newbasepath); + if(rv == -1) + return -errno; - error = 0; - tounlink.insert(srcmounts.begin(),srcmounts.end()); - for(size_t i = 0, ei = basepaths.size(); i != ei; i++) + error = -1; + for(size_t i = 0, ei = srcmounts.size(); i != ei; i++) { - fs::path::make(basepaths[i],oldfusepath,oldfullpath); - fs::path::make(basepaths[i],newfusepath,newfullpath); + const string &oldbasepath = srcmounts[i]; - tounlink.erase(basepaths[i]); - rv = ::rename(oldfullpath.c_str(),newfullpath.c_str()); - if(rv == -1) - error = errno; + _rename_create_path_one(oldbasepaths,oldbasepath,newbasepath, + oldfusepath,newfusepath, + newfusedirpath, + error,toremove); } if(error == 0) - _unlink(tounlink,newfusepath); + _remove(toremove); return -error; } static int -_rename(Policy::Func::Search searchFunc, - Policy::Func::Action actionFunc, - const vector &srcmounts, - const size_t minfreespace, - const string &oldfusepath, - const string &newfusepath) +_clonepath_if_would_create(Policy::Func::Search searchFunc, + Policy::Func::Create createFunc, + const vector &srcmounts, + const size_t minfreespace, + const string &oldbasepath, + const string &oldfusepath, + const string &newfusepath) { int rv; - vector oldbasepaths; + string newbasepath; + string newfusedirpath; - if(_different_dirname(oldfusepath,newfusepath)) - return -EXDEV; + newfusedirpath = fs::path::dirname(newfusepath); + + rv = createFunc(srcmounts,newfusedirpath,minfreespace,newbasepath); + if(rv != -1) + { + if(oldbasepath == newbasepath) + { + rv = searchFunc(srcmounts,newfusedirpath,minfreespace,newbasepath); + if(rv != -1) + { + const ugid::SetRootGuard ugidGuard; + fs::clonepath(newbasepath,oldbasepath,newfusedirpath); + } + } + else + { + rv = -1; + errno = EXDEV; + } + } + + return rv; +} + +static +void +_rename_preserve_path_one(Policy::Func::Search searchFunc, + Policy::Func::Create createFunc, + const vector &srcmounts, + const size_t minfreespace, + const vector &oldbasepaths, + const string &oldbasepath, + const string &oldfusepath, + const string &newfusepath, + int &error, + vector &toremove) +{ + int rv; + bool ismember; + string newfullpath; + + fs::path::make(oldbasepath,newfusepath,newfullpath); + + ismember = member(oldbasepaths,oldbasepath); + if(ismember) + { + string oldfullpath; + + fs::path::make(oldbasepath,oldfusepath,oldfullpath); + + rv = ::rename(oldfullpath.c_str(),newfullpath.c_str()); + if((rv == -1) && (errno == ENOENT)) + { + rv = _clonepath_if_would_create(searchFunc,createFunc, + srcmounts,minfreespace, + oldbasepath,oldfusepath,newfusepath); + if(rv != -1) + rv = ::rename(oldfullpath.c_str(),newfullpath.c_str()); + } + + error = _process_rv(rv,error,errno); + if(rv == -1) + toremove.push_back(oldfullpath); + } + else + { + toremove.push_back(newfullpath); + } +} + +static +int +_rename_preserve_path(Policy::Func::Search searchFunc, + Policy::Func::Action actionFunc, + Policy::Func::Create createFunc, + const vector &srcmounts, + const size_t minfreespace, + const string &oldfusepath, + const string &newfusepath) +{ + int rv; + int error; + vector toremove; + vector oldbasepaths; rv = actionFunc(srcmounts,oldfusepath,minfreespace,oldbasepaths); if(rv == -1) return -errno; - return _rename(srcmounts,oldbasepaths,oldfusepath,newfusepath); + error = -1; + for(size_t i = 0, ei = srcmounts.size(); i != ei; i++) + { + const string &oldbasepath = srcmounts[i]; + + _rename_preserve_path_one(searchFunc,createFunc, + srcmounts,minfreespace, + oldbasepaths,oldbasepath, + oldfusepath,newfusepath, + error,toremove); + } + + if(error == 0) + _remove(toremove); + + return -error; } namespace mergerfs @@ -135,12 +291,21 @@ namespace mergerfs const ugid::Set ugid(fc->uid,fc->gid); const rwlock::ReadGuard readlock(&config.srcmountslock); - return _rename(config.getattr, - config.rename, - config.srcmounts, - config.minfreespace, - oldpath, - newpath); + if(config.create != Policy::epmfs) + return _rename_create_path(config.rename, + config.create, + config.srcmounts, + config.minfreespace, + oldpath, + newpath); + + return _rename_preserve_path(config.getattr, + config.rename, + config.create, + config.srcmounts, + config.minfreespace, + oldpath, + newpath); } } }