Re: [PATCH RFC 06/12] migration: Use the new save_query_pending() API directly

[Juraj Marcin <jmarcin@redhat.com>]

On 2026-03-19 19:12, Peter Xu wrote:
> It's easier to use the new API directly in the migration iterations.  This
> also paves way for follow up patches to add new data to report directly to
> the iterator function.
> 
> When at it, merge the tracepoints too into one.
> 
> No functional change intended.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  migration/savevm.h     |  4 ----
>  migration/migration.c  | 16 +++++++---------
>  migration/savevm.c     | 23 ++---------------------
>  migration/trace-events |  1 +
>  4 files changed, 10 insertions(+), 34 deletions(-)
> 
> diff --git a/migration/savevm.h b/migration/savevm.h
> index b116933bce..3c39190fa4 100644
> --- a/migration/savevm.h
> +++ b/migration/savevm.h
> @@ -46,10 +46,6 @@ void qemu_savevm_state_cleanup(void);
>  void qemu_savevm_state_complete_postcopy(QEMUFile *f);
>  int qemu_savevm_state_complete_precopy(MigrationState *s);
>  void qemu_savevm_query_pending(MigPendingData *pending, bool fastpath);
> -void qemu_savevm_state_pending_exact(uint64_t *must_precopy,
> -                                     uint64_t *can_postcopy);
> -void qemu_savevm_state_pending_estimate(uint64_t *must_precopy,
> -                                        uint64_t *can_postcopy);
>  int qemu_savevm_state_complete_precopy_iterable(QEMUFile *f, bool in_postcopy);
>  bool qemu_savevm_state_postcopy_prepare(QEMUFile *f, Error **errp);
>  void qemu_savevm_state_end(QEMUFile *f);
> diff --git a/migration/migration.c b/migration/migration.c
> index dfc60372cf..99c4d09000 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -3204,17 +3204,17 @@ typedef enum {
>   */
>  static MigIterateState migration_iteration_run(MigrationState *s)
>  {
> -    uint64_t must_precopy, can_postcopy, pending_size;
>      Error *local_err = NULL;
>      bool in_postcopy = (s->state == MIGRATION_STATUS_POSTCOPY_DEVICE ||
>                          s->state == MIGRATION_STATUS_POSTCOPY_ACTIVE);
>      bool can_switchover = migration_can_switchover(s);
> +    MigPendingData pending = { };
> +    uint64_t pending_size;
>      bool complete_ready;
>  
>      /* Fast path - get the estimated amount of pending data */
> -    qemu_savevm_state_pending_estimate(&must_precopy, &can_postcopy);
> -    pending_size = must_precopy + can_postcopy;
> -    trace_migrate_pending_estimate(pending_size, must_precopy, can_postcopy);
> +    qemu_savevm_query_pending(&pending, true);
> +    pending_size = pending.precopy_bytes + pending.postcopy_bytes;
>  
>      if (in_postcopy) {
>          /*
> @@ -3243,14 +3243,12 @@ static MigIterateState migration_iteration_run(MigrationState *s)
>           * during postcopy phase.
>           */
>          if (pending_size <= s->threshold_size) {
> -            qemu_savevm_state_pending_exact(&must_precopy, &can_postcopy);
> -            pending_size = must_precopy + can_postcopy;
> -            trace_migrate_pending_exact(pending_size, must_precopy,
> -                                        can_postcopy);
> +            qemu_savevm_query_pending(&pending, false);
> +            pending_size = pending.precopy_bytes + pending.postcopy_bytes;
>          }
>  
>          /* Should we switch to postcopy now? */
> -        if (must_precopy <= s->threshold_size &&
> +        if (pending.precopy_bytes <= s->threshold_size &&
>              can_switchover && qatomic_read(&s->start_postcopy)) {
>              if (postcopy_start(s, &local_err)) {
>                  migrate_error_propagate(s, error_copy(local_err));
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 6268e68382..b3285d480f 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1779,28 +1779,9 @@ void qemu_savevm_query_pending(MigPendingData *pending, bool fastpath)
>          }
>          se->ops->save_query_pending(se->opaque, pending);
>      }
> -}
> -
> -void qemu_savevm_state_pending_estimate(uint64_t *must_precopy,
> -                                        uint64_t *can_postcopy)
> -{
> -    MigPendingData pending;
> -
> -    qemu_savevm_query_pending(&pending, true);
> -
> -    *must_precopy = pending.precopy_bytes;
> -    *can_postcopy = pending.postcopy_bytes;
> -}
> -
> -void qemu_savevm_state_pending_exact(uint64_t *must_precopy,
> -                                     uint64_t *can_postcopy)
> -{
> -    MigPendingData pending;
> -
> -    qemu_savevm_query_pending(&pending, false);
>  
> -    *must_precopy = pending.precopy_bytes;
> -    *can_postcopy = pending.postcopy_bytes;
> +    trace_qemu_savevm_query_pending(fastpath, pending->precopy_bytes,
> +                                    pending->postcopy_bytes);
>  }
>  
>  void qemu_savevm_state_cleanup(void)
> diff --git a/migration/trace-events b/migration/trace-events
> index 60e5087e38..5f836a8652 100644
> --- a/migration/trace-events
> +++ b/migration/trace-events
> @@ -7,6 +7,7 @@ qemu_loadvm_state_section_partend(uint32_t section_id) "%u"
>  qemu_loadvm_state_post_main(int ret) "%d"
>  qemu_loadvm_state_section_startfull(uint32_t section_id, const char *idstr, uint32_t instance_id, uint32_t version_id) "%u(%s) %u %u"
>  qemu_savevm_send_packaged(void) ""
> +qemu_savevm_query_pending(bool fast, uint64_t precopy, uint64_t postcopy) "fast=%d, precopy=%"PRIu64", postcopy=%"PRIu64

Should probably remove these two trace events, or is keeping them
intentional?

    migrate_pending_exact(uint64_t size, uint64_t pre, uint64_t post) "exact pending size %" PRIu64 " (pre = %" PRIu64 " post=%" PRIu64 ")"
    migrate_pending_estimate(uint64_t size, uint64_t pre, uint64_t post) "estimate pending size %" PRIu64 " (pre = %" PRIu64 " post=%" PRIu64 ")"

>  loadvm_state_switchover_ack_needed(unsigned int switchover_ack_pending_num) "Switchover ack pending num=%u"
>  loadvm_state_setup(void) ""
>  loadvm_state_cleanup(void) ""
> -- 
> 2.50.1
> 

With unused trace events comment resolved,

Reviewed-by: Juraj Marcin <jmarcin@redhat.com>