On 2021/7/23 16:02, Markus Armbruster wrote:
Daniel P. Berrangé <berra...@redhat.com> writes:
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");
Let's scratch "if provided".
I'd replace "must be equal to or greater than one" by "must be
positive", or maybe "must be greater than zero".
How about we use "must be greater than zero" ?
After a grep search of these two sentences in QEMU, they both show up
in several places. "must be positive" always reports an invalid value that
is "< 0". While the check in this patch actually reject an invalid zero
value.
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
"positive" again.
Thanks,
Yanan
+ 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
.
