| Message ID | 20240306095407.3058909-3-zhao1.liu@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Cleanup on SMP and its test | expand |
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
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;