Re: [PATCH v16 1/4] xen/domain: unify domain ID allocation

[dmkhn@proton.me]

On Thu, Aug 21, 2025 at 09:16:16AM +0200, Jan Beulich wrote:
> On 20.08.2025 01:58, dmkhn@proton.me wrote:
> > On Thu, Aug 14, 2025 at 09:11:11AM +0200, Jan Beulich wrote:
> >> On 13.08.2025 00:30, dmkhn@proton.me wrote:
> >>> From: Denis Mukhin <dmukhin@ford.com>
> >>>
> >>> Currently, there are two different domain ID allocation implementations:
> >>>
> >>>   1) Sequential IDs allocation in dom0less Arm code based on max_init_domid;
> >>>
> >>>   2) Sequential IDs allocation in XEN_DOMCTL_createdomain; does not use
> >>>      max_init_domid (both Arm and x86).
> >>>
> >>> The domain ID allocation covers dom0 or late hwdom, predefined domains,
> >>> post-boot domains, excluding Xen system domains (domid >=
> >>> DOMID_FIRST_RESERVED).
> >>>
> >>> It makes sense to have a common helper code for such task across architectures
> >>> (Arm and x86) and between dom0less / toolstack domU allocation.
> >>>
> >>> Note, fixing dependency on max_init_domid is out of scope of this patch.
> >>>
> >>> Wrap the domain ID allocation as an arch-independent function domid_alloc() in
> >>> new common/domid.c based on the bitmap.
> >>>
> >>> Allocation algorithm:
> >>> - If an explicit domain ID is provided, verify its availability and use it if
> >>>   ID is not used;
> >>> - If DOMID_INVALID is provided, search the range [1..DOMID_FIRST_RESERVED-1],
> >>>   starting from the last used ID.
> >>>   Implementation guarantees that two consecutive calls will never return the
> >>>   same ID. ID#0 is reserved for the first boot domain (currently, dom0) and
> >>>   excluded from the allocation range.
> >>>
> >>> Remove is_free_domid() helper as it is not needed now.
> >>>
> >>> No functional change intended.
> >>>
> >>> Signed-off-by: Denis Mukhin <dmukhin@ford.com>
> >>> Reviewed-by: Julien Grall <jgrall@amazon.com>
> >>> Reviewed-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
> >>> ---
> >>> Changes since v15:
> >>> - fixup for check after the first pass in the bitarray in domid_alloc()
> >>> - trivial renaming for the local variable in domid_alloc()
> >>> - kept Julien's R-b, added Alejandro's R-b
> >>
> >> Just to mention: My take is that this kind of a fix ought to invalidate all
> >> earlier R-b. It's not just a cosmetic change, after all.
> >
> > Sorry for the hiccup here, did not mean to overrule the review process.
> >
> > My bold assumption was that in case of small fixups like this it is
> > satisfactory to carry over previous acks.
> 
> Acks may be okay to keep, but imo R-b need dropping when an actual bug was
> fixed. Irrespective of how severe the bug was.
> 
> > I asked (matrix) both Julien and Alejandro to re-review and confirm.
> 
> While good to ask, that's of limited use. It'll be impossible later on to
> figure whether such a confirmation was given. Decisions (and acks and alike
> effectively fall into that category) need to be on the list, to be able to
> locate them later on.

Got it, thanks.

> 
> Jan
>