Re: [RFC PATCH 2/6] machine: Perform zero-check for the computed value of sockets

[Andrew Jones <drjones@redhat.com>]

On Fri, Jul 02, 2021 at 06:07:35PM +0800, Yanan Wang wrote:
> We currently perform zero-check (default the value to 1 if zeroed)
> for the computed values of cores/threads, to make sure they are at
> least 1. For consistency, we probably should also default sockets
> to 1 if the computed value is zero. Note that this won't affect
> any existing working cmdlines but will improve the error reporting
> of the invalid ones such as "-smp 8,maxcpus=9,cores=10,threads=1".

How does this help error checking? If the user input values that compute a
fractional (rounded down to zero with integer division) value, then we'll
catch that with the sockets*cores*threads == maxcpus test now, but we may
not catch that after this patch.

Thanks,
drew

> 
> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> ---
>  hw/core/machine.c | 1 +
>  hw/i386/pc.c      | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index f17bbe3275..1e194677cd 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -761,6 +761,7 @@ static void smp_parse(MachineState *ms, SMPConfiguration *config, Error **errp)
>          } else {
>              maxcpus = maxcpus > 0 ? maxcpus : cpus;
>              sockets = maxcpus / (cores * threads);
> +            sockets = sockets > 0 ? sockets : 1;
>          }
>      } else if (cores == 0) {
>          threads = threads > 0 ? threads : 1;
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index a9b22fdc01..a44511c937 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -729,6 +729,7 @@ static void pc_smp_parse(MachineState *ms, SMPConfiguration *config, Error **err
>          } else {
>              maxcpus = maxcpus > 0 ? maxcpus : cpus;
>              sockets = maxcpus / (dies * cores * threads);
> +            sockets = sockets > 0 ? sockets : 1;
>          }
>      } else if (cores == 0) {
>          threads = threads > 0 ? threads : 1;
> -- 
> 2.19.1
>