Re: [PATCH 1/6] xen: add a domain unique id to each domain

["Alejandro Vallejo" <alejandro.vallejo@xxxxxxxxx>]

On Wed Oct 23, 2024 at 2:10 PM BST, Juergen Gross wrote:
> Xenstore is referencing domains by their domid, but reuse of a domid
> can lead to the situation that Xenstore can't tell whether a domain
> with that domid has been deleted and created again without Xenstore
> noticing the domain is a new one now.
>
> Add a global domain creation unique id which is updated when creating
> a new domain, and store that value in struct domain of the new domain.
> The global unique id is initialized with the system time and updates
> are done via the xorshift algorithm which is used for pseudo random
> number generation, too (see https://en.wikipedia.org/wiki/Xorshift).
>
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> V1:
> - make unique_id local to function (Jan Beulich)
> - add lock (Julien Grall)
> - add comment (Julien Grall)
> ---
>  xen/common/domain.c     | 20 ++++++++++++++++++++
>  xen/include/xen/sched.h |  3 +++
>  2 files changed, 23 insertions(+)
>
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index 92263a4fbd..3948640fb0 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -562,6 +562,25 @@ static void _domain_destroy(struct domain *d)
>      free_domain_struct(d);
>  }
>  
> +static uint64_t get_unique_id(void)
> +{
> +    static uint64_t unique_id;
> +    static DEFINE_SPINLOCK(lock);
> +    uint64_t x = unique_id ? : NOW();
> +
> +    spin_lock(&lock);
> +
> +    /* Pseudo-randomize id in order to avoid consumers relying on sequence. 
> */
> +    x ^= x << 13;
> +    x ^= x >> 7;
> +    x ^= x << 17;
> +    unique_id = x;
> +
> +    spin_unlock(&lock);
> +
> +    return x;
> +}
> +
>  static int sanitise_domain_config(struct xen_domctl_createdomain *config)
>  {
>      bool hvm = config->flags & XEN_DOMCTL_CDF_hvm;
> @@ -654,6 +673,7 @@ struct domain *domain_create(domid_t domid,
>  
>      /* Sort out our idea of is_system_domain(). */
>      d->domain_id = domid;
> +    d->unique_id = get_unique_id();
>  
>      /* Holding CDF_* internal flags. */
>      d->cdf = flags;
> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> index 90666576c2..1dd8a425f9 100644
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -370,6 +370,9 @@ struct domain
>      domid_t          domain_id;
>  
>      unsigned int     max_vcpus;
> +
> +    uint64_t         unique_id;       /* Unique domain identifier */
> +

Why not xen_domain_handle_t handle, defined later on? That's meant to be a
UUID, so this feels like a duplicate field.

>      struct vcpu    **vcpu;
>  
>      shared_info_t   *shared_info;     /* shared data area */

Cheers,
Alejandro