Avoid re-setting (and sending) xattrs on a hard-linked file w/the same xattrs.
authorWayne Davison <wayned@samba.org>
Sat, 26 Mar 2011 16:59:14 +0000 (09:59 -0700)
committerWayne Davison <wayned@samba.org>
Sat, 26 Mar 2011 17:09:20 +0000 (10:09 -0700)
Improved the xattrs testing to include hard-linking.

.gitignore
Makefile.in
NEWS
generator.c
receiver.c
rsync.h
sender.c
testsuite/hardlinks.test
testsuite/xattrs.test

index 805af45..3e3bc5e 100644 (file)
@@ -39,4 +39,5 @@ config.status
 /support/savetransfer
 /testsuite/chown-fake.test
 /testsuite/devices-fake.test
 /support/savetransfer
 /testsuite/chown-fake.test
 /testsuite/devices-fake.test
+/testsuite/xattrs-hlink.test
 /patches
 /patches
index 055bc75..dfb4e07 100644 (file)
@@ -49,7 +49,7 @@ TLS_OBJ = tls.o syscall.o lib/compat.o lib/snprintf.o lib/permstring.o lib/sysxa
 CHECK_PROGS = rsync$(EXEEXT) tls$(EXEEXT) getgroups$(EXEEXT) getfsdev$(EXEEXT) \
        testrun$(EXEEXT) trimslash$(EXEEXT) t_unsafe$(EXEEXT) wildtest$(EXEEXT)
 
 CHECK_PROGS = rsync$(EXEEXT) tls$(EXEEXT) getgroups$(EXEEXT) getfsdev$(EXEEXT) \
        testrun$(EXEEXT) trimslash$(EXEEXT) t_unsafe$(EXEEXT) wildtest$(EXEEXT)
 
-CHECK_SYMLINKS = testsuite/chown-fake.test testsuite/devices-fake.test
+CHECK_SYMLINKS = testsuite/chown-fake.test testsuite/devices-fake.test testsuite/xattrs-hlink.test
 
 # Objects for CHECK_PROGS to clean
 CHECK_OBJS=tls.o testrun.o getgroups.o getfsdev.o t_stub.o t_unsafe.o trimslash.o wildtest.o
 
 # Objects for CHECK_PROGS to clean
 CHECK_OBJS=tls.o testrun.o getgroups.o getfsdev.o t_stub.o t_unsafe.o trimslash.o wildtest.o
@@ -256,6 +256,9 @@ testsuite/chown-fake.test:
 testsuite/devices-fake.test:
        ln -s devices.test $(srcdir)/testsuite/devices-fake.test
 
 testsuite/devices-fake.test:
        ln -s devices.test $(srcdir)/testsuite/devices-fake.test
 
+testsuite/xattrs-hlink.test:
+       ln -s xattrs.test $(srcdir)/testsuite/xattrs-hlink.test
+
 # This does *not* depend on building or installing: you can use it to
 # check a version installed from a binary or some other source tree,
 # if you want.
 # This does *not* depend on building or installing: you can use it to
 # check a version installed from a binary or some other source tree,
 # if you want.
diff --git a/NEWS b/NEWS
index 5fd125a..97b0200 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -95,8 +95,8 @@ Changes since 3.0.4:
     - When replacing a non-dir with a symlink/hard-link/device/special-file,
       the update should now be done in an atomic manner.
 
     - When replacing a non-dir with a symlink/hard-link/device/special-file,
       the update should now be done in an atomic manner.
 
-    - Fixed a free of the wrong pointer in uncache_tmp_xattrs() (which only
-      sometimes affects an --xattr transfer when --backup is used).
+    - Avoid re-sending xattr info for hard-linked files w/the same xattrs
+      (protocol 31).
 
     - When backing up a file, try to hard-link the file into place so that the
       upcoming replacement of the destination file will be atomic.
 
     - When backing up a file, try to hard-link the file into place so that the
       upcoming replacement of the destination file will be atomic.
