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

[Pádraig Brady]

On 09/02/11 12:10, Jim Meyering wrote:
> Pádraig Brady wrote:
>> I was looking at adding fallocate() to copy.c,
>> now that the fiemap code has gone in and
>> I noticed that if there was allocated space
>> at the end of a file, not accounted for
>> in st_size, then any holes would not be detected.
> 
> Good point.
> 
>> In what other cases does the sparse detection
>> heuristic fail BTW?
> 
> There are probably a few, but none that I know of.
> 
>> Anwyay, we don't need the heuristic with fiemap,
>> so I changed accordingly in the attached.
> ...
>> Subject: [PATCH] copy: adjust fiemap handling of sparse files
>>
>> Don't depend on heuristics to detect sparse files
>> if fiemap is available.  Also don't introduce new
>> holes unless --sparse=always has been specified.
> 
> Good change, in principle.
> 
>> * src/copy.c (extent_copy): Pass the user specified
>> sparse mode, and handle as described above.
>> Also a redundant lseek has been suppressed when
>> there is no hole between two extents.
> 
> Could that be done in two separate patches?
> I haven't looked closely, but when I tested it on x86_64 (F14),
> it failed like this:
> 
>     FAIL: cp/sparse-to-pipe (exit: 1)
>     =================================
>     ...
>     + truncate -s1M sparse
>     + timeout 10 cat pipe
>     + cp sparse pipe
>     cp: failed to extend `pipe': Invalid argument
>     + fail=1
>     + cmp sparse copy
>     cmp: EOF on copy
>     + fail=1
> 
> Sounds like the change provokes an lseek on the output FD,
> even though it's a pipe.

Oops, yes test pass here now.
I'll merge something like the following into the second patch.

cheers,
Pádraig.

diff --git a/src/copy.c b/src/copy.c
index 63a7b1e..cb6ab53 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -963,7 +963,8 @@ copy_reg (char const *src_name, char const *dst_name,
          '--sparse=never' option is specified, write all data but use
          any extents to read more efficiently.  */
       if (extent_copy (source_desc, dest_desc, buf, buf_size,
-                       src_open_sb.st_size, x->sparse_mode,
+                       src_open_sb.st_size,
+                       S_ISREG (sb.st_mode) ? x->sparse_mode : SPARSE_NEVER,
                        src_name, dst_name, &normal_copy_required))
         goto preserve_metadata;