* [PULL 0/1] Last minute fix for -rc4 @ 2021-08-16 21:06 Eduardo Habkost 2021-08-16 21:06 ` [PULL 1/1] machine: Disallow specifying topology parameters as zero Eduardo Habkost 2021-08-16 21:38 ` [PULL 0/1] Last minute fix for -rc4 Paolo Bonzini 0 siblings, 2 replies; 10+ messages in thread From: Eduardo Habkost @ 2021-08-16 21:06 UTC (permalink / raw) To: qemu-devel, Peter Maydell; +Cc: Paolo Bonzini, Eduardo Habkost The following changes since commit bd44d64a3879bb6b0ca19bff3be16e0093502fac: Merge remote-tracking branch 'remotes/thuth-gitlab/tags/pull-request-2021-08-11' into staging (2021-08-15 16:46:23 +0100) are available in the Git repository at: https://gitlab.com/ehabkost/qemu.git tags/machine-next-pull-request for you to fetch changes up to bbd0db9dc4751b6ab0884e92421fa4b2c3d3b532: machine: Disallow specifying topology parameters as zero (2021-08-16 16:55:39 -0400) ---------------------------------------------------------------- Last minute fix for -rc4 Bug fix: * Disallow specifying topology parameters as zero (Yanan Wang) ---------------------------------------------------------------- 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.31.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PULL 1/1] machine: Disallow specifying topology parameters as zero 2021-08-16 21:06 [PULL 0/1] Last minute fix for -rc4 Eduardo Habkost @ 2021-08-16 21:06 ` Eduardo Habkost 2021-08-16 21:37 ` Paolo Bonzini 2021-08-16 21:38 ` [PULL 0/1] Last minute fix for -rc4 Paolo Bonzini 1 sibling, 1 reply; 10+ messages in thread From: Eduardo Habkost @ 2021-08-16 21:06 UTC (permalink / raw) To: qemu-devel, Peter Maydell Cc: Andrew Jones, Daniel P . Berrange, Eduardo Habkost, Cornelia Huck, Yanan Wang, Paolo Bonzini From: Yanan Wang <wangyanan55@huawei.com> In the SMP configuration, we should either provide a topology parameter with a reasonable value (greater than zero) or just omit it and QEMU will compute the missing value. Users should have never provided a configuration with parameters as zero (e.g. -smp 8,sockets=0) which should be treated as invalid. But commit 1e63fe68580 (machine: pass QAPI struct to mc->smp_parse) has added some doc which implied that 'anything=0' is valid and has the same semantics as omitting a parameter. To avoid meaningless configurations possibly introduced by users in the future and consequently a necessary deprecation process, fix the doc and also add the corresponding sanity check. Fixes: 1e63fe68580 (machine: pass QAPI struct to mc->smp_parse) Suggested-by: Andrew Jones <drjones@redhat.com> Signed-off-by: Yanan Wang <wangyanan55@huawei.com> Reviewed-by: Daniel P. Berrange <berrange@redhat.com> Tested-by: Daniel P. Berrange <berrange@redhat.com> Reviewed-by: Andrew Jones <drjones@redhat.com> Reviewed-by: Cornelia Huck <cohuck@redhat.com> Message-Id: <20210816024522.143124-2-wangyanan55@huawei.com> Signed-off-by: Eduardo Habkost <ehabkost@redhat.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 54e040587dd..a7e119469aa 100644 --- a/hw/core/machine.c +++ b/hw/core/machine.c @@ -832,6 +832,20 @@ static void machine_set_smp(Object *obj, Visitor *v, const char *name, return; } + /* + * A specified topology parameter must be greater than zero, + * 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 c3210ee1fb2..9272cb3cf8b 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 83aa59a920f..aee622f577d 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -227,11 +227,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.31.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PULL 1/1] machine: Disallow specifying topology parameters as zero 2021-08-16 21:06 ` [PULL 1/1] machine: Disallow specifying topology parameters as zero Eduardo Habkost @ 2021-08-16 21:37 ` Paolo Bonzini 2021-08-17 12:02 ` Andrew Jones 0 siblings, 1 reply; 10+ messages in thread From: Paolo Bonzini @ 2021-08-16 21:37 UTC (permalink / raw) To: Eduardo Habkost Cc: Peter Maydell, Andrew Jones, Daniel P . Berrange, Cornelia Huck, qemu-devel, Yanan Wang [-- Attachment #1: Type: text/plain, Size: 4627 bytes --] How do we know that no one has ever used such configuration? The conversion was meant to be bug-compatible. Paolo Il lun 16 ago 2021, 23:06 Eduardo Habkost <ehabkost@redhat.com> ha scritto: > From: Yanan Wang <wangyanan55@huawei.com> > > In the SMP configuration, we should either provide a topology > parameter with a reasonable value (greater than zero) or just > omit it and QEMU will compute the missing value. Users should > have never provided a configuration with parameters as zero > (e.g. -smp 8,sockets=0) which should be treated as invalid. > > But commit 1e63fe68580 (machine: pass QAPI struct to mc->smp_parse) > has added some doc which implied that 'anything=0' is valid and > has the same semantics as omitting a parameter. > > To avoid meaningless configurations possibly introduced by users > in the future and consequently a necessary deprecation process, > fix the doc and also add the corresponding sanity check. > > Fixes: 1e63fe68580 (machine: pass QAPI struct to mc->smp_parse) > Suggested-by: Andrew Jones <drjones@redhat.com> > Signed-off-by: Yanan Wang <wangyanan55@huawei.com> > Reviewed-by: Daniel P. Berrange <berrange@redhat.com> > Tested-by: Daniel P. Berrange <berrange@redhat.com> > Reviewed-by: Andrew Jones <drjones@redhat.com> > Reviewed-by: Cornelia Huck <cohuck@redhat.com> > Message-Id: <20210816024522.143124-2-wangyanan55@huawei.com> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.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 54e040587dd..a7e119469aa 100644 > --- a/hw/core/machine.c > +++ b/hw/core/machine.c > @@ -832,6 +832,20 @@ static void machine_set_smp(Object *obj, Visitor *v, > const char *name, > return; > } > > + /* > + * A specified topology parameter must be greater than zero, > + * 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 c3210ee1fb2..9272cb3cf8b 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 83aa59a920f..aee622f577d 100644 > --- a/qemu-options.hx > +++ b/qemu-options.hx > @@ -227,11 +227,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.31.1 > > [-- Attachment #2: Type: text/html, Size: 6266 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PULL 1/1] machine: Disallow specifying topology parameters as zero 2021-08-16 21:37 ` Paolo Bonzini @ 2021-08-17 12:02 ` Andrew Jones 2021-08-17 12:06 ` Peter Maydell 0 siblings, 1 reply; 10+ messages in thread From: Andrew Jones @ 2021-08-17 12:02 UTC (permalink / raw) To: Paolo Bonzini Cc: Peter Maydell, Daniel P . Berrange, Eduardo Habkost, Cornelia Huck, qemu-devel, Yanan Wang On Mon, Aug 16, 2021 at 11:37:21PM +0200, Paolo Bonzini wrote: > How do we know that no one has ever used such configuration? The conversion > was meant to be bug-compatible. We don't. But we do know that a zero input value was never documented prior to 1e63fe68580, which has not yet been released. Can we claim that an undocumented input value has undefined behavior, giving us freedom to modify that behavior until it is documented? Thanks, drew > > Paolo > > Il lun 16 ago 2021, 23:06 Eduardo Habkost <ehabkost@redhat.com> ha scritto: > > > From: Yanan Wang <wangyanan55@huawei.com> > > > > In the SMP configuration, we should either provide a topology > > parameter with a reasonable value (greater than zero) or just > > omit it and QEMU will compute the missing value. Users should > > have never provided a configuration with parameters as zero > > (e.g. -smp 8,sockets=0) which should be treated as invalid. > > > > But commit 1e63fe68580 (machine: pass QAPI struct to mc->smp_parse) > > has added some doc which implied that 'anything=0' is valid and > > has the same semantics as omitting a parameter. > > > > To avoid meaningless configurations possibly introduced by users > > in the future and consequently a necessary deprecation process, > > fix the doc and also add the corresponding sanity check. > > > > Fixes: 1e63fe68580 (machine: pass QAPI struct to mc->smp_parse) > > Suggested-by: Andrew Jones <drjones@redhat.com> > > Signed-off-by: Yanan Wang <wangyanan55@huawei.com> > > Reviewed-by: Daniel P. Berrange <berrange@redhat.com> > > Tested-by: Daniel P. Berrange <berrange@redhat.com> > > Reviewed-by: Andrew Jones <drjones@redhat.com> > > Reviewed-by: Cornelia Huck <cohuck@redhat.com> > > Message-Id: <20210816024522.143124-2-wangyanan55@huawei.com> > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.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 54e040587dd..a7e119469aa 100644 > > --- a/hw/core/machine.c > > +++ b/hw/core/machine.c > > @@ -832,6 +832,20 @@ static void machine_set_smp(Object *obj, Visitor *v, > > const char *name, > > return; > > } > > > > + /* > > + * A specified topology parameter must be greater than zero, > > + * 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 c3210ee1fb2..9272cb3cf8b 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 83aa59a920f..aee622f577d 100644 > > --- a/qemu-options.hx > > +++ b/qemu-options.hx > > @@ -227,11 +227,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.31.1 > > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PULL 1/1] machine: Disallow specifying topology parameters as zero 2021-08-17 12:02 ` Andrew Jones @ 2021-08-17 12:06 ` Peter Maydell 2021-08-17 12:22 ` Andrew Jones 0 siblings, 1 reply; 10+ messages in thread From: Peter Maydell @ 2021-08-17 12:06 UTC (permalink / raw) To: Andrew Jones Cc: Daniel P . Berrange, Eduardo Habkost, Cornelia Huck, qemu-devel, Yanan Wang, Paolo Bonzini On Tue, 17 Aug 2021 at 13:02, Andrew Jones <drjones@redhat.com> wrote: > > On Mon, Aug 16, 2021 at 11:37:21PM +0200, Paolo Bonzini wrote: > > How do we know that no one has ever used such configuration? The conversion > > was meant to be bug-compatible. > > We don't. But we do know that a zero input value was never documented > prior to 1e63fe68580, which has not yet been released. Can we claim > that an undocumented input value has undefined behavior, giving us > freedom to modify that behavior until it is documented? Dunno; I definitely don't want a behaviour-change patch at this point in the release-cycle, though... -- PMM ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PULL 1/1] machine: Disallow specifying topology parameters as zero 2021-08-17 12:06 ` Peter Maydell @ 2021-08-17 12:22 ` Andrew Jones 2021-08-17 12:37 ` Peter Maydell 2021-08-17 13:29 ` wangyanan (Y) 0 siblings, 2 replies; 10+ messages in thread From: Andrew Jones @ 2021-08-17 12:22 UTC (permalink / raw) To: Peter Maydell Cc: Daniel P . Berrange, Eduardo Habkost, Cornelia Huck, qemu-devel, Yanan Wang, Paolo Bonzini On Tue, Aug 17, 2021 at 01:06:19PM +0100, Peter Maydell wrote: > On Tue, 17 Aug 2021 at 13:02, Andrew Jones <drjones@redhat.com> wrote: > > > > On Mon, Aug 16, 2021 at 11:37:21PM +0200, Paolo Bonzini wrote: > > > How do we know that no one has ever used such configuration? The conversion > > > was meant to be bug-compatible. > > > > We don't. But we do know that a zero input value was never documented > > prior to 1e63fe68580, which has not yet been released. Can we claim > > that an undocumented input value has undefined behavior, giving us > > freedom to modify that behavior until it is documented? > > Dunno; I definitely don't want a behaviour-change patch at this > point in the release-cycle, though... > Can we replace this patch with the following one for now? And then discuss this further before committing to supporting a zero input? Thanks, drew Author: Andrew Jones <drjones@redhat.com> Date: Tue Aug 17 14:17:29 2021 +0200 qapi/machine.json: Remove zero value reference from SMPConfiguration documentation Commit 1e63fe685804 ("machine: pass QAPI struct to mc->smp_parse") introduced documentation stating that a zero input value for an SMP parameter indicates that its value should be automatically configured. This is indeed how things work today, but we'd like to change that. Avoid documenting behaviors we want to leave undefined for the time being, giving us freedom to change it later. Fixes: 1e63fe685804 ("machine: pass QAPI struct to mc->smp_parse") Signed-off-by: Andrew Jones <drjones@redhat.com> diff --git a/qapi/machine.json b/qapi/machine.json index c3210ee1fb24..157712f00614 100644 --- a/qapi/machine.json +++ b/qapi/machine.json @@ -1288,7 +1288,7 @@ ## # @SMPConfiguration: # -# Schema for CPU topology configuration. "0" or a missing value lets +# 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PULL 1/1] machine: Disallow specifying topology parameters as zero 2021-08-17 12:22 ` Andrew Jones @ 2021-08-17 12:37 ` Peter Maydell 2021-08-17 12:54 ` Andrew Jones 2021-08-17 13:29 ` wangyanan (Y) 1 sibling, 1 reply; 10+ messages in thread From: Peter Maydell @ 2021-08-17 12:37 UTC (permalink / raw) To: Andrew Jones Cc: Daniel P . Berrange, Eduardo Habkost, Cornelia Huck, qemu-devel, Yanan Wang, Paolo Bonzini On Tue, 17 Aug 2021 at 13:22, Andrew Jones <drjones@redhat.com> wrote: > > On Tue, Aug 17, 2021 at 01:06:19PM +0100, Peter Maydell wrote: > > On Tue, 17 Aug 2021 at 13:02, Andrew Jones <drjones@redhat.com> wrote: > > > > > > On Mon, Aug 16, 2021 at 11:37:21PM +0200, Paolo Bonzini wrote: > > > > How do we know that no one has ever used such configuration? The conversion > > > > was meant to be bug-compatible. > > > > > > We don't. But we do know that a zero input value was never documented > > > prior to 1e63fe68580, which has not yet been released. Can we claim > > > that an undocumented input value has undefined behavior, giving us > > > freedom to modify that behavior until it is documented? > > > > Dunno; I definitely don't want a behaviour-change patch at this > > point in the release-cycle, though... > > > > Can we replace this patch with the following one for now? And then > discuss this further before committing to supporting a zero input? If you can agree on that and send out a patch and get it reviewed within the next three hours or so... -- PMM ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PULL 1/1] machine: Disallow specifying topology parameters as zero 2021-08-17 12:37 ` Peter Maydell @ 2021-08-17 12:54 ` Andrew Jones 0 siblings, 0 replies; 10+ messages in thread From: Andrew Jones @ 2021-08-17 12:54 UTC (permalink / raw) To: Peter Maydell Cc: Daniel P . Berrange, Eduardo Habkost, Cornelia Huck, qemu-devel, Yanan Wang, Paolo Bonzini On Tue, Aug 17, 2021 at 01:37:15PM +0100, Peter Maydell wrote: > On Tue, 17 Aug 2021 at 13:22, Andrew Jones <drjones@redhat.com> wrote: > > > > On Tue, Aug 17, 2021 at 01:06:19PM +0100, Peter Maydell wrote: > > > On Tue, 17 Aug 2021 at 13:02, Andrew Jones <drjones@redhat.com> wrote: > > > > > > > > On Mon, Aug 16, 2021 at 11:37:21PM +0200, Paolo Bonzini wrote: > > > > > How do we know that no one has ever used such configuration? The conversion > > > > > was meant to be bug-compatible. > > > > > > > > We don't. But we do know that a zero input value was never documented > > > > prior to 1e63fe68580, which has not yet been released. Can we claim > > > > that an undocumented input value has undefined behavior, giving us > > > > freedom to modify that behavior until it is documented? > > > > > > Dunno; I definitely don't want a behaviour-change patch at this > > > point in the release-cycle, though... > > > > > > > Can we replace this patch with the following one for now? And then > > discuss this further before committing to supporting a zero input? > > If you can agree on that and send out a patch and get it reviewed > within the next three hours or so... I posted the patch. Agreement/disagreement can come in the form of ack/nack. Hopefully in time... Thanks, drew ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PULL 1/1] machine: Disallow specifying topology parameters as zero 2021-08-17 12:22 ` Andrew Jones 2021-08-17 12:37 ` Peter Maydell @ 2021-08-17 13:29 ` wangyanan (Y) 1 sibling, 0 replies; 10+ messages in thread From: wangyanan (Y) @ 2021-08-17 13:29 UTC (permalink / raw) To: Andrew Jones, Peter Maydell Cc: Daniel P . Berrange, Eduardo Habkost, Cornelia Huck, qemu-devel, Paolo Bonzini On 2021/8/17 20:22, Andrew Jones wrote: > On Tue, Aug 17, 2021 at 01:06:19PM +0100, Peter Maydell wrote: >> On Tue, 17 Aug 2021 at 13:02, Andrew Jones <drjones@redhat.com> wrote: >>> On Mon, Aug 16, 2021 at 11:37:21PM +0200, Paolo Bonzini wrote: >>>> How do we know that no one has ever used such configuration? The conversion >>>> was meant to be bug-compatible. >>> We don't. But we do know that a zero input value was never documented >>> prior to 1e63fe68580, which has not yet been released. Can we claim >>> that an undocumented input value has undefined behavior, giving us >>> freedom to modify that behavior until it is documented? >> Dunno; I definitely don't want a behaviour-change patch at this >> point in the release-cycle, though... >> > Can we replace this patch with the following one for now? And then > discuss this further before committing to supporting a zero input? Yeah, I think at least the doc needs to be fixed in 6.1. Then we may think about deprecating this behavior since 6.2, I have implemented the deprecation locally and it can be post with the smp parsing improvement series. Thanks, Yanan . > Thanks, > drew > > Author: Andrew Jones <drjones@redhat.com> > Date: Tue Aug 17 14:17:29 2021 +0200 > > qapi/machine.json: Remove zero value reference from SMPConfiguration documentation > > Commit 1e63fe685804 ("machine: pass QAPI struct to mc->smp_parse") > introduced documentation stating that a zero input value for an SMP > parameter indicates that its value should be automatically configured. > This is indeed how things work today, but we'd like to change that. > Avoid documenting behaviors we want to leave undefined for the time > being, giving us freedom to change it later. > > Fixes: 1e63fe685804 ("machine: pass QAPI struct to mc->smp_parse") > Signed-off-by: Andrew Jones <drjones@redhat.com> > > diff --git a/qapi/machine.json b/qapi/machine.json > index c3210ee1fb24..157712f00614 100644 > --- a/qapi/machine.json > +++ b/qapi/machine.json > @@ -1288,7 +1288,7 @@ > ## > # @SMPConfiguration: > # > -# Schema for CPU topology configuration. "0" or a missing value lets > +# 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 > > . ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PULL 0/1] Last minute fix for -rc4 2021-08-16 21:06 [PULL 0/1] Last minute fix for -rc4 Eduardo Habkost 2021-08-16 21:06 ` [PULL 1/1] machine: Disallow specifying topology parameters as zero Eduardo Habkost @ 2021-08-16 21:38 ` Paolo Bonzini 1 sibling, 0 replies; 10+ messages in thread From: Paolo Bonzini @ 2021-08-16 21:38 UTC (permalink / raw) To: Eduardo Habkost; +Cc: Peter Maydell, qemu-devel [-- Attachment #1: Type: text/plain, Size: 1187 bytes --] AIUI this behavior existed before in 6.0 and therefore should be deprecated first. Paolo Il lun 16 ago 2021, 23:06 Eduardo Habkost <ehabkost@redhat.com> ha scritto: > The following changes since commit > bd44d64a3879bb6b0ca19bff3be16e0093502fac: > > Merge remote-tracking branch > 'remotes/thuth-gitlab/tags/pull-request-2021-08-11' into staging > (2021-08-15 16:46:23 +0100) > > are available in the Git repository at: > > https://gitlab.com/ehabkost/qemu.git tags/machine-next-pull-request > > for you to fetch changes up to bbd0db9dc4751b6ab0884e92421fa4b2c3d3b532: > > machine: Disallow specifying topology parameters as zero (2021-08-16 > 16:55:39 -0400) > > ---------------------------------------------------------------- > Last minute fix for -rc4 > > Bug fix: > * Disallow specifying topology parameters as zero > (Yanan Wang) > > ---------------------------------------------------------------- > > 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.31.1 > > > [-- Attachment #2: Type: text/html, Size: 1712 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-08-17 13:31 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-08-16 21:06 [PULL 0/1] Last minute fix for -rc4 Eduardo Habkost 2021-08-16 21:06 ` [PULL 1/1] machine: Disallow specifying topology parameters as zero Eduardo Habkost 2021-08-16 21:37 ` Paolo Bonzini 2021-08-17 12:02 ` Andrew Jones 2021-08-17 12:06 ` Peter Maydell 2021-08-17 12:22 ` Andrew Jones 2021-08-17 12:37 ` Peter Maydell 2021-08-17 12:54 ` Andrew Jones 2021-08-17 13:29 ` wangyanan (Y) 2021-08-16 21:38 ` [PULL 0/1] Last minute fix for -rc4 Paolo Bonzini
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.