Re: [XEN PATCH 6/7] xenstored: do_introduce: handle the late_init case

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Sat, 8 Jan 2022, Julien Grall wrote:
> Hi Stefano,
> 
> On 08/01/2022 00:49, Stefano Stabellini wrote:
> > From: Luca Miccio <lucmiccio@xxxxxxxxx>
> > 
> > If the function is called with late_init set then also notify the domain
> > using the xenstore event channel.
> > 
> > Signed-off-by: Luca Miccio <lucmiccio@xxxxxxxxx>
> > Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
> > CC: wl@xxxxxxx
> > CC: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> > CC: Juergen Gross <jgross@xxxxxxxx>
> > CC: julien@xxxxxxx
> > ---
> >   tools/xenstore/xenstored_domain.c | 15 ++++++++++-----
> 
> All the changes to the protocol should be reflected in docs/misc/xenstore.txt.
> However...
> 
> >   1 file changed, 10 insertions(+), 5 deletions(-)
> > 
> > diff --git a/tools/xenstore/xenstored_domain.c
> > b/tools/xenstore/xenstored_domain.c
> > index d03c7d93a9..17b8021ca8 100644
> > --- a/tools/xenstore/xenstored_domain.c
> > +++ b/tools/xenstore/xenstored_domain.c
> > @@ -429,7 +429,7 @@ static void domain_conn_reset(struct domain *domain)
> >     static struct domain *introduce_domain(const void *ctx,
> >                                    unsigned int domid,
> > -                                  evtchn_port_t port, bool restore)
> > +                                  evtchn_port_t port, bool restore, bool
> > late_init)
> >   {
> >     struct domain *domain;
> >     int rc;
> > @@ -461,6 +461,9 @@ static struct domain *introduce_domain(const void *ctx,
> >             /* Now domain belongs to its connection. */
> >             talloc_steal(domain->conn, domain);
> >   +         if (late_init)
> > +                   xenevtchn_notify(xce_handle, domain->port);
> 
> ... I am not convinced the parameter late_init is necessary. I believe it
> would be safe to always raise an event channel because a domain should be
> resilient (event channel are just to say "Please check the status", there are
> no data carried).

This is a fantastic idea. I gave it a quick try and it seems to work
fine. If everything checks out I'll make the change in the next version
and drop late_init (the new parameter to xs_introduce_domain) completely.


> If you really need late_init, then it should be made optional to avoid
> breaking existing user of Xenstore (IHMO the protocol is stable and should be
> backward compatible).