Re: [PATCH V3 11/16] migration: cpr-transfer mode

[Peter Xu <peterx@redhat.com>]

On Tue, Nov 19, 2024 at 02:50:40PM -0500, Steven Sistare wrote:
> On 11/14/2024 2:04 PM, Peter Xu wrote:
> > On Thu, Nov 14, 2024 at 01:36:00PM -0500, Steven Sistare wrote:
> > > On 11/13/2024 4:58 PM, Peter Xu wrote:
> > > > On Fri, Nov 01, 2024 at 06:47:50AM -0700, Steve Sistare wrote:
> > > > > Add the cpr-transfer migration mode.  Usage:
> > > > >     qemu-system-$arch -machine anon-alloc=memfd ...
> > > > > 
> > > > >     start new QEMU with "-incoming <uri-1> -cpr-uri <uri-2>"
> > > > > 
> > > > >     Issue commands to old QEMU:
> > > > >     migrate_set_parameter mode cpr-transfer
> > > > >     migrate_set_parameter cpr-uri <uri-2>
> > > > >     migrate -d <uri-1>
> > > > 
> > > > QMP command "migrate" already allows taking MigrationChannel lists, cpr can
> > > > be the 2nd supported channel besides "main".
> > > > 
> > > > I apologize on only noticing this until now.. I wished the incoming side
> > > > can do the same already (which also takes 'MigrationChannel') if monitors
> > > > init can be moved earlier, and if precreate worked out.  If not, we should
> > > > still consider doing that on source, because cpr-uri isn't usable on dest
> > > > anyway.. so they need to be treated separately even now.
> > > > 
> > > > Then after we make the monitor code run earlier in the future we could
> > > > introduce that to incoming side too, obsoleting -cpr-uri there.
> > > 
> > > I have already been shot down on precreate and monitors init, so we are
> > > left with specifying a "cpr" channel on the outgoing side, and -cpr-uri
> > > on the incoming side.  That will confuse users, will require more implementation
> > > and specification work than you perhaps realize to explain this to users,
> > 
> > What is the specification work?  Can you elaborate?
> > 
> > > and only gets us halfway to your desired end point of specifying everything
> > > using channels.  I don't like that plan!
> > > 
> > > If we ever get the ability to open the monitor early, then we can implement
> > > a complete and clean solution using channels and declare the other options
> > > obsolete.
> > 
> > The sender side doesn't need to wait for destination side to be ready?
> > Dest side isn't a reason to me on how we should make sender side work if
> > they're totally separate anyway.  Dest requires -cpr-uri because we don't
> > yet have a choice.
> > 
> > Is the only concern about code changes?  I'm expecting this change is far
> > less controversial comparing to many others in this series, even if I
> > confess that may still contain some diff. They should hopefully be
> > straightforward, unlike many of the changes elsewhere in the series.
> > 
> > If you prefer not writting that patch, I am OK, and I can write one patch
> > on top of your series after it lands if that is OK for you. I still want to
> > have this there when release 10.0 if I didn't misunderstood anything, so
> > I'll be able to remove cpr-uri directly in that patch too.
> 
> I made the changes:
>   * implementation
>   * documentation in CPR.rst and QAPI
>   * convert sample code in CPR.rst, commit messages, and cover letter to QMP,
>     because a channel cannot be specified using HMP.

Yeah we can leave HMP as of now; it can easily be added on top with
existing helpers like migrate_uri_parse().

>   * migration tests
> 
> New CPR.rst:
> 
> -------------------
>   ...
>   This mode requires a second migration channel named "cpr" in the
>   channel arguments on the outgoing side.  The channel must be a type,
>   such as unix socket, that supports SCM_RIGHTS.  However, the cpr
>   channel cannot be added to the list of channels for a migrate-incoming
>   command, because it must be read before new QEMU opens a monitor.
>   Instead, the user passes the equivalent URI for the channel as part of
>   the ``cpr-uri`` command-line argument to new QEMU.
>   ...
> 
>   Outgoing:                             Incoming:
> 
>   # qemu-kvm -qmp stdio
>   -object memory-backend-file,id=ram0,size=4G,
>   mem-path=/dev/shm/ram0,share=on -m 4G
>   -machine aux-ram-share=on
>   ...
>                                         # qemu-kvm -monitor stdio
>                                         -incoming tcp:0:44444
>                                         -cpr-uri unix:cpr.sock
>                                         ...
>   {"execute":"qmp_capabilities"}
> 
>   {"execute": "query-status"}
>   {"return": {"status": "running",
>               "running": true}}
> 
>   {"execute":"migrate-set-parameters",
>    "arguments":{"mode":"cpr-transfer"}}
> 
>   {"execute": "migrate", "arguments": { "channels": [
>     {"channel-type": "main",
>      "addr": { "transport": "socket", "type": "inet",
>                "host": "0", "port": "44444" }},
>     {"channel-type": "cpr",
>      "addr": { "transport": "socket", "type": "unix",
>                "path": "cpr.sock" }}]}}
> 
>                                         QEMU 9.2.50 monitor
>                                         (qemu) info status
>                                         VM status: running
> 
>   {"execute": "query-status"}
>   {"return": {"status": "postmigrate",
>               "running": false}}
> -------------------

Thank you, Steve!

-- 
Peter Xu