Subject: Re: bug#20826: SEEK_HOLE not supported for ext4 for kernel < 3.1

From: Eric Blake <[email-masked]>
Date: Tue, 16 Jun 2015 13:17:31 -0600

On 06/16/2015 07:06 AM, Pádraig Brady wrote:
> On 16/06/15 13:47, Eric Blake wrote:
>> [adding bug-gnulib]
>>
>> On 06/16/2015 06:28 AM, Johannes Meixner wrote:
>>>
>>>> From one of our (SUSE) kernel developers
>>> I even got a proposal for a workaround in grep:
>>>
>>> --- a/src/grep.c
>>> +++ b/src/grep.c
>>> @@ -575,6 +575,17 @@ file_textbin (char *buf, size_t size, in
>>>            off_t hole_start = lseek (fd, cur, SEEK_HOLE);
>>>            if (0 <= hole_start)
>>>              {
>>> +             /* if hole_start is identical with cur, it's more likely a
>>> buggy
>>> +              * lseek(SEEK_HOLE) implementation in kernel filesystem;
>>> +              * check whether it really reads a null byte.
>>> +              */
>>> +             if (hole_start == cur)
>>> +               {
>>> +                 char c;
>>> +                 if (read (fd, &c, 1) != 1 || c != 0)
>>> +                   hole_start = st->st_size; /* no hole */

You MUST lseek() back to the original spot (or use pread() to avoid
changing offset) for the workaround to work. There might also be ways to
cache whether the workaround has ever been needed, so that subsequent
calls aren't penalized (then again, when working around the bug, the
likely behavior is to just treat the file as all data, so you'll only
ever encounter the bug once per file, unless you are reading the file's
contents multiple times); and as the call is filesystem related, we
can't guarantee that it works or fails on all possible files served by
the kernel, when crossing device boundaries.  So yeah, we're probably
stuck with a per-file probing, minimized to the cases where a reported
hole sounds suspicious.

>>> +               }
>>> +
>>
>> Sounds like we need to update the gnulib lseek module to work around
>> bugs like this in the wild.
> 
> Interesting. The above runtime check wouldn't be appropriate I think
> for such an old fleeting problem. Also the read() as a side effect
> would be very much best avoided.  How about a linux kernel version check
> at build time?
> 
> As a general point, gnulib would really benefit from SEEK_HOLE, SEEK_DATA
> wrappers that degenerated to SEEK_END,0 and SEEK_SET,offset respectively.

Yeah, I've wanted those for several years now (I even mentioned it as a
"nice to have" in my Linux Plumber's Convention talk in 2012), but
haven't had time to work on it myself yet.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org