Re: [V9fs-developer] tar does not support partial reads

[Dominique Martinet]

Hi,

Daniel P. Berrangé wrote on Tue, Jul 20, 2021 at 06:14:53PM +0100:
> > > When attempting to read a file (other than a pipe or FIFO) that supports 
> > > non-blocking reads and has no data currently available:
> > > 
> > >     - If O_NONBLOCK is set, read() shall return -1 and set errno to 
> > > [EAGAIN].
> > > 
> > >     - If O_NONBLOCK is clear, read() shall block the calling thread until 
> > > some data becomes available.
> > > 
> > >     - The use of the O_NONBLOCK flag has no effect if there is some data 
> > > available.
> > > 
> > > and
> > > 
> > > [EAGAIN]
> > >     The file is neither a pipe, nor a FIFO, nor a socket, the O_NONBLOCK 
> > > flag is set for the file descriptor, and the thread would be delayed in 
> > > the read operation.

Not much time to reply now (will follow up in more details tomorrow),
but that was (unfortunately?) a voluntary change:

http://lkml.kernel.org/r/20200205003457.24340-2-l29ah@cock.li

with the argument that if some program goes out of its way to use
O_NONBLOCK on open it can also handle short reads (which by the way can
also happen without O_NONBLOCK as bugs on other filesystems, so in my
opinion is something that should be handled regardless of what we do
here -- I've seen enough data being eaten by programs that take
shortcuts on IO as soon as something goes wrong instead of erroring or
taking care of these)

Unfortunately GNU tar doesn't, and Nikos Tsipinakis (added to Ccs) sent
a patch to bug-tar@ in ... september last year which looked good enough
to me but didn't seem to be taken? I didn't check.


I agree this isn't posix, but it was discussed as a quirk that seemed
better than yet another weird mount option that e.g. NFS has for special
netfs behaviour.

The argument was for synthetic fs having a file that would stream things
and implementing tail -f like behaviour on it, problem being that if
they would make the fake-file a pipe it would stay local to the linux
client and not go through the 9p server, so it was deemed difficult to
emulate the behaviour.
If you have a practical way of doing this then I'll be happy to revert
Sergey's commit (also added to cc), as I can agree sticking to posix
when possible is better.

> > I was thinking to something like that (not tested yet):
> > 
> > --- a/fs/9p/vfs_file.c
> > +++ b/fs/9p/vfs_file.c
> > @@ -389,8 +389,22 @@ v9fs_file_read_iter(struct kiocb *iocb, struct 
> > iov_iter *t>
> >         p9_debug(P9_DEBUG_VFS, "count %zu offset %lld\n",
> >                  iov_iter_count(to), iocb->ki_pos);
> >  
> > -       if (iocb->ki_filp->f_flags & O_NONBLOCK)
> > +       if (iocb->ki_filp->f_flags & O_NONBLOCK) {
> > +               size_t count = iov_iter_count(to);
> > +
> >                 ret = p9_client_read_once(fid, iocb->ki_pos, to, &err);
> > +               if (!ret)
> > +                       return err;
> > +
> > +               /*
> > +                * POSIX requires to ignore O_NONBLOCK if some data is
> > +                * already available.
> > +                */
> > +               if (ret != count) {
> > +                       iocb->ki_pos += ret;
> > +                       ret = p9_client_read(fid, iocb->ki_pos, to, &err);
> > +               }

That seems overly complicated, just use p9_client_read as per the else
(e.g. remove the condition) if that's what you want.

-- 
Dominique