| Message ID | 20210803080527.156556-13-wangyanan55@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | machine: smp parsing fixes and improvement | expand |
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 <wangyanan55@huawei.com> > --- > hw/core/machine.c | 63 +++++++++++++++++++++++------------------------ > 1 file changed, 31 insertions(+), 32 deletions(-) > Reviewed-by: Andrew Jones <drjones@redhat.com>
> 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 <wangyanan55@huawei.com> > --- > 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 <pankaj.gupta@ionos.com>
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)
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 <wangyanan55@huawei.com> --- hw/core/machine.c | 63 +++++++++++++++++++++++------------------------ 1 file changed, 31 insertions(+), 32 deletions(-)