[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 01/33] migration: push Error **errp into qemu_loadvm_state()
From: |
Daniel P . Berrangé |
Subject: |
Re: [PATCH 01/33] migration: push Error **errp into qemu_loadvm_state() |
Date: |
Thu, 11 Mar 2021 12:38:30 +0000 |
User-agent: |
Mutt/2.0.5 (2021-01-21) |
On Fri, Feb 05, 2021 at 10:35:28AM +0100, Philippe Mathieu-Daudé wrote:
> On Fri, Feb 5, 2021 at 10:33 AM Daniel P. Berrangé <berrange@redhat.com>
> wrote:
> > 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.
>
> Fair. Ignore my other same comments in this. R-b tag stands.
On further investigation you are right. Not using error_setg_errno
has caused a regression in error quality as shown by Dave in a later
patch in this series.
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 :|
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [PATCH 01/33] migration: push Error **errp into qemu_loadvm_state(),
Daniel P . Berrangé <=