From: Zhao Liu <zhao1.liu@intel.com>
Currentlt, it was allowed for users to specify the unsupported
topology parameter as "1". For example, x86 PC machine doesn't
support drawer/book/cluster topology levels, but user could specify
"-smp drawers=1,books=1,clusters=1".
This is meaningless and confusing, so that the support for this kind of
configurations is marked depresated since 9.0. And report warning
message for such case like:
qemu-system-x86_64: warning: Deprecated CPU topology (considered invalid):
Unsupported clusters parameter mustn't be specified as 1
qemu-system-x86_64: warning: Deprecated CPU topology (considered invalid):
Unsupported books parameter mustn't be specified as 1
qemu-system-x86_64: warning: Deprecated CPU topology (considered invalid):
Unsupported drawers parameter mustn't be specified as 1
Users have to ensure that all the topology members described with -smp
are supported by the target machine.
Cc: devel@lists.libvirt.org
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
docs/about/deprecated.rst | 14 +++++++++
hw/core/machine-smp.c | 63 +++++++++++++++++++++++++++++----------
2 files changed, 61 insertions(+), 16 deletions(-)
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
index 872974640252..2e782e83e952 100644
--- a/docs/about/deprecated.rst
+++ b/docs/about/deprecated.rst
@@ -47,6 +47,20 @@ as short-form boolean values, and passed to plugins as ``arg_name=on``.
However, short-form booleans are deprecated and full explicit ``arg_name=on``
form is preferred.
+``-smp`` (Unsopported "parameter=1" SMP configurations) (since 9.0)
+'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
+
+Specified CPU topology parameters must be supported by the machine.
+
+In the SMP configuration, users should provide the CPU topology parameters that
+are supported by the target machine.
+
+However, historically it was allowed for users to specify the unsupported
+topology parameter as "1", which is meaningless. So support for this kind of
+configurations (e.g. -smp drawers=1,books=1,clusters=1 for x86 PC machine) is
+marked depresated since 9.0, users have to ensure that all the topology members
+described with -smp are supported by the target machine.
+
QEMU Machine Protocol (QMP) commands
------------------------------------
diff --git a/hw/core/machine-smp.c b/hw/core/machine-smp.c
index 96533886b14e..50a5a40dbc3d 100644
--- a/hw/core/machine-smp.c
+++ b/hw/core/machine-smp.c
@@ -112,30 +112,61 @@ void machine_parse_smp_config(MachineState *ms,
/*
* If not supported by the machine, a topology parameter must be
- * omitted or specified equal to 1.
+ * omitted.
*/
- if (!mc->smp_props.dies_supported && dies > 1) {
- error_setg(errp, "dies not supported by this machine's CPU topology");
- return;
- }
- if (!mc->smp_props.clusters_supported && clusters > 1) {
- error_setg(errp, "clusters not supported by this machine's CPU topology");
- return;
+ if (!mc->smp_props.clusters_supported && config->has_clusters) {
+ if (config->clusters > 1) {
+ error_setg(errp, "clusters not supported by this "
+ "machine's CPU topology");
+ return;
+ } else {
+ /* Here clusters only equals 1 since we've checked zero case. */
+ warn_report("Deprecated CPU topology (considered invalid): "
+ "Unsupported clusters parameter mustn't be "
+ "specified as 1");
+ }
}
+ clusters = clusters > 0 ? clusters : 1;
+ if (!mc->smp_props.dies_supported && config->has_dies) {
+ if (config->dies > 1) {
+ error_setg(errp, "dies not supported by this "
+ "machine's CPU topology");
+ return;
+ } else {
+ /* Here dies only equals 1 since we've checked zero case. */
+ warn_report("Deprecated CPU topology (considered invalid): "
+ "Unsupported dies parameter mustn't be "
+ "specified as 1");
+ }
+ }
dies = dies > 0 ? dies : 1;
- clusters = clusters > 0 ? clusters : 1;
- if (!mc->smp_props.books_supported && books > 1) {
- error_setg(errp, "books not supported by this machine's CPU topology");
- return;
+ if (!mc->smp_props.books_supported && config->has_books) {
+ if (config->books > 1) {
+ error_setg(errp, "books not supported by this "
+ "machine's CPU topology");
+ return;
+ } else {
+ /* Here books only equals 1 since we've checked zero case. */
+ warn_report("Deprecated CPU topology (considered invalid): "
+ "Unsupported books parameter mustn't be "
+ "specified as 1");
+ }
}
books = books > 0 ? books : 1;
- if (!mc->smp_props.drawers_supported && drawers > 1) {
- error_setg(errp,
- "drawers not supported by this machine's CPU topology");
- return;
+ if (!mc->smp_props.drawers_supported && config->has_drawers) {
+ if (config->drawers > 1) {
+ error_setg(errp, "drawers not supported by this "
+ "machine's CPU topology");
+ return;
+ } else {
+ /* Here drawers only equals 1 since we've checked zero case. */
+ warn_report("Deprecated CPU topology (considered invalid): "
+ "Unsupported drawers parameter mustn't be "
+ "specified as 1");
+ }
}
drawers = drawers > 0 ? drawers : 1;
--
2.34.1On 06/03/2024 10.53, Zhao Liu wrote:
> From: Zhao Liu <zhao1.liu@intel.com>
>
> Currentlt, it was allowed for users to specify the unsupported
s/Currentlt/Currently/
> topology parameter as "1". For example, x86 PC machine doesn't
> support drawer/book/cluster topology levels, but user could specify
> "-smp drawers=1,books=1,clusters=1".
>
> This is meaningless and confusing, so that the support for this kind of
> configurations is marked depresated since 9.0. And report warning
s/depresated/deprecated/
> message for such case like:
>
> qemu-system-x86_64: warning: Deprecated CPU topology (considered invalid):
> Unsupported clusters parameter mustn't be specified as 1
> qemu-system-x86_64: warning: Deprecated CPU topology (considered invalid):
> Unsupported books parameter mustn't be specified as 1
> qemu-system-x86_64: warning: Deprecated CPU topology (considered invalid):
> Unsupported drawers parameter mustn't be specified as 1
>
> Users have to ensure that all the topology members described with -smp
> are supported by the target machine.
>
> Cc: devel@lists.libvirt.org
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> ---
> docs/about/deprecated.rst | 14 +++++++++
> hw/core/machine-smp.c | 63 +++++++++++++++++++++++++++++----------
> 2 files changed, 61 insertions(+), 16 deletions(-)
>
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index 872974640252..2e782e83e952 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -47,6 +47,20 @@ as short-form boolean values, and passed to plugins as ``arg_name=on``.
> However, short-form booleans are deprecated and full explicit ``arg_name=on``
> form is preferred.
>
> +``-smp`` (Unsopported "parameter=1" SMP configurations) (since 9.0)
s/Unsopported/Unsupported/
> +'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
> +
> +Specified CPU topology parameters must be supported by the machine.
> +
> +In the SMP configuration, users should provide the CPU topology parameters that
> +are supported by the target machine.
> +
> +However, historically it was allowed for users to specify the unsupported
> +topology parameter as "1", which is meaningless. So support for this kind of
> +configurations (e.g. -smp drawers=1,books=1,clusters=1 for x86 PC machine) is
> +marked depresated since 9.0, users have to ensure that all the topology members
s/depresated/deprecated/
> +described with -smp are supported by the target machine.
> +
> QEMU Machine Protocol (QMP) commands
> ------------------------------------
>
> diff --git a/hw/core/machine-smp.c b/hw/core/machine-smp.c
> index 96533886b14e..50a5a40dbc3d 100644
> --- a/hw/core/machine-smp.c
> +++ b/hw/core/machine-smp.c
> @@ -112,30 +112,61 @@ void machine_parse_smp_config(MachineState *ms,
>
> /*
> * If not supported by the machine, a topology parameter must be
> - * omitted or specified equal to 1.
> + * omitted.
> */
> - if (!mc->smp_props.dies_supported && dies > 1) {
> - error_setg(errp, "dies not supported by this machine's CPU topology");
> - return;
> - }
> - if (!mc->smp_props.clusters_supported && clusters > 1) {
> - error_setg(errp, "clusters not supported by this machine's CPU topology");
> - return;
> + if (!mc->smp_props.clusters_supported && config->has_clusters) {
> + if (config->clusters > 1) {
> + error_setg(errp, "clusters not supported by this "
> + "machine's CPU topology");
> + return;
> + } else {
> + /* Here clusters only equals 1 since we've checked zero case. */
> + warn_report("Deprecated CPU topology (considered invalid): "
> + "Unsupported clusters parameter mustn't be "
> + "specified as 1");
> + }
> }
> + clusters = clusters > 0 ? clusters : 1;
>
> + if (!mc->smp_props.dies_supported && config->has_dies) {
> + if (config->dies > 1) {
> + error_setg(errp, "dies not supported by this "
> + "machine's CPU topology");
> + return;
> + } else {
> + /* Here dies only equals 1 since we've checked zero case. */
> + warn_report("Deprecated CPU topology (considered invalid): "
> + "Unsupported dies parameter mustn't be "
> + "specified as 1");
> + }
> + }
> dies = dies > 0 ? dies : 1;
> - clusters = clusters > 0 ? clusters : 1;
>
> - if (!mc->smp_props.books_supported && books > 1) {
> - error_setg(errp, "books not supported by this machine's CPU topology");
> - return;
> + if (!mc->smp_props.books_supported && config->has_books) {
> + if (config->books > 1) {
> + error_setg(errp, "books not supported by this "
> + "machine's CPU topology");
> + return;
> + } else {
> + /* Here books only equals 1 since we've checked zero case. */
> + warn_report("Deprecated CPU topology (considered invalid): "
> + "Unsupported books parameter mustn't be "
> + "specified as 1");
> + }
> }
> books = books > 0 ? books : 1;
>
> - if (!mc->smp_props.drawers_supported && drawers > 1) {
> - error_setg(errp,
> - "drawers not supported by this machine's CPU topology");
> - return;
> + if (!mc->smp_props.drawers_supported && config->has_drawers) {
> + if (config->drawers > 1) {
> + error_setg(errp, "drawers not supported by this "
> + "machine's CPU topology");
> + return;
> + } else {
> + /* Here drawers only equals 1 since we've checked zero case. */
> + warn_report("Deprecated CPU topology (considered invalid): "
> + "Unsupported drawers parameter mustn't be "
> + "specified as 1");
> + }
> }
> drawers = drawers > 0 ? drawers : 1;
Apart from the typos, patch looks fine. I recommend to run "checkpath.pl"
with the --codespell parameter, that helps to avoid those.
Thomas
On Thu, Mar 07, 2024 at 07:22:10AM +0100, Thomas Huth wrote:
> Date: Thu, 7 Mar 2024 07:22:10 +0100
> From: Thomas Huth <thuth@redhat.com>
> Subject: Re: [PATCH 02/14] hw/core/machine-smp: Deprecate unsupported
> "parameter=1" SMP configurations
>
> On 06/03/2024 10.53, Zhao Liu wrote:
> > From: Zhao Liu <zhao1.liu@intel.com>
> >
> > Currentlt, it was allowed for users to specify the unsupported
>
> s/Currentlt/Currently/
>
> > topology parameter as "1". For example, x86 PC machine doesn't
> > support drawer/book/cluster topology levels, but user could specify
> > "-smp drawers=1,books=1,clusters=1".
> >
> > This is meaningless and confusing, so that the support for this kind of
> > configurations is marked depresated since 9.0. And report warning
>
> s/depresated/deprecated/
>
> > message for such case like:
> >
> > qemu-system-x86_64: warning: Deprecated CPU topology (considered invalid):
> > Unsupported clusters parameter mustn't be specified as 1
> > qemu-system-x86_64: warning: Deprecated CPU topology (considered invalid):
> > Unsupported books parameter mustn't be specified as 1
> > qemu-system-x86_64: warning: Deprecated CPU topology (considered invalid):
> > Unsupported drawers parameter mustn't be specified as 1
> >
> > Users have to ensure that all the topology members described with -smp
> > are supported by the target machine.
> >
> > Cc: devel@lists.libvirt.org
> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> > ---
> > docs/about/deprecated.rst | 14 +++++++++
> > hw/core/machine-smp.c | 63 +++++++++++++++++++++++++++++----------
> > 2 files changed, 61 insertions(+), 16 deletions(-)
> >
> > diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> > index 872974640252..2e782e83e952 100644
> > --- a/docs/about/deprecated.rst
> > +++ b/docs/about/deprecated.rst
> > @@ -47,6 +47,20 @@ as short-form boolean values, and passed to plugins as ``arg_name=on``.
> > However, short-form booleans are deprecated and full explicit ``arg_name=on``
> > form is preferred.
> > +``-smp`` (Unsopported "parameter=1" SMP configurations) (since 9.0)
>
> s/Unsopported/Unsupported/
>
> > +'''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''
> > +
> > +Specified CPU topology parameters must be supported by the machine.
> > +
> > +In the SMP configuration, users should provide the CPU topology parameters that
> > +are supported by the target machine.
> > +
> > +However, historically it was allowed for users to specify the unsupported
> > +topology parameter as "1", which is meaningless. So support for this kind of
> > +configurations (e.g. -smp drawers=1,books=1,clusters=1 for x86 PC machine) is
> > +marked depresated since 9.0, users have to ensure that all the topology members
>
> s/depresated/deprecated/
>
> > +described with -smp are supported by the target machine.
> > +
> > QEMU Machine Protocol (QMP) commands
> > ------------------------------------
> > diff --git a/hw/core/machine-smp.c b/hw/core/machine-smp.c
> > index 96533886b14e..50a5a40dbc3d 100644
> > --- a/hw/core/machine-smp.c
> > +++ b/hw/core/machine-smp.c
> > @@ -112,30 +112,61 @@ void machine_parse_smp_config(MachineState *ms,
> > /*
> > * If not supported by the machine, a topology parameter must be
> > - * omitted or specified equal to 1.
> > + * omitted.
> > */
> > - if (!mc->smp_props.dies_supported && dies > 1) {
> > - error_setg(errp, "dies not supported by this machine's CPU topology");
> > - return;
> > - }
> > - if (!mc->smp_props.clusters_supported && clusters > 1) {
> > - error_setg(errp, "clusters not supported by this machine's CPU topology");
> > - return;
> > + if (!mc->smp_props.clusters_supported && config->has_clusters) {
> > + if (config->clusters > 1) {
> > + error_setg(errp, "clusters not supported by this "
> > + "machine's CPU topology");
> > + return;
> > + } else {
> > + /* Here clusters only equals 1 since we've checked zero case. */
> > + warn_report("Deprecated CPU topology (considered invalid): "
> > + "Unsupported clusters parameter mustn't be "
> > + "specified as 1");
> > + }
> > }
> > + clusters = clusters > 0 ? clusters : 1;
> > + if (!mc->smp_props.dies_supported && config->has_dies) {
> > + if (config->dies > 1) {
> > + error_setg(errp, "dies not supported by this "
> > + "machine's CPU topology");
> > + return;
> > + } else {
> > + /* Here dies only equals 1 since we've checked zero case. */
> > + warn_report("Deprecated CPU topology (considered invalid): "
> > + "Unsupported dies parameter mustn't be "
> > + "specified as 1");
> > + }
> > + }
> > dies = dies > 0 ? dies : 1;
> > - clusters = clusters > 0 ? clusters : 1;
> > - if (!mc->smp_props.books_supported && books > 1) {
> > - error_setg(errp, "books not supported by this machine's CPU topology");
> > - return;
> > + if (!mc->smp_props.books_supported && config->has_books) {
> > + if (config->books > 1) {
> > + error_setg(errp, "books not supported by this "
> > + "machine's CPU topology");
> > + return;
> > + } else {
> > + /* Here books only equals 1 since we've checked zero case. */
> > + warn_report("Deprecated CPU topology (considered invalid): "
> > + "Unsupported books parameter mustn't be "
> > + "specified as 1");
> > + }
> > }
> > books = books > 0 ? books : 1;
> > - if (!mc->smp_props.drawers_supported && drawers > 1) {
> > - error_setg(errp,
> > - "drawers not supported by this machine's CPU topology");
> > - return;
> > + if (!mc->smp_props.drawers_supported && config->has_drawers) {
> > + if (config->drawers > 1) {
> > + error_setg(errp, "drawers not supported by this "
> > + "machine's CPU topology");
> > + return;
> > + } else {
> > + /* Here drawers only equals 1 since we've checked zero case. */
> > + warn_report("Deprecated CPU topology (considered invalid): "
> > + "Unsupported drawers parameter mustn't be "
> > + "specified as 1");
> > + }
> > }
> > drawers = drawers > 0 ? drawers : 1;
>
> Apart from the typos, patch looks fine. I recommend to run "checkpath.pl"
> with the --codespell parameter, that helps to avoid those.
>
Oops...I didn't realize there were so many typos.
Maybe I'm relying too much on --codespell ;-), and these typos aren't in
the default dictionary used by --codespell so I didn't check them
before.
I'll refresh a new version (at Friday) for these typos.
Thanks,
Zhao
© 2016 - 2024 Red Hat, Inc.