Re: [PATCH 01/33] migration: push Error **errp into qemu_loadvm_state()

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

On Thu, Feb 04, 2021 at 10:57:20PM +0100, Philippe Mathieu-Daudé wrote:
> On 2/4/21 6:18 PM, Daniel P. Berrangé wrote:
> > This is an incremental step in converting vmstate loading code to report
> > via Error objects instead of printing directly to the console/monitor.
> > 
> > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> > ---
> >  migration/migration.c |  4 ++--
> >  migration/savevm.c    | 36 ++++++++++++++++++++----------------
> >  migration/savevm.h    |  2 +-
> >  3 files changed, 23 insertions(+), 19 deletions(-)
> ...
> 
> > diff --git a/migration/savevm.c b/migration/savevm.c
> > index 6b320423c7..c8d93eee1e 100644
> > --- a/migration/savevm.c
> > +++ b/migration/savevm.c
> > @@ -2638,40 +2638,49 @@ out:
> >      return ret;
> >  }
> >  
> > -int qemu_loadvm_state(QEMUFile *f)
> > +int qemu_loadvm_state(QEMUFile *f, Error **errp)
> >  {
> >      MigrationIncomingState *mis = migration_incoming_get_current();
> > -    Error *local_err = NULL;
> >      int ret;
> >  
> > -    if (qemu_savevm_state_blocked(&local_err)) {
> > -        error_report_err(local_err);
> > -        return -EINVAL;
> > +    if (qemu_savevm_state_blocked(errp)) {
> > +        return -1;
> >      }
> >  
> >      ret = qemu_loadvm_state_header(f);
> >      if (ret) {
> > -        return ret;
> > +        error_setg(errp, "Error %d while loading VM state", ret);
> 
> Using error_setg_errno() instead (multiple occurences):

I don't think we want todo that in general, because the code is
already not reliable at actually returning an errno value, sometimes
returning just "-1". At the end of this series it will almost always
be returning "-1", not an errno.  There are some places where an
errno is relevant though - specificially qemu_get_file_error calls.

> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> 

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