Re: [PULL v4 14/27] io: add qio_channel_readv_full_all_eof & qio_channel_readv_full_all helpers

[Jag Raman]

> On Feb 11, 2021, at 10:46 AM, Daniel P. Berrangé <berrange@redhat.com> wrote:
> 
> On Thu, Feb 11, 2021 at 04:34:40PM +0100, Max Reitz wrote:
>> On 10.02.21 10:26, Stefan Hajnoczi wrote:
>>> From: Elena Ufimtseva <elena.ufimtseva@oracle.com>
>>> 
>>> Adds qio_channel_readv_full_all_eof() and qio_channel_readv_full_all()
>>> to read both data and FDs. Refactors existing code to use these helpers.
>>> 
>>> Signed-off-by: Elena Ufimtseva <elena.ufimtseva@oracle.com>
>>> Signed-off-by: John G Johnson <john.g.johnson@oracle.com>
>>> Signed-off-by: Jagannathan Raman <jag.raman@oracle.com>
>>> Acked-by: Daniel P. Berrangé <berrange@redhat.com>
>>> Message-id: 
>>> b059c4cc0fb741e794d644c144cc21372cad877d.1611938319.git.jag.raman@oracle.com
>>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>>> ---
>>>  include/io/channel.h |  53 +++++++++++++++++++++++
>>>  io/channel.c         | 101 ++++++++++++++++++++++++++++++++++---------
>>>  2 files changed, 134 insertions(+), 20 deletions(-)
>> 
>> [...]
>> 
>>> diff --git a/io/channel.c b/io/channel.c
>>> index 0d4b8b5160..4555021b62 100644
>>> --- a/io/channel.c
>>> +++ b/io/channel.c
>> 
>> [...]
>> 
>>> @@ -135,20 +193,23 @@ int qio_channel_readv_all_eof(QIOChannel *ioc,
>>>      return ret;
>>>  }
>>> -int qio_channel_readv_all(QIOChannel *ioc,
>>> -                          const struct iovec *iov,
>>> -                          size_t niov,
>>> -                          Error **errp)
>>> +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");
>>> -    } else if (ret == 1) {
>>> -        ret = 0;
>>> +        error_prepend(errp,
>>> +                      "Unexpected end-of-file before all data were read.");
>>> +        return -1;
>> 
>> This change breaks iotest 083 (i.e., it segfaults), because
>> qio_channel_readv_full_all_eof() doesn’t set *errp when it returns 0, so
>> there is no error to prepend.
> 
> Opps, yes, this needs to be error_setg() not error_prepend()

Hi Max & Daniel,

We will send a patch shortly to address this issue.

Thank you very much!

> 
> 
> 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 :|
>