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

[Julien Grall <julien@xxxxxxx>]

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).
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).
Cheers,

--
Julien Grall