bug#6131: [PATCH]: fiemap support for efficient sparse file copy

[Jim Meyering]

Pádraig Brady wrote:

> On 16/06/10 07:45, Jim Meyering wrote:
>> Paul Eggert wrote:
>>> On 06/15/2010 02:43 PM, Jim Meyering wrote:
>>>> I think that copying physical holes via FIEMAP should be the default, when
>>>> possible.  One problem is that the current code on the fiemap-copy branch
>>>> does not honor --sparse=WHEN when in fiemap-copying mode.  The solution
>>>> would seem to be to change the regular-file-copying loop in the fiemap_copy
>>>> function to use the same hole-preserving code that is used in copy_reg.
>>>
>>> I assume that the solution would be used only with cp --sparse=always?
>>> (Otherwise, it would amount to copying logical holes by default.)
>>
>> Right.
>> I.e., use the same code that honors (or not) the "make_holes" variable.
>>
>>> If so, under this proposal, --sparse=always would copy logical holes,
>>> --sparse=never would never copy holes, and --sparse=auto (the default)
>>> would copy physical holes if supported, else it would copy logical
>>> holes if the file seems to have enough physical holes, and otherwise
>>> it would copy no holes.  (Whew!)
>>
>> Right.  With --sparse=always and fiemap support, it would take advantage
>> of existing extents to minimize copying time, and for the nontrivial
>> extents, it would detect/induce new holes when possible.
>>
>> Perhaps we need a new logical option that would make a difference
>> only when there are nontrivial fiemap extents:
>>   - read nontrivial extents, as is done now
>>   - read them, *and* search-for/induce-holes as we do in legacy
>>       copy for --sparse=always.
>>
>> Or, putting it another way, perhaps we need a new command line
>> option to control whether we even attempt a fiemap copy.
>> IMHO the default should be to enable it, once all of the
>> underlying bits are deemed to be stable enough.
>>
>>     --fiemap
>>     --no-fiemap
>>
>> Then, --fiemap --sparse=never would do what the existing fiemap_copy
>> function does, and --fiemap --sparse=always would work once the
>> internal-to-fiemap_copy copying code is adjusted to use the
>> hole-preserving code in copy_reg.
>>
>> Then, to guarantee the legacy --sparse=never behavior,
>> one would have to specify --no-fiemap --sparse=never
>
> I would suggest not to add options like this,
> and only do what's possible without new options
> as it would push too many implementation details
> to the docs/users IMHO.

Good point.  Smaller/simpler would be better.
It'd be great if we can eliminate as never-useful one of the
new combinations.

Currently on the fiemap-copy branch, once we get a single
successful ioctl, the code will not bother with the usual
hole-detecting/introducing technique implied by --sparse=always.

Should it do that ever?  Always?
Does this need an option?