Re: [XEN PATCH v2 2/5] xen: export get_free_port

[Julien Grall <julien@xxxxxxx>]

Hi Stefano,

On 13/01/2022 04:58, Stefano Stabellini wrote:
> From: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
>
> get_free_port will soon be used to allocate the xenstore event channel
> for dom0less domains.
>
> Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxxxxx>
> CC: Julien Grall <julien@xxxxxxx>
> CC: Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
> CC: Bertrand Marquis <bertrand.marquis@xxxxxxx>
> CC: Jan Beulich <jbeulich@xxxxxxxx>
> CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
>   xen/common/event_channel.c | 2 +-
>   xen/include/xen/event.h    | 3 +++
>   2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c
> index da88ad141a..5b0bcaaad4 100644
> --- a/xen/common/event_channel.c
> +++ b/xen/common/event_channel.c
> @@ -232,7 +232,7 @@ int evtchn_allocate_port(struct domain *d, evtchn_port_t 
> port)
>       return 0;
>   }
>   
> -static int get_free_port(struct domain *d)
> +int get_free_port(struct domain *d)
I dislike the idea to expose get_free_port() (or whichever name we 
decide) because this can be easily misused.
In fact looking at your next patch (#3), you are misusing it as it is 
meant to be called with d->event_lock. I know this doesn't much matter
in your situation because this is done at boot with no other domains 
running (or potentially any event channel allocation). However, I still 
think we should get the API right.
I am also not entirely happy of open-coding the allocation in 
domain_build.c. Instead, I would prefer if we provide a new helper to 
allocate an unbound event channel. This would be similar to your v1 (I 
still need to review the patch though).
Cheers,

--
Julien Grall