"Daniel P. Berrange" <berra...@redhat.com> writes: > Switch away from using OptsVisitor to parse the -numa > argument processing. This enables use of the modern > list syntax for specifying CPUs. e.g. the old syntax > > -numa node,nodeid=0,cpus=0-3,cpus=8-11,mem=107 > > is equivalent to > > -numa node,nodeid=0,cpus.0=0,cpus.1=1,cpus.2=2,cpus.3=3,\ > cpus.4=8,cpus.5=9,cpus.6=10,cpus.7=11,mem=107 > > Furthermore, the cli arg can now follow the QAPI schema > nesting, so the above is equivalent to the canonical > syntax: > > -numa type=node,data.nodeid=0,data.cpus.0=0,data.cpus.1=1,\ > data.cpus.2=2,data.cpus.3=3,data.cpus.4=8,data.cpus.5=9,\ > data.cpus.6=10,data.cpus.7=11,data.mem=107 > > A test case is added to cover argument parsing to validate > that both the old and new syntax is correctly handled. > > Signed-off-by: Daniel P. Berrange <berra...@redhat.com> > --- > include/sysemu/numa_int.h | 11 +++++ > numa.c | 36 +++++++++----- > stubs/Makefile.objs | 5 ++ > stubs/exec.c | 6 +++ > stubs/hostmem.c | 14 ++++++ > stubs/memory.c | 41 ++++++++++++++++ > stubs/qdev.c | 8 ++++ > stubs/vl.c | 8 ++++ > stubs/vmstate.c | 4 ++ > tests/Makefile.include | 2 + > tests/test-numa.c | 116 > ++++++++++++++++++++++++++++++++++++++++++++++ > 11 files changed, 240 insertions(+), 11 deletions(-) > create mode 100644 include/sysemu/numa_int.h > create mode 100644 stubs/exec.c > create mode 100644 stubs/hostmem.c > create mode 100644 stubs/memory.c > create mode 100644 stubs/qdev.c > create mode 100644 stubs/vl.c > create mode 100644 tests/test-numa.c > > diff --git a/include/sysemu/numa_int.h b/include/sysemu/numa_int.h > new file mode 100644 > index 0000000..93160da > --- /dev/null > +++ b/include/sysemu/numa_int.h > @@ -0,0 +1,11 @@ > +#ifndef SYSEMU_NUMA_INT_H > +#define SYSEMU_NUMA_INT_H > + > +#include "sysemu/numa.h" > + > +extern int have_memdevs; > +extern int max_numa_nodeid; > + > +int parse_numa(void *opaque, QemuOpts *opts, Error **errp); > + > +#endif > diff --git a/numa.c b/numa.c > index 6289f46..653ebf1 100644 > --- a/numa.c > +++ b/numa.c > @@ -23,14 +23,14 @@ > */ > > #include "qemu/osdep.h" > -#include "sysemu/numa.h" > +#include "sysemu/numa_int.h" > #include "exec/cpu-common.h" > #include "qemu/bitmap.h" > #include "qom/cpu.h" > #include "qemu/error-report.h" > #include "include/exec/cpu-common.h" /* for RAM_ADDR_FMT */ > #include "qapi-visit.h" > -#include "qapi/opts-visitor.h" > +#include "qapi/qobject-input-visitor.h" > #include "hw/boards.h" > #include "sysemu/hostmem.h" > #include "qmp-commands.h" > @@ -45,10 +45,10 @@ QemuOptsList qemu_numa_opts = { > .desc = { { 0 } } /* validated with OptsVisitor */ > }; > > -static int have_memdevs = -1; > -static int max_numa_nodeid; /* Highest specified NUMA node ID, plus one. > - * For all nodes, nodeid < max_numa_nodeid > - */ > +int have_memdevs = -1; > +int max_numa_nodeid; /* Highest specified NUMA node ID, plus one. > + * For all nodes, nodeid < max_numa_nodeid > + */ > int nb_numa_nodes; > NodeInfo numa_info[MAX_NODES]; > > @@ -189,6 +189,9 @@ static void numa_node_parse(NumaNodeOptions *node, > QemuOpts *opts, Error **errp) > if (node->has_mem) { > uint64_t mem_size = node->mem; > const char *mem_str = qemu_opt_get(opts, "mem"); > + if (!mem_str) { > + mem_str = qemu_opt_get(opts, "data.mem"); > + } > /* Fix up legacy suffix-less format */ > if (g_ascii_isdigit(mem_str[strlen(mem_str) - 1])) { > mem_size <<= 20; > @@ -211,16 +214,27 @@ static void numa_node_parse(NumaNodeOptions *node, > QemuOpts *opts, Error **errp) > max_numa_nodeid = MAX(max_numa_nodeid, nodenr + 1); > } > > -static int parse_numa(void *opaque, QemuOpts *opts, Error **errp) > +int parse_numa(void *opaque, QemuOpts *opts, Error **errp) > { > NumaOptions *object = NULL; > Error *err = NULL; > + Visitor *v; > > - { > - Visitor *v = opts_visitor_new(opts); > - visit_type_NumaOptions(v, NULL, &object, &err); > - visit_free(v); > + /* > + * Needs autocreate_list=true, permit_int_ranges=true and > + * permit_repeated_opts=true in order to support existing > + * syntax for 'cpus' parameter which is an int list. > + * > + * Needs autocreate_struct_levels=1, because existing syntax > + * used a nested struct in the QMP schema with a flat namespace > + * in the CLI args.
Which QAPI definition(s) do you allude to here? If it's just union NumaOptions, that's not ABI. I'd simply flatten it. Sketch appended. > + */ > + v = qobject_input_visitor_new_opts(opts, true, 1, true, true, &err); > + if (err) { > + goto end; > } > + visit_type_NumaOptions(v, NULL, &object, &err); > + visit_free(v); > > if (err) { > goto end; [Skipping test updates for now...] diff --git a/numa.c b/numa.c index 9c09e45..792fa2a 100644 --- a/numa.c +++ b/numa.c @@ -227,8 +227,8 @@ static int parse_numa(void *opaque, QemuOpts *opts, Error **errp) } switch (object->type) { - case NUMA_OPTIONS_KIND_NODE: - numa_node_parse(object->u.node.data, opts, &err); + case NUMA_OPTIONS_TYPE_NODE: + numa_node_parse(&object->u.node, opts, &err); if (err) { goto end; } diff --git a/qapi-schema.json b/qapi-schema.json index ded1179..4838258 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -4255,7 +4255,10 @@ # # Since 2.1 ## +{ 'enum': 'NumaOptionsType', 'data': ['node'] } { 'union': 'NumaOptions', + 'base': { 'type': 'NumaOptionsType' }, + 'discriminator': 'type', 'data': { 'node': 'NumaNodeOptions' }}