On Tue, 6 Jul 2021 at 11:39, Paolo Bonzini <[email protected]> wrote:
>
> Make -smp syntactic sugar for a compound property "-machine
> smp.{cores,threads,cpu,...}". machine_smp_parse is replaced by the
> setter for the property.
>
> numa-test will now cover the new syntax, while other tests
> still use -smp.
>
> Signed-off-by: Paolo Bonzini <[email protected]>
Hi; Coverity reports a bug in this function (CID 1458083):
> +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();
ERRP_GUARD() ensures that 'errp' is never NULL...
> +
> + if (!visit_type_SMPConfiguration(v, name, &config, errp)) {
> + return;
> + }
> +
> + mc->smp_parse(ms, config, errp);
> + if (errp) {
> + goto out_free;
...so this condition is always true, and we always take the goto...
> + }
...and the sanity-checks below here are dead code.
> +
> + /* 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",
> + current_machine->smp.max_cpus,
> + mc->name, mc->max_cpus);
> + }
> +
> +out_free:
> + qapi_free_SMPConfiguration(config);
> +}
With ERRP_GUARD(), the test you want for "is there an error?"
is "if (*errp) {..."
thanks
-- PMM
