Re: [PATCH 03/33] migration: push Error **errp into qemu_loadvm_state_setup()

[Markus Armbruster]

Daniel P. Berrangé <berrange@redhat.com> writes:

> 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/savevm.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 870199b629..f4ed14a230 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -2490,7 +2490,7 @@ static int qemu_loadvm_state_header(QEMUFile *f, Error 
> **errp)
>      return 0;
>  }
>  
> -static int qemu_loadvm_state_setup(QEMUFile *f)
> +static int qemu_loadvm_state_setup(QEMUFile *f, Error **errp)
>  {
>      SaveStateEntry *se;
>      int ret;
> @@ -2509,7 +2509,7 @@ static int qemu_loadvm_state_setup(QEMUFile *f)
>          ret = se->ops->load_setup(f, se->opaque);
>          if (ret < 0) {
>              qemu_file_set_error(f, ret);
> -            error_report("Load state of device %s failed", se->idstr);
> +            error_setg(errp, "Load state of device %s failed", se->idstr);
>              return ret;
>          }
>      }
> @@ -2656,8 +2656,7 @@ int qemu_loadvm_state(QEMUFile *f, Error **errp)
>          return -1;
>      }
>  
> -    if (qemu_loadvm_state_setup(f) != 0) {
> -        error_setg(errp, "Error %d while loading VM state", -EINVAL);
> +    if (qemu_loadvm_state_setup(f, errp) < 0) {
>          return -1;
>      }

Drive-by remark, *not* a demand: I don't like "0 on success, -1 on
failure".  When we return just one value on success and one on failure,
I prefer true and false.  Negative value on failure is of course fine
for returning error codes, and were we want to return arbitrary
non-negative values on success.