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

[Juergen Gross]

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;

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;

@@ -114,6 +117,27 @@ int main(int argc, char **argv)
          goto out;
      }

+    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);

/* Accept 0 or 1 argument */ + /* Create an entry in xenstore for each cpupool on the system */

... 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

OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

OpenPGP_signature
Description: OpenPGP digital signature