Re: [Xen-devel] [PATCH 2/6] xen: Move xenstore initialization to common location

[Jason Andryuk <jandryuk@xxxxxxxxx>]

On Wed, Mar 13, 2019 at 11:01 AM Paul Durrant <Paul.Durrant@xxxxxxxxxx> wrote:
>
> > -----Original Message-----
> > From: Jason Andryuk [mailto:jandryuk@xxxxxxxxx]
> > Sent: 11 March 2019 18:02
> > To: qemu-devel@xxxxxxxxxx
> > Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx; marmarek@xxxxxxxxxxxxxxxxxxxxxx; Jason 
> > Andryuk
> > <jandryuk@xxxxxxxxx>; Stefano Stabellini <sstabellini@xxxxxxxxxx>; Anthony 
> > Perard
> > <anthony.perard@xxxxxxxxxx>; Paul Durrant <Paul.Durrant@xxxxxxxxxx>; Paolo 
> > Bonzini
> > <pbonzini@xxxxxxxxxx>; Richard Henderson <rth@xxxxxxxxxxx>; Eduardo Habkost 
> > <ehabkost@xxxxxxxxxx>;
> > Michael S. Tsirkin <mst@xxxxxxxxxx>; Marcel Apfelbaum 
> > <marcel.apfelbaum@xxxxxxxxx>
> > Subject: [PATCH 2/6] xen: Move xenstore initialization to common location
> >
> > For the xen stubdom case, we'll want xenstore initialized, but we'll
> > want to skip the rest of xen_be_init.  Move the initialization to
> > xen_hvm_init so we can conditionalize calling xen_be_init.
> >
> > xs_domain_open() is deprecated for xs_open(0), so make the replacement
> > as well.
>
> Can you elaborate as to why you need to do this when the code at the top of 
> xen_hvm_init() already opens xenstore for its own purposes, and AFAICT 
> xenstore_update() is only needed if QEMU is implementing a PV backend?
>
>

Hi, Paul.  Thanks for reviewing.

I think you are right, that this basically shouldn't be needed if PV
backends are disabled.  This patch came out of OpenXT, where it is
needed for some out-of-tree patches.  But that doesn't make it
suitable for upstreaming.

However, while reviewing, it looks like the xen accelerator in
hw/xen/xen-common.c:xen_init() registers xen_change_state_handler().
xen_change_state_handler() uses the global xenstore handle and will
exit(1) if NULL.  I'm not sure how to get the XenIOState xenstore
handle over to the accelerator's xen_init.  Outside of that, I think
you are correct that xenstore_update doesn't need to be run when PV
backends are disabled.

Thanks,
Jason

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