[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v19 08/20] io: add qio_channel_readv_full_all_eof & qio_chann
From: |
Daniel P . Berrangé |
Subject: |
Re: [PATCH v19 08/20] io: add qio_channel_readv_full_all_eof & qio_channel_readv_full_all helpers |
Date: |
Thu, 14 Jan 2021 18:00:35 +0000 |
User-agent: |
Mutt/1.14.6 (2020-07-11) |
On Thu, Jan 14, 2021 at 12:55:58PM -0500, Jag Raman wrote:
>
>
> > On Jan 14, 2021, at 11:27 AM, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> >
> > On Thu, Jan 14, 2021 at 10:40:03AM -0500, Jagannathan Raman wrote:
> >> +int qio_channel_readv_full_all(QIOChannel *ioc,
> >> + const struct iovec *iov,
> >> + size_t niov,
> >> + int **fds, size_t *nfds,
> >> + Error **errp)
> >> {
> >> - int ret = qio_channel_readv_all_eof(ioc, iov, niov, errp);
> >> + int ret = qio_channel_readv_full_all_eof(ioc, iov, niov, fds, nfds,
> >> errp);
> >>
> >> if (ret == 0) {
> >> - ret = -1;
> >> error_setg(errp,
> >> "Unexpected end-of-file before all bytes were read");
> >
> > qio_channel_readv_full_all_eof() can read file descriptors but no data
> > and return 0.
> >
> > Here that case is converted into an error and the file descriptors
> > aren't closed, freed, and fds/nfds isn't cleared.
>
> That’s a valid point. I’m wondering if the fix for this case should be in
> qio_channel_readv_full_all_eof(), instead of here.
>
> qio_channel_readv_full_all_eof() should probably return error (-1) if the
> amount of data read does not match iov_size(). If the caller is only expecting
> to read fds, and not any data, it would indicate that by setting iov to NULL
> and/or setting niov=0. If the caller is setting these parameters, it means it
> is
> expecting data.Does that sound good?
The API spec for the existing _eof() methods says:
* The function will wait for all requested data
* to be read, yielding from the current coroutine
* if required.
*
* If end-of-file occurs before any data is read,
* no error is reported; otherwise, if it occurs
* before all requested data has been read, an error
* will be reported.
IOW, return '0' is *only* valid if we've not read anything. I consider
file descriptors to be something.
IOW, qio_channel_readv_full_all_eof must only return 0, if it didn't
read any data and also didn't receive any file descriptors. So yeah,
we must return -1 in the scenario Stefan describes
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 :|
- [PATCH v19 09/20] multi-process: define MPQemuMsg format and transmission functions, (continued)
- [PATCH v19 09/20] multi-process: define MPQemuMsg format and transmission functions, Jagannathan Raman, 2021/01/14
- [PATCH v19 14/20] multi-process: add proxy communication functions, Jagannathan Raman, 2021/01/14
- [PATCH v19 17/20] multi-process: Synchronize remote memory, Jagannathan Raman, 2021/01/14
- [PATCH v19 20/20] multi-process: perform device reset in the remote process, Jagannathan Raman, 2021/01/14
- [PATCH v19 18/20] multi-process: create IOHUB object to handle irq, Jagannathan Raman, 2021/01/14
- [PATCH v19 05/20] multi-process: setup PCI host bridge for remote device, Jagannathan Raman, 2021/01/14
- [PATCH v19 02/20] multi-process: add configure and usage information, Jagannathan Raman, 2021/01/14
- [PATCH v19 08/20] io: add qio_channel_readv_full_all_eof & qio_channel_readv_full_all helpers, Jagannathan Raman, 2021/01/14
- Re: [PATCH v19 08/20] io: add qio_channel_readv_full_all_eof & qio_channel_readv_full_all helpers, Stefan Hajnoczi, 2021/01/14
- Re: [PATCH v19 08/20] io: add qio_channel_readv_full_all_eof & qio_channel_readv_full_all helpers, Jag Raman, 2021/01/14
- Re: [PATCH v19 08/20] io: add qio_channel_readv_full_all_eof & qio_channel_readv_full_all helpers,
Daniel P . Berrangé <=
- Re: [PATCH v19 08/20] io: add qio_channel_readv_full_all_eof & qio_channel_readv_full_all helpers, Jag Raman, 2021/01/14
- Re: [PATCH v19 08/20] io: add qio_channel_readv_full_all_eof & qio_channel_readv_full_all helpers, Stefan Hajnoczi, 2021/01/15
- Re: [PATCH v19 08/20] io: add qio_channel_readv_full_all_eof & qio_channel_readv_full_all helpers, Jag Raman, 2021/01/15
- Re: [PATCH v19 08/20] io: add qio_channel_readv_full_all_eof & qio_channel_readv_full_all helpers, Jag Raman, 2021/01/15
- Re: [PATCH v19 08/20] io: add qio_channel_readv_full_all_eof & qio_channel_readv_full_all helpers, Stefan Hajnoczi, 2021/01/18
[PATCH v19 11/20] multi-process: Associate fd of a PCIDevice with its object, Jagannathan Raman, 2021/01/14
[PATCH v19 07/20] io: add qio_channel_writev_full_all helper, Jagannathan Raman, 2021/01/14
[PATCH v19 12/20] multi-process: setup memory manager for remote device, Jagannathan Raman, 2021/01/14
[PATCH v19 15/20] multi-process: Forward PCI config space acceses to the remote process, Jagannathan Raman, 2021/01/14
[PATCH v19 13/20] multi-process: introduce proxy object, Jagannathan Raman, 2021/01/14