On Thu, Jul 22, 2021 at 11:43:26PM +0800, Yanan Wang wrote: > In the SMP configuration, we should either specify a topology > parameter with a reasonable value (equal to or greater than 1) > or just leave it omitted and QEMU will calculate its value. > Configurations which explicitly specify the topology parameters > as zero like "sockets=0" are meaningless, so disallow them. > > However, the commit 1e63fe685804d > (machine: pass QAPI struct to mc->smp_parse) has documented that > '0' has the same semantics as omitting a parameter in the qapi > comment for SMPConfiguration. So this patch fixes the doc and > also adds the corresponding sanity check in the smp parsers. > > Suggested-by: Andrew Jones <drjo...@redhat.com> > Signed-off-by: Yanan Wang <wangyana...@huawei.com> > --- > hw/core/machine.c | 14 ++++++++++++++ > qapi/machine.json | 6 +++--- > qemu-options.hx | 12 +++++++----- > 3 files changed, 24 insertions(+), 8 deletions(-) > > diff --git a/hw/core/machine.c b/hw/core/machine.c > index 775add0795..db129d937b 100644 > --- a/hw/core/machine.c > +++ b/hw/core/machine.c > @@ -829,6 +829,20 @@ static void machine_set_smp(Object *obj, Visitor *v, > const char *name, > return; > } > > + /* > + * The topology parameters must be specified equal to or great than one > + * 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, "parameters must be equal to or greater than one if > provided");
I'd suggest a slight tweak since when seen it lacks context: $ ./qemu-system-x86_64 -smp 4,cores=0,sockets=2 qemu-system-x86_64: parameters must be equal to or greater than one if provided error_setg(errp, "CPU topology parameters must be equal to or greater than one if provided"); > diff --git a/qemu-options.hx b/qemu-options.hx > index 99ed5ec5f1..b0168f8c48 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -223,11 +223,13 @@ SRST > of computing the CPU maximum count. > > Either the initial CPU count, or at least one of the topology parameters > - must be specified. Values for any omitted parameters will be computed > - from those which are given. Historically preference was given to the > - coarsest topology parameters when computing missing values (ie sockets > - preferred over cores, which were preferred over threads), however, this > - behaviour is considered liable to change. > + must be specified. The specified parameters must be equal to or great s/great/greater/ > + than one, explicit configuration like "cpus=0" is not allowed. Values > + for any omitted parameters will be computed from those which are given. > + Historically preference was given to the coarsest topology parameters > + when computing missing values (ie sockets preferred over cores, which > + were preferred over threads), however, this behaviour is considered > + liable to change. > ERST If you make the text changes, then feel free to add this when posting v2: Reviewed-by: Daniel P. Berrangé <berra...@redhat.com> Tested-by: Daniel P. Berrangé <berra...@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|