index d17e3b9..e3890bb 100644 (file)
@@ -549,8 +549,10 @@ void itemize(const char *fnamecmp, struct file_struct *file, int ndx, int statre
 #ifdef SUPPORT_XATTRS
                        if (preserve_xattrs && do_xfers
                         && iflags & (ITEM_REPORT_XATTR|ITEM_TRANSFER)) {
 #ifdef SUPPORT_XATTRS
                        if (preserve_xattrs && do_xfers
                         && iflags & (ITEM_REPORT_XATTR|ITEM_TRANSFER)) {
-                               send_xattr_request(NULL, file,
-                                       iflags & ITEM_REPORT_XATTR ? sock_f_out : -1);
+                               int fd = iflags & ITEM_REPORT_XATTR
+                                     && (protocol_version < 31 || !BITS_SET(iflags, ITEM_XNAME_FOLLOWS|ITEM_LOCAL_CHANGE))
+                                      ? sock_f_out : -1;
+                               send_xattr_request(NULL, file, fd);
                        }
 #endif
                } else if (ndx >= 0) {
                        }
 #endif
                } else if (ndx >= 0) {
index a519247..7641b9b 100644 (file)
@@ -548,14 +548,16 @@ int recv_files(int f_in, int f_out, char *local_name)
                        rprintf(FINFO, "recv_files(%s)\n", fname);
 
 #ifdef SUPPORT_XATTRS
                        rprintf(FINFO, "recv_files(%s)\n", fname);
 
 #ifdef SUPPORT_XATTRS
-               if (iflags & ITEM_REPORT_XATTR && do_xfers)
+               if (preserve_xattrs && iflags & ITEM_REPORT_XATTR && do_xfers
+                && (protocol_version < 31 || !BITS_SET(iflags, ITEM_XNAME_FOLLOWS|ITEM_LOCAL_CHANGE)))
                        recv_xattr_request(file, f_in);
 #endif
 
                if (!(iflags & ITEM_TRANSFER)) {
                        maybe_log_item(file, iflags, itemizing, xname);
 #ifdef SUPPORT_XATTRS
                        recv_xattr_request(file, f_in);
 #endif
 
                if (!(iflags & ITEM_TRANSFER)) {
                        maybe_log_item(file, iflags, itemizing, xname);
 #ifdef SUPPORT_XATTRS
-                       if (preserve_xattrs && iflags & ITEM_REPORT_XATTR && do_xfers)
+                       if (preserve_xattrs && iflags & ITEM_REPORT_XATTR && do_xfers
+                        && !BITS_SET(iflags, ITEM_XNAME_FOLLOWS|ITEM_LOCAL_CHANGE))
                                set_file_attrs(fname, file, NULL, fname, 0);
 #endif
                        if (iflags & ITEM_IS_NEW) {
                                set_file_attrs(fname, file, NULL, fname, 0);
 #endif
                        if (iflags & ITEM_IS_NEW) {
diff --git a/rsync.h b/rsync.h
index ed60005..79402cd 100644 (file)
--- a/rsync.h
+++ b/rsync.h
 /* This is used when working on a new protocol version in CVS, and should
  * be a new non-zero value for each CVS change that affects the protocol.
  * It must ALWAYS be 0 when the protocol goes final (and NEVER before)! */
 /* This is used when working on a new protocol version in CVS, and should
  * be a new non-zero value for each CVS change that affects the protocol.
  * It must ALWAYS be 0 when the protocol goes final (and NEVER before)! */
-#define SUBPROTOCOL_VERSION 12
+#define SUBPROTOCOL_VERSION 13
 
 /* We refuse to interoperate with versions that are not in this range.
  * Note that we assume we'll work with later versions: the onus is on
 
 /* We refuse to interoperate with versions that are not in this range.
  * Note that we assume we'll work with later versions: the onus is on
index 600ad84..7b6d313 100644 (file)
--- a/sender.c
+++ b/sender.c
@@ -155,7 +155,8 @@ static void write_ndx_and_attrs(int f_out, int ndx, int iflags,
        if (iflags & ITEM_XNAME_FOLLOWS)
                write_vstring(f_out, buf, len);
 #ifdef SUPPORT_XATTRS
        if (iflags & ITEM_XNAME_FOLLOWS)
                write_vstring(f_out, buf, len);
 #ifdef SUPPORT_XATTRS
-       if (preserve_xattrs && iflags & ITEM_REPORT_XATTR && do_xfers)
+       if (preserve_xattrs && iflags & ITEM_REPORT_XATTR && do_xfers
+        && (protocol_version < 31 || !BITS_SET(iflags, ITEM_XNAME_FOLLOWS|ITEM_LOCAL_CHANGE)))
                send_xattr_request(fname, file, f_out);
 #endif
 }
                send_xattr_request(fname, file, f_out);
 #endif
 }
@@ -236,7 +237,8 @@ void send_files(int f_in, int f_out)
                        rprintf(FINFO, "send_files(%d, %s%s%s)\n", ndx, path,slash,fname);
 
 #ifdef SUPPORT_XATTRS
                        rprintf(FINFO, "send_files(%d, %s%s%s)\n", ndx, path,slash,fname);
 
 #ifdef SUPPORT_XATTRS
-               if (preserve_xattrs && iflags & ITEM_REPORT_XATTR && do_xfers)
+               if (preserve_xattrs && iflags & ITEM_REPORT_XATTR && do_xfers
+                && (protocol_version < 31 || !BITS_SET(iflags, ITEM_XNAME_FOLLOWS|ITEM_LOCAL_CHANGE)))
                        recv_xattr_request(file, f_in);
 #endif
 
                        recv_xattr_request(file, f_in);
 #endif
 
index caa0823..55131fa 100644 (file)
@@ -28,7 +28,7 @@ name2="$fromdir/name2"
 name3="$fromdir/name3"
 name4="$fromdir/name4"
 echo "This is the file" > "$name1"
 name3="$fromdir/name3"
 name4="$fromdir/name4"
 echo "This is the file" > "$name1"
-ln "$name1" "$name2" || fail "Can't create hardlink"
+ln "$name1" "$name2" || test_skipped "Can't create hardlink"
 ln "$name2" "$name3" || fail "Can't create hardlink"
 cp "$name2" "$name4" || fail "Can't copy file"
 cat $srcdir/*.c >"$fromdir/text"
 ln "$name2" "$name3" || fail "Can't create hardlink"
 cp "$name2" "$name4" || fail "Can't copy file"
 cat $srcdir/*.c >"$fromdir/text"
index 7200f25..9a14584 100644 (file)
@@ -6,6 +6,7 @@
 # Test that rsync handles basic xattr preservation.
 
 . $suitedir/rsync.fns
 # Test that rsync handles basic xattr preservation.
 
 . $suitedir/rsync.fns
+lnkdir="$tmpdir/lnk"
 
 $RSYNC --version | grep ", xattrs" >/dev/null || test_skipped "Rsync is configured without xattr support"
 
 
 $RSYNC --version | grep ", xattrs" >/dev/null || test_skipped "Rsync is configured without xattr support"
 
@@ -38,7 +39,7 @@ case "`xattr 2>&1`" in
     ;;
 esac
 
     ;;
 esac
 
-makepath "$fromdir/foo/bar"
+makepath "$lnkdir" "$fromdir/foo/bar"
 echo now >"$fromdir/file0"
 echo something >"$fromdir/file1"
 echo else >"$fromdir/file2"
 echo now >"$fromdir/file0"
 echo something >"$fromdir/file1"
 echo else >"$fromdir/file2"
@@ -73,32 +74,60 @@ xset user.dir1 'need to test directory xattrs too' foo
 xset user.dir2 'another xattr' foo
 xset user.dir3 'this is one last one for the moment' foo
 
 xset user.dir2 'another xattr' foo
 xset user.dir3 'this is one last one for the moment' foo
 
+xset user.dir4 'another dir test' foo/bar
+xset user.dir5 'one last one' foo/bar
+
 xset user.foo 'new foo' foo/file3 foo/bar/file5
 xset user.bar 'new bar' foo/file3 foo/bar/file5
 xset user.long 'this is also a long attribute that will be truncated in the initial data send' foo/file3 foo/bar/file5
 xset $RUSR.equal 'this long attribute should remain the same and not need to be transferred' foo/file3 foo/bar/file5
 
 xset user.foo 'new foo' foo/file3 foo/bar/file5
 xset user.bar 'new bar' foo/file3 foo/bar/file5
 xset user.long 'this is also a long attribute that will be truncated in the initial data send' foo/file3 foo/bar/file5
 xset $RUSR.equal 'this long attribute should remain the same and not need to be transferred' foo/file3 foo/bar/file5
 
+xset user.dir0 'old extra value' "$chkdir/foo"
+xset user.dir1 'old dir value' "$chkdir/foo"
+
 xset user.short 'old short' "$chkdir/file1"
 xset user.extra 'remove me' "$chkdir/file1"
 
 xset user.foo 'old foo' "$chkdir/foo/file3"
 xset $RUSR.equal 'this long attribute should remain the same and not need to be transferred' "$chkdir/foo/file3"
 
 xset user.short 'old short' "$chkdir/file1"
 xset user.extra 'remove me' "$chkdir/file1"
 
 xset user.foo 'old foo' "$chkdir/foo/file3"
 xset $RUSR.equal 'this long attribute should remain the same and not need to be transferred' "$chkdir/foo/file3"
 
+case $0 in
+*hlink*)
+    ln foo/bar/file5 foo/bar/file6 || test_skipped "Can't create hardlink"
+    files="$files foo/bar/file6"
+    dashH='-H'
+    altDest='--link-dest'
+    ;;
+*)
+    dashH=''
+    altDest='--copy-dest'
+    ;;
+esac
+
 xls $dirs $files >"$scratchdir/xattrs.txt"
 
 # OK, let's try a simple xattr copy.
 xls $dirs $files >"$scratchdir/xattrs.txt"
 
 # OK, let's try a simple xattr copy.
-checkit "$RSYNC -avX --super . '$chkdir/'" "$fromdir" "$chkdir"
+checkit "$RSYNC -avX $dashH --super . '$chkdir/'" "$fromdir" "$chkdir"
 
 cd "$chkdir"
 xls $dirs $files | diff $diffopt "$scratchdir/xattrs.txt" -
 
 cd "$fromdir"
 
 
 cd "$chkdir"
 xls $dirs $files | diff $diffopt "$scratchdir/xattrs.txt" -
 
 cd "$fromdir"
 
-checkit "$RSYNC -aiX --super --copy-dest=../chk . ../to" "$fromdir" "$todir"
+if [ "$dashH" ]; then
+    for fn in $files; do
+       name=`basename $fn`
+       ln $fn ../lnk/$name
+    done
+fi
+
+checkit "$RSYNC -aiX $dashH --super $altDest=../chk . ../to" "$fromdir" "$todir"
 
 cd "$todir"
 xls $dirs $files | diff $diffopt "$scratchdir/xattrs.txt" -
 
 
 cd "$todir"
 xls $dirs $files | diff $diffopt "$scratchdir/xattrs.txt" -
 
+[ "$dashH" ] && rm -rf "$lnkdir"
+
 cd "$fromdir"
 rm -rf "$todir"
 
 cd "$fromdir"
 rm -rf "$todir"
 
@@ -106,7 +135,7 @@ xset user.nice 'this is nice, but different' file1
 
 xls $dirs $files >"$scratchdir/xattrs.txt"
 
 
 xls $dirs $files >"$scratchdir/xattrs.txt"
 
-checkit "$RSYNC -aiX --fake-super --link-dest=../chk . ../to" "$chkdir" "$todir"
+checkit "$RSYNC -aiX $dashH --fake-super --link-dest=../chk . ../to" "$chkdir" "$todir"
 
 cd "$todir"
 xls $dirs $files | diff $diffopt "$scratchdir/xattrs.txt" -
 
 cd "$todir"
 xls $dirs $files | diff $diffopt "$scratchdir/xattrs.txt" -
@@ -136,7 +165,7 @@ cd "$fromdir"
 rm -rf "$todir"
 
 # When run by a non-root tester, this checks if no-user-perm files/dirs can be copied.
 rm -rf "$todir"
 
 # When run by a non-root tester, this checks if no-user-perm files/dirs can be copied.
-checkit "$RSYNC -aiX --fake-super --chmod=a= . ../to" "$chkdir" "$todir" # 2>"$scratchdir/errors.txt"
+checkit "$RSYNC -aiX $dashH --fake-super --chmod=a= . ../to" "$chkdir" "$todir" # 2>"$scratchdir/errors.txt"
 
 cd "$todir"
 xls $dirs $files | diff $diffopt "$scratchdir/xattrs.txt" -
 
 cd "$todir"
 xls $dirs $files | diff $diffopt "$scratchdir/xattrs.txt" -
@@ -148,10 +177,13 @@ $RSYNC -aX file1 file2
 $RSYNC -aX file1 file2 ../chk/
 $RSYNC -aX --del ../chk/ .
 $RSYNC -aX file1 ../lnk/
 $RSYNC -aX file1 file2 ../chk/
 $RSYNC -aX --del ../chk/ .
 $RSYNC -aX file1 ../lnk/
+[ "$dashH" ] && ln "$chkdir/file1" ../lnk/extra-link
 
 xls file1 file2 >"$scratchdir/xattrs.txt"
 
 
 xls file1 file2 >"$scratchdir/xattrs.txt"
 
-checkit "$RSYNC -aiiX --copy-dest=../lnk . ../to" "$chkdir" "$todir"
+checkit "$RSYNC -aiiX $dashH $altDest=../lnk . ../to" "$chkdir" "$todir"
+
+[ "$dashH" ] && rm ../lnk/extra-link
 
 cd "$todir"
 xls file1 file2 | diff $diffopt "$scratchdir/xattrs.txt" -
 
 cd "$todir"
 xls file1 file2 | diff $diffopt "$scratchdir/xattrs.txt" -