rsync - Sep 2006 - DO NOT REPLY [Bug 4079] New: rsync fails with --inplace, --link-dest and --no-whole-file

https://bugzilla.samba.org/show_bug.cgi?id=4079

           Summary: rsync fails with --inplace, --link-dest and --no-whole-
                    file
           Product: rsync
           Version: 2.6.8
          Platform: x86
        OS/Version: Linux
            Status: NEW
          Severity: major
          Priority: P3
         Component: core
        AssignedTo: wayned@samba.org
        ReportedBy: roger@planbit.co.uk
         QAContact: rsync-qa@samba.org


System: Debian sarge (x86) with a locally compiled 2.6.8 version of rsync with
the ACL patches applied.  Kernel updated to 2.6.16.20.

The command below fails if there is a reference file in "/tmp/test/b"
that is
slightly different to the file in "/tmp/test/a" and the destination
file does
not exist in "/tmp/test/c".

rsync -a --no-whole-file --inplace --link-dest=/tmp/test/b tmp/test/a/
tmp/test/c/

If rsync is run with -vvvv, the RSYNC algorithm appears to run correctly and
determine the blocks in the file that are missing.  Unfortunately, only the
changed data is written to the file in "/tmp/test/c/" and the rest of
the file
is zeros. 

I suspect this is because an empty destination file is created when the file
doesn't exist rather than hard-linking the destination file to the link-dest
file before performing the in-place copy.


-- 
Configure bugmail: https://bugzilla.samba.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.

https://bugzilla.samba.org/show_bug.cgi?id=4079





------- Comment #1 from paul@debian.org  2006-09-04 10:44 MST -------
I can confirm this with the standard Debian version 2.6.7-1 and 2.6.8-2 (no ACL
patches).


-- 
Configure bugmail: https://bugzilla.samba.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.

https://bugzilla.samba.org/show_bug.cgi?id=4079





------- Comment #2 from roger@planbit.co.uk  2006-09-04 10:57 MST -------
The patch below will detect the condition where we are "inplace", have
a
destination file that does not exist and an existing reference file.  When this
happens, the destination file will be hardlinked to the reference file before
being opened.  This resolves the bug.  There is probably a cleaner way of
detecting the missing destination file than trying to open it read-only and
handling the fail state.

There is also the question of whether this is the correct behaviour since the
modifications also get applied to the reference file because of the hard-link. 
For me, the hard-link is what I would want to happen, but other people may
prefer to have rsync copy the matching data from the reference file to the
destination file so that the reference file is left unchanged.

diff -aur rsync-2.6.8/receiver.c wip/receiver.c
--- rsync-2.6.8/receiver.c      2006-09-04 16:48:46.000000000 +0100
+++ wip/receiver.c      2006-09-04 16:46:43.000000000 +0100
@@ -560,6 +560,21 @@

                /* We now check to see if we are writing file
"inplace" */
                if (inplace)  {
+
+                       if ((fd1 != -1) && (fname != fnamecmp))
+                       {
+                               fd2 = do_open(fname, O_RDONLY, 0);
+                               if (fd2 < 0)
+                               {
+                                       rprintf(FINFO,"Adding hard-link %s
->
%s\n", fname, fnamecmp);
+                                       do_link(fnamecmp, fname);
+                               }
+                   else
+                               {
+                                       close(fd2);
+                               }
+                       }
+
                        fd2 = do_open(fname, O_WRONLY|O_CREAT, 0600);
                        if (fd2 == -1) {
                                rsyserr(FERROR, errno, "open %s
failed",


-- 
Configure bugmail: https://bugzilla.samba.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.

https://bugzilla.samba.org/show_bug.cgi?id=4079





------- Comment #3 from paul@debian.org  2006-09-04 11:21 MST -------
Hardlinking and then modifying the existing file in the --link-dest directory
is absolutely the wrong thing to do IMNSHO, this will break many
"incremental"
backup strategies such as dirvish. (Dirvish at this time does not use
--inplace,
although I was just testing that out over the last couple of days... I guess I
can throw those backups away now :-( )

The manpage says:

     --link-dest=DIR         hardlink to files in DIR when unchanged

However, in these cases, the file _is_ changed, so hardlinking is the wrong
thing to do.


-- 
Configure bugmail: https://bugzilla.samba.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.

https://bugzilla.samba.org/show_bug.cgi?id=4079





------- Comment #4 from roger@planbit.co.uk  2006-09-04 11:59 MST -------
Fair comment.  I don't feel comfortable digging into rsync to try and
optimally
copy over the reference file to the destination, however, so I am afraid that
at this point I will have to request that someone more familiar with the rsync
source fix this one.


-- 
Configure bugmail: https://bugzilla.samba.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.

https://bugzilla.samba.org/show_bug.cgi?id=4079





------- Comment #5 from wayned@samba.org  2006-09-17 18:18 MST -------
Created an attachment (id=2142)
 --> (https://bugzilla.samba.org/attachment.cgi?id=2142&action=view)
Fix --inplace when basis file doesn't match dest file

Here's a patch that should fix the bug.


-- 
Configure bugmail: https://bugzilla.samba.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.

https://bugzilla.samba.org/show_bug.cgi?id=4079


wayned@samba.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
Attachment #2142 is|0                           |1
           obsolete|                            |




------- Comment #6 from wayned@samba.org  2006-09-17 18:25 MST -------
Created an attachment (id=2143)
 --> (https://bugzilla.samba.org/attachment.cgi?id=2143&action=view)
Fix --inplace when basis file doesn't match dest file

Actually, that one change prior to the ftruncate() call isn't right.  This
patch leaves that out.


-- 
Configure bugmail: https://bugzilla.samba.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.

https://bugzilla.samba.org/show_bug.cgi?id=4079


wayned@samba.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED




------- Comment #7 from wayned@samba.org  2006-09-17 18:28 MST -------
Fix is now in CVS.


-- 
Configure bugmail: https://bugzilla.samba.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.