Re: [Xen-devel] [PATCH 1 of 3] libxl: remove libxl_domain_create_info.poolname

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2012-01-23 at 14:43 +0000, Ian Campbell wrote:
> # HG changeset patch
> # User Ian Campbell <ian.campbell@xxxxxxxxxx>
> # Date 1327329346 0
> # Node ID c91bee33280debdfe602d28e48318c03ddf0f4c9
> # Parent  4bb2b6a04ec02c3502c1825e2736df54870229e5
> libxl: remove libxl_domain_create_info.poolname
> 
> It is redundant with poolid and allowing the user to specify both
> opens up the possibility of a disconnect.
> 
> Also default c_info->poolid to -1 (no pool) instead of defaulting to 0 in the
> library and overriding in the toolstack.

The above seems to be a false assumption -- the correct default is 0,
libxl does not actually treat -1 as special and xen always creates a
single pool on boot.

Found by actually testing without using a pool option which also showed
up that the option parsing in xl was wrong, we cannot call
libxl_cpupoolid_to_name on an invalid pool (which is the case if no pool
is given).

Corrected patch follows.

8<---------------------------------------------------------------------

# HG changeset patch
# User Ian Campbell <ian.campbell@xxxxxxxxxx>
# Date 1327510469 0
# Node ID 4b54a3680947f9cf24340a3f7e458afc948c7165
# Parent  3f0d2ed701104e7e37560a3262bc7bcd7da5b90b
libxl: remove libxl_domain_create_info.poolname

It is redundant with poolid and allowing the user to specify both
opens up the possibility of a disconnect.

Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

diff -r 3f0d2ed70110 -r 4b54a3680947 tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c        Wed Jan 25 16:28:58 2012 +0000
+++ b/tools/libxl/libxl_create.c        Wed Jan 25 16:54:29 2012 +0000
@@ -438,8 +438,9 @@ retry_transaction:
 
     xs_write(ctx->xsh, t, libxl__sprintf(gc, "%s/uuid", vm_path), uuid_string, 
strlen(uuid_string));
     xs_write(ctx->xsh, t, libxl__sprintf(gc, "%s/name", vm_path), info->name, 
strlen(info->name));
-    if (info->poolname)
-        xs_write(ctx->xsh, t, libxl__sprintf(gc, "%s/pool_name", vm_path), 
info->poolname, strlen(info->poolname));
+    if (info->poolid != -1)
+        libxl__xs_write(gc, t, libxl__sprintf(gc, "%s/pool_name", vm_path),
+                        "%s", libxl__cpupoolid_to_name(gc, info->poolid));
 
     libxl__xs_writev(gc, t, dom_path, info->xsdata);
     libxl__xs_writev(gc, t, libxl__sprintf(gc, "%s/platform", dom_path), 
info->platformdata);
diff -r 3f0d2ed70110 -r 4b54a3680947 tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl       Wed Jan 25 16:28:58 2012 +0000
+++ b/tools/libxl/libxl_types.idl       Wed Jan 25 16:54:29 2012 +0000
@@ -156,7 +156,6 @@ libxl_domain_create_info = Struct("domai
     ("xsdata",       libxl_key_value_list),
     ("platformdata", libxl_key_value_list),
     ("poolid",       uint32),
-    ("poolname",     string),
     ])
 
 libxl_domain_build_info = Struct("domain_build_info",[
diff -r 3f0d2ed70110 -r 4b54a3680947 tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c  Wed Jan 25 16:28:58 2012 +0000
+++ b/tools/libxl/xl_cmdimpl.c  Wed Jan 25 16:54:29 2012 +0000
@@ -315,7 +315,7 @@ static void printf_info(int domid,
         printf("\t(uuid <unknown>)\n");
     }
 
-    printf("\t(cpupool %s)\n", c_info->poolname);
+    printf("\t(cpupool %s)\n", libxl_cpupoolid_to_name(ctx, c_info->poolid));
     if (c_info->xsdata)
         printf("\t(xsdata contains data)\n");
     else
@@ -643,8 +643,7 @@ static void parse_config_data(const char
         c_info->poolid = -1;
         cpupool_qualifier_to_cpupoolid(buf, &c_info->poolid, NULL);
     }
-    c_info->poolname = libxl_cpupoolid_to_name(ctx, c_info->poolid);
-    if (!c_info->poolname) {
+    if (!libxl_cpupoolid_to_name(ctx, c_info->poolid)) {
         fprintf(stderr, "Illegal pool specified\n");
         exit(1);
     }



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel