Re: [PATCH] copy: adjust fiemap handling of sparse files

[Pádraig Brady]

On 16/03/11 19:18, Mike Frysinger wrote:
> On Wednesday, March 16, 2011 11:26:40 Pádraig Brady wrote:
>> On 14/02/11 06:05, Jim Meyering wrote:
>>> Pádraig Brady wrote:
>>>> For my own reference, I can probably skip performance
>>>> tests on (older) btrfs by checking `filefrag` output.
>>>> Also in `cp`, if we see an "unwritten extent" we should
>>>> probably create a hole rather than an empty allocation
>>>> by default.  It's better to decrease file allocation
>>>> than increase it.
>>>
>>> Makes sense.
>>
>> Thinking a bit more about it, I'm not sure I should do the above,
>> as one would be more surprised if cp by default introduced holes into
>> a copy of a fully allocated file.
>>
>> The previously applied patch changes behavior on BTRFS on Fedora 14,
>> in that it will convert a file with holes to a fully allocated one
>> with zeros. But that is due to an already fixed bug in BTRFS
>> where it reports holes as empty extents.
>>
>> I'm inclined to leave this as is, because this stuff is tricky enough,
>> without introducing work arounds for buggy file systems.
>> There is no data loss in this case, just bigger files
>> (which one can avoid with cp --sparse=always).
>> Also it will not be common to overlap future coreutils releases,
>> with older BTRFS implementations.
> 
> well, in the bug report i was working with, we were seeing data loss.  i 
> wonder if it'd be possible to detect the fs/kernel version and error out when 
> versions are found that are known to be broken ?

That was a different issue.

It would be nice, but I don't think it would be practical to try and detect
and work around such file system issues in cp.

There are always going to be teething problems with a large
body of new logic, so I think the best approach with file systems
is to increase trust in the gradually over time.

Personally I'd consider BTRFS for my backup drive at present,
which it should be particularly good at given its snapshot
capabilities.

cheers,
Pádraig.