> On Jan 14, 2021, at 1:00 PM, Daniel P. Berrangé <berra...@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 <stefa...@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 :|
>
