> On 15 Feb 2022, at 10:33, Juergen Gross <jgr...@suse.com> wrote:
>
> 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.fance...@arm.com>
>> ---
>> 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;
Hi Juergen,
>
> pool_string seems to be unused.
I will remove it
>
>> + 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?
Why would you like more this approach? I was trying to avoid allocation/free
operations in a loop that would need also more code to check cases in which
memory is not allocated. Instead with the buffers in the stack we don’t have
problems.
>
>> + 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.
Yes you are right, it would simplify the code
>
>> + }
>> + } 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?
This serie is more about cpupools, maybe I can do that in another commit out
of this serie.
Thanks for your review.
Cheers,
Luca
>
>
> Juergen
> <OpenPGP_0xB0DE9DD628BF132F.asc>
