[PATCH 02/14] hw/core/machine-smp: Deprecate unsupported "parameter=1" SMP configurations
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.1
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. 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
participants (2)
-
Thomas Huth -
Zhao Liu