Re: [Qemu-devel] [PATCH 06/11] savevm: update error reporting for qemu_loadvm_state()

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1739 bytes --]

On 04/16/2013 10:05 AM, Pavel Hrdina wrote:
> Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
> ---
>  include/sysemu/sysemu.h |  2 +-
>  migration.c             |  9 +++----
>  savevm.c                | 64 ++++++++++++++++++++++++-------------------------
>  3 files changed, 37 insertions(+), 38 deletions(-)
> 

> +++ b/migration.c
> @@ -93,12 +93,13 @@ void qemu_start_incoming_migration(const char *uri, Error **errp)
>  static void process_incoming_migration_co(void *opaque)
>  {
>      QEMUFile *f = opaque;
> -    int ret;
> +    Error *local_err = NULL;
>  
> -    ret = qemu_loadvm_state(f);
> +    qemu_loadvm_state(f, &local_err);
>      qemu_fclose(f);
> -    if (ret < 0) {
> -        fprintf(stderr, "load of migration failed\n");
> +    if (error_is_set(&local_err)) {
> +        fprintf(stderr, "%s\n", error_get_pretty(local_err));
> +        error_free(local_err);
>          exit(0);

Pre-existing, but WHY are we exiting with successful status even when
incoming migration failed?  This feels SOOOO wrong.  Exiting non-zero
would at least give management apps like libvirt a reason to inspect the
output for the error message.

>      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");
> -        return -ENOTSUP;
> +        error_setg(errp, "saveVM v2 format is obsolete and don't work anymore");

As long as you are touching this, fix the grammar:

s/don't/doesn't/

As my findings were pre-existing problems:

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]