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

[Daniel P . Berrangé]

On Tue, Jul 20, 2021 at 04:58:03PM +0200, Greg Kurz wrote:
> On Tue, 20 Jul 2021 16:37:01 +0200
> Greg Kurz <groug@kaod.org> wrote:
> 
> > On Tue, 20 Jul 2021 13:26:50 +0200
> > 
> > Looking more closely at what POSIX says about O_NONBLOCK:
> > 
> > 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.
> > 
> > The case of the reported issue is thus "O_NONBLOCK is set and some data
> > is available", which should lead O_NONBLOCK to be ignored, i.e. switch
> > to a full read instead of propagating the short read IIUC.
> > 
> > Makes sense ?
> > 
> 
> 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);
> +               }
> +       }
>         else
>                 ret = p9_client_read(fid, iocb->ki_pos, to, &err);
>         if (!ret)

Yes, this looks like it would better match the semantics documented
for O_NONBLOCK and expected by tar.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|