Re: [PATCH v7 5/5] migration: add MULTIFD_RECV_SYNC migration command

[Peter Xu <peterx@redhat.com>]

On Wed, Mar 05, 2025 at 04:51:00PM +0530, Prasad Pandit wrote:
> Hi,
> 
> On Tue, 4 Mar 2025 at 20:05, Peter Xu <peterx@redhat.com> wrote:
> > I think we need the header, the ram is a module.
> > Do similarly like qemu_savevm_state_complete_precopy_iterable() but do
> > whatever a vmstate hander wants, so it'll be with a header.
> 
> * I don't fully see yet how this shall work.

Another option is add another event for precopy_notifier_list, it can be
PRECOPY_NOTIFY_SWITCH_POSTCOPY, then make RAM register to it, send the
header itself, and do the flush and sync.

Let me know if you want me to write the patches.  That's almost the only
thing left to make it clearer..

> 
> > Please consider adding details like "we need message AAA on BBB channel to
> > serialize with CCC" and details.  Not asking that as required to merge, but
> > my understanding is that that's what is missing and that's why none of yet
> > versions can make sure of it in code.  Maybe that'll help you to understand
> > how that was serialized.
> 
> * Okay, will try.
> 
> >
> > MULTIFD_SYNC_LOCAL will not invoke MULTIFD_FLAG_SYNC, which we need.
> ...
> > So is it your intention to not send MULTIFD_FLAG_SYNC above?
> > In all cases, I still think that's not the right way to do.
> 
> * It makes little difference; MULTIFD_FLAG_SYNC is also used to
> increase 'multifd_recv_state->sem_sync' semaphore on the destination
> side, which then unblocks the 'main' thread waiting on it.

AFAIU that's the whole difference and whole point of doing such..

> ===
> diff --git a/migration/migration.c b/migration/migration.c
> index 65fc4f5eed..d8c4ea0ad1 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -3402,7 +3402,7 @@ static MigIterateState
> migration_iteration_run(MigrationState *s)
>          && can_switchover && qatomic_read(&s->start_postcopy)) {
>          if (migrate_multifd()) {
>              multifd_send_flush();
> -            multifd_send_sync_main(MULTIFD_SYNC_LOCAL);
> +            multifd_send_sync_main(MULTIFD_SYNC_ALL);
>              qemu_savevm_send_multifd_recv_sync(s->to_dst_file);
>              multifd_send_shutdown();
>          }
> diff --git a/migration/multifd.c b/migration/multifd.c
> index 8928ca2611..2b5bc2d478 100644
> --- a/migration/multifd.c
> +++ b/migration/multifd.c
> @@ -1265,7 +1265,7 @@ static void *multifd_recv_thread(void *opaque)
> 
>      rcu_unregister_thread();
>      trace_multifd_recv_thread_end(p->id, p->packets_recved);
> -    qemu_sem_post(&multifd_recv_state->sem_sync);
> +//  qemu_sem_post(&multifd_recv_state->sem_sync);
> 
>      return NULL;
>  }
> ===
> host-1] 63/63 qemu:qtest+qtest-x86_64 / qtest-x86_64/migration-test
>              OK             159.46s   79 subtests passed
> host-2] 63/63 qemu:qtest+qtest-x86_64 / qtest-x86_64/migration-test
>              OK             164.55s   79 subtests passed
> ===
> 
> * I tried the above patch and it also works the same. I'll use this, no issues.

We can't introduce a migration global cmd just to work the same as
RAM_SAVE_FLAG_MULTIFD_FLUSH, which is a sub-cmd for a module.

> 
> Thank you.
> ---
>   - Prasad
> 

-- 
Peter Xu