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

[Pádraig Brady]

On 14/07/10 07:58, jeff.liu wrote:
> Pádraig Brady wrote:
> 
> Hello All,
> 
> I am very sorry for the late response! I have an urgent task need to deliver 
> in the past few weeks.
> 
> Thanks for all your suggestions, I would like to improve the fiemap-copy 
> accordingly, so does the
> following sentence is the final decision?
> 
>> 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).

Joel, overlapped with my response and concurred.

> Current code handle 'cp --sparse=auto' and 'cp --sparse=always' in same way 
> since these two options
> all setup 'make_holes' to True, though '--sparse=auto' use a heuristic to 
> determine whether SRC_NAME
> contains any sparse blocks.
> 
> For 'cp --sparse=never', when detected holes from SRC file, do not lseek(2) 
> against DST file,
> instead, write ZEROs to DST file, Am I right?

right

> 
> If so, IMHO, we can pass 'sparse_mode' to fiemap_copy(), then decide how to 
> do operation against DST
> file according to its mode.
> 
> In addition to this, I'd like to fix another issue in current code as Paul 
> suggested,
> 1. Do not allocate buffer in fiemap_copy() for data pass between SRC and DST 
> files, instead, take
> advantage of the buffer allocated outside.

good

> 2. Performance optimization, invoke fallocate(2) if an extent flag is 
> UNWRITTEN
> For this case, maybe we have to wait until fallocate interface become stable 
> just as Pádraig
> methoned before:
> http://lists.gnu.org/archive/html/bug-coreutils/2009-05/msg00260.html

If you decide to do that, then please do it as a separate patch.

cheers,
Pádraig.