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

[Pádraig Brady]

On 17/03/11 07:24, Mike Frysinger wrote:
> On Wednesday, March 16, 2011 19:55:56 Pádraig Brady wrote:
>> On 16/03/11 19:18, Mike Frysinger wrote:
>>> well, in the bug report i was working with, we were seeing data loss.  i
>>> wonder if it'd be possible to detect the fs/kernel version and error out
>>> when versions are found that are known to be broken ?
>>
>> That was a different issue.
>>
>> It would be nice, but I don't think it would be practical to try and detect
>> and work around such file system issues in cp.
>>
>> There are always going to be teething problems with a large
>> body of new logic, so I think the best approach with file systems
>> is to increase trust in the gradually over time.
> 
> while this is true, i'd understand if the issue were bugs in coreutils' cp.
> they'd get fixed and a new release made, no problem.  but we're talking about
> silent errors in the kernel, so anyone running a newer coreutils with a kernel
> from two days ago is going hit issues.  if we were looking at basic read/write
> functionality, i'd understand not bothering, but we're talking purely about
> optimization paths in coreutils' cp.
> 
> on the Gentoo side, i guess i'll run with a hack like so:
> 
> --- a/src/copy.c
> +++ b/src/copy.c
> @@ -22,6 +22,7 @@
>  #include <sys/ioctl.h>
>  #include <sys/types.h>
>  #include <selinux/selinux.h>
> +#include <sys/utsname.h>
>  
>  #if HAVE_HURD_H
>  # include <hurd.h>
> @@ -930,7 +931,32 @@ copy_reg (char const *src_name, char const *dst_name,
>               the file is a hole.  */
>            if (x->sparse_mode == SPARSE_AUTO && S_ISREG (src_open_sb.st_mode)
>                && ST_NBLOCKS (src_open_sb) < src_open_sb.st_size / 
> ST_NBLOCKSIZE)
> -            make_holes = true;
> +            {
> +              make_holes = true;
> +
> +# ifdef __linux__
> +              static int safe_kernel = -1;
> +
> +              if (safe_kernel == -1)
> +                {
> +                  struct utsname name;
> +
> +                  safe_kernel = 1;
> +
> +                  if (uname (&name) != -1 && strncmp (name.release, "2.6.", 
> 4) == 0)
> +                    {
> +                      int kver = atoi (name.release + 4);
> +
> +                      /* ext4 & btrfs are known to be broken */
> +                      if (kver < 38)
> +                        safe_kernel = 0;
> +                    }
> +                }
> +
> +              if (safe_kernel == 0)
> +                make_holes = false;
> +# endif
> +            }
>  #endif
>          }
>  
> -mike

This is the kind of patch appropriate for a distro,
as they may or may not have a fix backported to their kernel.

I presume you're referring to coreutils bug 8001.
I didn't realise ext4 was also affected.
Is this specific to the compress flag?
What was the specific fix to btrfs &/or ext4 in 2.6.38?

Also note the make_holes heuristic variable is no
longer used in extent_copy due to this patch which
came after the 8.10
http://git.sv.gnu.org/gitweb/?p=coreutils.git;a=commitdiff;h=275c831a

cheers,
Pádraig