Bug 7194 - Getting --inplace and --sparse to work together (+patch)
Summary: Getting --inplace and --sparse to work together (+patch)
Status: RESOLVED FIXED
Alias: None
Product: rsync
Classification: Unclassified
Component: core (show other bugs)
Version: 3.0.7
Hardware: All All
: P3 enhancement (vote)
Target Milestone: ---
Assignee: Wayne Davison
QA Contact: Rsync QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-03-02 01:43 UTC by Arne Jansen
Modified: 2020-07-26 10:01 UTC (History)
3 users (show)

See Also:


Attachments
Patch as described (3.06 KB, patch)
2010-03-02 01:44 UTC, Arne Jansen
no flags Details
updated patch (4.44 KB, patch)
2010-11-12 01:46 UTC, Arne Jansen
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Arne Jansen 2010-03-02 01:43:33 UTC
I created a small patch to get --inplace and --sparse to work together. The main problem with this is, as far as I can see, that the write code just skips blocks of zeroes instead of writing them, as is requested by the --sparse switch. If the target already exists and has data in places where the source has zeroes, this data will never get erased.
My patch simply reads these regions first and makes sure they really are zero. If they are not, it clears them.
It would be good if someone with a deeper knowledge of the implications could have a look at the patch to see if I missed something.
Comment 1 Arne Jansen 2010-03-02 01:44:39 UTC
Created attachment 5440 [details]
Patch as described
Comment 2 Ildar Muyukov 2010-11-05 15:40:19 UTC
https://bugzilla.samba.org/show_bug.cgi?id=7778 might be related
Comment 3 Arne Jansen 2010-11-12 01:46:03 UTC
Created attachment 6065 [details]
updated patch

This updated patch contains a small bugfix. With this version we have successfully copied many TB of data snapshot-by-snapshot and verified the result with gdiff.
Comment 4 Wayne Davison 2011-01-13 09:53:54 UTC
An improvement like this seems good for 3.1.0.  Your patch looks nice, but I do want to make sure that it is leveraging knowledge from the generator -- if we're doing a normal (not whole-file) transfer, then a seek over identical bytes doesn't need to read the bytes again.  Does you patch take that into consideration?

Also, there's already a change in the 3.1.0dev code in sparse_end() that replaces that write-a-0-at-the-end-of-the-file code with an ftruncate() call (if ftruncate() is available), which will avoid allocating an extra block at the end of the file to hold the 0.
Comment 5 Arne Jansen 2011-01-14 08:39:26 UTC
I assume that when write_file is called, the receiver already has determined that the buffer has to be written to disk. In case of inplace, we cannot assume that the file contains zeros in this location, nor it's sparse. So we need to make sure it really contains what we require.
I don't see how any information from the generator can help to avoid this step. Can you please elaborate on your question?
Comment 6 Arne Jansen 2011-05-30 08:51:05 UTC
Is it ready to be pulled into 3.1, or do you need anything else?
Comment 7 Ildar Muyukov 2011-11-07 07:23:06 UTC
Wayne?
Comment 8 David Taylor 2013-09-08 09:50:35 UTC
Is there any update available on what's stopping this patch being merged?

It would be very useful...
Comment 9 David Taylor 2013-09-08 09:53:13 UTC
Is there any update available on what's stopping this patch being merged?

It would be very useful...
Comment 10 Wayne Davison 2020-07-26 10:01:00 UTC
This was fixed in an earlier release.