> -----Original Message----- > From: Jason Andryuk [mailto:jandr...@gmail.com] > Sent: 13 March 2019 18:12 > To: Paul Durrant <paul.durr...@citrix.com> > Cc: qemu-devel@nongnu.org; xen-de...@lists.xenproject.org; > marma...@invisiblethingslab.com; Stefano > Stabellini <sstabell...@kernel.org>; Anthony Perard > <anthony.per...@citrix.com>; Paolo Bonzini > <pbonz...@redhat.com>; Richard Henderson <r...@twiddle.net>; Eduardo Habkost > <ehabk...@redhat.com>; > Michael S. Tsirkin <m...@redhat.com>; Marcel Apfelbaum > <marcel.apfelb...@gmail.com> > Subject: Re: [PATCH 2/6] xen: Move xenstore initialization to common location > > On Wed, Mar 13, 2019 at 11:01 AM Paul Durrant <paul.durr...@citrix.com> wrote: > > > > > -----Original Message----- > > > From: Jason Andryuk [mailto:jandr...@gmail.com] > > > Sent: 11 March 2019 18:02 > > > To: qemu-devel@nongnu.org > > > Cc: xen-de...@lists.xenproject.org; marma...@invisiblethingslab.com; > > > Jason Andryuk > > > <jandr...@gmail.com>; Stefano Stabellini <sstabell...@kernel.org>; > > > Anthony Perard > > > <anthony.per...@citrix.com>; Paul Durrant <paul.durr...@citrix.com>; > > > Paolo Bonzini > > > <pbonz...@redhat.com>; Richard Henderson <r...@twiddle.net>; Eduardo > > > Habkost <ehabk...@redhat.com>; > > > Michael S. Tsirkin <m...@redhat.com>; Marcel Apfelbaum > > > <marcel.apfelb...@gmail.com> > > > 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 see it yes. TBH signalling state via xenstore should go away as it is incompatible with deprivileging, and I think Anthony might have some patches for that? In the meantime I suggest just doing a local xs_open(0) in that function. > I'm not sure how to get the XenIOState xenstore > handle over to the accelerator's xen_init. That would not be appropriate as the machine type may not be xenfv and hence xen_hvm_init() may not have been called. Paul > Outside of that, I think > you are correct that xenstore_update doesn't need to be run when PV > backends are disabled. > > Thanks, > Jason
