[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: |
Jag Raman |
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 13:24:37 -0500 |
> On Jan 14, 2021, at 1:00 PM, Daniel P. Berrangé <berrange@redhat.com> wrote:
>
> 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
That makes sense to me. Reading “fds" is something, which is different
from our previous understanding. I thought data only meant iov, and not fds.
So the return values for qio_channel_readv_full_all_eof() would be:
- ‘0’ only if EOF is reached without reading any fds and data.
- ‘1’ if all data that the caller expects are read (even if the caller reads
fds exclusively, without any iovs)
- ‘-1’ otherwise, considered as error
qio_channel_readv_full_all() would return:
- ‘0’ if all the data that caller expects are read
- ‘-1’ otherwise, considered as error
Hey Stefan,
Does this sound good to you?
Thank you!
--
Jag
>
> 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 14/20] multi-process: add proxy communication functions, (continued)
- [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é, 2021/01/14
- Re: [PATCH v19 08/20] io: add qio_channel_readv_full_all_eof & qio_channel_readv_full_all helpers,
Jag Raman <=
- 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
[PATCH v19 19/20] multi-process: Retrieve PCI info from remote process, Jagannathan Raman, 2021/01/14