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

["Marc-André Lureau" <marcandre.lureau@redhat.com>]

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

Hi

On Mon, Jul 21, 2025 at 3:30 PM Arun Menon <armenon@redhat.com> 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.
>
> Signed-off-by: Arun Menon <armenon@redhat.com>
> ---
>  migration/savevm.c | 25 ++++++++++++++++---------
>  1 file changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/migration/savevm.c b/migration/savevm.c
> index
> ab947620f724874f325fb9fb59bef50b7c16fb51..162fb05933fae5993eeef107811f97cb08726ac3
> 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -2814,35 +2814,42 @@ 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",
>

As Akihiko said in patch 2 review, better "not to have line breaks in
string literals" .. here, there is a missing space now.

+                   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,"
>

here too


> +                       "%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 +3126,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) {
>          return ret;
>      }
>
> --
> 2.50.0
>
>

[-- Attachment #2: Type: text/html, Size: 4436 bytes --]