Re: [PATCH V2 13/13] migration: cpr-transfer mode

[Peter Xu <peterx@redhat.com>]

On Tue, Oct 08, 2024 at 03:28:30PM -0300, Fabiano Rosas wrote:
> >>> +    /* Close cpr socket to tell source that we are listening */
> >>> +    cpr_state_close();
> >> 
> >> Would it be possible to use some explicit reply message to mark this?  
> >
> > In theory yes, but I fear that using a return channel with message parsing and
> > dispatch adds more code than it is worth.
> 
> I think this approach is fine for now, but I wonder whether we could
> reuse the current return path (RP) by starting it earlier and take
> benefit from it already having the message passing infrastructure in
> place. I'm actually looking ahead to the migration handshake thread[1],
> which could be thought to have some similarity with the early cpr
> channel. So having a generic channel in place early on to handle
> handshake, CPR, RP, etc. could be a good idea.

The current design relies on CPR stage happens before device realize()s, so
I assume migration channel (including RP) isn't easily applicable at as
early as this stage.

However I think dest qemu can directly write back to the cpr_uri channel
instead if we want and then follow a protocol simple enough (even though
it'll be separate from the migration stream protocol).

What worries me more (besides using HUP as of now..) is cpr_state_save() is
currently synchronous and can block the main iothread.  It means if cpr
destination is not properly setup, it can hang the main thread (including
e.g. QMP monitor) at qio_channel_socket_connect_sync().  Ideally we
shouldn't block the main thread.

If async-mode can be done, it might be even easier, e.g. if we do
cpr_state_save() in a thread, after qemu_put*() we can directly qemu_get*()
in the same context with the pairing return qemufile.

But maybe we can do it in two steps, merging HUP first.  Then when a better
protocol (plus async mode) ready, one can boost QEMU_CPR_FILE_VERSION.
I'll see how Steve wants to address it.

> 
> Anyway, I'm probing on this a bit so I can start drafting something. I
> got surprised that we don't even have the capability bits in the stream
> in a useful way (currently, configuration_validate_capabilities() does
> kind of nothing).
> 
> 1- https://wiki.qemu.org/ToDo/LiveMigration#Migration_handshake

Happy to know this. I was thinking whether I should work on this even
earlier, so if you're looking at that it'll be great.

The major pain to me is the channel establishment part where we now have
all kinds of channels, so we should really fix that sooner (e.g., we hope
to enable multifd + postcopy very soon, that requires multifd and preempt
channels appear in the same time).  It was reasonable the vfio/multifd
series tried to fix it.

-- 
Peter Xu