Re: [PATCH v8 03/27] migration: push Error **errp into qemu_loadvm_state_header()

[Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>]

On 2025/07/31 22:20, Arun Menon wrote:
> This is an incremental step in converting vmstate loading
> code to report error via Error objects instead of directly
> printing it to console/monitor.
> It is ensured that qemu_loadvm_state_header() must report an error
> in errp, in case of failure.
> 
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Arun Menon <armenon@redhat.com>
> ---
>   migration/savevm.c | 27 ++++++++++++++++++---------
>   1 file changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/migration/savevm.c b/migration/savevm.c
> index ab947620f724874f325fb9fb59bef50b7c16fb51..842ff3dc6d5ccb05f7d33cef9f7319b141419501 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -2814,35 +2814,44 @@ qemu_loadvm_section_part_end(QEMUFile *f, uint8_t type)
>       return 0;
>   }
>   
> -static int qemu_loadvm_state_header(QEMUFile *f)
> +static int qemu_loadvm_state_header(QEMUFile *f, Error **errp)
>   {
>       unsigned int v;
>       int ret;
>   
>       v = qemu_get_be32(f);
>       if (v != QEMU_VM_FILE_MAGIC) {
> -        error_report("Not a migration stream");
> +        error_setg(errp, "Not a migration stream, magic: %x != %x",
> +                   v, QEMU_VM_FILE_MAGIC);
>           return -EINVAL;
>       }
>   
>       v = qemu_get_be32(f);
>       if (v == QEMU_VM_FILE_VERSION_COMPAT) {
> -        error_report("SaveVM v2 format is obsolete and don't work anymore");
> +        error_setg(errp,
> +                   "SaveVM v2 format is obsolete and no longer supported, "
> +                   "file version %x != %x",
> +                   v, QEMU_VM_FILE_VERSION_COMPAT);
> +
>           return -ENOTSUP;
>       }
>       if (v != QEMU_VM_FILE_VERSION) {
> -        error_report("Unsupported migration stream version");
> +        error_setg(errp, "Unsupported migration stream version, "
> +                   "file version %x != %x",
> +                   v, QEMU_VM_FILE_VERSION);
>           return -ENOTSUP;
>       }
>   
>       if (migrate_get_current()->send_configuration) {
> -        if (qemu_get_byte(f) != QEMU_VM_CONFIGURATION) {
> -            error_report("Configuration section missing");
> +        v = qemu_get_byte(f);
> +        if (v != QEMU_VM_CONFIGURATION) {
> +            error_setg(errp, "Configuration section missing, %x != %x",
> +                       v, QEMU_VM_CONFIGURATION);
>               return -EINVAL;
>           }
> -        ret = vmstate_load_state(f, &vmstate_configuration, &savevm_state, 0,
> -                                 NULL);
>   
> +        ret = vmstate_load_state(f, &vmstate_configuration, &savevm_state, 0,
> +                                 errp);
>           if (ret) {
>               return ret;
>           }
> @@ -3119,7 +3128,7 @@ int qemu_loadvm_state(QEMUFile *f)
>   
>       qemu_loadvm_thread_pool_create(mis);
>   
> -    ret = qemu_loadvm_state_header(f);
> +    ret = qemu_loadvm_state_header(f, NULL);
>       if (ret) {

I have another comment: the error should be reported with 
error_report_err() or the messages converted from error_report() to 
error_setg() will be temporarily gone.

I'm sorry that I missed this in the last email (this and the problem I 
mentioned in the last email was there since v4 [the first version I got 
CCed] and I failed to notice them until now...)

>           return ret;
>       }
>