coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Pádraig Brady
Subject: Re: [PATCH] copy: adjust fiemap handling of sparse files
Date: Wed, 30 Mar 2011 23:25:32 +0100
User-agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.1.8) Gecko/20100227 Thunderbird/3.0.3

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

I'm going to use this "2.6.38" check to only enable FIEMAP_FLAG_SYNC
before Linux kernel 2.6.38.  It's always worth avoiding sync if possible.
Proposed patch attached.
I'll submit my 3 outstanding fiemap patches tomorrow.

cheers,
Pádraig.

Attachment: fiemap-sync.diff
Description: Text Data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]