xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0 of 3] libxl: tweak the cpupool interface slightly
@ 2012-01-23 14:43 Ian Campbell
  2012-01-23 14:43 ` [PATCH 1 of 3] libxl: remove libxl_domain_create_info.poolname Ian Campbell
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Ian Campbell @ 2012-01-23 14:43 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson, juergen.gross

Just a few changes I noticed while playing with cpupools on Friday.

There is no need for both poolid and poolname in the libxl API. Remove
poolname.

Use libxl_cpupool_create not libxl_create_cpupool for consistency with
other functions.

Do not write "pool_name" to xenstore. I couldn't find any reader of
this node but I left it till last so it can easily be dropped if
necessary.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1 of 3] libxl: remove libxl_domain_create_info.poolname
  2012-01-23 14:43 [PATCH 0 of 3] libxl: tweak the cpupool interface slightly Ian Campbell
@ 2012-01-23 14:43 ` Ian Campbell
  2012-01-24  7:21   ` Juergen Gross
  2012-01-25 16:54   ` Ian Campbell
  2012-01-23 14:43 ` [PATCH 2 of 3] libxl: name libxl_create_cpupool consistent with other functions Ian Campbell
  2012-01-23 14:43 ` [PATCH 3 of 3] libxl: do not write/maintain "pool_name" in XenStore Ian Campbell
  2 siblings, 2 replies; 8+ messages in thread
From: Ian Campbell @ 2012-01-23 14:43 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson, juergen.gross

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# 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.

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

diff -r 4bb2b6a04ec0 -r c91bee33280d tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c	Fri Jan 20 17:12:17 2012 +0000
+++ b/tools/libxl/libxl_create.c	Mon Jan 23 14:35:46 2012 +0000
@@ -60,7 +60,7 @@ int libxl_init_create_info(libxl_ctx *ct
     c_info->type = LIBXL_DOMAIN_TYPE_HVM;
     c_info->oos = 1;
     c_info->ssidref = 0;
-    c_info->poolid = 0;
+    c_info->poolid = -1;
     return 0;
 }
 
@@ -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 4bb2b6a04ec0 -r c91bee33280d tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl	Fri Jan 20 17:12:17 2012 +0000
+++ b/tools/libxl/libxl_types.idl	Mon Jan 23 14:35:46 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 4bb2b6a04ec0 -r c91bee33280d tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Fri Jan 20 17:12:17 2012 +0000
+++ b/tools/libxl/xl_cmdimpl.c	Mon Jan 23 14:35:46 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
@@ -640,11 +640,9 @@ static void parse_config_data(const char
         c_info->oos = l;
 
     if (!xlu_cfg_get_string (config, "pool", &buf, 0)) {
-        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);
     }

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 2 of 3] libxl: name libxl_create_cpupool consistent with other functions
  2012-01-23 14:43 [PATCH 0 of 3] libxl: tweak the cpupool interface slightly Ian Campbell
  2012-01-23 14:43 ` [PATCH 1 of 3] libxl: remove libxl_domain_create_info.poolname Ian Campbell
@ 2012-01-23 14:43 ` Ian Campbell
  2012-01-24  7:21   ` Juergen Gross
  2012-01-23 14:43 ` [PATCH 3 of 3] libxl: do not write/maintain "pool_name" in XenStore Ian Campbell
  2 siblings, 1 reply; 8+ messages in thread
From: Ian Campbell @ 2012-01-23 14:43 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson, juergen.gross

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1327329347 0
# Node ID 10f5656caaaa2bdfd6a7ae9aada903da8bddcbb6
# Parent  c91bee33280debdfe602d28e48318c03ddf0f4c9
libxl: name libxl_create_cpupool consistent with other functions.

The pattern for the other cpupool functions is libxl_cpupool_<ACTION>
and in general we use libxl_<THING>_<ACTION>

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

diff -r c91bee33280d -r 10f5656caaaa tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Mon Jan 23 14:35:46 2012 +0000
+++ b/tools/libxl/libxl.c	Mon Jan 23 14:35:47 2012 +0000
@@ -3172,7 +3172,7 @@ int libxl_get_freecpus(libxl_ctx *ctx, l
     return 0;
 }
 
