Re: [RFC V1 00/14] precreate phase

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

On 10/25/2024 9:43 AM, Daniel P. Berrangé wrote:
> On Fri, Oct 25, 2024 at 09:33:51AM -0400, Steven Sistare wrote:
>> On 10/25/2024 4:46 AM, Daniel P. Berrangé wrote:
>>> On Thu, Oct 24, 2024 at 05:16:14PM -0400, Steven Sistare wrote:
>>>>
>>>> Regarding: "what you want is effectively to execute monitor commands
>>>> from the migration stream"
>>>>
>>>> That is not the goal of this series.  It could be someone else's goal, when
>>>> fully developing a precreate phase, and in that context I understand and
>>>> agree with your comments.  I have a narrower immediate problem to solve,
>>>> however.
>>>>
>>>> For CPR, src qemu sends file descriptors to dst qemu using SCM_RIGHTS over
>>>> a dedicated channel, then src qemu sends migration state over the normal
>>>> migration channel.
>>>>
>>>> Dst qemu reads the fds early, then calls the backend and device creation
>>>> functions which use them.  Dst qemu then accepts and reads the migration
>>>> channel.
>>>>
>>>> We need a way to send monitor commands that set dst migration capabilities,
>>>> before src qemu starts the migration.  Hence the dst cannot proceed to
>>>> backend and device creation because the src has not sent fd's yet.  Hence
>>>> we need a dst monitor before device creation.  The precreate phase does that.
>>>
>>> Sigh, what we obviously need here, is what we've always talked about as our
>>> long term design goal:
>>>
>>> A way to launch QEMU with the CLI only specifying the QMP socket, and every
>>> other config aspect done by issuing QMP commands, which are processed in the
>>> order the mgmt app sends them, so QEMU hasn't have to hardcode processing
>>> of different pieces in different phases.
>>>
>>> Anything that isn't that, is piling more hacks on top of our existing
>>> mountain of hacks. That's OK if it does something useful as a side effect
>>> that moves us incrementally closer towards that desired end goal.
>>>
>>>> Regarding: "This series makes this much more complex."
>>>>
>>>> I could simplify it if I abandon CPR for chardevs.  Then qemu_create_early_backends
>>>> and other early dependencies can remain as is.  I would drop the notion of
>>>> a precreate phase, and instead leverage the preconfig phase.  I would move
>>>> qemu_create_late_backends, and a small part at the end of qemu_init, to
>>>> qmp_x_exit_preconfig.
>>>
>>> Is CPR still going to useful enough in the real world if you drop chardev
>>> support ? Every VM has at least one chardev for a serial device doesn't
>>> it, and often more since we wire chardevs into all kinds of places.
>>
>> CPR for chardev is not as useful for cpr-transfer mode because the mgmt layer already
>> knows how to create and manage new connections to dest qemu, as it would for normal
>> migration.
>>
>> CPR for chardev is very useful for cpr-exec mode.  And cpr-exec mode does not need any
>> of these monitor patches, because old qemu exec's new qemu, and they are never active
>> at the same time.  One must completely specify the migration using src qemu before
>> initiating the exec.  I mourn cpr-exec mode.
>>
>> Which begs the question, do we really need to allow migration parameters to be set
>> in the dest monitor when using cpr?  CPR is a very restricted mode of migration.
>> Let me discuss this with Peter.
> 
> The migration QAPI design has always felt rather odd to me, in that we
> have perfectly good commands "migrate" & "migrate-incoming" that are able
> to accept an arbitrary list of parameters when invoked. Instead of passing
> parameters to them though, we instead require apps use the separate
> migreate-set-parameters/capabiltiies commands many times over to set
> global variables which the later 'migrate' command then uses.
> 
> The reason for this is essentially a historical mistake - we copied the
> way we did it from HMP, which was this way because HMP was bad at supporting
> arbitrary customizable paramters to commands. I wish we hadn't copied this
> design over to QMP.
> 
> To bring it back on topic, we need QMP on the dest to set parameters,
> because -incoming  was limited to only take the URI.
> 
> If the "migrate-incoming" command accepted all parameters directly,
> then we could use QAPI visitor to usupport a "-incoming ..." command
> that took an arbitrary JSON document and turned it into a call to
> "migrate-incoming".
> 
> With that we would never need QMP on the target for cpr-exec, avoiding
> this ordering poblem you're facing....assuming we put processing of
> -incoming at the right point in the code flow
> 
> Can we fix this design and expose the full configurability on the
> CLI using QAPI schema & inline JSON, like we do for other QAPI-ified
> CLI args.
> 
> It seems entirely practical to me to add parameters to 'migrate-incoming'
> in a backwards compatible manner and deprecate set-parameters/capabilities

Hi Daniel, should we ever need to set caps or parameters for CPR, that sounds
like a good way forward.  And a good idea independently of CPR.  However, I
am hoping to proceed with CPR with the initial restriction that one cannot
set them. The case that motivated my exploration of precreate is artificial --
qtest wanting to enable migration events -- and I can fix that.  I know of no
real cases where caps must be set for CPR.

The other screw case which motivated this thread is a dynamically chosen TCP
port number for the migration listen socket.  One must query dest qemu to get it.
Your suggestions here for new incoming syntax would not help.  However, for CPR,
we always migrate to the same host, so a unix domain socket can be used.

- Steve