Re: [PATCH v2 22/24] migration: Make qemu_savevm_state_non_iterable() take errp

[Fabiano Rosas <farosas@suse.de>]

Peter Xu <peterx@redhat.com> writes:

> Let the function report errors to upper layers.  Out of three current
> users, two of them already process the errors, except one outlier,
> qemu_savevm_state_complete_precopy(), where we do it manually for now with
> a comment for TODO.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  migration/savevm.h    |  2 +-
>  migration/migration.c |  8 ++++----
>  migration/savevm.c    | 13 +++++++------
>  3 files changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/migration/savevm.h b/migration/savevm.h
> index f2750eca09..6a589b2990 100644
> --- a/migration/savevm.h
> +++ b/migration/savevm.h
> @@ -74,7 +74,7 @@ int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis,
>                             Error **errp);
>  int qemu_load_device_state(QEMUFile *f, Error **errp);
>  int qemu_loadvm_approve_switchover(void);
> -int qemu_savevm_state_non_iterable(QEMUFile *f);
> +int qemu_savevm_state_non_iterable(QEMUFile *f, Error **errp);
>  int qemu_savevm_state_non_iterable_early(QEMUFile *f,
>                                           JSONWriter *vmdesc,
>                                           Error **errp);
> diff --git a/migration/migration.c b/migration/migration.c
> index ddf6faab46..22cfa5f19f 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -2545,9 +2545,9 @@ static int postcopy_start(MigrationState *ms, Error **errp)
>       */
>      qemu_savevm_send_postcopy_listen(fb);
>  
> -    ret = qemu_savevm_state_non_iterable(fb);
> +    ret = qemu_savevm_state_non_iterable(fb, errp);
>      if (ret) {
> -        error_setg(errp, "Postcopy save non-iterable device states failed");
> +        error_prepend(errp, "Postcopy save non-iterable states failed: ");
>          goto fail_closefb;
>      }
>  
> @@ -3675,8 +3675,8 @@ static void *bg_migration_thread(void *opaque)
>          goto fail_with_bql;
>      }
>  
> -    if (qemu_savevm_state_non_iterable(fb)) {
> -        error_setg(&local_err, "Failed to save non-iterable devices");
> +    if (qemu_savevm_state_non_iterable(fb, &local_err)) {
> +        error_prepend(&local_err, "Failed to save non-iterable devices");

No space or punctuation at the end?

>          goto fail_with_bql;
>      }
>  
> diff --git a/migration/savevm.c b/migration/savevm.c
> index c16951b532..130b9764a7 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1710,13 +1710,12 @@ void qemu_savevm_state_end_precopy(MigrationState *s, QEMUFile *f)
>      qemu_savevm_state_vm_desc(s, f);
>  }
>  
> -int qemu_savevm_state_non_iterable(QEMUFile *f)
> +int qemu_savevm_state_non_iterable(QEMUFile *f, Error **errp)
>  {
>      MigrationState *ms = migrate_get_current();
>      int64_t start_ts_each, end_ts_each;
>      JSONWriter *vmdesc = ms->vmdesc;
>      SaveStateEntry *se;
> -    Error *local_err = NULL;
>      int ret;
>  
>      /* Making sure cpu states are synchronized before saving non-iterable */
> @@ -1730,10 +1729,8 @@ int qemu_savevm_state_non_iterable(QEMUFile *f)
>  
>          start_ts_each = qemu_clock_get_us(QEMU_CLOCK_REALTIME);
>  
> -        ret = vmstate_save(f, se, vmdesc, &local_err);
> +        ret = vmstate_save(f, se, vmdesc, errp);
>          if (ret) {
> -            migrate_error_propagate(ms, error_copy(local_err));
> -            error_report_err(local_err);
>              return ret;
>          }
>  
> @@ -1750,6 +1747,7 @@ int qemu_savevm_state_non_iterable(QEMUFile *f)
>  int qemu_savevm_state_complete_precopy(MigrationState *s)
>  {
>      QEMUFile *f = s->to_dst_file;
> +    Error *local_err = NULL;
>      int ret;
>  
>      ret = qemu_savevm_state_complete_precopy_iterable(f, false);
> @@ -1757,8 +1755,11 @@ int qemu_savevm_state_complete_precopy(MigrationState *s)
>          return ret;
>      }
>  
> -    ret = qemu_savevm_state_non_iterable(f);
> +    /* TODO: pass error upper */
> +    ret = qemu_savevm_state_non_iterable(f, &local_err);
>      if (ret) {
> +        migrate_error_propagate(s, error_copy(local_err));
> +        error_report_err(local_err);
>          return ret;
>      }