Message ID | 1520860275-101576-3-git-send-email-imammedo@redhat.com |
---|---|
State | New |
Headers | show |
Series | enable numa configuration before machine_init() from QMP | expand |
On Mon, Mar 12, 2018 at 02:11:08PM +0100, Igor Mammedov wrote: > it will allow to reuse parse_NumaOptions() for parsing > configuration commands received via QMP interface > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > --- > include/sysemu/numa.h | 1 + > numa.c | 48 +++++++++++++++++++++++++++++------------------- > 2 files changed, 30 insertions(+), 19 deletions(-) > > diff --git a/include/sysemu/numa.h b/include/sysemu/numa.h > index 21713b7..7a0ae75 100644 > --- a/include/sysemu/numa.h > +++ b/include/sysemu/numa.h > @@ -22,6 +22,7 @@ struct NumaNodeMem { > }; > > extern NodeInfo numa_info[MAX_NODES]; > +int parse_numa(void *opaque, QemuOpts *opts, Error **errp); > void parse_numa_opts(MachineState *ms); > void numa_complete_configuration(MachineState *ms); > void query_numa_node_mem(NumaNodeMem node_mem[]); > diff --git a/numa.c b/numa.c > index 126c649..2b1d292 100644 > --- a/numa.c > +++ b/numa.c > @@ -169,28 +169,11 @@ static void parse_numa_distance(NumaDistOptions *dist, Error **errp) > have_numa_distance = true; > } > > -static int parse_numa(void *opaque, QemuOpts *opts, Error **errp) > +static > +void parse_NumaOptions(MachineState *ms, NumaOptions *object, Error **errp) I wonder if we should rename the parse_numa_{node,distance}() functions to configure_numa_{node,distance}(), and this one configure_numa(). These functions don't parse anything, anymore. > { > - NumaOptions *object = NULL; > - MachineState *ms = opaque; > Error *err = NULL; > > - { > - Visitor *v = opts_visitor_new(opts); > - visit_type_NumaOptions(v, NULL, &object, &err); > - visit_free(v); > - } > - > - if (err) { > - goto end; > - } > - > - /* Fix up legacy suffix-less format */ > - if ((object->type == NUMA_OPTIONS_TYPE_NODE) && object->u.node.has_mem) { > - const char *mem_str = qemu_opt_get(opts, "mem"); > - qemu_strtosz_MiB(mem_str, NULL, &object->u.node.mem); > - } > - > switch (object->type) { > case NUMA_OPTIONS_TYPE_NODE: > parse_numa_node(ms, &object->u.node, &err); > @@ -224,6 +207,33 @@ static int parse_numa(void *opaque, QemuOpts *opts, Error **errp) > } > > end: > + if (err) { > + error_propagate(errp, err); > + } "if (err)" is not necessary here. See scripts/coccinelle/error_propagate_null.cocci. > +} > + > +int parse_numa(void *opaque, QemuOpts *opts, Error **errp) > +{ > + NumaOptions *object = NULL; > + MachineState *ms = MACHINE(opaque); > + Error *err = NULL; > + Visitor *v = opts_visitor_new(opts); > + > + visit_type_NumaOptions(v, NULL, &object, &err); > + visit_free(v); > + if (err) { > + goto end; > + } > + > + /* Fix up legacy suffix-less format */ > + if ((object->type == NUMA_OPTIONS_TYPE_NODE) && object->u.node.has_mem) { > + const char *mem_str = qemu_opt_get(opts, "mem"); > + qemu_strtosz_MiB(mem_str, NULL, &object->u.node.mem); > + } > + > + parse_NumaOptions(ms, object, &err); > + > +end: > qapi_free_NumaOptions(object); > if (err) { > error_report_err(err); We can fix this one too while at it. The rest of the patch looks good.
On 03/23/2018 03:42 PM, Eduardo Habkost wrote: > On Mon, Mar 12, 2018 at 02:11:08PM +0100, Igor Mammedov wrote: >> it will allow to reuse parse_NumaOptions() for parsing >> configuration commands received via QMP interface >> >> Signed-off-by: Igor Mammedov <imammedo@redhat.com> >> --- >> end: >> + if (err) { >> + error_propagate(errp, err); >> + } > > "if (err)" is not necessary here. See > scripts/coccinelle/error_propagate_null.cocci. > >> + parse_NumaOptions(ms, object, &err); >> + >> +end: >> qapi_free_NumaOptions(object); >> if (err) { >> error_report_err(err); > > We can fix this one too while at it. Hmm - this is the same script mentioned here: https://lists.gnu.org/archive/html/qemu-devel/2018-03/msg06293.html Except that patch didn't pick up this file. Why is Coccinelle not seeing this?
On Fri, Mar 23, 2018 at 03:49:38PM -0500, Eric Blake wrote: > On 03/23/2018 03:42 PM, Eduardo Habkost wrote: > > On Mon, Mar 12, 2018 at 02:11:08PM +0100, Igor Mammedov wrote: > > > it will allow to reuse parse_NumaOptions() for parsing > > > configuration commands received via QMP interface > > > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > > --- > > > > end: > > > + if (err) { > > > + error_propagate(errp, err); > > > + } > > > > "if (err)" is not necessary here. See > > scripts/coccinelle/error_propagate_null.cocci. > > > > > > + parse_NumaOptions(ms, object, &err); > > > + > > > +end: > > > qapi_free_NumaOptions(object); > > > if (err) { > > > error_report_err(err); > > > > We can fix this one too while at it. > > Hmm - this is the same script mentioned here: > https://lists.gnu.org/archive/html/qemu-devel/2018-03/msg06293.html > > Except that patch didn't pick up this file. Why is Coccinelle not seeing > this? I don't know. I've seen Coccinelle being confused by some of our preprocessor magic before, and in those cases it simply skipped some files.
On 23/03/2018 21:49, Eric Blake wrote: > On 03/23/2018 03:42 PM, Eduardo Habkost wrote: >> On Mon, Mar 12, 2018 at 02:11:08PM +0100, Igor Mammedov wrote: >>> it will allow to reuse parse_NumaOptions() for parsing >>> configuration commands received via QMP interface >>> >>> Signed-off-by: Igor Mammedov <imammedo@redhat.com> >>> --- > >>> end: >>> + if (err) { >>> + error_propagate(errp, err); >>> + } >> >> "if (err)" is not necessary here. See >> scripts/coccinelle/error_propagate_null.cocci. >> > >>> + parse_NumaOptions(ms, object, &err); >>> + >>> +end: >>> qapi_free_NumaOptions(object); >>> if (err) { >>> error_report_err(err); >> >> We can fix this one too while at it. > > Hmm - this is the same script mentioned here: > https://lists.gnu.org/archive/html/qemu-devel/2018-03/msg06293.html > > Except that patch didn't pick up this file. Why is Coccinelle not > seeing this? > The script only catch error_propagate(), not error_report_err(). And error_report_err() doesn't check if err is NULL. Thanks, Laurent
On 03/26/2018 03:38 AM, Laurent Vivier wrote: >>>> end: >>>> + if (err) { >>>> + error_propagate(errp, err); >>>> + } >>> >>> "if (err)" is not necessary here. See >>> scripts/coccinelle/error_propagate_null.cocci. >>> >> >>>> + parse_NumaOptions(ms, object, &err); >>>> + >>>> +end: >>>> qapi_free_NumaOptions(object); >>>> if (err) { >>>> error_report_err(err); >>> >>> We can fix this one too while at it. >> >> Hmm - this is the same script mentioned here: >> https://lists.gnu.org/archive/html/qemu-devel/2018-03/msg06293.html >> >> Except that patch didn't pick up this file. Why is Coccinelle not >> seeing this? >> > > The script only catch error_propagate(), not error_report_err(). And > error_report_err() doesn't check if err is NULL. Aha - chalk it up to reviewing late in the day; I saw an 'if (err)' but didn't pay close attention to what was being guarded in the two different conditionals.
On Fri, 23 Mar 2018 17:42:18 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: > On Mon, Mar 12, 2018 at 02:11:08PM +0100, Igor Mammedov wrote: > > it will allow to reuse parse_NumaOptions() for parsing > > configuration commands received via QMP interface > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > --- > > include/sysemu/numa.h | 1 + > > numa.c | 48 +++++++++++++++++++++++++++++------------------- > > 2 files changed, 30 insertions(+), 19 deletions(-) > > > > diff --git a/include/sysemu/numa.h b/include/sysemu/numa.h > > index 21713b7..7a0ae75 100644 > > --- a/include/sysemu/numa.h > > +++ b/include/sysemu/numa.h > > @@ -22,6 +22,7 @@ struct NumaNodeMem { > > }; > > > > extern NodeInfo numa_info[MAX_NODES]; > > +int parse_numa(void *opaque, QemuOpts *opts, Error **errp); > > void parse_numa_opts(MachineState *ms); > > void numa_complete_configuration(MachineState *ms); > > void query_numa_node_mem(NumaNodeMem node_mem[]); > > diff --git a/numa.c b/numa.c > > index 126c649..2b1d292 100644 > > --- a/numa.c > > +++ b/numa.c > > @@ -169,28 +169,11 @@ static void parse_numa_distance(NumaDistOptions *dist, Error **errp) > > have_numa_distance = true; > > } > > > > -static int parse_numa(void *opaque, QemuOpts *opts, Error **errp) > > +static > > +void parse_NumaOptions(MachineState *ms, NumaOptions *object, Error **errp) > > I wonder if we should rename the parse_numa_{node,distance}() > functions to configure_numa_{node,distance}(), and this one > configure_numa(). These functions don't parse anything, anymore. I'd preffer to do it in another patch on top of this series (added it my TODO list) > > { > > - NumaOptions *object = NULL; > > - MachineState *ms = opaque; > > Error *err = NULL; > > > > - { > > - Visitor *v = opts_visitor_new(opts); > > - visit_type_NumaOptions(v, NULL, &object, &err); > > - visit_free(v); > > - } > > - > > - if (err) { > > - goto end; > > - } > > - > > - /* Fix up legacy suffix-less format */ > > - if ((object->type == NUMA_OPTIONS_TYPE_NODE) && object->u.node.has_mem) { > > - const char *mem_str = qemu_opt_get(opts, "mem"); > > - qemu_strtosz_MiB(mem_str, NULL, &object->u.node.mem); > > - } > > - > > switch (object->type) { > > case NUMA_OPTIONS_TYPE_NODE: > > parse_numa_node(ms, &object->u.node, &err); > > @@ -224,6 +207,33 @@ static int parse_numa(void *opaque, QemuOpts *opts, Error **errp) > > } > > > > end: > > + if (err) { > > + error_propagate(errp, err); > > + } > > "if (err)" is not necessary here. See > scripts/coccinelle/error_propagate_null.cocci. fixed > > +} > > + > > +int parse_numa(void *opaque, QemuOpts *opts, Error **errp) > > +{ > > + NumaOptions *object = NULL; > > + MachineState *ms = MACHINE(opaque); > > + Error *err = NULL; > > + Visitor *v = opts_visitor_new(opts); > > + > > + visit_type_NumaOptions(v, NULL, &object, &err); > > + visit_free(v); > > + if (err) { > > + goto end; > > + } > > + > > + /* Fix up legacy suffix-less format */ > > + if ((object->type == NUMA_OPTIONS_TYPE_NODE) && object->u.node.has_mem) { > > + const char *mem_str = qemu_opt_get(opts, "mem"); > > + qemu_strtosz_MiB(mem_str, NULL, &object->u.node.mem); > > + } > > + > > + parse_NumaOptions(ms, object, &err); > > + > > +end: > > qapi_free_NumaOptions(object); > > if (err) { > > error_report_err(err); > > We can fix this one too while at it. error_report_err() doesn't check for NULL value, 'if(err)' is needed here > The rest of the patch looks good. >
On Tue, Mar 27, 2018 at 03:08:27PM +0200, Igor Mammedov wrote: > On Fri, 23 Mar 2018 17:42:18 -0300 > Eduardo Habkost <ehabkost@redhat.com> wrote: > > > On Mon, Mar 12, 2018 at 02:11:08PM +0100, Igor Mammedov wrote: > > > it will allow to reuse parse_NumaOptions() for parsing > > > configuration commands received via QMP interface > > > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > > --- > > > include/sysemu/numa.h | 1 + > > > numa.c | 48 +++++++++++++++++++++++++++++------------------- > > > 2 files changed, 30 insertions(+), 19 deletions(-) > > > > > > diff --git a/include/sysemu/numa.h b/include/sysemu/numa.h > > > index 21713b7..7a0ae75 100644 > > > --- a/include/sysemu/numa.h > > > +++ b/include/sysemu/numa.h > > > @@ -22,6 +22,7 @@ struct NumaNodeMem { > > > }; > > > > > > extern NodeInfo numa_info[MAX_NODES]; > > > +int parse_numa(void *opaque, QemuOpts *opts, Error **errp); > > > void parse_numa_opts(MachineState *ms); > > > void numa_complete_configuration(MachineState *ms); > > > void query_numa_node_mem(NumaNodeMem node_mem[]); > > > diff --git a/numa.c b/numa.c > > > index 126c649..2b1d292 100644 > > > --- a/numa.c > > > +++ b/numa.c > > > @@ -169,28 +169,11 @@ static void parse_numa_distance(NumaDistOptions *dist, Error **errp) > > > have_numa_distance = true; > > > } > > > > > > -static int parse_numa(void *opaque, QemuOpts *opts, Error **errp) > > > +static > > > +void parse_NumaOptions(MachineState *ms, NumaOptions *object, Error **errp) > > > > I wonder if we should rename the parse_numa_{node,distance}() > > functions to configure_numa_{node,distance}(), and this one > > configure_numa(). These functions don't parse anything, anymore. > I'd preffer to do it in another patch on top of this series > (added it my TODO list) I agree with renaming parse_numa*() later, but the new function you are creating can have a more reasonable name as it doesn't parse anything. > > > > > { > > > - NumaOptions *object = NULL; > > > - MachineState *ms = opaque; > > > Error *err = NULL; > > > > > > - { > > > - Visitor *v = opts_visitor_new(opts); > > > - visit_type_NumaOptions(v, NULL, &object, &err); > > > - visit_free(v); > > > - } > > > - > > > - if (err) { > > > - goto end; > > > - } > > > - > > > - /* Fix up legacy suffix-less format */ > > > - if ((object->type == NUMA_OPTIONS_TYPE_NODE) && object->u.node.has_mem) { > > > - const char *mem_str = qemu_opt_get(opts, "mem"); > > > - qemu_strtosz_MiB(mem_str, NULL, &object->u.node.mem); > > > - } > > > - > > > switch (object->type) { > > > case NUMA_OPTIONS_TYPE_NODE: > > > parse_numa_node(ms, &object->u.node, &err); > > > @@ -224,6 +207,33 @@ static int parse_numa(void *opaque, QemuOpts *opts, Error **errp) > > > } > > > > > > end: > > > + if (err) { > > > + error_propagate(errp, err); > > > + } > > > > "if (err)" is not necessary here. See > > scripts/coccinelle/error_propagate_null.cocci. > fixed Thanks! > > > > +} > > > + > > > +int parse_numa(void *opaque, QemuOpts *opts, Error **errp) > > > +{ > > > + NumaOptions *object = NULL; > > > + MachineState *ms = MACHINE(opaque); > > > + Error *err = NULL; > > > + Visitor *v = opts_visitor_new(opts); > > > + > > > + visit_type_NumaOptions(v, NULL, &object, &err); > > > + visit_free(v); > > > + if (err) { > > > + goto end; > > > + } > > > + > > > + /* Fix up legacy suffix-less format */ > > > + if ((object->type == NUMA_OPTIONS_TYPE_NODE) && object->u.node.has_mem) { > > > + const char *mem_str = qemu_opt_get(opts, "mem"); > > > + qemu_strtosz_MiB(mem_str, NULL, &object->u.node.mem); > > > + } > > > + > > > + parse_NumaOptions(ms, object, &err); > > > + > > > +end: > > > qapi_free_NumaOptions(object); > > > if (err) { > > > error_report_err(err); > > > > We can fix this one too while at it. > error_report_err() doesn't check for NULL value, > 'if(err)' is needed here Sorry, my mistake.
On Wed, 28 Mar 2018 15:54:28 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: > On Tue, Mar 27, 2018 at 03:08:27PM +0200, Igor Mammedov wrote: > > On Fri, 23 Mar 2018 17:42:18 -0300 > > Eduardo Habkost <ehabkost@redhat.com> wrote: > > > > > On Mon, Mar 12, 2018 at 02:11:08PM +0100, Igor Mammedov wrote: > > > > it will allow to reuse parse_NumaOptions() for parsing > > > > configuration commands received via QMP interface > > > > > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > > > --- > > > > include/sysemu/numa.h | 1 + > > > > numa.c | 48 +++++++++++++++++++++++++++++------------------- > > > > 2 files changed, 30 insertions(+), 19 deletions(-) > > > > > > > > diff --git a/include/sysemu/numa.h b/include/sysemu/numa.h > > > > index 21713b7..7a0ae75 100644 > > > > --- a/include/sysemu/numa.h > > > > +++ b/include/sysemu/numa.h > > > > @@ -22,6 +22,7 @@ struct NumaNodeMem { > > > > }; > > > > > > > > extern NodeInfo numa_info[MAX_NODES]; > > > > +int parse_numa(void *opaque, QemuOpts *opts, Error **errp); > > > > void parse_numa_opts(MachineState *ms); > > > > void numa_complete_configuration(MachineState *ms); > > > > void query_numa_node_mem(NumaNodeMem node_mem[]); > > > > diff --git a/numa.c b/numa.c > > > > index 126c649..2b1d292 100644 > > > > --- a/numa.c > > > > +++ b/numa.c > > > > @@ -169,28 +169,11 @@ static void parse_numa_distance(NumaDistOptions *dist, Error **errp) > > > > have_numa_distance = true; > > > > } > > > > > > > > -static int parse_numa(void *opaque, QemuOpts *opts, Error **errp) > > > > +static > > > > +void parse_NumaOptions(MachineState *ms, NumaOptions *object, Error **errp) > > > > > > I wonder if we should rename the parse_numa_{node,distance}() > > > functions to configure_numa_{node,distance}(), and this one > > > configure_numa(). These functions don't parse anything, anymore. > > I'd preffer to do it in another patch on top of this series > > (added it my TODO list) > > I agree with renaming parse_numa*() later, but the new function > you are creating can have a more reasonable name as it doesn't > parse anything. how about: s/parse_NumaOptions/set_NumaOptions/
On Thu, Mar 29, 2018 at 03:05:09PM +0200, Igor Mammedov wrote: > On Wed, 28 Mar 2018 15:54:28 -0300 > Eduardo Habkost <ehabkost@redhat.com> wrote: > > > On Tue, Mar 27, 2018 at 03:08:27PM +0200, Igor Mammedov wrote: > > > On Fri, 23 Mar 2018 17:42:18 -0300 > > > Eduardo Habkost <ehabkost@redhat.com> wrote: > > > > > > > On Mon, Mar 12, 2018 at 02:11:08PM +0100, Igor Mammedov wrote: > > > > > it will allow to reuse parse_NumaOptions() for parsing > > > > > configuration commands received via QMP interface > > > > > > > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > > > > --- > > > > > include/sysemu/numa.h | 1 + > > > > > numa.c | 48 +++++++++++++++++++++++++++++------------------- > > > > > 2 files changed, 30 insertions(+), 19 deletions(-) > > > > > > > > > > diff --git a/include/sysemu/numa.h b/include/sysemu/numa.h > > > > > index 21713b7..7a0ae75 100644 > > > > > --- a/include/sysemu/numa.h > > > > > +++ b/include/sysemu/numa.h > > > > > @@ -22,6 +22,7 @@ struct NumaNodeMem { > > > > > }; > > > > > > > > > > extern NodeInfo numa_info[MAX_NODES]; > > > > > +int parse_numa(void *opaque, QemuOpts *opts, Error **errp); > > > > > void parse_numa_opts(MachineState *ms); > > > > > void numa_complete_configuration(MachineState *ms); > > > > > void query_numa_node_mem(NumaNodeMem node_mem[]); > > > > > diff --git a/numa.c b/numa.c > > > > > index 126c649..2b1d292 100644 > > > > > --- a/numa.c > > > > > +++ b/numa.c > > > > > @@ -169,28 +169,11 @@ static void parse_numa_distance(NumaDistOptions *dist, Error **errp) > > > > > have_numa_distance = true; > > > > > } > > > > > > > > > > -static int parse_numa(void *opaque, QemuOpts *opts, Error **errp) > > > > > +static > > > > > +void parse_NumaOptions(MachineState *ms, NumaOptions *object, Error **errp) > > > > > > > > I wonder if we should rename the parse_numa_{node,distance}() > > > > functions to configure_numa_{node,distance}(), and this one > > > > configure_numa(). These functions don't parse anything, anymore. > > > I'd preffer to do it in another patch on top of this series > > > (added it my TODO list) > > > > I agree with renaming parse_numa*() later, but the new function > > you are creating can have a more reasonable name as it doesn't > > parse anything. > > > how about: s/parse_NumaOptions/set_NumaOptions/ No strong preference. "register", "configure", "set" would all be good enough to me. I would avoid the weird underline_and_CamelCase naming style, though (this style seems to be used only by generated QAPI code).
On Thu, 29 Mar 2018 13:31:13 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: > On Thu, Mar 29, 2018 at 03:05:09PM +0200, Igor Mammedov wrote: > > On Wed, 28 Mar 2018 15:54:28 -0300 > > Eduardo Habkost <ehabkost@redhat.com> wrote: > > > > > On Tue, Mar 27, 2018 at 03:08:27PM +0200, Igor Mammedov wrote: > > > > On Fri, 23 Mar 2018 17:42:18 -0300 > > > > Eduardo Habkost <ehabkost@redhat.com> wrote: > > > > > > > > > On Mon, Mar 12, 2018 at 02:11:08PM +0100, Igor Mammedov wrote: > > > > > > it will allow to reuse parse_NumaOptions() for parsing > > > > > > configuration commands received via QMP interface > > > > > > > > > > > > Signed-off-by: Igor Mammedov <imammedo@redhat.com> > > > > > > --- > > > > > > include/sysemu/numa.h | 1 + > > > > > > numa.c | 48 +++++++++++++++++++++++++++++------------------- > > > > > > 2 files changed, 30 insertions(+), 19 deletions(-) > > > > > > > > > > > > diff --git a/include/sysemu/numa.h b/include/sysemu/numa.h > > > > > > index 21713b7..7a0ae75 100644 > > > > > > --- a/include/sysemu/numa.h > > > > > > +++ b/include/sysemu/numa.h > > > > > > @@ -22,6 +22,7 @@ struct NumaNodeMem { > > > > > > }; > > > > > > > > > > > > extern NodeInfo numa_info[MAX_NODES]; > > > > > > +int parse_numa(void *opaque, QemuOpts *opts, Error **errp); > > > > > > void parse_numa_opts(MachineState *ms); > > > > > > void numa_complete_configuration(MachineState *ms); > > > > > > void query_numa_node_mem(NumaNodeMem node_mem[]); > > > > > > diff --git a/numa.c b/numa.c > > > > > > index 126c649..2b1d292 100644 > > > > > > --- a/numa.c > > > > > > +++ b/numa.c > > > > > > @@ -169,28 +169,11 @@ static void parse_numa_distance(NumaDistOptions *dist, Error **errp) > > > > > > have_numa_distance = true; > > > > > > } > > > > > > > > > > > > -static int parse_numa(void *opaque, QemuOpts *opts, Error **errp) > > > > > > +static > > > > > > +void parse_NumaOptions(MachineState *ms, NumaOptions *object, Error **errp) > > > > > > > > > > I wonder if we should rename the parse_numa_{node,distance}() > > > > > functions to configure_numa_{node,distance}(), and this one > > > > > configure_numa(). These functions don't parse anything, anymore. > > > > I'd preffer to do it in another patch on top of this series > > > > (added it my TODO list) > > > > > > I agree with renaming parse_numa*() later, but the new function > > > you are creating can have a more reasonable name as it doesn't > > > parse anything. > > > > > > how about: s/parse_NumaOptions/set_NumaOptions/ > > No strong preference. "register", "configure", "set" would all > be good enough to me. I would avoid the weird > underline_and_CamelCase naming style, though (this style seems to > be used only by generated QAPI code). I'll fix it on respin
diff --git a/include/sysemu/numa.h b/include/sysemu/numa.h index 21713b7..7a0ae75 100644 --- a/include/sysemu/numa.h +++ b/include/sysemu/numa.h @@ -22,6 +22,7 @@ struct NumaNodeMem { }; extern NodeInfo numa_info[MAX_NODES]; +int parse_numa(void *opaque, QemuOpts *opts, Error **errp); void parse_numa_opts(MachineState *ms); void numa_complete_configuration(MachineState *ms); void query_numa_node_mem(NumaNodeMem node_mem[]); diff --git a/numa.c b/numa.c index 126c649..2b1d292 100644 --- a/numa.c +++ b/numa.c @@ -169,28 +169,11 @@ static void parse_numa_distance(NumaDistOptions *dist, Error **errp) have_numa_distance = true; } -static int parse_numa(void *opaque, QemuOpts *opts, Error **errp) +static +void parse_NumaOptions(MachineState *ms, NumaOptions *object, Error **errp) { - NumaOptions *object = NULL; - MachineState *ms = opaque; Error *err = NULL; - { - Visitor *v = opts_visitor_new(opts); - visit_type_NumaOptions(v, NULL, &object, &err); - visit_free(v); - } - - if (err) { - goto end; - } - - /* Fix up legacy suffix-less format */ - if ((object->type == NUMA_OPTIONS_TYPE_NODE) && object->u.node.has_mem) { - const char *mem_str = qemu_opt_get(opts, "mem"); - qemu_strtosz_MiB(mem_str, NULL, &object->u.node.mem); - } - switch (object->type) { case NUMA_OPTIONS_TYPE_NODE: parse_numa_node(ms, &object->u.node, &err); @@ -224,6 +207,33 @@ static int parse_numa(void *opaque, QemuOpts *opts, Error **errp) } end: + if (err) { + error_propagate(errp, err); + } +} + +int parse_numa(void *opaque, QemuOpts *opts, Error **errp) +{ + NumaOptions *object = NULL; + MachineState *ms = MACHINE(opaque); + Error *err = NULL; + Visitor *v = opts_visitor_new(opts); + + visit_type_NumaOptions(v, NULL, &object, &err); + visit_free(v); + if (err) { + goto end; + } + + /* Fix up legacy suffix-less format */ + if ((object->type == NUMA_OPTIONS_TYPE_NODE) && object->u.node.has_mem) { + const char *mem_str = qemu_opt_get(opts, "mem"); + qemu_strtosz_MiB(mem_str, NULL, &object->u.node.mem); + } + + parse_NumaOptions(ms, object, &err); + +end: qapi_free_NumaOptions(object); if (err) { error_report_err(err);
it will allow to reuse parse_NumaOptions() for parsing configuration commands received via QMP interface Signed-off-by: Igor Mammedov <imammedo@redhat.com> --- include/sysemu/numa.h | 1 + numa.c | 48 +++++++++++++++++++++++++++++------------------- 2 files changed, 30 insertions(+), 19 deletions(-)