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.

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;
 }

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 */
+}

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;

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",

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);
 	}
 

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)
 

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