rework rename algo to minimize likelihood of EXDEV being returned. closes #187

README.md

@@ -1,6 +1,6 @@
 % mergerfs(1) mergerfs user manual
 % Antonio SJ Musumeci <trapexit@spawn.link>
-% 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.
     

src/rename.cpp

@@ -1,38 +1,38 @@
 /*
-   The MIT License (MIT)
-
-   Copyright (c) 2014 Antonio SJ Musumeci <trapexit@spawn.link>
-
-   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 <fuse.h>
+  Copyright (c) 2016 Antonio SJ Musumeci <trapexit@spawn.link>
+
+  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 <stdio.h>
 #include <errno.h>
+#include <stdio.h>
 #include <unistd.h>
 
+#include <algorithm>
+#include <set>
 #include <string>
 #include <vector>
-#include <set>
 
 #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<string> &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<string> &tounlink,
-        const string      &newfusepath)
+_remove(const vector<string> &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<string> &oldbasepaths,
+                        const string         &oldbasepath,
+                        const string         &newbasepath,
+                        const string         &oldfusepath,
+                        const string         &newfusepath,
+                        const string         &newfusedirpath,
+                        int                  &error,
+                        vector<string>       &tounlink)
 {
   int rv;
-  string fullpath;
+  bool ismember;
+  string oldfullpath;
+  string newfullpath;
+
+  fs::path::make(oldbasepath,newfusepath,newfullpath);
 
-  for(set<string>::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<string> &srcmounts,
-        const vector<string> &basepaths,
-        const string         &oldfusepath,
-        const string         &newfusepath)
+_rename_create_path(Policy::Func::Search  searchFunc,
+                    Policy::Func::Action  actionFunc,
+                    const vector<string> &srcmounts,
+                    const size_t          minfreespace,
+                    const string         &oldfusepath,
+                    const string         &newfusepath)
 {
   int rv;
   int error;
-  string oldfullpath;
-  string newfullpath;
-  set<string> tounlink;
+  string newbasepath;
+  vector<string> toremove;
+  vector<string> 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<string> &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<string> &srcmounts,
+                           const size_t          minfreespace,
+                           const string         &oldbasepath,
+                           const string         &oldfusepath,
+                           const string         &newfusepath)
 {
   int rv;
-  vector<string> 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<string> &srcmounts,
+                          const size_t          minfreespace,
+                          const vector<string> &oldbasepaths,
+                          const string         &oldbasepath,
+                          const string         &oldfusepath,
+                          const string         &newfusepath,
+                          int                  &error,
+                          vector<string>       &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<string> &srcmounts,
+                      const size_t          minfreespace,
+                      const string         &oldfusepath,
+                      const string         &newfusepath)
+{
+  int rv;
+  int error;
+  vector<string> toremove;
+  vector<string> 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);
     }
   }
 }