Improved the backup code:

- Backups do not interfere with an atomic update (when possible).
- Backing up a file will remove a directory that is in the way
  and visa versa.
- Unify the backup-dir and non-backup-dir code in backup.c.
- Improved the backup tests a little bit.

diff --git a/backup.c b/backup.c
index 8907b25..a81f947 100644 (file)
--- a/backup.c
+++ b/backup.c
@@ -51,43 +51,6 @@ char *get_backup_name(const char *fname)
        return NULL;
 }
 
-/* simple backup creates a backup with a suffix in the same directory */
-static int make_simple_backup(const char *fname)
-{
-       int rename_errno;
-       const char *fnamebak = get_backup_name(fname);
-
-       if (!fnamebak)
-               return 0;
-
-       while (1) {
-               if (do_rename(fname, fnamebak) == 0) {
-                       if (INFO_GTE(BACKUP, 1)) {
-                               rprintf(FINFO, "backed up %s to %s\n",
-                                       fname, fnamebak);
-                       }
-                       break;
-               }
-               /* cygwin (at least version b19) reports EINVAL */
-               if (errno == ENOENT || errno == EINVAL)
-                       break;
-
-               rename_errno = errno;
-               if (errno == EISDIR && do_rmdir(fnamebak) == 0)
-                       continue;
-               if (errno == ENOTDIR && do_unlink(fnamebak) == 0)
-                       continue;
-
-               rsyserr(FERROR, rename_errno, "rename %s to backup %s",
-                       fname, fnamebak);
-               errno = rename_errno;
-               return 0;
-       }
-
-       return 1;
-}
-
-
 /****************************************************************************
 Create a directory given an absolute path, perms based upon another directory
 path
@@ -172,50 +135,87 @@ int make_bak_dir(const char *fullpath)
        return 0;
 }
 
-/* robustly move a file, creating new directory structures if necessary */
-static int robust_move(const char *src, char *dst)
+/* Has same return codes as make_backup(). */
+static inline int link_or_rename(const char *from, const char *to,
+                                BOOL prefer_rename, STRUCT_STAT *stp)
 {
-       if (robust_rename(src, dst, NULL, 0755) < 0) {
-               int save_errno = errno ? errno : EINVAL; /* 0 paranoia */
-               if (errno == ENOENT && make_bak_dir(dst) == 0) {
-                       if (robust_rename(src, dst, NULL, 0755) < 0)
-                               save_errno = errno ? errno : save_errno;
-                       else
-                               save_errno = 0;
-               }
-               if (save_errno) {
-                       errno = save_errno;
-                       return -1;
+       if (S_ISLNK(stp->st_mode)) {
+               if (prefer_rename)
+                       goto do_rename;
+#ifndef CAN_HARDLINK_SYMLINK
+               return 0; /* Use copy code. */
+#endif
+       }
+       if (IS_SPECIAL(stp->st_mode) || IS_DEVICE(stp->st_mode)) {
+               if (prefer_rename)
+                       goto do_rename;
+#ifndef CAN_HARDLINK_SPECIAL
+               return 0; /* Use copy code. */
+#endif
+       }
+#ifdef SUPPORT_HARD_LINKS
+       if (!S_ISDIR(stp->st_mode)) {
+               if (do_link(from, to) == 0)
+                       return 2;
+               return 0;
+       }
+#endif
+  do_rename:
+       if (do_rename(from, to) == 0) {
+               if (stp->st_nlink > 1 && !S_ISDIR(stp->st_mode)) {
+                       /* If someone has hard-linked the file into the backup
+                        * dir, rename() might return success but do nothing! */
+                       robust_unlink(to); /* Just in case... */
                }
+               return 1;
        }
        return 0;
 }
 
-
-/* If we have a --backup-dir, then we get here from make_backup().
- * We will move the file to be deleted into a parallel directory tree. */
-static int keep_backup(const char *fname)
+/* Hard-link, rename, or copy an item to the backup name.  Returns 2 if item
+ * was duplicated into backup area, 1 if item was moved, or 0 for failure.*/
+int make_backup(const char *fname, BOOL prefer_rename)
 {
        stat_x sx;
        struct file_struct *file;
-       char *buf;
-       int save_preserve_xattrs = preserve_xattrs;
-       int kept = 0;
-       int ret_code;
+       int save_preserve_xattrs;
+       char *buf = get_backup_name(fname);
+       int ret = 0;
+
+       if (!buf)
+               return 0;
 
        init_stat_x(&sx);
        /* Return success if no file to keep. */
        if (x_lstat(fname, &sx.st, NULL) < 0)
                return 1;
 
-       if (!(file = make_file(fname, NULL, NULL, 0, NO_FILTERS)))
-               return 1; /* the file could have disappeared */
-
-       if (!(buf = get_backup_name(fname))) {
-               unmake_file(file);
-               return 0;
+       /* Try a hard-link or a rename first.  Using rename is not atomic, but
+        * is more efficient than forcing a copy for larger files when no hard-
+        * linking is possible. */
+       if ((ret = link_or_rename(fname, buf, prefer_rename, &sx.st)) != 0)
+               goto success;
+       if (errno == EEXIST) {
+               STRUCT_STAT bakst;
+               if (do_lstat(buf, &bakst) == 0) {
+                       int flags = get_del_for_flag(bakst.st_mode) | DEL_FOR_BACKUP | DEL_RECURSE;
+                       if (delete_item(buf, bakst.st_mode, flags) != 0)
+                               return 0;
+               }
+               if ((ret = link_or_rename(fname, buf, prefer_rename, &sx.st)) != 0)
+                       goto success;
+       } else if (backup_dir && errno == ENOENT) {
+               /* If the backup dir is missing, try again after making it. */
+               if (make_bak_dir(buf) != 0)
+                       return 0;
+               if ((ret = link_or_rename(fname, buf, prefer_rename, &sx.st)) != 0)
+                       goto success;
        }
 
+       /* Fall back to making a copy. */
+       if (!(file = make_file(fname, NULL, &sx.st, 0, NO_FILTERS)))
+               return 1; /* the file could have disappeared */
+
 #ifdef SUPPORT_ACLS
        if (preserve_acls && !S_ISLNK(file->mode)) {
                get_acl(fname, &sx);
@@ -235,7 +235,6 @@ static int keep_backup(const char *fname)
        if ((am_root && preserve_devices && IS_DEVICE(file->mode))
         || (preserve_specials && IS_SPECIAL(file->mode))) {
                int save_errno;
-               do_unlink(buf);
                if (do_mknod(buf, file->mode, sx.st.st_rdev) < 0) {
                        save_errno = errno ? errno : EINVAL; /* 0 paranoia */
                        if (errno == ENOENT && make_bak_dir(buf) == 0) {
@@ -254,11 +253,11 @@ static int keep_backup(const char *fname)
                        rprintf(FINFO, "make_backup: DEVICE %s successful.\n",
                                fname);
                }
-               kept = 1;
-               do_unlink(fname);
+               ret = 2;
        }
 
-       if (!kept && S_ISDIR(file->mode)) {
+       if (!ret && S_ISDIR(file->mode)) {
+               int ret_code;
                /* make an empty directory */
                if (do_mkdir(buf, file->mode) < 0) {
                        int save_errno = errno ? errno : EINVAL; /* 0 paranoia */
@@ -279,20 +278,19 @@ static int keep_backup(const char *fname)
                        rprintf(FINFO, "make_backup: RMDIR %s returns %i\n",
                                full_fname(fname), ret_code);
                }
-               kept = 1;
+               ret = 2;
        }
 
 #ifdef SUPPORT_LINKS
-       if (!kept && preserve_links && S_ISLNK(file->mode)) {
+       if (!ret && preserve_links && S_ISLNK(file->mode)) {
                const char *sl = F_SYMLINK(file);
                if (safe_symlinks && unsafe_symlink(sl, buf)) {
                        if (INFO_GTE(SYMSAFE, 1)) {
                                rprintf(FINFO, "ignoring unsafe symlink %s -> %s\n",
                                        full_fname(buf), sl);
                        }
-                       kept = 1;
+                       ret = 2;
                } else {
-                       do_unlink(buf);
                        if (do_symlink(sl, buf) < 0) {
                                int save_errno = errno ? errno : EINVAL; /* 0 paranoia */
                                if (errno == ENOENT && make_bak_dir(buf) == 0) {
@@ -306,47 +304,40 @@ static int keep_backup(const char *fname)
                                                full_fname(buf), sl);
                                }
                        }
-                       do_unlink(fname);
-                       kept = 1;
+                       ret = 2;
                }
        }
 #endif
 
-       if (!kept && !S_ISREG(file->mode)) {
+       if (!ret && !S_ISREG(file->mode)) {
                rprintf(FINFO, "make_bak: skipping non-regular file %s\n",
                        fname);
                unmake_file(file);
-               return 1;
+               return 2;
        }
 
-       /* move to keep tree if a file */
-       if (!kept) {
-               if (robust_move(fname, buf) != 0) {
+       /* Copy to backup tree if a file. */
+       if (!ret) {
+               if (copy_file(fname, buf, -1, file->mode, 1) < 0) {
                        rsyserr(FERROR, errno, "keep_backup failed: %s -> \"%s\"",
                                full_fname(fname), buf);
-               } else if (sx.st.st_nlink > 1) {
-                       /* If someone has hard-linked the file into the backup
-                        * dir, rename() might return success but do nothing! */
-                       robust_unlink(fname); /* Just in case... */
+                       unmake_file(file);
+                       return 0;
                }
+               ret = 2;
        }
+
+       save_preserve_xattrs = preserve_xattrs;
        preserve_xattrs = 0;
        set_file_attrs(buf, file, NULL, fname, 0);
        preserve_xattrs = save_preserve_xattrs;
+
        unmake_file(file);
 
+  success:
        if (INFO_GTE(BACKUP, 1)) {
                rprintf(FINFO, "backed up %s to %s\n",
                        fname, buf);
        }
-       return 1;
-}
-
-
-/* main backup switch routine */
-int make_backup(const char *fname)
-{
-       if (backup_dir)
-               return keep_backup(fname);
-       return make_simple_backup(fname);
+       return ret;
 }

diff --git a/delete.c b/delete.c
index 4d7220e..33fdd0e 100644 (file)
--- a/delete.c
+++ b/delete.c
@@ -169,12 +169,18 @@ enum delret delete_item(char *fbuf, uint16 mode, uint16 flags)
        if (S_ISDIR(mode)) {
                what = "rmdir";
                ok = do_rmdir(fbuf) == 0;
-       } else if (make_backups > 0 && (backup_dir || !is_backup_file(fbuf))) {
-               what = "make_backup";
-               ok = make_backup(fbuf);
        } else {
-               what = "unlink";
-               ok = robust_unlink(fbuf) == 0;
+               if (make_backups > 0 && (backup_dir || !is_backup_file(fbuf))) {
+                       what = "make_backup";
+                       ok = make_backup(fbuf, True);
+                       if (ok == 2) {
+                               what = "unlink";
+                               ok = robust_unlink(fbuf) == 0;
+                       }
+               } else {
+                       what = "unlink";
+                       ok = robust_unlink(fbuf) == 0;
+               }
        }
 
        if (ok) {
@@ -219,8 +225,24 @@ enum delret delete_item(char *fbuf, uint16 mode, uint16 flags)
                case DEL_FOR_SPECIAL: desc = "special file"; break;
                default: exit_cleanup(RERR_UNSUPPORTED); /* IMPOSSIBLE */
                }
-               rprintf(FERROR_XFER, "could not make way for new %s: %s\n",
+               rprintf(FERROR_XFER, "could not make way for %s %s: %s\n",
+                       flags & DEL_FOR_BACKUP ? "backup" : "new",
                        desc, fbuf);
        }
        return ret;
 }
+
+uint16 get_del_for_flag(uint16 mode)
+{
+       if (S_ISREG(mode))
+               return DEL_FOR_FILE;
+       if (S_ISDIR(mode))
+               return DEL_FOR_DIR;
+       if (S_ISLNK(mode))
+               return DEL_FOR_SYMLINK;
+       if (IS_DEVICE(mode))
+               return DEL_FOR_DEVICE;
+       if (IS_SPECIAL(mode))
+               return DEL_FOR_SPECIAL;
+       exit_cleanup(RERR_UNSUPPORTED); /* IMPOSSIBLE */
+}

diff --git a/hlink.c b/hlink.c
index 6c9010d..c9eb33a 100644 (file)
--- a/hlink.c
+++ b/hlink.c
@@ -217,6 +217,7 @@ static int maybe_hard_link(struct file_struct *file, int ndx,
                           const char *realname, int itemizing, enum logcode code)
 {
        if (statret == 0) {
+               int ok = 0;
                if (sxp->st.st_dev == old_stp->st_dev
                 && sxp->st.st_ino == old_stp->st_ino) {
                        if (itemizing) {
@@ -229,10 +230,9 @@ static int maybe_hard_link(struct file_struct *file, int ndx,
                        file->flags |= FLAG_HLINK_DONE;
                        return 0;
                }
-               if (make_backups > 0) {
-                       if (!make_backup(fname))
-                               return -1;
-               } else if (robust_unlink(fname)) {
+               if (make_backups > 0 && (ok = make_backup(fname, True)) == 0)
+                       return -1;
+               if (ok != 1 && robust_unlink(fname) && errno != ENOENT) {
                        rsyserr(FERROR_XFER, errno, "unlink %s failed",
                                full_fname(fname));
                        return -1;

diff --git a/receiver.c b/receiver.c
index c1ea608..0135e42 100644 (file)
--- a/receiver.c
+++ b/receiver.c
@@ -331,7 +331,7 @@ static void handle_delayed_updates(char *local_name)
                struct file_struct *file = cur_flist->files[ndx];
                fname = local_name ? local_name : f_name(file, NULL);
                if ((partialptr = partial_dir_fname(fname)) != NULL) {
-                       if (make_backups > 0 && !make_backup(fname))
+                       if (make_backups > 0 && !make_backup(fname, False))
                                continue;
                        if (DEBUG_GTE(RECV, 1)) {
                                rprintf(FINFO, "renaming %s to %s\n",

diff --git a/rsync.c b/rsync.c
index c420cf8..2c026a2 100644 (file)
--- a/rsync.c
+++ b/rsync.c
@@ -561,9 +561,10 @@ int finish_transfer(const char *fname, const char *fnametmp,
        }
 
        if (make_backups > 0 && overwriting_basis) {
-               if (!make_backup(fname))
+               int ok = make_backup(fname, False);
+               if (!ok)
                        return 1;
-               if (fnamecmp == fname)
+               if (ok == 1 && fnamecmp == fname)
                        fnamecmp = get_backup_name(fname);
        }
 

diff --git a/rsync.h b/rsync.h
index 57bae83..088f90e 100644 (file)
--- a/rsync.h
+++ b/rsync.h
@@ -249,6 +249,7 @@ enum msgcode {
 #define DEL_FOR_SYMLINK        (1<<5) /* making room for a replacement symlink */
 #define DEL_FOR_DEVICE         (1<<6) /* making room for a replacement device */
 #define DEL_FOR_SPECIAL        (1<<7) /* making room for a replacement special */
+#define DEL_FOR_BACKUP         (1<<8) /* the delete is for a backup operation */
 
 #define DEL_MAKE_ROOM (DEL_FOR_FILE|DEL_FOR_DIR|DEL_FOR_SYMLINK|DEL_FOR_DEVICE|DEL_FOR_SPECIAL)
 

diff --git a/testsuite/backup.test b/testsuite/backup.test
index 4227b86..e28dd54 100644 (file)
--- a/testsuite/backup.test
+++ b/testsuite/backup.test
@@ -11,7 +11,7 @@
 
 bakdir="$tmpdir/bak"
 
-makepath "$fromdir/deep" "$bakdir"
+makepath "$fromdir/deep" "$bakdir/dname"
 name1="$fromdir/deep/name1"
 name2="$fromdir/deep/name2"
 
@@ -20,13 +20,13 @@ outfile="$scratchdir/rsync.out"
 cat "$srcdir"/[gr]*.[ch] > "$name1"
 cat "$srcdir"/[et]*.[ch] > "$name2"
 
-checkit "$RSYNC -avv '$fromdir/' '$todir/'" "$fromdir" "$todir"
+checkit "$RSYNC -ai --info=backup '$fromdir/' '$todir/'" "$fromdir" "$todir"
 
-checkit "$RSYNC -avv '$fromdir/' '$chkdir/'" "$fromdir" "$chkdir"
+checkit "$RSYNC -ai --info=backup '$fromdir/' '$chkdir/'" "$fromdir" "$chkdir"
 cat "$srcdir"/[fgpr]*.[ch] > "$name1"
 cat "$srcdir"/[etw]*.[ch] > "$name2"
 
-$RSYNC -avv --no-whole-file --backup "$fromdir/" "$todir/" \
+$RSYNC -ai --info=backup --no-whole-file --backup "$fromdir/" "$todir/" \
     | tee "$outfile"
 for fn in deep/name1 deep/name2; do
     grep "backed up $fn to $fn~" "$outfile" >/dev/null || test_fail "no backup message output for $fn"
@@ -38,7 +38,7 @@ done
 echo deleted-file >"$todir/dname"
 cp_touch "$todir/dname" "$chkdir"
 
-checkit "$RSYNC -avv --no-whole-file --delete-delay \
+checkit "$RSYNC -ai --info=backup --no-whole-file --delete-delay \
     --backup --backup-dir='$bakdir' '$fromdir/' '$todir/'" "$fromdir" "$todir" \
     | tee "$outfile"
 
@@ -48,11 +48,11 @@ done
 diff -r $diffopt "$chkdir" "$bakdir" || test_fail "backup dir contents are bogus"
 rm "$bakdir/dname"
 
-checkit "$RSYNC -avv --del '$fromdir/' '$chkdir/'" "$fromdir" "$chkdir"
+checkit "$RSYNC -ai --info=backup --del '$fromdir/' '$chkdir/'" "$fromdir" "$chkdir"
 cat "$srcdir"/[efgr]*.[ch] > "$name1"
 cat "$srcdir"/[ew]*.[ch] > "$name2"
 
-checkit "$RSYNC -avv --inplace --no-whole-file --backup --backup-dir='$bakdir' '$fromdir/' '$todir/'" "$fromdir" "$todir" \
+checkit "$RSYNC -ai --info=backup --inplace --no-whole-file --backup --backup-dir='$bakdir' '$fromdir/' '$todir/'" "$fromdir" "$todir" \
     | tee "$outfile"
 
 for fn in deep/name1 deep/name2; do
@@ -60,7 +60,7 @@ for fn in deep/name1 deep/name2; do
 done
 diff -r $diffopt "$chkdir" "$bakdir" || test_fail "backup dir contents are bogus"
 
-checkit "$RSYNC -avv --inplace --no-whole-file '$fromdir/' '$bakdir/'" "$fromdir" "$bakdir"
+checkit "$RSYNC -ai --info=backup --inplace --no-whole-file '$fromdir/' '$bakdir/'" "$fromdir" "$bakdir"
 
 # The script would have aborted on error, so getting here means we've won.
 exit 0