Re: [RFC PATCH 2/2] tools/cpupools: Give a name to unnamed cpupools

From: Juergen Gross <jgross@suse.com>
To: Luca Fancellu <luca.fancellu@arm.com>
Cc: Xen-devel <xen-devel@lists.xenproject.org>,
	Bertrand Marquis <bertrand.marquis@arm.com>,
	wei.chen@arm.com, Ian Jackson <iwj@xenproject.org>,
	Wei Liu <wl@xen.org>, Anthony PERARD <anthony.perard@citrix.com>
Subject: Re: [RFC PATCH 2/2] tools/cpupools: Give a name to unnamed cpupools
Date: Wed, 17 Nov 2021 13:06:41 +0100	[thread overview]
Message-ID: <3ecbae61-ade8-955f-a1f9-bf7478e2821a@suse.com> (raw)
In-Reply-To: <D14A26F2-487D-42B6-BD3B-552CEFA84294@arm.com>

[-- Attachment #1.1.1: Type: text/plain, Size: 2456 bytes --]

On 17.11.21 12:52, Luca Fancellu wrote:
> 
> 
>> On 17 Nov 2021, at 11:10, Juergen Gross <jgross@suse.com> wrote:
>>
>> On 17.11.21 10:57, Luca Fancellu wrote:
>>> With the introduction of boot time cpupools, Xen can
>>> create at boot time many cpupools different from the
>>> cpupool with id 0.
>>> Since these newly created cpupools can't have an
>>> entry in Xenstore, name them with the same convention
>>> used for the cpupool 0: Pool-<cpupool id>.
>>> Signed-off-by: Luca Fancellu <luca.fancellu@arm.com>
>>> ---
>>>   tools/libs/light/libxl_utils.c | 13 ++++++++-----
>>>   1 file changed, 8 insertions(+), 5 deletions(-)
>>> diff --git a/tools/libs/light/libxl_utils.c b/tools/libs/light/libxl_utils.c
>>> index 4699c4a0a3..d97d91ca98 100644
>>> --- a/tools/libs/light/libxl_utils.c
>>> +++ b/tools/libs/light/libxl_utils.c
>>> @@ -147,13 +147,16 @@ int libxl_cpupool_qualifier_to_cpupoolid(libxl_ctx *ctx, const char *p,
>>>   char *libxl_cpupoolid_to_name(libxl_ctx *ctx, uint32_t poolid)
>>>   {
>>>       unsigned int len;
>>> -    char path[strlen("/local/pool") + 12];
>>> +    char buffer[strlen("/local/pool") + 12];
>>>       char *s;
>>>   -    snprintf(path, sizeof(path), "/local/pool/%d/name", poolid);
>>> -    s = xs_read(ctx->xsh, XBT_NULL, path, &len);
>>> -    if (!s && (poolid == 0))
>>> -        return strdup("Pool-0");
>>> +    snprintf(buffer, sizeof(buffer), "/local/pool/%d/name", poolid);
>>> +    s = xs_read(ctx->xsh, XBT_NULL, buffer, &len);
>>> +    if (!s)
>>> +    {
>>> +        snprintf(buffer, sizeof(buffer), "Pool-%d", poolid);
>>> +        return strdup(buffer);
>>> +    }
>>>       return s;
>>>   }
>>>   
>>
>> This breaks libxl_cpupoolid_is_valid(), as it will now return always
>> true, regardless whether the poolid is existing or not.
> 
> Hi Juergen,
> 
> Yes right, do you think I can use safely xc_cpupool_getinfo(…) when there is no entry
> in xenstore?
> I would check that the returned cpupool id is the same and if it isn’t or if I get a null
> result, then I will return NULL to ensure libxl_cpupoolid_is_valid(…) works again.

An alternative might be to let tools/helpers/xen-init-dom0.c let write
the missing cpupool entries (including for Pool-0) and drop the
poolid == 0 special casing from libxl_cpupoolid_to_name().

This should be rather easy by using xc_cpupool_getinfo() until it finds
no further cpupool.

Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3135 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]