Re: [PATCH 06/14] migration: Introduce stopcopy_bytes in save_query_pending()

[Juraj Marcin <jmarcin@redhat.com>]

On 2026-04-16 13:20, Peter Xu wrote:
> On Thu, Apr 09, 2026 at 07:36:51PM +0200, Juraj Marcin wrote:
> > Hi Peter,
> > 
> > actually, I do have one question, see inline
> 
> [...]
> 
> > shouldn't also the condition that triggers postcopy migration be updated?
> > As total_bytes is calculated as sum of all three
> > (precopy_bytes + stopcopy_bytes + postcopy_bytes), this implies to me
> > that stopcopy_bytes is not subset of precopy_bytes and would also need
> > to be migrated during switchover before postcopy.
> 
> For now it shouldn't matter when VFIO never works with postcpoy yet, but
> it's a good point. We'd better make it right from the start.
> 
> When looking at this, I also found we may be reporting wrong things in the
> query results when pmem is available on postcopy bits, it's about when this
> hits:
> 
> static bool ram_has_postcopy(void *opaque)
> {
>     RAMBlock *rb;
>     RAMBLOCK_FOREACH_NOT_IGNORED(rb) {
>         if (ram_block_is_pmem(rb)) {
>             info_report("Block: %s, host: %p is a nvdimm memory, postcopy"
>                          "is not supported now!", rb->idstr, rb->host);
>             return false;
>         }
>     }
> 
>     return migrate_postcopy_ram();
> }
> 
> So I think we should also report differently based on whether pmem is
> present in ramblocks.. IOW, I think the module should make sure its
> save_query_pending() to match its has_postcopy() when they're both present.
> Or, maybe we don't even need has_postcopy()..

Yeah, it looks like ram_state_pending() should use ram_has_postcopy()
instead of just migrate_postcopy_ram().

> 
> If it's a problem, it should be an old problem.  Let me address the
> comments so far on this patch, so a fixup planned to be squashed (I also
> added the trace parameter Avihai requested), feel free to comment before I
> repost, thanks.
> 
> From 594b85b66b2d1abd9a38fae4051e01ffc73aa8ff Mon Sep 17 00:00:00 2001
> From: Peter Xu <peterx@redhat.com>
> Date: Thu, 16 Apr 2026 13:09:16 -0400
> Subject: [PATCH] fixup! migration: Introduce stopcopy_bytes in
>  save_query_pending()
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>  migration/migration.c  | 13 +++++++++++--
>  migration/savevm.c     |  3 ++-
>  migration/trace-events |  2 +-
>  3 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index c2aa145106..62299ff3c0 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -3276,6 +3276,16 @@ static void migration_iteration_go_next(MigPendingData *pending)
>      }
>  }
>  
> +static bool postcopy_should_start(MigrationState *s, MigPendingData *pending)
> +{
> +    /* If postcopy's switchver will violate user specified downtime, stop */
> +    if (pending->precopy_bytes + pending->stopcopy_bytes > s->threshold_size) {
> +        return false;
> +    }
> +
> +    return qatomic_read(&s->start_postcopy);
> +}
> +
>  /*
>   * Return true if continue to the next iteration directly, false
>   * otherwise.
> @@ -3323,8 +3333,7 @@ static MigIterateState migration_iteration_run(MigrationState *s)
>          }
>  
>          /* Should we switch to postcopy now? */
> -        if (pending.precopy_bytes <= s->threshold_size &&
> -            can_switchover && qatomic_read(&s->start_postcopy)) {
> +        if (can_switchover && postcopy_should_start(s, &pending)) {
>              if (postcopy_start(s, &local_err)) {
>                  migrate_error_propagate(s, error_copy(local_err));
>                  error_report_err(local_err);
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 1d3fce45b9..7f38be0ee1 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -1804,7 +1804,8 @@ void qemu_savevm_query_pending(MigPendingData *pending, bool exact)
>  
>      trace_qemu_savevm_query_pending(exact, pending->precopy_bytes,
>                                      pending->stopcopy_bytes,
> -                                    pending->postcopy_bytes);
> +                                    pending->postcopy_bytes,
> +                                    pending->total_bytes);
>  }
>  
>  void qemu_savevm_state_cleanup(MigrationState *s)
> diff --git a/migration/trace-events b/migration/trace-events
> index 2f86ad448e..d2134af862 100644
> --- a/migration/trace-events
> +++ b/migration/trace-events
> @@ -7,7 +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 exact, uint64_t precopy, uint64_t stopcopy, uint64_t postcopy) "exact=%d, precopy=%"PRIu64", stopcopy=%"PRIu64", postcopy=%"PRIu64
> +qemu_savevm_query_pending(bool exact, uint64_t precopy, uint64_t stopcopy, uint64_t postcopy, uint64_t total) "exact=%d, precopy=%"PRIu64", stopcopy=%"PRIu64", postcopy=%"PRIu64", total=%"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.53.0

The fixup looks good, thanks!

> 
> 
> -- 
> Peter Xu
>