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;