Re: [PATCH 1/5] tools/cpupools: Give a name to unnamed cpupools

[Juergen Gross <jgross@xxxxxxxx>]

On 15.02.22 11:15, Luca Fancellu wrote:
> With the introduction of boot time cpupools, Xen can create many
> different cpupools at boot time other than cpupool with id 0.
>
> Since these newly created cpupools can't have an
> entry in Xenstore, create the entry using xen-init-dom0
> helper with the usual convention: Pool-<cpupool id>.
>
> Given the change, remove the check for poolid == 0 from
> libxl_cpupoolid_to_name(...).
>
> Signed-off-by: Luca Fancellu <luca.fancellu@xxxxxxx>
> ---
>   tools/helpers/xen-init-dom0.c  | 26 +++++++++++++++++++++++++-
>   tools/libs/light/libxl_utils.c |  3 +--
>   2 files changed, 26 insertions(+), 3 deletions(-)
>
> diff --git a/tools/helpers/xen-init-dom0.c b/tools/helpers/xen-init-dom0.c
> index c99224a4b607..3539f56faeb0 100644
> --- a/tools/helpers/xen-init-dom0.c
> +++ b/tools/helpers/xen-init-dom0.c
> @@ -43,7 +43,10 @@ int main(int argc, char **argv)
>       int rc;
>       struct xs_handle *xsh = NULL;
>       xc_interface *xch = NULL;
> -    char *domname_string = NULL, *domid_string = NULL;
> +    char *domname_string = NULL, *domid_string = NULL, *pool_string = NULL;
pool_string seems to be unused.

> +    char pool_path[strlen("/local/pool") + 12], pool_name[strlen("Pool-") + 5];
I don't like that. Why don't you use pointers and ...

> +    xc_cpupoolinfo_t *xcinfo;
> +    unsigned int pool_id = 0;
>       libxl_uuid uuid;
>   
>       /* Accept 0 or 1 argument */
> @@ -114,6 +117,27 @@ int main(int argc, char **argv)
>           goto out;
>       }
>   
> +    /* Create an entry in xenstore for each cpupool on the system */
> +    do {
> +        xcinfo = xc_cpupool_getinfo(xch, pool_id);
> +        if (xcinfo != NULL) {
> +            if (xcinfo->cpupool_id != pool_id)
> +                pool_id = xcinfo->cpupool_id;
> +            snprintf(pool_path, sizeof(pool_path), "/local/pool/%d/name",
> +                     pool_id);
> +            snprintf(pool_name, sizeof(pool_name), "Pool-%d", pool_id);
... use asprintf() here for allocating the strings in the needed size?

> +            pool_id++;
> +            if (!xs_write(xsh, XBT_NULL, pool_path, pool_name,
> +                          strlen(pool_name))) {
> +                fprintf(stderr, "cannot set pool name\n");
> +                rc = 1;
> +            }
> +            xc_cpupool_infofree(xch, xcinfo);
> +            if (rc)
> +                goto out;
Moving the call of xc_cpupool_infofree() ahead of the call of xs_write()
would drop the need for this last if statement, as you could add the
goto to the upper if.

> +        }
> +    } while(xcinfo != NULL);
> +
With doing all of this for being able to assign other domains created
at boot to cpupools, shouldn't you add names for other domains than dom0
here, too?


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature