Some (not only) cpupool related fixes and improvements
v2 cover Some (not only) cpupool related fixes and improvements
Hi,
Here's the take two of this:
http://www.gossamer-threads.com/lists/xen/devel/370516
I think I addressed all the comments from v1's review.
Sample output for the new `xl list' can be found here:
Available as a git branch here:
git://xenbits.xen.org/people/dariof/xen.git rel/cpupools/allow-ranges-v1
http://xenbits.xen.org/gitweb/?p=people/dariof/xen.git;a=shortlog;h=refs/heads/rel/cpupools/allow-ranges-v2
Thanks and Regards,
Dario
---
Dario Faggioli (7):
xl: turn some int local variable into bool
xl: add -c/--cpupool option to `xl list'
libxl: introduce libxl_cpupool_cpu{add,remove}_cpumap()
xl: enable using ranges of pCPUs when manipulating cpupools
xl: enable using ranges of pCPUs when creating cpupools
xl: make error reporting of cpupool subcommands consistent
xl: use libxl_cpupoolinfo_list_free() in main_cpupoolnumasplit
docs/man/xl.pod.1 | 29 +++++ docs/man/xlcpupool.cfg.pod.5 | 22 ++++ tools/libxl/libxl.c | 56 +++++++++- tools/libxl/libxl.h | 13 ++ tools/libxl/xl_cmdimpl.c | 225 ++++++++++++++++++++++-------------------- tools/libxl/xl_cmdtable.c | 1 6 files changed, 222 insertions(+), 124 deletions(-)
PATCH v2 1/7 xl: turn some int local variable into bool
more specifically, the ones used as argument presence flags in `xl list'.
---
Changes from v1:
* use true/false for actual parameters, as requested during review
--- tools/libxl/xl_cmdimpl.c | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-)
tools/libxl/xl_cmdimpl.c +14 -12
@@ -3505,7 +3505,7 @@ static void print_bitmap(uint8_t *map, int maplen, FILE *stream) } } -static void list_domains (int verbose,int context,int claim,int numa, +static void list_domains (bool verbose,bool context,bool claim,bool numa,const libxl_dominfo *info,int nb_domain) {int i;
@@ -4489,10 +4489,11 @@ int main_reboot(int argc, char **argv)int main_list (int argc,char **argv) { -int opt, verbose =0 ; -int context =0 ; -int details =0 ; -int numa =0 ; +int opt; +bool verbose = false; +bool context = false; +bool details = false; +bool numa = false;static struct option opts[] = { {"long" ,0 ,0 ,'l' }, {"verbose" ,0 ,0 ,'v' },
@@ -4508,16 +4509,16 @@ int main_list(int argc, char **argv) SWITCH_FOREACH_OPT(opt, "lvhZn", opts, "list", 0) {case 'l' : - details =1 ; + details = true;break ;case 'v' : - verbose =1 ; + verbose = true;break ;case 'Z' : - context =1 ; + context = true;break ;case 'n' : - numa =1 ; + numa = true;break ; }
@@ -4550,7 +4551,8 @@ int main_list(int argc, char **argv)if (details)list_domains_details (info, nb_domain);else -list_domains (verbose, context,0 /* claim */ , numa, info, nb_domain); +list_domains (verbose, context, false/* claim */ , numa, + info, nb_domain);if (info_free)libxl_dominfo_list_free (info, nb_domain);
@@ -6600,8 +6602,8 @@ int main_claims(int argc, char **argv)return 1 ; } -list_domains (0 /* verbose */ ,0 /* context */ ,1 /* claim */ , -0 /* numa */ , info, nb_domain); +list_domains (false/* verbose */ , false/* context */ , true/* claim */ , + false/* numa */ , info, nb_domain);libxl_dominfo_list_free (info, nb_domain);return 0 ;
PATCH v2 2/7 xl: add -c/--cpupool option to `xl list'
In reply to Dario Faggioliwhich, if provided, makes the command print a column with the name of the cpupool of the listed domain(s).
---
Changes from v1:
* use true/false for actual parameters, as requested during review
--- docs/man/xl.pod.1 | 4 ++++ tools/libxl/xl_cmdimpl.c | 19 +++++++++++++++---- tools/libxl/xl_cmdtable.c | 1 + 3 files changed, 20 insertions(+), 4 deletions(-)
docs/man/xl.pod.1 +4 -0
@@ -304,6 +304,10 @@ Also prints the security labels. Also prints the domain UUIDs, the shutdown reason and security labels. +=item B<-c>, <--cpupool> + +Also prints the cpupool the domain belong to. + =item B<-n>, <--numa> Also prints the domain NUMA node affinity.
tools/libxl/xl_cmdimpl.c +15 -4
@@ -3506,7 +3506,7 @@ static void print_bitmap(uint8_t *map, int maplen, FILE *stream) }static void list_domains (bool verbose,bool context,bool claim,bool numa, -const libxl_dominfo *info,int nb_domain) +bool cpupool,const libxl_dominfo *info,int nb_domain) {int i;static const char shutdown_reason_letters[]="-rscw" ;
@@ -3520,6 +3520,7 @@ static void list_domains(bool verbose, bool context, bool claim, bool numa,if (verbose)printf (" UUID Reason-Code\tSecurity Label" );if (context && !verbose)printf (" Security Label" );if (claim)printf (" Claimed" ); +if (cpupool)printf (" Cpupool" );if (numa) {if (libxl_node_bitmap_alloc (ctx, &nodemap,0 )) {fprintf (stderr,"libxl_node_bitmap_alloc_failed.\n" );
@@ -3564,6 +3565,11 @@ static void list_domains(bool verbose, bool context, bool claim, bool numa,printf (" %5lu" , (unsigned long )info[i].outstanding_memkb /1024 );if (verbose || context)printf (" %16s" , info[i].ssid_label ? :"-" ); +if (cpupool) { +char *poolname =libxl_cpupoolid_to_name (ctx, info[i].cpupool); +printf ("%16s" , poolname); +free (poolname); + }if (numa) {libxl_domain_get_nodeaffinity (ctx, info[i].domid, &nodemap);
@@ -4493,11 +4499,13 @@ int main_list(int argc, char **argv)bool verbose = false;bool context = false;bool details = false; +bool cpupool = false;bool numa = false;static struct option opts[] = { {"long" ,0 ,0 ,'l' }, {"verbose" ,0 ,0 ,'v' }, {"context" ,0 ,0 ,'Z' }, + {"cpupool" ,0 ,0 ,'c' }, {"numa" ,0 ,0 ,'n' },COMMON_LONG_OPTS , {0 ,0 ,0 ,0 }
@@ -4507,7 +4515,7 @@ int main_list(int argc, char **argv)libxl_dominfo *info, *info_free=0 ;int nb_domain, rc; - SWITCH_FOREACH_OPT(opt, "lvhZn", opts, "list", 0) { + SWITCH_FOREACH_OPT(opt, "lvhZcn", opts, "list", 0) {case 'l' : details = true;break ;
@@ -4517,6 +4525,9 @@ int main_list(int argc, char **argv)case 'Z' : context = true;break ; +case 'c' : + cpupool = true; +break ;case 'n' : numa = true;break ;
@@ -4551,7 +4562,7 @@ int main_list(int argc, char **argv)if (details)list_domains_details (info, nb_domain);else -list_domains (verbose, context, false/* claim */ , numa, +list_domains (verbose, context, false/* claim */ , numa, cpupool, info, nb_domain);if (info_free)
@@ -6603,7 +6614,7 @@ int main_claims(int argc, char **argv) }list_domains (false/* verbose */ , false/* context */ , true/* claim */ , - false/* numa */ , info, nb_domain); + false/* numa */ , false/* cpupool */ , info, nb_domain);libxl_dominfo_list_free (info, nb_domain);return 0 ;
tools/libxl/xl_cmdtable.c +1 -0
@@ -53,6 +53,7 @@ struct cmd_spec cmd_table[] = {"-l, --long Output all VM details\n" "-v, --verbose Prints out UUIDs and security context\n" "-Z, --context Prints out security context\n" +"-c, --cpupool Prints the cpupool the domain is in\n" "-n, --numa Prints out NUMA node affinity" }, { "destroy",
PATCH v2 3/7 libxl: introduce libxl_cpupool_cpu{add, remove}_cpumap()
In reply to Dario FaggioliTo add (removes) to (from) a cpupool all the pCPUs corresponding to the bits that are set in the passed bitmap.
This is convenient and useful in order to implement, in xl, the possibility of specifying ranges of pCPUs to be added (removed) to (from) a cpupool, in the relevant commands.
While there, convert libxl_cpupool_cpu{add,remove} to use the appropriate logging macro (LOGE()).
---
Changes from v1:
* added LIBXL_HAVE_CPUPOOL_ADD_REM_CPUMAP, as requested during review;
* libxl_cpupool_cpu{add,remove} now conform more to the 'goto out:' error handling style (although there is no need for any actual goto! :-D), as requested during review;
* libxl_cpupool_cpu{add,remove}_cpumap() now return 0 or ERROR_FAIL, as requested during review.
--- tools/libxl/libxl.c | 56 ++++++++++++++++++++++++++++++++++++++++++--------- tools/libxl/libxl.h | 13 ++++++++++++ 2 files changed, 59 insertions(+), 10 deletions(-)
tools/libxl/libxl.c +46 -10
@@ -6358,15 +6358,33 @@ out:int libxl_cpupool_cpuadd (libxl_ctx *ctx,uint32_t poolid,int cpu) { -int rc; +GC_INIT (ctx); +int rc =0 ; rc =xc_cpupool_addcpu (ctx->xch, poolid, cpu);if (rc) { -LIBXL__LOG_ERRNOVAL (ctx,LIBXL__LOG_ERROR , rc, -"Error moving cpu to cpupool" ); -return ERROR_FAIL ; +LOGE (ERROR ,"Error moving cpu %d to cpupool" , cpu); + rc =ERROR_FAIL ; } -return 0 ; + +GC_FREE ; +return rc; +} + +int libxl_cpupool_cpuadd_cpumap (libxl_ctx *ctx,uint32_t poolid, +const libxl_bitmap *cpumap) +{ +int c, ncpus =0 , rc =0 ; + +libxl_for_each_set_bit (c, *cpumap) { +if (!libxl_cpupool_cpuadd (ctx, poolid, c)) + ncpus++; + } + +if (ncpus !=libxl_bitmap_count_set (cpumap)) + rc =ERROR_FAIL ; + +return rc; }int libxl_cpupool_cpuadd_node (libxl_ctx *ctx,uint32_t poolid,int node,int *cpus)
@@ -6403,15 +6421,33 @@ out:int libxl_cpupool_cpuremove (libxl_ctx *ctx,uint32_t poolid,int cpu) { -int rc; +GC_INIT (ctx); +int rc =0 ; rc =xc_cpupool_removecpu (ctx->xch, poolid, cpu);if (rc) { -LIBXL__LOG_ERRNOVAL (ctx,LIBXL__LOG_ERROR , rc, -"Error removing cpu from cpupool" ); -return ERROR_FAIL ; +LOGE (ERROR ,"Error removing cpu %d from cpupool" , cpu); + rc =ERROR_FAIL ; } -return 0 ; + +GC_FREE ; +return rc; +} + +int libxl_cpupool_cpuremove_cpumap (libxl_ctx *ctx,uint32_t poolid, +const libxl_bitmap *cpumap) +{ +int c, ncpus =0 , rc =0 ; + +libxl_for_each_set_bit (c, *cpumap) { +if (!libxl_cpupool_cpuremove (ctx, poolid, c)) + ncpus++; + } + +if (ncpus !=libxl_bitmap_count_set (cpumap)) + rc =ERROR_FAIL ; + +return rc; }int libxl_cpupool_cpuremove_node (libxl_ctx *ctx,uint32_t poolid,int node,int *cpus)
tools/libxl/libxl.h +13 -0
@@ -83,6 +83,15 @@*/ #define LIBXL_HAVE_CPUPOOL_QUALIFIER_TO_CPUPOOLID 1 +/* LIBXL_HAVE_CPUPOOL_ADD_REM_CPUMAP +* +* If this is defined, libxl has two library functions called +* libxl_cpupool_cpuadd_cpumap and libxl_cpupool_cpuremove_cpumap, +* which allow to add to or remove from a cpupool all the cpus +* specified in a bitmap. +*/ +#define LIBXL_HAVE_CPUPOOL_ADD_REM_CPUMAP 1 +/* * LIBXL_HAVE_FIRMWARE_PASSTHROUGH indicates the feature for * passing in SMBIOS and ACPI firmware to HVM guests is present
@@ -1468,8 +1477,12 @@ int libxl_cpupool_destroy(libxl_ctx *ctx, uint32_t poolid);int libxl_cpupool_rename (libxl_ctx *ctx,const char *name,uint32_t poolid);int libxl_cpupool_cpuadd (libxl_ctx *ctx,uint32_t poolid,int cpu);int libxl_cpupool_cpuadd_node (libxl_ctx *ctx,uint32_t poolid,int node,int *cpus); +int libxl_cpupool_cpuadd_cpumap (libxl_ctx *ctx,uint32_t poolid, +const libxl_bitmap *cpumap);int libxl_cpupool_cpuremove (libxl_ctx *ctx,uint32_t poolid,int cpu);int libxl_cpupool_cpuremove_node (libxl_ctx *ctx,uint32_t poolid,int node,int *cpus); +int libxl_cpupool_cpuremove_cpumap (libxl_ctx *ctx,uint32_t poolid, +const libxl_bitmap *cpumap);int libxl_cpupool_movedomain (libxl_ctx *ctx,uint32_t poolid,uint32_t domid);int libxl_cpupool_info (libxl_ctx *ctx,libxl_cpupoolinfo *info,uint32_t poolid);
Re: [PATCH v2 3/7] libxl: introduce libxl_cpupool_cpu{add, remove}_cpumap()
Quoted 10 lines · Dario Faggioli
To add (removes) to (from) a cpupool all the pCPUs corresponding to the bits that are set in the passed bitmap.
This is convenient and useful in order to implement, in xl, the possibility of specifying ranges of pCPUs to be added (removed) to (from) a cpupool, in the relevant commands.
While there, convert libxl_cpupool_cpu{add,remove} to use the appropriate logging macro (
LOGE()).
Quoted 6 lines
Signed-off-by Dario Faggioli <[email protected]>Cc Ian Campbell <[email protected]>Cc Ian Jackson <[email protected]>Cc Stefano Stabellini <[email protected]>Cc Wei Liu <[email protected]>Cc Juergen Gross <[email protected]>
PATCH v2 4/7 xl: enable using ranges of pCPUs when manipulating cpupools
In reply to Dario Faggioliin fact, right now, xl sub-commands 'cpupool-cpu-add' and 'cpupool-cpu-remove' only accept the specification of one pCPU to be added or removed to/from a cpupool.
With this change, they can deal with ranges, like "4-8", or "node:1,12-18,^14". The syntax is exactly the same one that is supported by the 'vcpu-pin' subcommand, and specifying just one pCPU still works, of course.
This make things more flexible, more consistent, and also improves error handling, as the pCPU range parsing routine already present in xl is more reliable than just a call to atoi().
---
Changes from v1:
* reword the man page change, in line with what was requested during review.
--- docs/man/xl.pod.1 | 25 +++++++++++-- tools/libxl/xl_cmdimpl.c | 92 ++++++++++++++++++++-------------------------- 2 files changed, 60 insertions(+), 57 deletions(-)
docs/man/xl.pod.1 +21 -4
@@ -1147,13 +1147,30 @@ This is possible only if no domain is active in the cpu-pool. Renames a cpu-pool to I<newname>. -=item B<cpupool-cpu-add> I<cpu-pool> I<cpu-nr|node:node-nr> +=item B<cpupool-cpu-add> I<cpu-pool> I<cpus|node:nodes> -Adds a cpu or all cpus of a numa node to a cpu-pool. +Adds one or more CPUs or NUMA nodes to I<cpu-pool>. CPUs and NUMA +nodes can be specified as single CPU/node IDs or as ranges. -=item B<cpupool-cpu-remove> I<cpu-nr|node:node-nr> +For example: + + (a) xl cpupool-cpu-add mypool 4 + (b) xl cpupool-cpu-add mypool 1,5,10-16,^13 + (c) xl cpupool-cpu-add mypool node:0,nodes:2-3,^10-12,8 + +means adding CPU 4 to mypool, in (a); adding CPUs 1,5,10,11,12,14,15 +and 16, in (b); and adding all the CPUs of NUMA nodes 0, 2 and 3, +plus CPU 8, but keeping out CPUs 10,11,12, in (c). + +All the specified CPUs that can be added to the cpupool will be added +to it. If some CPU can't (e.g., because they're already part of another +cpupool), an error is reported about each one of them. + +=item B<cpupool-cpu-remove> I<cpus|node:nodes> -Removes a cpu or all cpus of a numa node from a cpu-pool. +Removes one or more CPUs or NUMA nodes from I<cpu-pool>. CPUs and NUMA +nodes can be specified as single CPU/node IDs or as ranges, using the +exact same syntax as in B<cpupool-cpu-add> above. =item B<cpupool-migrate> I<domain> I<cpu-pool>
tools/libxl/xl_cmdimpl.c +39 -53
162 lines not on this page · raw message
Re: [PATCH v2 4/7] xl: enable using ranges of pCPUs when manipulating cpupools
Quoted 14 lines · Dario Faggioli
in fact, right now, xl sub-commands 'cpupool-cpu-add' and 'cpupool-cpu-remove' only accept the specification of one pCPU to be added or removed to/from a cpupool.
With this change, they can deal with ranges, like "4-8", or "node:1,12-18,^14". The syntax is exactly the same one that is supported by the 'vcpu-pin' subcommand, and specifying just one pCPU still works, of course.
This make things more flexible, more consistent, and also improves error handling, as the pCPU range parsing routine already present in xl is more reliable than just a call to
atoi().
Quoted 6 lines
Signed-off-by Dario Faggioli <[email protected]>Cc Ian Campbell <[email protected]>Cc Ian Jackson <[email protected]>Cc Stefano Stabellini <[email protected]>Cc Wei Liu <[email protected]>Cc Juergen Gross <[email protected]>
PATCH v2 5/7 xl: enable using ranges of pCPUs when creating cpupools
In reply to Dario Faggioliinstead of just list of single pCPUs or NUMA node IDs, as it happens right now.
On the other hand, after this change, strings containing pCPUs and NUMA node ranges is supported. The syntax is the same one supported by the "cpus" and "cpus_soft" config switch, i.e., "4-8" or "node:1,12-18,^14".
This make things more flexible, more consistent, and also improves error handling, as the pCPU range parsing routine already present in xl is more reliable than just a call to atoi().
While there, remove a redundant error check in the legacy syntax handling (libxl_bitmap_test() already checks the index being within the size of the bitmap).
--- docs/man/xlcpupool.cfg.pod.5 | 22 +++++++++++++++++++--- tools/libxl/xl_cmdimpl.c | 17 ++++++++++++++--- 2 files changed, 33 insertions(+), 6 deletions(-)
docs/man/xlcpupool.cfg.pod.5 +19 -3
@@ -93,10 +93,26 @@ Specifies the cpus of the NUMA-nodes given in C<NODES> (an integer or a list of integers) to be member of the cpupool. The free cpus in the specified nodes are allocated in the new cpupool. -=item B<cpus="CPUS"> +=item B<cpus="CPU-LIST"> -The specified C<CPUS> are allocated in the new cpupool. All cpus must -be free. Must not be specified together with B<nodes>. +Specifies the cpus that will be member of the cpupool. All the specified +cpus must be free, or creation will fail. C<CPU-LIST> may be specified +as follows: + +=over 4 + +=item ["2", "3", "5"] + +means that cpus 2,3,5 will be member of the cpupool. + +=item "0-3,5,^1" + +means that cpus 0,2,3 and 5 will be member of the cpupool. A "node:" or +"nodes:" modifier can be used. E.g., "0,node:1,nodes:2-3,^10-13" means +that pcpus 0, plus all the cpus of NUMA nodes 1,2,3 with the exception +of cpus 10,11,12,13 will be memeber of the cpupool. + +=back If neither B<nodes> nor B<cpus> are specified only the first free cpu found will be allocated in the new cpupool.
tools/libxl/xl_cmdimpl.c +14 -3
@@ -7148,18 +7148,29 @@ int main_cpupoolcreate(int argc, char **argv)fprintf (stderr,"no free cpu found\n" ); goto out_cfg; } - }else if (!xlu_cfg_get_list (config,"cpus" , &cpus,0 ,0 )) { + }else if (!xlu_cfg_get_list (config,"cpus" , &cpus,0 ,1 )) { n_cpus =0 ;while ((buf =xlu_cfg_get_listitem (cpus, n_cpus)) !=NULL ) { i =atoi (buf); -if ((i <0 ) || (i >= freemap.size *8 ) || - !libxl_bitmap_test (&freemap, i)) { +if ((i <0 ) || !libxl_bitmap_test (&freemap, i)) {fprintf (stderr,"cpu %d illegal or not free\n" , i); goto out_cfg; }libxl_bitmap_set (&cpumap, i); n_cpus++; } + }else if (!xlu_cfg_get_string (config,"cpus" , &buf,0 )) { +if (cpurange_parse (buf, &cpumap)) + goto out_cfg; + + n_cpus =0 ; +libxl_for_each_set_bit (i, cpumap) { +if (!libxl_bitmap_test (&freemap, i)) { +fprintf (stderr,"cpu %d illegal or not free\n" , i); + goto out_cfg; + } + n_cpus++; + } }else n_cpus =0 ;
Re: [PATCH v2 5/7] xl: enable using ranges of pCPUs when creating cpupools
Quoted 17 lines · Dario Faggioli
instead of just list of single pCPUs or NUMA node IDs, as it happens right now.
On the other hand, after this change, strings containing pCPUs and NUMA node ranges is supported. The syntax is the same one supported by the "cpus" and "
cpus_soft" config switch, i.e., "4-8" or "node:1,12-18,^14".This make things more flexible, more consistent, and also improves error handling, as the pCPU range parsing routine already present in xl is more reliable than just a call to
atoi().While there, remove a redundant error check in the legacy syntax handling (
libxl_bitmap_test()already checks the index being within the size of the bitmap).
Quoted 6 lines
Signed-off-by Dario Faggioli <[email protected]>Cc Ian Campbell <[email protected]>Cc Ian Jackson <[email protected]>Cc Stefano Stabellini <[email protected]>Cc Wei Liu <[email protected]>Cc Juergen Gross <[email protected]>
PATCH v2 6/7 xl: make error reporting of cpupool subcommands consistent
In reply to Dario Faggioliwith the rest of the file, where we return 1 on 0, rather than using libxl error codes.
--- tools/libxl/xl_cmdimpl.c | 69 ++++++++++++++++++++++++---------------------- 1 file changed, 36 insertions(+), 33 deletions(-)
tools/libxl/xl_cmdimpl.c +36 -33
@@ -7019,7 +7019,7 @@ int main_cpupoolcreate(int argc, char **argv)libxl_bitmap cpumap;libxl_uuid uuid;libxl_cputopology *topology; -int rc = -ERROR_FAIL ; +int rc =1 ; SWITCH_FOREACH_OPT(opt, "hnf:", opts, "cpupool-create", 0) {case 'f' :
@@ -7213,7 +7213,6 @@ int main_cpupoollist(int argc, char **argv)int n_pools, p, c, n;uint32_t poolid;char *name; -int ret =0 ; SWITCH_FOREACH_OPT(opt, "hc", opts, "cpupool-list", 0) {case 'c' :
@@ -7225,14 +7224,14 @@ int main_cpupoollist(int argc, char **argv) pool = argv[optind];if (libxl_name_to_cpupoolid (ctx, pool, &poolid)) {fprintf (stderr,"Pool \'%s\' does not exist\n" , pool); -return -ERROR_FAIL ; +return 1 ; } } poolinfo =libxl_list_cpupool (ctx, &n_pools);if (!poolinfo) {fprintf (stderr,"error getting cpupool info\n" ); -return -ERROR_NOMEM ; +return 1 ; }printf ("%-19s" ,"Name" );
@@ -7242,7 +7241,7 @@ int main_cpupoollist(int argc, char **argv)printf ("CPUs Sched Active Domain count\n" );for (p =0 ; p < n_pools; p++) { -if (!ret && (!pool || (poolinfo[p].poolid == poolid))) { +if (!pool || (poolinfo[p].poolid == poolid)) { name = poolinfo[p].pool_name;printf ("%-19s" , name); n =0 ;
@@ -7263,7 +7262,7 @@ int main_cpupoollist(int argc, char **argv)libxl_cpupoolinfo_list_free (poolinfo, n_pools); -return ret; +return 0 ; }int main_cpupooldestroy (int argc,char **argv)
@@ -7280,11 +7279,14 @@ int main_cpupooldestroy(int argc, char **argv)if (libxl_cpupool_qualifier_to_cpupoolid (ctx, pool, &poolid,NULL ) || !libxl_cpupoolid_is_valid (ctx, poolid)) { -fprintf (stderr,"unknown cpupool \'%s\'\n" , pool); -return -ERROR_FAIL ; +fprintf (stderr,"unknown cpupool '%s'\n" , pool); +return 1 ; } -return -libxl_cpupool_destroy (ctx, poolid); +if (libxl_cpupool_destroy (ctx, poolid)) +return 1 ; + +return 0 ; }int main_cpupoolrename (int argc,char **argv)
@@ -7302,14 +7304,14 @@ int main_cpupoolrename(int argc, char **argv)if (libxl_cpupool_qualifier_to_cpupoolid (ctx, pool, &poolid,NULL ) || !libxl_cpupoolid_is_valid (ctx, poolid)) { -fprintf (stderr,"unknown cpupool \'%s\'\n" , pool); -return -ERROR_FAIL ; +fprintf (stderr,"unknown cpupool '%s'\n" , pool); +return 1 ; } new_name = argv[optind];if (libxl_cpupool_rename (ctx, new_name, poolid)) { -fprintf (stderr,"Can't rename cpupool '%s'.\n" , pool); +fprintf (stderr,"Can't rename cpupool '%s'\n" , pool);return 1 ; }
@@ -7409,22 +7411,25 @@ int main_cpupoolmigrate(int argc, char **argv)if (libxl_domain_qualifier_to_domid (ctx, dom, &domid) || !libxl_domid_to_name (ctx, domid)) { -fprintf (stderr,"unknown domain \'%s\'\n" , dom); -return -ERROR_FAIL ; +fprintf (stderr,"unknown domain '%s'\n" , dom); +return 1 ; }if (libxl_cpupool_qualifier_to_cpupoolid (ctx, pool, &poolid,NULL ) || !libxl_cpupoolid_is_valid (ctx, poolid)) { -fprintf (stderr,"unknown cpupool \'%s\'\n" , pool); -return -ERROR_FAIL ; +fprintf (stderr,"unknown cpupool '%s'\n" , pool); +return 1 ; } -return -libxl_cpupool_movedomain (ctx, poolid, domid); +if (libxl_cpupool_movedomain (ctx, poolid, domid)) +return 1 ; + +return 0 ; }int main_cpupoolnumasplit (int argc,char **argv) { -int ret; +int rc;int opt;int p;int c;
@@ -7445,13 +7450,13 @@ int main_cpupoolnumasplit(int argc, char **argv)/* No options */ } - ret =0 ; + rc =1 ;libxl_bitmap_init (&cpumap); poolinfo =libxl_list_cpupool (ctx, &n_pools);if (!poolinfo) {fprintf (stderr,"error getting cpupool info\n" ); -return -ERROR_NOMEM ; +return 1 ; } poolid = poolinfo[0 ].poolid; sched = poolinfo[0 ].sched;
@@ -7460,19 +7465,19 @@ int main_cpupoolnumasplit(int argc, char **argv) }if (n_pools >1 ) {fprintf (stderr,"splitting not possible, already cpupools in use\n" ); -return -ERROR_FAIL ; +return 1 ; } topology =libxl_get_cpu_topology (ctx, &n_cpus);if (topology ==NULL ) {fprintf (stderr,"libxl_get_topologyinfo failed\n" ); -return -ERROR_FAIL ; +return 1 ; }if (libxl_cpu_bitmap_alloc (ctx, &cpumap,0 )) {fprintf (stderr,"Failed to allocate cpumap\n" );libxl_cputopology_list_free (topology, n_cpus); -return -ERROR_FAIL ; +return 1 ; }/* Reset Pool-0 to 1st node: first add cpus, then remove cpus to avoid
@@ -7481,12 +7486,11 @@ int main_cpupoolnumasplit(int argc, char **argv) node = topology[0 ].node;if (libxl_cpupool_cpuadd_node (ctx,0 , node, &n)) {fprintf (stderr,"error on adding cpu to Pool 0\n" ); -return -ERROR_FAIL ; +return 1 ; }snprintf (name,15 ,"Pool-node%d" , node); - ret = -libxl_cpupool_rename (ctx, name,0 ); -if (ret) { +if (libxl_cpupool_rename (ctx, name,0 )) {fprintf (stderr,"error on renaming Pool 0\n" ); goto out; }
@@ -7525,8 +7529,7 @@ int main_cpupoolnumasplit(int argc, char **argv) } node = topology[c].node; - ret = -libxl_cpupool_cpuremove_node (ctx,0 , node, &n); -if (ret) { +if (libxl_cpupool_cpuremove_node (ctx,0 , node, &n)) {fprintf (stderr,"error on removing cpu from Pool 0\n" ); goto out; }
@@ -7534,14 +7537,12 @@ int main_cpupoolnumasplit(int argc, char **argv)snprintf (name,15 ,"Pool-node%d" , node);libxl_uuid_generate (&uuid); poolid =0 ; - ret = -libxl_cpupool_create (ctx, name, sched, cpumap, &uuid, &poolid); -if (ret) { +if (libxl_cpupool_create (ctx, name, sched, cpumap, &uuid, &poolid)) {fprintf (stderr,"error on creating cpupool\n" ); goto out; } - ret = -libxl_cpupool_cpuadd_node (ctx, poolid, node, &n); -if (ret) { +if (libxl_cpupool_cpuadd_node (ctx, poolid, node, &n)) {fprintf (stderr,"error on adding cpus to cpupool\n" ); goto out; }
@@ -7553,11 +7554,13 @@ int main_cpupoolnumasplit(int argc, char **argv) } } + rc =0 ; + out:libxl_cputopology_list_free (topology, n_cpus);libxl_bitmap_dispose (&cpumap); -return ret; +return rc; }int main_getenforce (int argc,char **argv)
PATCH v2 7/7 xl: use libxl_cpupoolinfo_list_free() in main_cpupoolnumasplit
In reply to Dario Faggioliinstead of manually freeing the elements of the list, which is exactly the purpose of the said function.
Trade also a couple of 'return'-s with 'goto out'-s, which is more in line with libxl usage paradigm.
---
Changes from v1:
* grammar fixes in the changelog, as requested during review.
--- tools/libxl/xl_cmdimpl.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)
tools/libxl/xl_cmdimpl.c +4 -6
@@ -7460,9 +7460,8 @@ int main_cpupoolnumasplit(int argc, char **argv) } poolid = poolinfo[0 ].poolid; sched = poolinfo[0 ].sched; -for (p =0 ; p < n_pools; p++) { -libxl_cpupoolinfo_dispose (poolinfo + p); - } +libxl_cpupoolinfo_list_free (poolinfo, n_pools); +if (n_pools >1 ) {fprintf (stderr,"splitting not possible, already cpupools in use\n" );return 1 ;
@@ -7476,8 +7475,7 @@ int main_cpupoolnumasplit(int argc, char **argv)if (libxl_cpu_bitmap_alloc (ctx, &cpumap,0 )) {fprintf (stderr,"Failed to allocate cpumap\n" ); -libxl_cputopology_list_free (topology, n_cpus); -return 1 ; + goto out; }/* Reset Pool-0 to 1st node: first add cpus, then remove cpus to avoid
@@ -7486,7 +7484,7 @@ int main_cpupoolnumasplit(int argc, char **argv) node = topology[0 ].node;if (libxl_cpupool_cpuadd_node (ctx,0 , node, &n)) {fprintf (stderr,"error on adding cpu to Pool 0\n" ); -return 1 ; + goto out; }snprintf (name,15 ,"Pool-node%d" , node);
Quoted 8 lines · Dario Faggioli
Dario Faggioli (7):
xl: turn some int local variable into bool
xl: add -c/--cpupool option to `xl list'
libxl: introduce libxl_cpupool_cpu{add,remove}_cpumap()
xl: enable using ranges of pCPUs when manipulating cpupools
xl: enable using ranges of pCPUs when creating cpupools
xl: make error reporting of cpupool subcommands consistent
xl: uselibxl_cpupoolinfo_list_free()inmain_cpupoolnumasplit
Applied with Wei's acks.
Ian.