[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.
fiemap-sync.diff
Description: Text Data
- Re: [PATCH] copy: adjust fiemap handling of sparse files, (continued)
- Re: [PATCH] copy: adjust fiemap handling of sparse files, Eric Sandeen, 2011/03/18
- Re: [PATCH] copy: adjust fiemap handling of sparse files, Jim Meyering, 2011/03/18
- Re: [PATCH] copy: adjust fiemap handling of sparse files, Pádraig Brady, 2011/03/22
- Re: [PATCH] copy: adjust fiemap handling of sparse files, Pádraig Brady, 2011/03/18
- Re: [PATCH] copy: adjust fiemap handling of sparse files, Jim Meyering, 2011/03/19
- Re: [PATCH] copy: adjust fiemap handling of sparse files, Pádraig Brady, 2011/03/19
- Re: [PATCH] copy: adjust fiemap handling of sparse files,
Pádraig Brady <=
- Re: [PATCH] copy: adjust fiemap handling of sparse files, Jim Meyering, 2011/03/31