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

[Jason Andryuk <jason.andryuk@xxxxxxx>]

On 2025-03-06 05:29, Orzel, Michal wrote:
> 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...
You are correct.

With xl create, cpus 0..vcpus-1 are marked online and vcpus...maxvcpus-1 
are marked offline.  I was trying to match that.  To online or offline 
cpus, xl vcpu-set keeps them densely packed, 0..$m.
I think libxl_list_vcpu() should give the needed information to set 
these correctly.  I'll try that.
Thanks for taking a look.

-Jason