Re: [RFC V1 00/14] precreate phase

[Peter Xu <peterx@redhat.com>]

On Fri, Oct 25, 2024 at 02:43:06PM +0100, 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.

Just to mention, we will still need some special parameters that can change
during migration, like max-bandwidth, max-downtime etc.  So not all of them
can be made into "migrate"/"migrate-incoming" arguments.

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

Fabiano started working on handshake recently.  After that is ready, we
should logically also achieve similar goal at least for dest qemu so no
cap/parameter is ever needed there.

Thanks,

-- 
Peter Xu