Bug 4561 - Add options --tweak, --no-tweak, --no-tweak-hlinked
Add options --tweak, --no-tweak, --no-tweak-hlinked
Product: rsync
Classification: Unclassified
Component: core
All All
: P3 enhancement
: ---
Assigned To: Wayne Davison
Rsync QA Contact
Depends on:
Blocks: 5448
  Show dependency treegraph
Reported: 2007-04-26 20:48 UTC by Matt McCutchen
Modified: 2010-02-07 17:53 UTC (History)
2 users (show)

See Also:

Implementation of --tweak options (13.11 KB, patch)
2007-11-04 14:51 UTC, Matt McCutchen
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Matt McCutchen 2007-04-26 20:48:28 UTC
Please add --tweak, --no-tweak, and --no-tweak-hlinked options to specify whether rsync should tweak or recreate a destination file whose data matches that of the source file but whose preserved attributes differ.

--tweak (default): always tweak
--no-tweak: always recreate
--no-tweak-hlinked: recreate if the destination file is hard-linked (i.e., st_nlink > 1)

--no-tweak-hlinked makes it safe to copy into a backup that contains files hard-linked from older backups, as in "rsnapshot sync".  --no-tweak is useful if the attributes at the destination path need to change all at the same time.  I'm thinking there might conceivably be other cases in which --no-tweak is important, but I can't come up with an example.

It would be interesting if --no-tweak-hlinked could selectively override --inplace to recreate hard-linked files but edit other files in place.

See comments here:


In "I doubt anyone wants to undertake them now", I was referring to the major changes, while I think these three options would be a tractable addition.
Comment 1 Matt McCutchen 2007-11-04 14:51:51 UTC
Created attachment 2960 [details]
Implementation of --tweak options

Here, finally, is an implementation of the --tweak options!  Now that bug 5051 and the omission of mtimes from unchanged_attrs have been fixed, the implementation is pretty simple; Wayne, perhaps you guessed from the comment change I inadvertently left in my patch for bug 5051 that this was what I was up to.

The options seem to work and itemize properly, and there is even a test case.  The clever interaction between --no-tweak-hlinked and --inplace is not implemented, but it can come later (if at all).  Please add some form of this to patches/ .  I will start using --no-tweak-hlinked in my sync_first-mode rsnapshot runs.
Comment 2 Erik Logtenberg 2007-11-26 15:04:52 UTC
I have no idea if this is appreciated, so I hereby apologize in advance if it isn't, but I would like to support Matts request for inclusion of these patches.
These added options for rsync would seriously contribute to the usability of rsnapshot, essentially making rsync an even better tool for making backups.

And Matt, thank you for working on this, it's appreciated.
Comment 3 Matt McCutchen 2008-02-06 21:33:32 UTC
Marking WONTFIX to reflect what Wayne said in bug 4768 comment 5.
Comment 4 Matt McCutchen 2008-03-17 22:14:20 UTC
I have posted a revised version of the patch that covers non-regular files and has slightly improved documentation in branch "tweak-opts" of my repository at:

Comment 5 Carl E. Thompson 2008-05-07 20:19:48 UTC
See bug #5448.
Comment 6 Matt McCutchen 2008-05-07 23:20:06 UTC
Note that, as currently implemented, --no-tweak does not prevent tweaking of non-directories when the destination is subject to malicious concurrent modification: someone can replace a directory or new file whose attributes rsync is about to set with a hard link to an existing file.  The following set of changes would fix this:

1. Use a secure --temp-dir.
2. Use temporary files for all non-directories in the same way as for regular files.
3. To tweak a directory, open it, fstat the fd to check that it refers to a directory (i.e., the directory wasn't concurrently replaced), and then set attributes with f* calls on the fd.

This is secure because all rsync's attribute-setting calls are either in the temp dir, where a concurrent rsync won't replace files, or on fds that rsync has verified to refer to directories.  For extra protection, the wrappers for path-based attribute-setting calls in syscall.c could be enhanced to check that the path is in the temp dir.

#1 is its own option.  A simple approach to integrating #2 and #3 would be to simply add them to --no-tweak.  However, #2 is useful even without --no-tweak to ensure that a destination path always exists while it is being updated.  Furthermore, --no-tweak is useful without #3 when there is no concurrent modification but a process that opens a destination file and fstats it twice must not see the attributes change.  Thus, for maximum flexibility, I propose making #2 its own option, --stage-all, and adding an option --secure-no-tweak that includes --no-tweak, --stage-all, #3, and a check that the user specified a temp dir that appears to be outside the destination (begins with / or ..).

The daemon parameter in bug 5448 would correspond to --secure-no-tweak and would come with a daemon temp dir that appears to be outside the *module*; of course, that is only possible when the module path isn't purely a chroot.
Comment 7 Carl E. Thompson 2008-05-08 07:48:05 UTC
I think you mean to reference bug 5449 (not 5448).
Comment 8 Matt McCutchen 2008-05-08 07:54:10 UTC
No, I do mean bug 5448, because the daemon parameter there is another form of the --no-tweak/--secure-no-tweak option here.  The daemon link-dest parameter of bug 5449 is not directly related to the --*tweak options although they are useful together.
Comment 9 Wayne Davison 2008-07-26 10:08:25 UTC
Am considering this.
Comment 10 Matt McCutchen 2010-02-07 17:53:14 UTC
I noticed that this should block bug 5448, as stated in comment #8.  Carl, please do not remove this relationship again without stating a reason.