* [PATCH for-6.1 v5 0/1] machine: Disallow specifying topology parameters as zero @ 2021-07-23 9:57 Yanan Wang 2021-07-23 9:57 ` [PATCH for-6.1 v5 1/1] " Yanan Wang 0 siblings, 1 reply; 3+ messages in thread From: Yanan Wang @ 2021-07-23 9:57 UTC (permalink / raw) To: qemu-devel Cc: Peter Maydell, Andrew Jones, Daniel P . Berrangé, Eduardo Habkost, Pierre Morel, Pankaj Gupta, wanghaibin.wang, Cornelia Huck, Markus Armbruster, Yanan Wang, Cleber Rosa, yuzenghui, Paolo Bonzini 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. This patch originly comes form [1], and it was suggested that this patch fixing the doc should be sent for 6.1 to avoid a deprecation process in the future. [1] https://lore.kernel.org/qemu-devel/YPWsThPiZa3mF+zp@redhat.com/ v4->v5: - tweak the error message (Markus) - v4: https://lore.kernel.org/qemu-devel/20210723073122.7720-1-wangyanan55@huawei.com/ v3->v4: - fix the typos - v3: https://lore.kernel.org/qemu-devel/20210723020002.18456-1-wangyanan55@huawei.com/ v2->v3: - improve the error message - v2: https://lore.kernel.org/qemu-devel/20210722154326.1464-1-wangyanan55@huawei.com/ v1->v2: - move the check to machine_set_smp - update qemu-option.hx - v1: https://lore.kernel.org/qemu-devel/20210722021512.2600-1-wangyanan55@huawei.com/ Yanan Wang (1): machine: Disallow specifying topology parameters as zero hw/core/machine.c | 14 ++++++++++++++ qapi/machine.json | 6 +++--- qemu-options.hx | 12 +++++++----- 3 files changed, 24 insertions(+), 8 deletions(-) -- 2.19.1 ^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH for-6.1 v5 1/1] machine: Disallow specifying topology parameters as zero 2021-07-23 9:57 [PATCH for-6.1 v5 0/1] machine: Disallow specifying topology parameters as zero Yanan Wang @ 2021-07-23 9:57 ` Yanan Wang 2021-08-06 8:27 ` wangyanan (Y) 0 siblings, 1 reply; 3+ messages in thread From: Yanan Wang @ 2021-07-23 9:57 UTC (permalink / raw) To: qemu-devel Cc: Peter Maydell, Andrew Jones, Daniel P . Berrangé, Eduardo Habkost, Pierre Morel, Pankaj Gupta, wanghaibin.wang, Cornelia Huck, Markus Armbruster, Yanan Wang, Cleber Rosa, yuzenghui, Paolo Bonzini 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. Reviewed-by: Cornelia Huck <cohuck@redhat.com> Reviewed-by: Andrew Jones <drjones@redhat.com> Reviewed-by: Daniel P. Berrange <berrange@redhat.com> Tested-by: Daniel P. Berrange <berrange@redhat.com> Suggested-by: Andrew Jones <drjones@redhat.com> Signed-off-by: Yanan Wang <wangyanan55@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..e1533dfc47 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 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; + } + mc->smp_parse(ms, config, errp); if (errp) { goto out_free; diff --git a/qapi/machine.json b/qapi/machine.json index c3210ee1fb..9272cb3cf8 100644 --- a/qapi/machine.json +++ b/qapi/machine.json @@ -1288,8 +1288,8 @@ ## # @SMPConfiguration: # -# Schema for CPU topology configuration. "0" or a missing value lets -# QEMU figure out a suitable value based on the ones that are provided. +# Schema for CPU topology configuration. A missing value lets QEMU +# figure out a suitable value based on the ones that are provided. # # @cpus: number of virtual CPUs in the virtual machine # @@ -1297,7 +1297,7 @@ # # @dies: number of dies per socket in the CPU topology # -# @cores: number of cores per thread in the CPU topology +# @cores: number of cores per die in the CPU topology # # @threads: number of threads per core in the CPU topology # diff --git a/qemu-options.hx b/qemu-options.hx index 99ed5ec5f1..e077aa80bf 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 greater than zero, + 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 DEF("numa", HAS_ARG, QEMU_OPTION_numa, -- 2.19.1 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH for-6.1 v5 1/1] machine: Disallow specifying topology parameters as zero 2021-07-23 9:57 ` [PATCH for-6.1 v5 1/1] " Yanan Wang @ 2021-08-06 8:27 ` wangyanan (Y) 0 siblings, 0 replies; 3+ messages in thread From: wangyanan (Y) @ 2021-08-06 8:27 UTC (permalink / raw) To: qemu-devel, Eduardo Habkost, Marcel Apfelbaum, Paolo Bonzini Cc: Peter Maydell, Andrew Jones, Daniel P . Berrangé, Pierre Morel, Pankaj Gupta, Cornelia Huck, Markus Armbruster, Cleber Rosa, yuzenghui, wanghaibin.wang A gentle ping ... Will this patch be picked up soon for 6.1 or is there anything else to tweak? Thanks, Yanan On 2021/7/23 17:57, 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. > > Reviewed-by: Cornelia Huck <cohuck@redhat.com> > Reviewed-by: Andrew Jones <drjones@redhat.com> > Reviewed-by: Daniel P. Berrange <berrange@redhat.com> > Tested-by: Daniel P. Berrange <berrange@redhat.com> > Suggested-by: Andrew Jones <drjones@redhat.com> > Signed-off-by: Yanan Wang <wangyanan55@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..e1533dfc47 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 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; > + } > + > mc->smp_parse(ms, config, errp); > if (errp) { > goto out_free; > diff --git a/qapi/machine.json b/qapi/machine.json > index c3210ee1fb..9272cb3cf8 100644 > --- a/qapi/machine.json > +++ b/qapi/machine.json > @@ -1288,8 +1288,8 @@ > ## > # @SMPConfiguration: > # > -# Schema for CPU topology configuration. "0" or a missing value lets > -# QEMU figure out a suitable value based on the ones that are provided. > +# Schema for CPU topology configuration. A missing value lets QEMU > +# figure out a suitable value based on the ones that are provided. > # > # @cpus: number of virtual CPUs in the virtual machine > # > @@ -1297,7 +1297,7 @@ > # > # @dies: number of dies per socket in the CPU topology > # > -# @cores: number of cores per thread in the CPU topology > +# @cores: number of cores per die in the CPU topology > # > # @threads: number of threads per core in the CPU topology > # > diff --git a/qemu-options.hx b/qemu-options.hx > index 99ed5ec5f1..e077aa80bf 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 greater than zero, > + 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 > > DEF("numa", HAS_ARG, QEMU_OPTION_numa, ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-08-06 8:28 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-07-23 9:57 [PATCH for-6.1 v5 0/1] machine: Disallow specifying topology parameters as zero Yanan Wang 2021-07-23 9:57 ` [PATCH for-6.1 v5 1/1] " Yanan Wang 2021-08-06 8:27 ` wangyanan (Y)
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.