[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 5/6] NUMA: convert -numa option to use OptsVi
From: |
Eduardo Habkost |
Subject: |
Re: [Qemu-devel] [PATCH v2 5/6] NUMA: convert -numa option to use OptsVisitor |
Date: |
Wed, 26 Feb 2014 15:08:05 -0300 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Wed, Feb 26, 2014 at 03:40:58PM +0100, Igor Mammedov wrote:
> From: Wanlong Gao <address@hidden>
>
> Signed-off-by: Wanlong Gao <address@hidden>
> Signed-off-by: Paolo Bonzini <address@hidden>
> Signed-off-by: Igor Mammedov <address@hidden>
> ---
> v2:
> - altered doc comment according to Eric Blake <address@hidden>
> suggestions.
>
> Signed-off-by: Igor Mammedov <address@hidden>
I didn't know QemuOptsVisitor finally supports ranges when parsing
integer lists. Nice.
I also confirm that it keeps compatibility with previous behavior when
the size suffix is omitted (defaulting to "M").
Tested-by: Eduardo Habkost <address@hidden>
Reviewed-by: Eduardo Habkost <address@hidden>
> ---
> include/sysemu/sysemu.h | 3 +-
> numa.c | 147
> +++++++++++++++++++++++------------------------
> qapi-schema.json | 32 ++++++++++
> vl.c | 11 +++-
> 4 files changed, 115 insertions(+), 78 deletions(-)
>
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index d873b42..20b05a3 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -140,9 +140,10 @@ typedef struct node_info {
> DECLARE_BITMAP(node_cpu, MAX_CPUMASK_BITS);
> } NodeInfo;
> extern NodeInfo numa_info[MAX_NODES];
> -void numa_add(const char *optarg);
> void set_numa_nodes(void);
> void set_numa_modes(void);
> +extern QemuOptsList qemu_numa_opts;
> +int numa_init_func(QemuOpts *opts, void *opaque);
>
> #define MAX_OPTION_ROMS 16
> typedef struct QEMUOptionRom {
> diff --git a/numa.c b/numa.c
> index c3eca78..40c28b3 100644
> --- a/numa.c
> +++ b/numa.c
> @@ -24,101 +24,96 @@
> */
>
> #include "sysemu/sysemu.h"
> -
> -static void numa_node_parse_cpus(int nodenr, const char *cpus)
> +#include "qapi-visit.h"
> +#include "qapi/opts-visitor.h"
> +#include "qapi/dealloc-visitor.h"
> +QemuOptsList qemu_numa_opts = {
> + .name = "numa",
> + .implied_opt_name = "type",
> + .head = QTAILQ_HEAD_INITIALIZER(qemu_numa_opts.head),
> + .desc = { { 0 } } /* validated with OptsVisitor */
> +};
> +
> +static int numa_node_parse(NumaNodeOptions *node, QemuOpts *opts)
> {
> - char *endptr;
> - unsigned long long value, endvalue;
> -
> - /* Empty CPU range strings will be considered valid, they will simply
> - * not set any bit in the CPU bitmap.
> - */
> - if (!*cpus) {
> - return;
> - }
> + uint16_t nodenr;
> + uint16List *cpus = NULL;
>
> - if (parse_uint(cpus, &value, &endptr, 10) < 0) {
> - goto error;
> - }
> - if (*endptr == '-') {
> - if (parse_uint_full(endptr + 1, &endvalue, 10) < 0) {
> - goto error;
> - }
> - } else if (*endptr == '\0') {
> - endvalue = value;
> + if (node->has_nodeid) {
> + nodenr = node->nodeid;
> } else {
> - goto error;
> + nodenr = nb_numa_nodes;
> }
>
> - if (endvalue >= MAX_CPUMASK_BITS) {
> - endvalue = MAX_CPUMASK_BITS - 1;
> - fprintf(stderr,
> - "qemu: NUMA: A max of %d VCPUs are supported\n",
> - MAX_CPUMASK_BITS);
> + if (nodenr >= MAX_NODES) {
> + fprintf(stderr, "qemu: Max number of NUMA nodes reached: %"
> + PRIu16 "\n", nodenr);
> + return -1;
> }
>
> - if (endvalue < value) {
> - goto error;
> + for (cpus = node->cpus; cpus; cpus = cpus->next) {
> + if (cpus->value > MAX_CPUMASK_BITS) {
> + fprintf(stderr, "qemu: cpu number %" PRIu16 " is bigger than %d",
> + cpus->value, MAX_CPUMASK_BITS);
> + continue;
> + }
> + bitmap_set(numa_info[nodenr].node_cpu, cpus->value, 1);
> }
>
> - bitmap_set(numa_info[nodenr].node_cpu, value, endvalue - value + 1);
> - return;
> + if (node->has_mem) {
> + uint64_t mem_size = node->mem;
> + const char *mem_str = qemu_opt_get(opts, "mem");
> + /* Fix up legacy suffix-less format */
> + if (g_ascii_isdigit(mem_str[strlen(mem_str) - 1])) {
> + mem_size <<= 20;
> + }
> + numa_info[nodenr].node_mem = mem_size;
> + }
>
> -error:
> - fprintf(stderr, "qemu: Invalid NUMA CPU range: %s\n", cpus);
> - exit(1);
> + return 0;
> }
>
> -void numa_add(const char *optarg)
> +int numa_init_func(QemuOpts *opts, void *opaque)
> {
> - char option[128];
> - char *endptr;
> - unsigned long long nodenr;
> -
> - optarg = get_opt_name(option, 128, optarg, ',');
> - if (*optarg == ',') {
> - optarg++;
> + NumaOptions *object = NULL;
> + Error *err = NULL;
> + int ret = 0;
> +
> + {
> + OptsVisitor *ov = opts_visitor_new(opts);
> + visit_type_NumaOptions(opts_get_visitor(ov), &object, NULL, &err);
> + opts_visitor_cleanup(ov);
> }
> - if (!strcmp(option, "node")) {
> -
> - if (nb_numa_nodes >= MAX_NODES) {
> - fprintf(stderr, "qemu: too many NUMA nodes\n");
> - exit(1);
> - }
>
> - if (get_param_value(option, 128, "nodeid", optarg) == 0) {
> - nodenr = nb_numa_nodes;
> - } else {
> - if (parse_uint_full(option, &nodenr, 10) < 0) {
> - fprintf(stderr, "qemu: Invalid NUMA nodeid: %s\n", option);
> - exit(1);
> - }
> - }
> -
> - if (nodenr >= MAX_NODES) {
> - fprintf(stderr, "qemu: invalid NUMA nodeid: %llu\n", nodenr);
> - exit(1);
> - }
> + if (error_is_set(&err)) {
> + fprintf(stderr, "qemu: %s\n", error_get_pretty(err));
> + error_free(err);
> + ret = -1;
> + goto error;
> + }
>
> - if (get_param_value(option, 128, "mem", optarg) == 0) {
> - numa_info[nodenr].node_mem = 0;
> - } else {
> - int64_t sval;
> - sval = strtosz(option, &endptr);
> - if (sval < 0 || *endptr) {
> - fprintf(stderr, "qemu: invalid numa mem size: %s\n", optarg);
> - exit(1);
> - }
> - numa_info[nodenr].node_mem = sval;
> - }
> - if (get_param_value(option, 128, "cpus", optarg) != 0) {
> - numa_node_parse_cpus(nodenr, option);
> + switch (object->kind) {
> + case NUMA_OPTIONS_KIND_NODE:
> + ret = numa_node_parse(object->node, opts);
> + if (ret) {
> + goto error;
> }
> nb_numa_nodes++;
> - } else {
> - fprintf(stderr, "Invalid -numa option: %s\n", option);
> - exit(1);
> + break;
> + default:
> + fprintf(stderr, "qemu: Invalid NUMA options type.\n");
> + ret = -1;
> }
> +
> +error:
> + if (object) {
> + QapiDeallocVisitor *dv = qapi_dealloc_visitor_new();
> + visit_type_NumaOptions(qapi_dealloc_get_visitor(dv),
> + &object, NULL, NULL);
> + qapi_dealloc_visitor_cleanup(dv);
> + }
> +
> + return ret;
> }
>
> void set_numa_nodes(void)
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 7cfb5e5..75095a9 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -4420,3 +4420,35 @@
> # Since: 1.7
> ##
> { 'command': 'blockdev-add', 'data': { 'options': 'BlockdevOptions' } }
> +
> +##
> +# @NumaOptions
> +#
> +# A discriminated record of NUMA options. (for OptsVisitor)
> +#
> +# Since 2.0
> +##
> +{ 'union': 'NumaOptions',
> + 'data': {
> + 'node': 'NumaNodeOptions' }}
> +
> +##
> +# @NumaNodeOptions
> +#
> +# Create a guest NUMA node. (for OptsVisitor)
> +#
> +# @nodeid: #optional NUMA node ID (increase by 1 from 0 if omitted)
> +#
> +# @cpus: #optional VCPUs belonging to this node (assign VCPUS round-robin
> +# if omitted)
> +#
> +# @mem: #optional memory size of this node (equally divide total memory among
> +# nodes if omitted)
> +#
> +# Since: 2.0
> +##
> +{ 'type': 'NumaNodeOptions',
> + 'data': {
> + '*nodeid': 'uint16',
> + '*cpus': ['uint16'],
> + '*mem': 'size' }}
> diff --git a/vl.c b/vl.c
> index 915f8b7..e070649 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2765,6 +2765,7 @@ int main(int argc, char **argv, char **envp)
> qemu_add_opts(&qemu_tpmdev_opts);
> qemu_add_opts(&qemu_realtime_opts);
> qemu_add_opts(&qemu_msg_opts);
> + qemu_add_opts(&qemu_numa_opts);
>
> runstate_init();
>
> @@ -2952,7 +2953,10 @@ int main(int argc, char **argv, char **envp)
> }
> break;
> case QEMU_OPTION_numa:
> - numa_add(optarg);
> + opts = qemu_opts_parse(qemu_find_opts("numa"), optarg, 1);
> + if (!opts) {
> + exit(1);
> + }
> break;
> case QEMU_OPTION_display:
> display_type = select_display(optarg);
> @@ -4042,6 +4046,11 @@ int main(int argc, char **argv, char **envp)
>
> register_savevm_live(NULL, "ram", 0, 4, &savevm_ram_handlers, NULL);
>
> + if (qemu_opts_foreach(qemu_find_opts("numa"), numa_init_func,
> + NULL, 1) != 0) {
> + exit(1);
> + }
> +
> set_numa_nodes();
>
> if (qemu_opts_foreach(qemu_find_opts("mon"), mon_init_func, NULL, 1) !=
> 0) {
> --
> 1.7.1
>
>
--
Eduardo
- [Qemu-devel] [PATCH 0/6] convert -numa to QemuOpts/OptsVisitor, Igor Mammedov, 2014/02/17
- [Qemu-devel] [PATCH 4/6] NUMA: Add numa_info structure to contain numa nodes info, Igor Mammedov, 2014/02/17
- [Qemu-devel] [PATCH 3/6] NUMA: check if the total numa memory size is equal to ram_size, Igor Mammedov, 2014/02/17
- [Qemu-devel] [PATCH 6/6] NUMA: expand MAX_NODES from 64 to 128, Igor Mammedov, 2014/02/17
- [Qemu-devel] [PATCH 2/6] NUMA: move numa related code to new file numa.c, Igor Mammedov, 2014/02/17
- Re: [Qemu-devel] [PATCH 0/6] convert -numa to QemuOpts/OptsVisitor, Michael S. Tsirkin, 2014/02/26