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

[Pádraig Brady]

On 16/06/10 09:49, Jim Meyering wrote:
> 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?

I don't see the need for new options TBH
I see fiemap just as a way to efficiently detect/read holes,
and should have no bearing on the destination.

cp --sparse=auto (this is currently what cp does by default)
  recreate the original fiemap holes or resort to existing
  heuristic if fiemap not available
cp --sparse=never
  write all data, but use fiemap if available to efficiently read
cp --sparse=always
  recreate original holes and perhaps extend add to them for
  other runs of zero bytes. Without having looked at the code
  I see this as a little tricky to mix with fiemap.
  Now since fiemap is only an optimization we can skip it
  completely for this uncommon case if too tricky (just add a FIXME for now).

cheers,
Pádraig.