Re: RFC: should lseek(SEEK_DATA/SEEK_HOLE) return ENOTTY?

Ian Lepore wrote:
>On Sun, 2019-08-11 at 09:12 -0600, Alan Somers wrote:
>> On Sun, Aug 11, 2019 at 8:57 AM Ian Lepore <ian_at_freebsd.org> wrote:
>> >
>> > On Sun, 2019-08-11 at 09:04 +0200, Gary Jennejohn wrote:
>> > > On Sun, 11 Aug 2019 02:03:10 +0000
>> > > Rick Macklem <rmacklem_at_uoguelph.ca> wrote:
>> > >
>> > > > Hi,
>> > > >
>> > > > I've noticed that, if you do a lseek(SEEK_DATA/SEEK_HOLE) on a
>> > > > file
>> > > > that
>> > > > resides in a file system that does not support holes, ENOTTY is
>> > > > returned.
>> > > >
>> > > > This error isn't listed for lseek() and seems a liitle weird.
>> > > >
>> > >
>> > > ENOTTY is the standard error return for an unimplemented
>> > > ioctl(2),
>> > > and SEEK_HOLE ultimately becomes a call to fo_ioctl().
That's true and explains why it returns ENOTTY. However, lseek(2) is not ioctl(2)
and it doesn't list ENOTTY as an error.
(Just to make things confusing, lseek(2) using SEEK_DATA/SEEK_HOLE appears to
 be only a POSIX draft at this point, so POSIX doesn't really help w.r.t. what errors
 should be returned for this case.)

>> > >
>> > > > I can see a couple of alternatives to this:
>> > > > 1 - Return a different error. Maybe ENXIO?
>> > > > or
>> > > > 2 - Have lseek() do the trivial implementation when the
>> > > > VOP_IOCTL()
>> > > > fails.
>> > > >    - For SEEK_DATA, just return the offset given as argument
>> > > > and
>> > > > for SEEK_HOLE
>> > > >       return the file's size as the offset.
>> > > >
>> > > > What do others think? rick
>> > > > ps: The man page should be updated, whatever is done w.r.t.
>> > > > this.
>> > > >
>> > >
>> > > I also vote for option 2
>> > >
>> >
>> > If SEEK_DATA and SEEK_HOLE don't return the standard "ioctl not
>> > supported" error code and return a fake result, how are you
>> > supposed to
>> > determine at runtime whether SEEK_HOLE is supported or not?
>> >
>> > -- Ian
>>
>> pathconf(2) will tell you.
>>
>
>Ahh, I wasn't aware of that.
>
>For option 2, lseek() has to not just return the info, but must also
>actually set the file position accordingly, and has to treat offset >=
>filesize as an error.
Yes, this check is done below the VOP_IOCTL() layer for the file system
(using vn_bmap_seekhole() or custom code).

I think the easiest way to implement #2 is create a vop_stdioctl() and put it into
sys/kern/vfs_default.c. It would need to do this check.

Interestingly, I had assumed the discussion would have been between leaving
the errno alone vs changing the errno. I only threw in #2 for completeness
sake.
--> Now, it appears that #2 is the favourite.

I'll wait for more responses before I propose a patch.

Thanks for the comments, rick