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

[Steven Sistare <steven.sistare@oracle.com>]

On 11/19/2024 3:16 PM, Peter Xu wrote:
> 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().

This begs the question, should we allow channels to be specified in hmp migrate
commands and for -incoming, in a very simple way?  Like with a prefix naming
the channel.  And eliminate the -cpr-uri argument. Examples:

(qemu) migrate -d main:tcp:0:44444,cpr:unix:cpr.sock

qemu -incoming main:tcp:0:44444,cpr:unix:cpr.sock
qemu -incoming main:defer,cpr:unix:cpr.sock

- Steve

>>    * 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!
>