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

[Julien Grall <julien@xen.org>]

Hi Stefano,

On 08/01/2022 00:49, Stefano Stabellini wrote:
> From: Luca Miccio <lucmiccio@gmail.com>
> 
> 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@gmail.com>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xilinx.com>
> CC: wl@xen.org
> CC: Anthony PERARD <anthony.perard@citrix.com>
> CC: Juergen Gross <jgross@suse.com>
> CC: julien@xen.org
> ---
>   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