Re: [PATCH v2 16/24] migration: Split qemu_savevm_state_complete_precopy_non_iterable()

[Fabiano Rosas <farosas@suse.de>]

Peter Xu <peterx@redhat.com> writes:

> Split the function, making itself to be the helper to dump all non-iterable
> device states (early_vmsd excluded).  Move the precopy end logic out to the
> two callers that need it.
>
> With it, we can remove the in_postcopy parameter.  Meanwhile, renaming the
> function to be qemu_savevm_state_non_iterable(): we don't need the keyword
> "complete" because non-iterable doesn't iterate anyway, and we don't need
> precopy because we moved precopy specialties out.
>
> NOTE: this patch introduced one new migrate_get_current() user; will be
> removed in follow up patch.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  migration/savevm.h    |  3 +--
>  migration/migration.c |  7 +++++--
>  migration/savevm.c    | 12 ++++--------
>  3 files changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/migration/savevm.h b/migration/savevm.h
> index f957f851ef..57b96133d5 100644
> --- a/migration/savevm.h
> +++ b/migration/savevm.h
> @@ -74,8 +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_complete_precopy_non_iterable(QEMUFile *f,
> -        bool in_postcopy);
> +int qemu_savevm_state_non_iterable(QEMUFile *f);
>  
>  bool qemu_loadvm_load_state_buffer(const char *idstr, uint32_t instance_id,
>                                     char *buf, size_t len, Error **errp);
> diff --git a/migration/migration.c b/migration/migration.c
> index e3f1cc7b2e..c6e54d2a3f 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -2545,7 +2545,7 @@ static int postcopy_start(MigrationState *ms, Error **errp)
>       */
>      qemu_savevm_send_postcopy_listen(fb);
>  
> -    ret = qemu_savevm_state_complete_precopy_non_iterable(fb, true);
> +    ret = qemu_savevm_state_non_iterable(fb);
>      if (ret) {
>          error_setg(errp, "Postcopy save non-iterable device states failed");
>          goto fail_closefb;
> @@ -3678,9 +3678,12 @@ static void *bg_migration_thread(void *opaque)
>          goto fail;
>      }
>  
> -    if (qemu_savevm_state_complete_precopy_non_iterable(fb, false)) {
> +    if (qemu_savevm_state_non_iterable(fb)) {
>          goto fail;
>      }
> +
> +    qemu_savevm_state_end_precopy(s, fb);
> +
>      /*
>       * Since we are going to get non-iterable state data directly
>       * from s->bioc->data, explicit flush is needed here.
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 41560b97a4..e1918d4f38 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1691,8 +1691,7 @@ void qemu_savevm_state_end_precopy(MigrationState *s, QEMUFile *f)
>      qemu_savevm_state_vm_desc(s, f);
>  }
>  
> -int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
> -                                                    bool in_postcopy)
> +int qemu_savevm_state_non_iterable(QEMUFile *f)
>  {
>      MigrationState *ms = migrate_get_current();
>      int64_t start_ts_each, end_ts_each;
> @@ -1724,11 +1723,6 @@ int qemu_savevm_state_complete_precopy_non_iterable(QEMUFile *f,
>                                      end_ts_each - start_ts_each);
>      }
>  
> -    if (!in_postcopy) {
> -        /* Postcopy stream will still be going */
> -        qemu_savevm_state_end_precopy(ms, f);
> -    }
> -
>      trace_vmstate_downtime_checkpoint("src-non-iterable-saved");
>  
>      return 0;
> @@ -1743,11 +1737,13 @@ int qemu_savevm_state_complete_precopy(QEMUFile *f)
>          return ret;
>      }
>  
> -    ret = qemu_savevm_state_complete_precopy_non_iterable(f, false);
> +    ret = qemu_savevm_state_non_iterable(f);
>      if (ret) {
>          return ret;
>      }
>  
> +    qemu_savevm_state_end_precopy(migrate_get_current(), f);
> +
>      return qemu_fflush(f);
>  }

Reviewed-by: Fabiano Rosas <farosas@suse.de>