Re: [PATCH] tools/init-dom0less: Fix cpus > 1 and xenstore entries

["Orzel, Michal" <michal.orzel@xxxxxxx>]

On 05/03/2025 22:58, Jason Andryuk wrote:
> 
> 
> The trailing / in the path is incorrect and generates an error when
> writing to xenstore:
> Checking domid: 1
> Init dom0less domain: 1
> init-dom0less: writing to xenstore: No error information
> 
> init-dom0less exits without finishing initialization.
> 
> vcpu_max_id is an inclusive value, so it should be included in the
> loop's range to include all vcpus.  Without this, no xenstore entries
> are created for a 1 vcpu domain.
> 
> Finally, use vcpu_online, the count of online vcpus, to determine online
> vs. offline.  info->cpupool is a cpupool id and not a bitmask.
> 
> Fixes: ec53e0c4ea ("tools: add example application to initialize dom0less PV 
> drivers")
> Signed-off-by: Jason Andryuk <jason.andryuk@xxxxxxx>
> ---
>  tools/helpers/init-dom0less.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/helpers/init-dom0less.c b/tools/helpers/init-dom0less.c
> index 17579fe2e8..c569a890a0 100644
> --- a/tools/helpers/init-dom0less.c
> +++ b/tools/helpers/init-dom0less.c
> @@ -182,13 +182,13 @@ retry_transaction:
>      if (!do_xs_write_dom(xsh, t, domid, "vm", vm_val_str)) goto err;
>      if (!do_xs_write_dom(xsh, t, domid, "name", dom_name_str)) goto err;
>      if (!do_xs_write_dom(xsh, t, domid, "cpu", "")) goto err;
> -    for (i = 0; i < info->vcpu_max_id; i++) {
> -        rc = snprintf(cpu_str, STR_MAX_LENGTH, "cpu/%u/availability/", i);
> +    for (i = 0; i <= info->vcpu_max_id; i++) {
> +        rc = snprintf(cpu_str, STR_MAX_LENGTH, "cpu/%u/availability", i);
Up until this point:
Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>

That said...

>          if (rc < 0 || rc >= STR_MAX_LENGTH)
>              goto err;
>          rc = -EIO;
>          if (!do_xs_write_dom(xsh, t, domid, cpu_str,
> -                             (info->cpupool & (1 << i)) ? "online" : 
> "offline"))
> +                             i < info->vcpu_online ? "online" : "offline"))
I struggle with this one. Let's say that a dom0less domU starts with 4 vCPUs and
later on (before executing init-dom0less from dom0), decides to kill it's 2nd
vCPU. So domU is running vCPU{0,2,3}. With your patch, after executing the
script, the 4th vCPU will have its availability set to offline and domU will get
notified to kill its 4th vCPU. That does not sound right...

~Michal