On 17/06/18 17:38, Adam Borowski wrote: > Control: tags -1 +patch > > Here's a patch. > > I find upstream's decision to not default to --reflink=auto weird. Not > reflinking when it's supported is harmful: the stated reason of "resiliency" > against disk failures is bogus: common types of failure don't work this way. > Neither does "guaranteeing disk space" work: a CoW filesystem has to CoW all > files you modify, even briefly. In most cases the old version will be kept > by a snapshot anyway. Even if not, if the user uses any method of > deduplication, the damage is obvious. Even more (un-)hilarious if you have > some kind of inline deduplication (for btrfs, bees or that recently posted > patch set, for zfs that's even the only implemented mode of deduplication) > where the entire file is read and hashed for no reason whatsoever. > > With my hat of a btrfs mailing list/IRC regular and an occasional > kernel/btrfs-progs contributor, the only case where non-reflinking might > possibly be useful are non-CoW files. But: > touch foo > chattr +C foo > dd if=/dev/zero of=foo bs=1048576 count=10 > cp --reflink=always foo bar > cp: failed to clone 'bar' from 'foo': Invalid argument > So it's already blocked! > > > Thus, I doubt there's ever a reason to use a value of --reflink other than > "auto". But, if there is, you'd want to be able to temporarily override > your alias. Which is what the attached patch does.
Upstream is probably going to change to trying reflink by default in a new major release of coreutils (v9). I agree with the arguments you have presented, though it's a change with significant behavior change and best done in a major version bump. As for --reflink=never, that will be needed as you say, and useful now in the edge case of people using --reflink=auto in aliases. I will apply this for the upcoming 8.30 release. thanks, Pádraig.