qemu-devel
[Top][All Lists]
Advanced

[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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]