Re: [RFC V1 00/14] precreate phase

[Daniel P . Berrangé]

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

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|