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

["Daniel P. Berrangé" <berrange@redhat.com>]

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()


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