Re: [PATCH 2/2] python/qemu/machine: accept QMP connection asynchronously

[Daniel P . Berrangé]

On Wed, Jun 29, 2022 at 07:54:08PM -0400, John Snow wrote:
> On Tue, Jun 28, 2022 at 10:17 AM Daniel P. Berrangé <berrange@redhat.com> 
> wrote:
> >
> > On Tue, Jun 28, 2022 at 05:49:39PM +0400, marcandre.lureau@redhat.com wrote:
> > > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> > >
> > > QMP accept is currently synchronous. If qemu dies before the connection
> > > is established, it will wait there. Instead turn the code to do
> > > concurrently accept() and wait(). Returns when the first task is
> > > completed to determine whether a connection was established.
> >
> > If the spawned QEMU process was given -daemonize, won't this code
> > mistakenly think the subprocess has quit ?
> 
> Do we use daemonize with this code anywhere? Is it important that we
> are able to?

Well what's the intended breadth of use cases this code wants to
target ?

If you don't daemonize QEMU, then QEMU will (likely) get killed off
when the parent python process terminates. That can be ok for adhoc
testing scenarios where the QEMU process is transient and throwaway,
or for using QEMU as an embedded technology (think libguestfs). If
you want your QEMU to run full OS workloads, then generally you won't
want it to die when the mgmt app is restarted (eg for software upgrade),
whereupon you want to daemonize.

> Many of the shutdown routines I wrote expect to work directly with a
> launched process ... at least, that expectation exists in my head. I
> suppose a lot of this code may actually just coincidentally work with
> -daemonize and I wouldn't have noticed. I certainly haven't been
> testing it explicitly. I definitely make no accommodations for it, so
> I would expect some stale processes in various cases at a minimum.

Looking at the code it probably works by accident - the shutdown()
methods kinda assumes we're talking to a direct child, but it'll
happen to work right now as it'll simply cleanup the defunct
intermediate child, while QEMU stays running.

> If we want to expand to accommodate this feature, can we do that
> later? Machine needs a bit of a remodel anyway. (I want to write an
> 'idiomatic' asyncio version to match the QMP lib. I have some
> questions to work out WRT which portions of this appliance can be
> upstreamed and which need to remain only in our testing tree. We can
> talk about those pieces later, just throwing it out there that it's on
> my list.)

The machine class is probably the part that looks least ready to be
published as an API on pypi. Its code really shows its origins as an
adhoc testing framework, rather than a general purpose API for running
and managing QEMU from python.

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