[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 2/2] tools/ocaml: Fix stubs the introduction of domain_create.cpupool_id


  • To: Luca Fancellu <Luca.Fancellu@xxxxxxx>
  • From: Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>
  • Date: Wed, 18 May 2022 11:34:07 +0000
  • Accept-language: en-GB, en-US
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=Kr8WqZ1XN6M2i3dml6O22rlME34Dkh62jD+RCBBQlVw=; b=XGWGqR77YubtC1wQOIWFGdikz4bUcEYdnYhXNlQgl2V3amQLv5kc4n945zyPVYFxPrxMnGGAr2gcSqMiS2NqBkRUiCTqxHk3C2R3tr5ZIMb8q+xQWs6EPOGyM6F4vi5X2F+i7n+xCtOWpRYpqMFZeQKIokOPgg+IRRVcN0kkygw/VqgkP8IdZJ3Iru13m58aVEyIZgVHzAqh1Pl/ejeXQzCB68ylocUr3Z1Q2SgIaObjD6llvDAFrIDprEN1GDq3IdBZNurYSwcWfWvO2pMKH94hVXjxyB5KfHxyGMUSrlM6bh1AKq/KbF+GUbehVvqDm6AhZuDp5gSejN7Vv2Z9vw==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=PqgWM9yu8Q2GqfHRd3FXEnksF5LkqRWHEV6jC/1ZvbHNa9kf/FluyQdqKSLN+himCfG1JVGhSl47inL8J282WB9fvHUtPNRHWBCPNq9fqCPy40cPnmUxwY6aEeuZVLicjCZa2Zovfs5SlTS99/CXJWV8eSmEvuQsEnuj5fOLH8xYlFvL2qT26hYmhuW6JLhlhbEA44grpZ6hn26AVY1FDGB5n+uI6pwX24iHTei1TgBuXAWRFshhXhf8o08dlIDK04G8mYsJknPrVgxRixTF9rg0PRXf7H+jrE/obBn7TTNjh65W3qk1wTiY37BTkVE4QwCG8Q2FsWCBIDzkuUMgbQ==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
  • Cc: Edwin Torok <edvin.torok@xxxxxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Christian Lindig <christian.lindig@xxxxxxxxxx>
  • Delivery-date: Wed, 18 May 2022 11:34:24 +0000
  • Ironport-data: A9a23:hObT8q71slpheg8QMfs+ZgxRtFPGchMFZxGqfqrLsTDasY5as4F+v jZNWG+Ab62DZDb2eNpzOYS0808PsZGHmtBhS1c5+3tkHi5G8cbLO4+Ufxz6V8+wwmwvb67FA +E2MISowBUcFyeEzvuVGuG96yE6j8lkf5KkYAL+EnkZqTRMFWFw0HqPp8Zj2tQy2YXgU1vU0 T/Pi5a31GGNimYc3l08s8pvmDs31BglkGpF1rCWTakjUG72zxH5PrpGTU2CByKQrr1vNvy7X 47+IISRpQs1yfuP5uSNyd4XemVSKlLb0JPnZnB+A8BOiTAazsA+PzpS2FPxpi67hh3Q9+2dx umhurTtVwINYfePqt8UVjZCGi1aYKNA57nIdC3XXcy7lyUqclPK6tA3VgQcG91d/ex6R2ZT6 fYfNTYBKAiZgP67y666Te8qgdk/KM7sP8UUvXQIITPxVK56B8ycBfiXo4YHh1/chegXdRraT +MfZSBic1LrZBpXN01MIJk/gP2plj/0dDgwRFe9+vJmvjiIklwZPL7FIeTZS8CVa9dprGnb+ Fvf2VTjIxsdHYnKodaC2jf27gPVpgvfRYkbUpOx8PVnhFmO7mUJDVsdUl7Tiem0jAuyVsxSL 2QQ+zEytu4i+UqzVN7/Uhak5nmesXYht8F4FuQ77ESHzPrS6gPAXGwcFGceM5ohqdM8QiEs2 hmRhdT1CDdzsbqTD3WA6rOTqjD0Mi8QRYMfWRI5ocI+y4GLiOkOYtjnF76PzIbdYgXJJAzN
  • Ironport-hdrordr: A9a23:iWTtta0z3UUrH3XoJ5FAwAqjBetxeYIsimQD101hICG9Lfb0qy n+pp4mPEHP4wr5AEtQ4uxpOMG7MBDhHQYc2/hcAV7QZnidhILOFvAs0WKC+UysJ8SazIJgPM hbAs9D4bHLbGSSyPyKmDVQcOxQjuVvkprY49s2pk0FJW4FV0gj1XYBNu/xKDwVeOAyP+tcKH Pq3Lsjm9PPQxQqR/X+IkNAc/nIptXNmp6jSwUBHQQb5A6Hii7twKLmEjCDty1uEw9n8PMHyy zoggb57qKsv7WQ0RnHzVLe6JxQhZ/I1sZDPsqRkcIYQw+cyTpAJb4RGYFqjgpF5N1H22xa1+ UkZC1Qefib3kmhO11dZyGdgjUIngxes0MKgmXo/EcL6faJOA7STfAxxL6xOyGplXbJ9rtHod 129nPcuJxNARzamiPho9DOShFxj0Kx5WEviOgJkhVkIMMjgZJq3PoiFXluYd499ePBmfIaOf grCNuZ6OddcFucYXyctm5zwMa0VnB2GhudWEANtsGczjATxRlCvgEl7d1amm1F+IM2SpFC6e iBOqN0lKtWRstTaa5mHu8OTca+F2SISxPRN2CZJ0jhCcg8Sjnwgo+y5K9w6PCheZQOwpd3kJ PdUElAvWp3YE7qAd3m5uw9zvkMehTIYd3A8LAv23EigMyMeFPCC1zxdHk+1829vv4YHsrXH/ 6uJZM+OY6XEVfT
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Thread-index: AQHYaiYkF2Fwt7YEY0iLRkyR5qKZ760kZYcAgAAF1QCAAATZgIAAEdyA
  • Thread-topic: [PATCH 2/2] tools/ocaml: Fix stubs the introduction of domain_create.cpupool_id

On 18/05/2022 11:30, Luca Fancellu wrote:
>> On 18 May 2022, at 11:12, Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx> 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.

>  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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.