On Thu Aug 21, 2025 at 9:16 AM CEST, Jan Beulich wrote: > On 20.08.2025 01:58, dm...@proton.me wrote: >> On Thu, Aug 14, 2025 at 09:11:11AM +0200, Jan Beulich wrote: >>> On 13.08.2025 00:30, dm...@proton.me wrote: >>>> From: Denis Mukhin <dmuk...@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 <dmuk...@ford.com> >>>> Reviewed-by: Julien Grall <jgr...@amazon.com> >>>> Reviewed-by: Alejandro Vallejo <alejandro.garciavall...@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.
I don't know. Unless the bugfix involves a change in the code with wide reaching consequences I'd say it's reasonable to keep them. But that's something for you (the committers) to decide, and this just my .02 cents. > Irrespective of how severe the bug was. It's not so much about the severity (imo), as the behavioural differences involved in the fixup. In this case (afaics?) it's a straight s/==/>=/, which is self-contained and has no wide-reaching side effects at all. > >> 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. > > Jan He meant he reached out to ask for an in-list confirmation. As far as I'm concerned, my R-by still holds. Cheers, Alejandro
