Re: [Qemu-devel] [PATCH 12/18] savevm: add error parameter to qemu_loadvm_state()

[Pavel Hrdina]

On 08/30/2012 07:09 PM, Luiz Capitulino wrote:
> On Wed, 15 Aug 2012 09:41:53 +0200
> Pavel Hrdina <address@hidden> wrote:
>
> > Signed-off-by: Pavel Hrdina <address@hidden>
> > ---
> >   migration.c |  2 +-
> >   savevm.c    | 44 ++++++++++++++++++++++++++++----------------
> >   sysemu.h    |  3 ++-
> >   3 files changed, 31 insertions(+), 18 deletions(-)
> >
> > diff --git a/migration.c b/migration.c
> > index ec2f267..f048faf 100644
> > --- a/migration.c
> > +++ b/migration.c
> > @@ -88,7 +88,7 @@ int qemu_start_incoming_migration(const char *uri, Error 
> > **errp)
> >   
> >   void process_incoming_migration(QEMUFile *f)
> >   {
> > -    if (qemu_loadvm_state(f) < 0) {
> > +    if (qemu_loadvm_state(f, NULL) < 0) {
> >           fprintf(stderr, "load of migration failed\n");
> >           exit(0);
> >       }
> > diff --git a/savevm.c b/savevm.c
> > index 0d54115..500eb72 100644
> > --- a/savevm.c
> > +++ b/savevm.c
> > @@ -1916,7 +1916,8 @@ typedef struct LoadStateEntry {
> >       int version_id;
> >   } LoadStateEntry;
> >   
> > -int qemu_loadvm_state(QEMUFile *f)
> > +int qemu_loadvm_state(QEMUFile *f,
> > +                      Error **errp)
> >   {
> >       QLIST_HEAD(, LoadStateEntry) loadvm_handlers =
> >           QLIST_HEAD_INITIALIZER(loadvm_handlers);
> > @@ -1925,21 +1926,26 @@ int qemu_loadvm_state(QEMUFile *f)
> >       unsigned int v;
> >       int ret;
> >   
> > -    if (qemu_savevm_state_blocked(NULL)) {
> > -        return -EINVAL;
> > +    if (qemu_savevm_state_blocked(errp)) {
> > +        return -ENOTSUP;
> >       }
> >   
> >       v = qemu_get_be32(f);
> > -    if (v != QEMU_VM_FILE_MAGIC)
> > +    if (v != QEMU_VM_FILE_MAGIC) {
> > +        error_set(errp, ERROR_CLASS_GENERIC_ERROR,
> > +                  "Unknown vm-state file magic");
> >           return -EINVAL;
> > +    }
> >   
> >       v = qemu_get_be32(f);
> >       if (v == QEMU_VM_FILE_VERSION_COMPAT) {
> > -        fprintf(stderr, "SaveVM v2 format is obsolete and don't work 
> > anymore\n");
> > +        error_set(errp, QERR_NOT_SUPPORTED);
> >           return -ENOTSUP;
> >       }
> > -    if (v != QEMU_VM_FILE_VERSION)
> > +    if (v != QEMU_VM_FILE_VERSION) {
> > +        error_set(errp, QERR_NOT_SUPPORTED);
> >           return -ENOTSUP;
> > +    }
> >   
> >       while ((section_type = qemu_get_byte(f)) != QEMU_VM_EOF) {
> >           uint32_t instance_id, version_id, section_id;
> > @@ -1961,15 +1967,18 @@ int qemu_loadvm_state(QEMUFile *f)
> >               /* Find savevm section */
> >               se = find_se(idstr, instance_id);
> >               if (se == NULL) {
> > -                fprintf(stderr, "Unknown savevm section or instance '%s' 
> > %d\n", idstr, instance_id);
> > +                error_set(errp, ERROR_CLASS_GENERIC_ERROR,
> > +                          "Unknown savevm section or instance '%s' %d",
> > +                          idstr, instance_id);
> >                   ret = -EINVAL;
> >                   goto out;
> >               }
> >   
> >               /* Validate version */
> >               if (version_id > se->version_id) {
> > -                fprintf(stderr, "savevm: unsupported version %d for '%s' 
> > v%d\n",
> > -                        version_id, idstr, se->version_id);
> > +                error_set(errp, ERROR_CLASS_GENERIC_ERROR,
> > +                          "savevm: unsupported version %d for '%s' v%d",
> > +                          version_id, idstr, se->version_id);
> >                   ret = -EINVAL;
> >                   goto out;
> >               }
> > @@ -1984,8 +1993,7 @@ int qemu_loadvm_state(QEMUFile *f)
> >   
> >               ret = vmstate_load(f, le->se, le->version_id);
> >               if (ret < 0) {
> > -                fprintf(stderr, "qemu: warning: error while loading state for 
> > instance 0x%x of device '%s'\n",
> > -                        instance_id, idstr);
> > +                error_set(errp, QERR_GENERIC_ERROR, ret);
> >                   goto out;
> >               }
> >               break;
> > @@ -1999,20 +2007,21 @@ int qemu_loadvm_state(QEMUFile *f)
> >                   }
> >               }
> >               if (le == NULL) {
> > -                fprintf(stderr, "Unknown savevm section %d\n", section_id);
> > +                error_set(errp, ERROR_CLASS_GENERIC_ERROR,
> > +                          "Unknown savevm section %d", section_id);
> You sure that this error message will be printed to the terminal? This has
> to be done by the caller.

While loading snapshot yes, but while migrating no. I think, that we 
should rewrite migration error propagation too.
> >                   ret = -EINVAL;
> >                   goto out;
> >               }
> >   
> >               ret = vmstate_load(f, le->se, le->version_id);
> >               if (ret < 0) {
> > -                fprintf(stderr, "qemu: warning: error while loading state section 
> > id %d\n",
> > -                        section_id);
> > +                error_set(errp, QERR_GENERIC_ERROR, ret);
> >                   goto out;
> >               }
> >               break;
> >           default:
> > -            fprintf(stderr, "Unknown savevm section type %d\n", section_type);
> > +            error_set(errp, ERROR_CLASS_GENERIC_ERROR,
> > +                      "Unknown savevm section type %d", section_type);
> >               ret = -EINVAL;
> >               goto out;
> >           }
> > @@ -2030,6 +2039,9 @@ out:
> >   
> >       if (ret == 0) {
> >           ret = qemu_file_get_error(f);
> > +        if (ret < 0) {
> > +            error_set(errp, QERR_GENERIC_ERROR, ret);
> > +        }
> >       }
> >   
> >       return ret;
> > @@ -2297,7 +2309,7 @@ int load_vmstate(const char *name)
> >       }
> >   
> >       qemu_system_reset(VMRESET_SILENT);
> > -    ret = qemu_loadvm_state(f);
> > +    ret = qemu_loadvm_state(f, NULL);
> >   
> >       qemu_fclose(f);
> >       if (ret < 0) {
> > diff --git a/sysemu.h b/sysemu.h
> > index 35d6d67..651d96e 100644
> > --- a/sysemu.h
> > +++ b/sysemu.h
> > @@ -85,7 +85,8 @@ int qemu_savevm_state_iterate(QEMUFile *f,
> >   int qemu_savevm_state_complete(QEMUFile *f,
> >                                  Error **errp);
> >   void qemu_savevm_state_cancel(QEMUFile *f);
> > -int qemu_loadvm_state(QEMUFile *f);
> > +int qemu_loadvm_state(QEMUFile *f,
> > +                      Error **errp);
> >   
> >   /* SLIRP */
> >   void do_info_slirp(Monitor *mon);