+ Adding toolstack maintainer > On 18 May 2022, at 12:34, Andrew Cooper <andrew.coop...@citrix.com> wrote: > > On 18/05/2022 11:30, Luca Fancellu wrote: >>> On 18 May 2022, at 11:12, Andrew Cooper <andrew.coop...@citrix.com> wrote: >>> >>> On 18/05/2022 10:51, Edwin Torok wrote: >>>>> diff --git a/tools/ocaml/libs/xc/xenctrl.ml >>>>> b/tools/ocaml/libs/xc/xenctrl.ml >>>>> index 7503031d8f61..8eab6f60eb14 100644 >>>>> --- a/tools/ocaml/libs/xc/xenctrl.ml >>>>> +++ b/tools/ocaml/libs/xc/xenctrl.ml >>>>> @@ -85,6 +85,7 @@ type domctl_create_config = >>>>> max_grant_frames: int; >>>>> max_maptrack_frames: int; >>>>> max_grant_version: int; >>>>> + cpupool_id: int32; >>>> What are the valid values for a CPU pool id, in particular what value >>>> should be passed here to get back the behaviour prior to these changes in >>>> Xen? >>>> (i.e. would it be cpu pool id 0 or -1 if cpu pools aren't otherwise >>>> explicitly configured on the system) >>> cpupools are a non-optional construct in Xen. >>> >>> By default, one cpupool exists, with the id 0, using the default >>> scheduler covering all pCPUs, and dom0 is constructed in this cpupool. >>> >>> Passing 0 here is the backwards compatible option. >>> >>> And on that note, Luca, you ought to patch xl/libxl to apply the pool= >>> setting directly during domain create, rather than depending on cpupool >>> 0 existing and moving the domain later. >> Is it an enhancement or a bug fix? > > This isn't a binary option. > > Your series added an optimisation to DOMCTL_createdomain, then didn't > adjust libxl to use the optimisation (which would have reduced the > number of hypercalls to create the domain, and reduce the number of > dynamic memory allocations in the hypervisor. Marginal, certainly, but > clearly a nice-to-have). > > Therefore, you created technical debt, which is option 3. > > By default, as the contributor, you are expected to address the > technical debt, because it is an important difference between hacking a > feature up for yourself, and integrating the feature nicely for everyone. > > You can of course negotiate with the tools maintainer to see if they > care, and right now that's a bit difficult. It's quite possible that > noone other than me cares, and I'm not libxl maintainer. > > Either you need to pay off the technical debt, or someone else will have > to. Someone else is going to have to start with digging into source > history, which means it's more expensive than you doing it now. > > At an absolute minimum, you need to be aware of where/when you are > creating technical debt.
Ok, we've just created a task to handle this work so that we can track it, we will handle it in the future. Cheers, Luca > >> From what I know, please correct me if I’m wrong, cpupool0 >> Is always present, so there won’t be problem on assuming its existence > > From what I can see, your series has reduced the magic involved with > cpupool0, which is good. > > But the fact that it still has magic properties is still technical debt > that someone is going to have to pay off eventually. > > ~Andrew
