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

[Prasad Pandit <ppandit@redhat.com>]

Hello Peter,

On Mon, 3 Mar 2025 at 20:20, Peter Xu <peterx@redhat.com> wrote:
> We need the header.

* We need a section type, which is sent by qemu_savevm_command_send()
as 'QEMU_VM_COMMAND'.

>  Maybe the easiest as of now is one more hook like
> qemu_savevm_state_complete_precopy_prepare(), and only use it in RAM as of
> now.

* What will this helper do?

> > * But earlier we discussed 'flush and sync' is enough for that, no?
>
> Yes it's ok I think, but this patch didn't do that.
>
> +            multifd_send_flush();
> +            multifd_send_sync_main(MULTIFD_SYNC_LOCAL);
> +            qemu_savevm_send_multifd_recv_sync(s->to_dst_file);
>
> I don't think it sent RAM_SAVE_FLAG_MULTIFD_FLUSH.  IIUC you need the
> complete multifd_ram_flush_and_sync(), and the new message not needed.

* If we look at multifd_ram_flush_and_sync(), it does:
     1. multifd_send()                       <= this patch does it via
multifd_send_flush()
     2. multifd_send_sync_main()    <= this patch also calls it above
     3. send RAM_SAVE_FLAG_MULTIFD_FLUSH  <= this patch sends
MIG_CMD_MULTIFD_RECV_SYNC

* What is missing?

> Instead of I prepare the patch and whole commit message, please take your
> time and think about it, justify it, and if you also think it works put
> explanation into commit message and then we can go with it.

* The commit message does explain about flush and sync and how the
migration command helps. What else do we need to add?

> > * And multifd_recv_sync_main() function on the destination blocks the
> > 'main' thread until all multfd_recv_threads (mig/dst/recv_x) have
> > exited, only then it proceeds to accept the incoming new postcopy
> > connection.
>
> I don't think it makes sure threads have exited,

* 'multifd_recv_sync_main()' blocks the main thread on
'multifd_recv_state->sem_sync' semaphore. It is increased when
multifd_recv threads exit due to the shutdown message. ie. the 'main'
thread unblocks when all 'mig/dst/recv_x' threads have exited.

Thank you.
---
  - Prasad