Re: [Xen-devel] [PATCH v3 31/31] libxl: QEMU startup sync based on QMP

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Wed, Jun 27, 2018 at 05:16:07PM +0100, Ian Jackson wrote:
> Anthony PERARD writes ("[PATCH v3 31/31] libxl: QEMU startup sync based on 
> QMP"):
> > This is only activated when dm_restrict=1, as explained in the previous
> > patch "libxl_dm: Pre-open QMP socket for QEMU"
> ...
> > @@ -1603,11 +1603,16 @@ struct libxl__spawn_state {
> >      libxl__spawn_confirm_cb *confirm_cb;
> >      libxl__spawn_detached_cb *detached_cb;
> >  
> > +    /* If qmp_domid != INVALID_DOMID, then libxl__spawn_spawn will also 
> > use QMP
> > +     * to find out when the process is started */
> > +    uint32_t qmp_domid;
> > +
> 
> I think this is a layering violation.  libxl__spawn_* is a thing for
> double forking and shouldn't know about qmp.

Yes, I think I agree with that now, and I think I can move the QMP
calls to libxl__dm_spawn_*.

> I think you need to
> handle this the way the xenstore readiness is handled.

That is what I actually tried to do... but the way it is done is weird,
both libxl__dm_spawn_* and libxl__spawn_* do some setup of xenstore, the
first one setup some parameters, but the second one does start the
watch for event.

Anyway, I've managed to move the QMP stuff to libxl__dm_spawn_*. Result
in v4.

Thanks,

-- 
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel