Re: [PATCH for-6.2 v4 12/14] machine: Put all sanity-check in the generic SMP parser
> Put both sanity-check of the input SMP configuration and sanity-check > of the output SMP configuration uniformly in the generic parser. Then > machine_set_smp() will become cleaner, also all the invalid scenarios > can be tested only by calling the parser. > > Signed-off-by: Yanan Wang > --- > hw/core/machine.c | 63 +++ > 1 file changed, 31 insertions(+), 32 deletions(-) > > diff --git a/hw/core/machine.c b/hw/core/machine.c > index 443ae5aa1b..3dd6c6f81e 100644 > --- a/hw/core/machine.c > +++ b/hw/core/machine.c > @@ -811,6 +811,20 @@ static void smp_parse(MachineState *ms, SMPConfiguration > *config, Error **errp) > unsigned threads = config->has_threads ? config->threads : 0; > unsigned maxcpus = config->has_maxcpus ? config->maxcpus : 0; > > +/* > + * A specified topology parameter must be greater than zero, > + * explicit configuration like "cpus=0" is not allowed. > + */ > +if ((config->has_cpus && config->cpus == 0) || > +(config->has_sockets && config->sockets == 0) || > +(config->has_dies && config->dies == 0) || > +(config->has_cores && config->cores == 0) || > +(config->has_threads && config->threads == 0) || > +(config->has_maxcpus && config->maxcpus == 0)) { > +error_setg(errp, "CPU topology parameters must be greater than > zero"); > +return; > +} > + > /* > * If not supported by the machine, a topology parameter must be > * omitted or specified equal to 1. > @@ -889,6 +903,22 @@ static void smp_parse(MachineState *ms, SMPConfiguration > *config, Error **errp) > topo_msg, maxcpus, cpus); > return; > } > + > +if (ms->smp.cpus < mc->min_cpus) { > +error_setg(errp, "Invalid SMP CPUs %d. The min CPUs " > + "supported by machine '%s' is %d", > + ms->smp.cpus, > + mc->name, mc->min_cpus); > +return; > +} > + > +if (ms->smp.max_cpus > mc->max_cpus) { > +error_setg(errp, "Invalid SMP CPUs %d. The max CPUs " > + "supported by machine '%s' is %d", > + ms->smp.max_cpus, > + mc->name, mc->max_cpus); > +return; > +} > } > > static void machine_get_smp(Object *obj, Visitor *v, const char *name, > @@ -911,7 +941,6 @@ static void machine_get_smp(Object *obj, Visitor *v, > const char *name, > static void machine_set_smp(Object *obj, Visitor *v, const char *name, > void *opaque, Error **errp) > { > -MachineClass *mc = MACHINE_GET_CLASS(obj); > MachineState *ms = MACHINE(obj); > SMPConfiguration *config; > ERRP_GUARD(); > @@ -920,40 +949,10 @@ static void machine_set_smp(Object *obj, Visitor *v, > const char *name, > return; > } > > -/* > - * The CPU topology parameters must be specified greater than zero or > - * just omitted, explicit configuration like "cpus=0" is not allowed. > - */ > -if ((config->has_cpus && config->cpus == 0) || > -(config->has_sockets && config->sockets == 0) || > -(config->has_dies && config->dies == 0) || > -(config->has_cores && config->cores == 0) || > -(config->has_threads && config->threads == 0) || > -(config->has_maxcpus && config->maxcpus == 0)) { > -error_setg(errp, "CPU topology parameters must be greater than > zero"); > -goto out_free; > -} > - > smp_parse(ms, config, errp); > if (errp) { > -goto out_free; > +qapi_free_SMPConfiguration(config); > } > - > -/* sanity-check smp_cpus and max_cpus against mc */ > -if (ms->smp.cpus < mc->min_cpus) { > -error_setg(errp, "Invalid SMP CPUs %d. The min CPUs " > - "supported by machine '%s' is %d", > - ms->smp.cpus, > - mc->name, mc->min_cpus); > -} else if (ms->smp.max_cpus > mc->max_cpus) { > -error_setg(errp, "Invalid SMP CPUs %d. The max CPUs " > - "supported by machine '%s' is %d", > - ms->smp.max_cpus, > - mc->name, mc->max_cpus); > -} > - > -out_free: > -qapi_free_SMPConfiguration(config); > } > > static void machine_class_init(ObjectClass *oc, void *data) Looks good. Reviewed-by: Pankaj Gupta
Re: [PATCH for-6.2 v4 12/14] machine: Put all sanity-check in the generic SMP parser
On Tue, Aug 03, 2021 at 04:05:25PM +0800, Yanan Wang wrote: > Put both sanity-check of the input SMP configuration and sanity-check > of the output SMP configuration uniformly in the generic parser. Then > machine_set_smp() will become cleaner, also all the invalid scenarios > can be tested only by calling the parser. > > Signed-off-by: Yanan Wang > --- > hw/core/machine.c | 63 +++ > 1 file changed, 31 insertions(+), 32 deletions(-) > Reviewed-by: Andrew Jones
[PATCH for-6.2 v4 12/14] machine: Put all sanity-check in the generic SMP parser
Put both sanity-check of the input SMP configuration and sanity-check of the output SMP configuration uniformly in the generic parser. Then machine_set_smp() will become cleaner, also all the invalid scenarios can be tested only by calling the parser. Signed-off-by: Yanan Wang --- hw/core/machine.c | 63 +++ 1 file changed, 31 insertions(+), 32 deletions(-) diff --git a/hw/core/machine.c b/hw/core/machine.c index 443ae5aa1b..3dd6c6f81e 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -811,6 +811,20 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp) unsigned threads = config->has_threads ? config->threads : 0; unsigned maxcpus = config->has_maxcpus ? config->maxcpus : 0; +/* + * A specified topology parameter must be greater than zero, + * explicit configuration like "cpus=0" is not allowed. + */ +if ((config->has_cpus && config->cpus == 0) || +(config->has_sockets && config->sockets == 0) || +(config->has_dies && config->dies == 0) || +(config->has_cores && config->cores == 0) || +(config->has_threads && config->threads == 0) || +(config->has_maxcpus && config->maxcpus == 0)) { +error_setg(errp, "CPU topology parameters must be greater than zero"); +return; +} + /* * If not supported by the machine, a topology parameter must be * omitted or specified equal to 1. @@ -889,6 +903,22 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp) topo_msg, maxcpus, cpus); return; } + +if (ms->smp.cpus < mc->min_cpus) { +error_setg(errp, "Invalid SMP CPUs %d. The min CPUs " + "supported by machine '%s' is %d", + ms->smp.cpus, + mc->name, mc->min_cpus); +return; +} + +if (ms->smp.max_cpus > mc->max_cpus) { +error_setg(errp, "Invalid SMP CPUs %d. The max CPUs " + "supported by machine '%s' is %d", + ms->smp.max_cpus, + mc->name, mc->max_cpus); +return; +} } static void machine_get_smp(Object *obj, Visitor *v, const char *name, @@ -911,7 +941,6 @@ static void machine_get_smp(Object *obj, Visitor *v, const char *name, static void machine_set_smp(Object *obj, Visitor *v, const char *name, void *opaque, Error **errp) { -MachineClass *mc = MACHINE_GET_CLASS(obj); MachineState *ms = MACHINE(obj); SMPConfiguration *config; ERRP_GUARD(); @@ -920,40 +949,10 @@ static void machine_set_smp(Object *obj, Visitor *v, const char *name, return; } -/* - * The CPU topology parameters must be specified greater than zero or - * just omitted, explicit configuration like "cpus=0" is not allowed. - */ -if ((config->has_cpus && config->cpus == 0) || -(config->has_sockets && config->sockets == 0) || -(config->has_dies && config->dies == 0) || -(config->has_cores && config->cores == 0) || -(config->has_threads && config->threads == 0) || -(config->has_maxcpus && config->maxcpus == 0)) { -error_setg(errp, "CPU topology parameters must be greater than zero"); -goto out_free; -} - smp_parse(ms, config, errp); if (errp) { -goto out_free; +qapi_free_SMPConfiguration(config); } - -/* sanity-check smp_cpus and max_cpus against mc */ -if (ms->smp.cpus < mc->min_cpus) { -error_setg(errp, "Invalid SMP CPUs %d. The min CPUs " - "supported by machine '%s' is %d", - ms->smp.cpus, - mc->name, mc->min_cpus); -} else if (ms->smp.max_cpus > mc->max_cpus) { -error_setg(errp, "Invalid SMP CPUs %d. The max CPUs " - "supported by machine '%s' is %d", - ms->smp.max_cpus, - mc->name, mc->max_cpus); -} - -out_free: -qapi_free_SMPConfiguration(config); } static void machine_class_init(ObjectClass *oc, void *data) -- 2.19.1