Re: [RFC V1 00/14] precreate phase

["Daniel P. Berrangé" <berrange@redhat.com>]

On Fri, Oct 25, 2024 at 10:32:15AM -0400, Steven Sistare wrote:
> 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.

FWIW, at least in libvirt usage, IIRC, libvirt will choose the listener
port number upfront itself

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