Re: [PATCH v6 5/7] xenstored: send an evtchn notification on introduce_domain

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 11 May 2022, Julien Grall wrote:
> On 05/05/2022 01:16, Stefano Stabellini wrote:
> > From: Luca Miccio <lucmiccio@xxxxxxxxx>
> > 
> > When xs_introduce_domain is called, send out a notification on the
> > xenstore event channel so that any (dom0less) domain waiting for the
> > xenstore interface to be ready can continue with the initialization.
> > Before sending the notification, clear XENSTORE_RECONNECTING.
> > 
> > The extra notification is harmless for domains that don't require it.
> > 
> > In xs_wire.h update the commment on top of XENSTORE_RECONNECTING to
> > generalize its meaning to suit the dom0less use-case better.
> > 
> > Signed-off-by: Luca Miccio <lucmiccio@xxxxxxxxx>
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
> > CC: jgross@xxxxxxxx
> > CC: anthony.perard@xxxxxxxxxx
> > CC: wl@xxxxxxx
> > ---
> > If you have better suggestions for the wording in xs_wire.h please
> > suggest!
> > 
> > 
> > Changes in v6:
> > - use XENSTORE_CONNECTED instead of 0x0
> > - update xs_wire.h
> > 
> > Changes in v5:
> > - reset XS_CONNECTION_STATE_RECONNECTING before notifying the domU
> > 
> > Changes in v2:
> > - drop the new late_init parameter
> > ---
> >   tools/xenstore/xenstored_domain.c | 4 ++++
> >   xen/include/public/io/xs_wire.h   | 2 +-
> 
> I am not entirely sure this is the right place to mention it. But I couldn't
> find a better one.
> 
> The documentation (docs/misc/xenstore-misc.txt) states that the field is valid
> when the server advertised ``Connection State``.
> 
> Is there any guarantee the field will be 0 for any previous {C, O}xenstored
> implementation? If not, then I think we need to set the feature flag so Linux
> knows the field can be used.
> 
> If yes, then the documentation should be relaxed so an OS knows it can safely
> use the field without checking the feature flag.

The xenstore page is allocated by the toolstack which zeros the page,
*xenstored wouldn't set it, so I think we can assume the field has
always been zero.


> > diff --git a/xen/include/public/io/xs_wire.h
> > b/xen/include/public/io/xs_wire.h
> > index 953a0050a3..c1ec7c73e3 100644
> > --- a/xen/include/public/io/xs_wire.h
> > +++ b/xen/include/public/io/xs_wire.h
> > @@ -141,7 +141,7 @@ struct xenstore_domain_interface {
> >     /* Valid values for the connection field */
> >   #define XENSTORE_CONNECTED 0 /* the steady-state */
> > -#define XENSTORE_RECONNECT 1 /* guest has initiated a reconnect */
> > +#define XENSTORE_RECONNECT 1 /* reconnect in progress */
> 
> The definition in the docs needs to be updated.

I wanted to do that but I am very unfamiliar with the xenstore docs.
Can you point me to the place where I need to change the definition? I
cannot find where XENSTORE_RECONNECT is defined...