-int libxl_create_cpupool(libxl_ctx *ctx, const char *name, int schedid,
+int libxl_cpupool_create(libxl_ctx *ctx, const char *name, int schedid,
                          libxl_cpumap cpumap, libxl_uuid *uuid,
                          uint32_t *poolid)
 {
diff -r c91bee33280d -r 10f5656caaaa tools/libxl/libxl.h
--- a/tools/libxl/libxl.h	Mon Jan 23 14:35:46 2012 +0000
+++ b/tools/libxl/libxl.h	Mon Jan 23 14:35:47 2012 +0000
@@ -605,7 +605,7 @@ int libxl_tmem_shared_auth(libxl_ctx *ct
 int libxl_tmem_freeable(libxl_ctx *ctx);
 
 int libxl_get_freecpus(libxl_ctx *ctx, libxl_cpumap *cpumap);
-int libxl_create_cpupool(libxl_ctx *ctx, const char *name, int schedid,
+int libxl_cpupool_create(libxl_ctx *ctx, const char *name, int schedid,
                          libxl_cpumap cpumap, libxl_uuid *uuid,
                          uint32_t *poolid);
 int libxl_cpupool_destroy(libxl_ctx *ctx, uint32_t poolid);
diff -r c91bee33280d -r 10f5656caaaa tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c	Mon Jan 23 14:35:46 2012 +0000
+++ b/tools/libxl/xl_cmdimpl.c	Mon Jan 23 14:35:47 2012 +0000
@@ -5332,7 +5332,7 @@ int main_cpupoolcreate(int argc, char **
         return 0;
 
     poolid = 0;
-    if (libxl_create_cpupool(ctx, name, schedid, cpumap, &uuid, &poolid)) {
+    if (libxl_cpupool_create(ctx, name, schedid, cpumap, &uuid, &poolid)) {
         fprintf(stderr, "error on creating cpupool\n");
         return -ERROR_FAIL;
     }
@@ -5706,7 +5706,7 @@ int main_cpupoolnumasplit(int argc, char
         snprintf(name, 15, "Pool-node%d", node);
         libxl_uuid_generate(&uuid);
         poolid = 0;
-        ret = -libxl_create_cpupool(ctx, name, schedid, cpumap, &uuid, &poolid);
+        ret = -libxl_cpupool_create(ctx, name, schedid, cpumap, &uuid, &poolid);
         if (ret) {
             fprintf(stderr, "error on creating cpupool\n");
             goto out;

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 3 of 3] libxl: do not write/maintain "pool_name" in XenStore
  2012-01-23 14:43 [PATCH 0 of 3] libxl: tweak the cpupool interface slightly Ian Campbell
  2012-01-23 14:43 ` [PATCH 1 of 3] libxl: remove libxl_domain_create_info.poolname Ian Campbell
  2012-01-23 14:43 ` [PATCH 2 of 3] libxl: name libxl_create_cpupool consistent with other functions Ian Campbell
@ 2012-01-23 14:43 ` Ian Campbell
  2012-01-24  8:07   ` Juergen Gross
  2 siblings, 1 reply; 8+ messages in thread
From: Ian Campbell @ 2012-01-23 14:43 UTC (permalink / raw)
  To: xen-devel; +Cc: ian.jackson, juergen.gross

# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# Date 1327329565 0
# Node ID d8efdb16b1ef06013061da1f47c2dbce4b042992
# Parent  10f5656caaaa2bdfd6a7ae9aada903da8bddcbb6
libxl: do not write/maintain "pool_name" in XenStore

Nothing that I can find ever reads this key.

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

diff -r 10f5656caaaa -r d8efdb16b1ef tools/libxl/libxl.c
--- a/tools/libxl/libxl.c	Mon Jan 23 14:35:47 2012 +0000
+++ b/tools/libxl/libxl.c	Mon Jan 23 14:39:25 2012 +0000
@@ -3434,16 +3434,6 @@ int libxl_cpupool_movedomain(libxl_ctx *
 {
     GC_INIT(ctx);
     int rc;
-    char *dom_path;
-    char *vm_path;
-    char *poolname;
-    xs_transaction_t t;
-
-    dom_path = libxl__xs_get_dompath(gc, domid);
-    if (!dom_path) {
-        GC_FREE;
-        return ERROR_FAIL;
-    }
 
     rc = xc_cpupool_movedomain(ctx->xch, poolid, domid);
     if (rc) {
@@ -3453,21 +3443,6 @@ int libxl_cpupool_movedomain(libxl_ctx *
         return ERROR_FAIL;
     }
 
-    for (;;) {
-        t = xs_transaction_start(ctx->xsh);
-
-        poolname = libxl__cpupoolid_to_name(gc, poolid);
-        vm_path = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/vm", dom_path));
-        if (!vm_path)
-            break;
-
-        libxl__xs_write(gc, t, libxl__sprintf(gc, "%s/pool_name", vm_path),
-                        "%s", poolname);
-
-        if (xs_transaction_end(ctx->xsh, t, 0) || (errno != EAGAIN))
-            break;
-    }
-
     GC_FREE;
     return 0;
 }
diff -r 10f5656caaaa -r d8efdb16b1ef tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c	Mon Jan 23 14:35:47 2012 +0000
+++ b/tools/libxl/libxl_create.c	Mon Jan 23 14:39:25 2012 +0000
@@ -438,9 +438,6 @@ 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->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);

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1 of 3] libxl: remove libxl_domain_create_info.poolname
  2012-01-23 14:43 ` [PATCH 1 of 3] libxl: remove libxl_domain_create_info.poolname Ian Campbell
@ 2012-01-24  7:21   ` Juergen Gross
  2012-01-25 16:54   ` Ian Campbell
  1 sibling, 0 replies; 8+ messages in thread
From: Juergen Gross @ 2012-01-24  7:21 UTC (permalink / raw)
  To: Ian Campbell; +Cc: ian.jackson, xen-devel

On 01/23/2012 03:43 PM, Ian Campbell wrote:
> # HG changeset patch
> # User Ian Campbell<ian.campbell@citrix.com>
> # 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.
>
> Signed-off-by: Ian Campbell<ian.campbell@citrix.com>
Acked-by: juergen.gross@ts.fujitsu.com
> diff -r 4bb2b6a04ec0 -r c91bee33280d tools/libxl/libxl_create.c
> --- a/tools/libxl/libxl_create.c	Fri Jan 20 17:12:17 2012 +0000
> +++ b/tools/libxl/libxl_create.c	Mon Jan 23 14:35:46 2012 +0000
> @@ -60,7 +60,7 @@ int libxl_init_create_info(libxl_ctx *ct
>       c_info->type = LIBXL_DOMAIN_TYPE_HVM;
>       c_info->oos = 1;
>       c_info->ssidref = 0;
> -    c_info->poolid = 0;
> +    c_info->poolid = -1;
>       return 0;
>   }
>
> @@ -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 4bb2b6a04ec0 -r c91bee33280d tools/libxl/libxl_types.idl
> --- a/tools/libxl/libxl_types.idl	Fri Jan 20 17:12:17 2012 +0000
> +++ b/tools/libxl/libxl_types.idl	Mon Jan 23 14:35:46 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 4bb2b6a04ec0 -r c91bee33280d tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c	Fri Jan 20 17:12:17 2012 +0000
> +++ b/tools/libxl/xl_cmdimpl.c	Mon Jan 23 14:35:46 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
> @@ -640,11 +640,9 @@ static void parse_config_data(const char
>           c_info->oos = l;
>
>       if (!xlu_cfg_get_string (config, "pool",&buf, 0)) {
> -        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@lists.xensource.com
> http://lists.xensource.com/xen-devel
>
>


-- 
Juergen Gross                 Principal Developer Operating Systems
PDG ES&S SWE OS6                       Telephone: +49 (0) 89 3222 2967
Fujitsu Technology Solutions              e-mail: juergen.gross@ts.fujitsu.com
Domagkstr. 28                           Internet: ts.fujitsu.com
D-80807 Muenchen                 Company details: ts.fujitsu.com/imprint.html

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2 of 3] libxl: name libxl_create_cpupool consistent with other functions
  2012-01-23 14:43 ` [PATCH 2 of 3] libxl: name libxl_create_cpupool consistent with other functions Ian Campbell
@ 2012-01-24  7:21   ` Juergen Gross
  0 siblings, 0 replies; 8+ messages in thread
From: Juergen Gross @ 2012-01-24  7:21 UTC (permalink / raw)
  To: Ian Campbell; +Cc: ian.jackson, xen-devel

On 01/23/2012 03:43 PM, Ian Campbell wrote:
> # HG changeset patch
> # User Ian Campbell<ian.campbell@citrix.com>
> # Date 1327329347 0
> # Node ID 10f5656caaaa2bdfd6a7ae9aada903da8bddcbb6
> # Parent  c91bee33280debdfe602d28e48318c03ddf0f4c9
> libxl: name libxl_create_cpupool consistent with other functions.
>
> The pattern for the other cpupool functions is libxl_cpupool_<ACTION>
> and in general we use libxl_<THING>_<ACTION>
>
> Signed-off-by: Ian Campbell<ian.campbell@citrix.com>
Acked-by: juergen.gross@ts.fujitsu.com
> diff -r c91bee33280d -r 10f5656caaaa tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c	Mon Jan 23 14:35:46 2012 +0000
> +++ b/tools/libxl/libxl.c	Mon Jan 23 14:35:47 2012 +0000
> @@ -3172,7 +3172,7 @@ int libxl_get_freecpus(libxl_ctx *ctx, l
>       return 0;
>   }
>
> -int libxl_create_cpupool(libxl_ctx *ctx, const char *name, int schedid,
> +int libxl_cpupool_create(libxl_ctx *ctx, const char *name, int schedid,
>                            libxl_cpumap cpumap, libxl_uuid *uuid,
>                            uint32_t *poolid)
>   {
> diff -r c91bee33280d -r 10f5656caaaa tools/libxl/libxl.h
> --- a/tools/libxl/libxl.h	Mon Jan 23 14:35:46 2012 +0000
> +++ b/tools/libxl/libxl.h	Mon Jan 23 14:35:47 2012 +0000
> @@ -605,7 +605,7 @@ int libxl_tmem_shared_auth(libxl_ctx *ct
>   int libxl_tmem_freeable(libxl_ctx *ctx);
>
>   int libxl_get_freecpus(libxl_ctx *ctx, libxl_cpumap *cpumap);
> -int libxl_create_cpupool(libxl_ctx *ctx, const char *name, int schedid,
> +int libxl_cpupool_create(libxl_ctx *ctx, const char *name, int schedid,
>                            libxl_cpumap cpumap, libxl_uuid *uuid,
>                            uint32_t *poolid);
>   int libxl_cpupool_destroy(libxl_ctx *ctx, uint32_t poolid);
> diff -r c91bee33280d -r 10f5656caaaa tools/libxl/xl_cmdimpl.c
> --- a/tools/libxl/xl_cmdimpl.c	Mon Jan 23 14:35:46 2012 +0000
> +++ b/tools/libxl/xl_cmdimpl.c	Mon Jan 23 14:35:47 2012 +0000
> @@ -5332,7 +5332,7 @@ int main_cpupoolcreate(int argc, char **
>           return 0;
>
>       poolid = 0;
> -    if (libxl_create_cpupool(ctx, name, schedid, cpumap,&uuid,&poolid)) {
> +    if (libxl_cpupool_create(ctx, name, schedid, cpumap,&uuid,&poolid)) {
>           fprintf(stderr, "error on creating cpupool\n");
>           return -ERROR_FAIL;
>       }
> @@ -5706,7 +5706,7 @@ int main_cpupoolnumasplit(int argc, char
>           snprintf(name, 15, "Pool-node%d", node);
>           libxl_uuid_generate(&uuid);
>           poolid = 0;
> -        ret = -libxl_create_cpupool(ctx, name, schedid, cpumap,&uuid,&poolid);
> +        ret = -libxl_cpupool_create(ctx, name, schedid, cpumap,&uuid,&poolid);
>           if (ret) {
>               fprintf(stderr, "error on creating cpupool\n");
>               goto out;
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>
>


-- 
Juergen Gross                 Principal Developer Operating Systems
PDG ES&S SWE OS6                       Telephone: +49 (0) 89 3222 2967
Fujitsu Technology Solutions              e-mail: juergen.gross@ts.fujitsu.com
Domagkstr. 28                           Internet: ts.fujitsu.com
D-80807 Muenchen                 Company details: ts.fujitsu.com/imprint.html

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 3 of 3] libxl: do not write/maintain "pool_name" in XenStore
  2012-01-23 14:43 ` [PATCH 3 of 3] libxl: do not write/maintain "pool_name" in XenStore Ian Campbell
@ 2012-01-24  8:07   ` Juergen Gross
  0 siblings, 0 replies; 8+ messages in thread
From: Juergen Gross @ 2012-01-24  8:07 UTC (permalink / raw)
  To: Ian Campbell; +Cc: ian.jackson, xen-devel

On 01/23/2012 03:43 PM, Ian Campbell wrote:
> # HG changeset patch
> # User Ian Campbell<ian.campbell@citrix.com>
> # Date 1327329565 0
> # Node ID d8efdb16b1ef06013061da1f47c2dbce4b042992
> # Parent  10f5656caaaa2bdfd6a7ae9aada903da8bddcbb6
> libxl: do not write/maintain "pool_name" in XenStore
>
> Nothing that I can find ever reads this key.
>
> Signed-off-by: Ian Campbell<ian.campbell@citrix.com>

Acked-by: juergen.gross@ts.fujitsu.com

> diff -r 10f5656caaaa -r d8efdb16b1ef tools/libxl/libxl.c
> --- a/tools/libxl/libxl.c	Mon Jan 23 14:35:47 2012 +0000
> +++ b/tools/libxl/libxl.c	Mon Jan 23 14:39:25 2012 +0000
> @@ -3434,16 +3434,6 @@ int libxl_cpupool_movedomain(libxl_ctx *
>   {
>       GC_INIT(ctx);
>       int rc;
> -    char *dom_path;
> -    char *vm_path;
> -    char *poolname;
> -    xs_transaction_t t;
> -
> -    dom_path = libxl__xs_get_dompath(gc, domid);
> -    if (!dom_path) {
> -        GC_FREE;
> -        return ERROR_FAIL;
> -    }
>
>       rc = xc_cpupool_movedomain(ctx->xch, poolid, domid);
>       if (rc) {
> @@ -3453,21 +3443,6 @@ int libxl_cpupool_movedomain(libxl_ctx *
>           return ERROR_FAIL;
>       }
>
> -    for (;;) {
> -        t = xs_transaction_start(ctx->xsh);
> -
> -        poolname = libxl__cpupoolid_to_name(gc, poolid);
> -        vm_path = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/vm", dom_path));
> -        if (!vm_path)
> -            break;
> -
> -        libxl__xs_write(gc, t, libxl__sprintf(gc, "%s/pool_name", vm_path),
> -                        "%s", poolname);
> -
> -        if (xs_transaction_end(ctx->xsh, t, 0) || (errno != EAGAIN))
> -            break;
> -    }
> -
>       GC_FREE;
>       return 0;
>   }
> diff -r 10f5656caaaa -r d8efdb16b1ef tools/libxl/libxl_create.c
> --- a/tools/libxl/libxl_create.c	Mon Jan 23 14:35:47 2012 +0000
> +++ b/tools/libxl/libxl_create.c	Mon Jan 23 14:39:25 2012 +0000
> @@ -438,9 +438,6 @@ 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->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);
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>
>


-- 
Juergen Gross                 Principal Developer Operating Systems
PDG ES&S SWE OS6                       Telephone: +49 (0) 89 3222 2967
Fujitsu Technology Solutions              e-mail: juergen.gross@ts.fujitsu.com
Domagkstr. 28                           Internet: ts.fujitsu.com
D-80807 Muenchen                 Company details: ts.fujitsu.com/imprint.html

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1 of 3] libxl: remove libxl_domain_create_info.poolname
  2012-01-23 14:43 ` [PATCH 1 of 3] libxl: remove libxl_domain_create_info.poolname Ian Campbell
  2012-01-24  7:21   ` Juergen Gross
@ 2012-01-25 16:54   ` Ian Campbell
  1 sibling, 0 replies; 8+ messages in thread
From: Ian Campbell @ 2012-01-25 16:54 UTC (permalink / raw)
  To: xen-devel@lists.xensource.com; +Cc: juergen.gross@ts.fujitsu.com, Ian Jackson

On Mon, 2012-01-23 at 14:43 +0000, Ian Campbell wrote:
> # HG changeset patch
> # User Ian Campbell <ian.campbell@citrix.com>
> # 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@citrix.com>
# 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@citrix.com>

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2012-01-25 16:54 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-01-23 14:43 [PATCH 0 of 3] libxl: tweak the cpupool interface slightly Ian Campbell
2012-01-23 14:43 ` [PATCH 1 of 3] libxl: remove libxl_domain_create_info.poolname Ian Campbell
2012-01-24  7:21   ` Juergen Gross
2012-01-25 16:54   ` Ian Campbell
2012-01-23 14:43 ` [PATCH 2 of 3] libxl: name libxl_create_cpupool consistent with other functions Ian Campbell
2012-01-24  7:21   ` Juergen Gross
2012-01-23 14:43 ` [PATCH 3 of 3] libxl: do not write/maintain "pool_name" in XenStore Ian Campbell
2012-01-24  8:07   ` Juergen Gross

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).