[PATCH] smp: drop support for deprecated (invalid topologies)

Igor Mammedov posted 1 patch 1 week ago
Failed in applying to current master (apply log)
docs/system/deprecated.rst | 26 +++++++++++++-------------
hw/core/machine.c          | 16 ++++------------
hw/i386/pc.c               | 16 ++++------------
3 files changed, 21 insertions(+), 37 deletions(-)

[PATCH] smp: drop support for deprecated (invalid topologies)

Posted by Igor Mammedov 1 week ago
it's was deprecated since 3.1

Support for invalid topologies is removed, the user must ensure
that topologies described with -smp include all possible cpus,
i.e. (sockets * cores * threads) == maxcpus or QEMU will
exit with error.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 docs/system/deprecated.rst | 26 +++++++++++++-------------
 hw/core/machine.c          | 16 ++++------------
 hw/i386/pc.c               | 16 ++++------------
 3 files changed, 21 insertions(+), 37 deletions(-)

diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
index 122717cfee..d737728fab 100644
--- a/docs/system/deprecated.rst
+++ b/docs/system/deprecated.rst
@@ -47,19 +47,6 @@ The 'file' driver for drives is no longer appropriate for character or host
 devices and will only accept regular files (S_IFREG). The correct driver
 for these file types is 'host_cdrom' or 'host_device' as appropriate.
 
-``-smp`` (invalid topologies) (since 3.1)
-'''''''''''''''''''''''''''''''''''''''''
-
-CPU topology properties should describe whole machine topology including
-possible CPUs.
-
-However, historically it was possible to start QEMU with an incorrect topology
-where *n* <= *sockets* * *cores* * *threads* < *maxcpus*,
-which could lead to an incorrect topology enumeration by the guest.
-Support for invalid topologies will be removed, the user must ensure
-topologies described with -smp include all possible cpus, i.e.
-*sockets* * *cores* * *threads* = *maxcpus*.
-
 ``-vnc acl`` (since 4.0.0)
 ''''''''''''''''''''''''''
 
@@ -618,6 +605,19 @@ New machine versions (since 5.1) will not accept the option but it will still
 work with old machine types. User can check the QAPI schema to see if the legacy
 option is supported by looking at MachineInfo::numa-mem-supported property.
 
+``-smp`` (invalid topologies) (removed 5.2)
+'''''''''''''''''''''''''''''''''''''''''''
+
+CPU topology properties should describe whole machine topology including
+possible CPUs.
+
+However, historically it was possible to start QEMU with an incorrect topology
+where *n* <= *sockets* * *cores* * *threads* < *maxcpus*,
+which could lead to an incorrect topology enumeration by the guest.
+Support for invalid topologies is removed, the user must ensure
+topologies described with -smp include all possible cpus, i.e.
+*sockets* * *cores* * *threads* = *maxcpus*.
+
 Block devices
 -------------
 
diff --git a/hw/core/machine.c b/hw/core/machine.c
index ea26d61237..09aee4ea52 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -754,23 +754,15 @@ static void smp_parse(MachineState *ms, QemuOpts *opts)
             exit(1);
         }
 
-        if (sockets * cores * threads > ms->smp.max_cpus) {
-            error_report("cpu topology: "
-                         "sockets (%u) * cores (%u) * threads (%u) > "
-                         "maxcpus (%u)",
+        if (sockets * cores * threads != ms->smp.max_cpus) {
+            error_report("Invalid CPU topology: "
+                         "sockets (%u) * cores (%u) * threads (%u) "
+                         "!= maxcpus (%u)",
                          sockets, cores, threads,
                          ms->smp.max_cpus);
             exit(1);
         }
 
-        if (sockets * cores * threads != ms->smp.max_cpus) {
-            warn_report("Invalid CPU topology deprecated: "
-                        "sockets (%u) * cores (%u) * threads (%u) "
-                        "!= maxcpus (%u)",
-                        sockets, cores, threads,
-                        ms->smp.max_cpus);
-        }
-
         ms->smp.cpus = cpus;
         ms->smp.cores = cores;
         ms->smp.threads = threads;
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index d071da787b..fbde6b04e6 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -746,23 +746,15 @@ void pc_smp_parse(MachineState *ms, QemuOpts *opts)
             exit(1);
         }
 
-        if (sockets * dies * cores * threads > ms->smp.max_cpus) {
-            error_report("cpu topology: "
-                         "sockets (%u) * dies (%u) * cores (%u) * threads (%u) > "
-                         "maxcpus (%u)",
+        if (sockets * dies * cores * threads != ms->smp.max_cpus) {
+            error_report("Invalid CPU topology deprecated: "
+                         "sockets (%u) * dies (%u) * cores (%u) * threads (%u) "
+                         "!= maxcpus (%u)",
                          sockets, dies, cores, threads,
                          ms->smp.max_cpus);
             exit(1);
         }
 
-        if (sockets * dies * cores * threads != ms->smp.max_cpus) {
-            warn_report("Invalid CPU topology deprecated: "
-                        "sockets (%u) * dies (%u) * cores (%u) * threads (%u) "
-                        "!= maxcpus (%u)",
-                        sockets, dies, cores, threads,
-                        ms->smp.max_cpus);
-        }
-
         ms->smp.cpus = cpus;
         ms->smp.cores = cores;
         ms->smp.threads = threads;
-- 
2.27.0

Re: [PATCH] smp: drop support for deprecated (invalid topologies)

Posted by Michael S. Tsirkin 1 week ago
On Fri, Sep 11, 2020 at 09:32:02AM -0400, Igor Mammedov wrote:
> it's was deprecated since 3.1
> 
> Support for invalid topologies is removed, the user must ensure
> that topologies described with -smp include all possible cpus,
> i.e. (sockets * cores * threads) == maxcpus or QEMU will
> exit with error.
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>

Acked-by: 

memory tree I guess?

> ---
>  docs/system/deprecated.rst | 26 +++++++++++++-------------
>  hw/core/machine.c          | 16 ++++------------
>  hw/i386/pc.c               | 16 ++++------------
>  3 files changed, 21 insertions(+), 37 deletions(-)
> 
> diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
> index 122717cfee..d737728fab 100644
> --- a/docs/system/deprecated.rst
> +++ b/docs/system/deprecated.rst
> @@ -47,19 +47,6 @@ The 'file' driver for drives is no longer appropriate for character or host
>  devices and will only accept regular files (S_IFREG). The correct driver
>  for these file types is 'host_cdrom' or 'host_device' as appropriate.
>  
> -``-smp`` (invalid topologies) (since 3.1)
> -'''''''''''''''''''''''''''''''''''''''''
> -
> -CPU topology properties should describe whole machine topology including
> -possible CPUs.
> -
> -However, historically it was possible to start QEMU with an incorrect topology
> -where *n* <= *sockets* * *cores* * *threads* < *maxcpus*,
> -which could lead to an incorrect topology enumeration by the guest.
> -Support for invalid topologies will be removed, the user must ensure
> -topologies described with -smp include all possible cpus, i.e.
> -*sockets* * *cores* * *threads* = *maxcpus*.
> -
>  ``-vnc acl`` (since 4.0.0)
>  ''''''''''''''''''''''''''
>  
> @@ -618,6 +605,19 @@ New machine versions (since 5.1) will not accept the option but it will still
>  work with old machine types. User can check the QAPI schema to see if the legacy
>  option is supported by looking at MachineInfo::numa-mem-supported property.
>  
> +``-smp`` (invalid topologies) (removed 5.2)
> +'''''''''''''''''''''''''''''''''''''''''''
> +
> +CPU topology properties should describe whole machine topology including
> +possible CPUs.
> +
> +However, historically it was possible to start QEMU with an incorrect topology
> +where *n* <= *sockets* * *cores* * *threads* < *maxcpus*,
> +which could lead to an incorrect topology enumeration by the guest.
> +Support for invalid topologies is removed, the user must ensure
> +topologies described with -smp include all possible cpus, i.e.
> +*sockets* * *cores* * *threads* = *maxcpus*.
> +
>  Block devices
>  -------------
>  
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index ea26d61237..09aee4ea52 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -754,23 +754,15 @@ static void smp_parse(MachineState *ms, QemuOpts *opts)
>              exit(1);
>          }
>  
> -        if (sockets * cores * threads > ms->smp.max_cpus) {
> -            error_report("cpu topology: "
> -                         "sockets (%u) * cores (%u) * threads (%u) > "
> -                         "maxcpus (%u)",
> +        if (sockets * cores * threads != ms->smp.max_cpus) {
> +            error_report("Invalid CPU topology: "
> +                         "sockets (%u) * cores (%u) * threads (%u) "
> +                         "!= maxcpus (%u)",
>                           sockets, cores, threads,
>                           ms->smp.max_cpus);
>              exit(1);
>          }
>  
> -        if (sockets * cores * threads != ms->smp.max_cpus) {
> -            warn_report("Invalid CPU topology deprecated: "
> -                        "sockets (%u) * cores (%u) * threads (%u) "
> -                        "!= maxcpus (%u)",
> -                        sockets, cores, threads,
> -                        ms->smp.max_cpus);
> -        }
> -
>          ms->smp.cpus = cpus;
>          ms->smp.cores = cores;
>          ms->smp.threads = threads;
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index d071da787b..fbde6b04e6 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -746,23 +746,15 @@ void pc_smp_parse(MachineState *ms, QemuOpts *opts)
>              exit(1);
>          }
>  
> -        if (sockets * dies * cores * threads > ms->smp.max_cpus) {
> -            error_report("cpu topology: "
> -                         "sockets (%u) * dies (%u) * cores (%u) * threads (%u) > "
> -                         "maxcpus (%u)",
> +        if (sockets * dies * cores * threads != ms->smp.max_cpus) {
> +            error_report("Invalid CPU topology deprecated: "
> +                         "sockets (%u) * dies (%u) * cores (%u) * threads (%u) "
> +                         "!= maxcpus (%u)",
>                           sockets, dies, cores, threads,
>                           ms->smp.max_cpus);
>              exit(1);
>          }
>  
> -        if (sockets * dies * cores * threads != ms->smp.max_cpus) {
> -            warn_report("Invalid CPU topology deprecated: "
> -                        "sockets (%u) * dies (%u) * cores (%u) * threads (%u) "
> -                        "!= maxcpus (%u)",
> -                        sockets, dies, cores, threads,
> -                        ms->smp.max_cpus);
> -        }
> -
>          ms->smp.cpus = cpus;
>          ms->smp.cores = cores;
>          ms->smp.threads = threads;
> -- 
> 2.27.0


Re: [PATCH] smp: drop support for deprecated (invalid topologies)

Posted by Igor Mammedov 1 week ago
On Fri, 11 Sep 2020 11:04:47 -0400
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Fri, Sep 11, 2020 at 09:32:02AM -0400, Igor Mammedov wrote:
> > it's was deprecated since 3.1
> > 
> > Support for invalid topologies is removed, the user must ensure
> > that topologies described with -smp include all possible cpus,
> > i.e. (sockets * cores * threads) == maxcpus or QEMU will
> > exit with error.
> > 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>  
> 
> Acked-by: 
> 
> memory tree I guess?

It would be better for Paolo to take it since he has
queued numa deprecations, due to context confilict in
deprecated.rst.

Paolo,
can you queue this patch as well?

> 
> > ---
> >  docs/system/deprecated.rst | 26 +++++++++++++-------------
> >  hw/core/machine.c          | 16 ++++------------
> >  hw/i386/pc.c               | 16 ++++------------
> >  3 files changed, 21 insertions(+), 37 deletions(-)
> > 
> > diff --git a/docs/system/deprecated.rst b/docs/system/deprecated.rst
> > index 122717cfee..d737728fab 100644
> > --- a/docs/system/deprecated.rst
> > +++ b/docs/system/deprecated.rst
> > @@ -47,19 +47,6 @@ The 'file' driver for drives is no longer appropriate for character or host
> >  devices and will only accept regular files (S_IFREG). The correct driver
> >  for these file types is 'host_cdrom' or 'host_device' as appropriate.
> >  
> > -``-smp`` (invalid topologies) (since 3.1)
> > -'''''''''''''''''''''''''''''''''''''''''
> > -
> > -CPU topology properties should describe whole machine topology including
> > -possible CPUs.
> > -
> > -However, historically it was possible to start QEMU with an incorrect topology
> > -where *n* <= *sockets* * *cores* * *threads* < *maxcpus*,
> > -which could lead to an incorrect topology enumeration by the guest.
> > -Support for invalid topologies will be removed, the user must ensure
> > -topologies described with -smp include all possible cpus, i.e.
> > -*sockets* * *cores* * *threads* = *maxcpus*.
> > -
> >  ``-vnc acl`` (since 4.0.0)
> >  ''''''''''''''''''''''''''
> >  
> > @@ -618,6 +605,19 @@ New machine versions (since 5.1) will not accept the option but it will still
> >  work with old machine types. User can check the QAPI schema to see if the legacy
> >  option is supported by looking at MachineInfo::numa-mem-supported property.
> >  
> > +``-smp`` (invalid topologies) (removed 5.2)
> > +'''''''''''''''''''''''''''''''''''''''''''
> > +
> > +CPU topology properties should describe whole machine topology including
> > +possible CPUs.
> > +
> > +However, historically it was possible to start QEMU with an incorrect topology
> > +where *n* <= *sockets* * *cores* * *threads* < *maxcpus*,
> > +which could lead to an incorrect topology enumeration by the guest.
> > +Support for invalid topologies is removed, the user must ensure
> > +topologies described with -smp include all possible cpus, i.e.
> > +*sockets* * *cores* * *threads* = *maxcpus*.
> > +
> >  Block devices
> >  -------------
> >  
> > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > index ea26d61237..09aee4ea52 100644
> > --- a/hw/core/machine.c
> > +++ b/hw/core/machine.c
> > @@ -754,23 +754,15 @@ static void smp_parse(MachineState *ms, QemuOpts *opts)
> >              exit(1);
> >          }
> >  
> > -        if (sockets * cores * threads > ms->smp.max_cpus) {
> > -            error_report("cpu topology: "
> > -                         "sockets (%u) * cores (%u) * threads (%u) > "
> > -                         "maxcpus (%u)",
> > +        if (sockets * cores * threads != ms->smp.max_cpus) {
> > +            error_report("Invalid CPU topology: "
> > +                         "sockets (%u) * cores (%u) * threads (%u) "
> > +                         "!= maxcpus (%u)",
> >                           sockets, cores, threads,
> >                           ms->smp.max_cpus);
> >              exit(1);
> >          }
> >  
> > -        if (sockets * cores * threads != ms->smp.max_cpus) {
> > -            warn_report("Invalid CPU topology deprecated: "
> > -                        "sockets (%u) * cores (%u) * threads (%u) "
> > -                        "!= maxcpus (%u)",
> > -                        sockets, cores, threads,
> > -                        ms->smp.max_cpus);
> > -        }
> > -
> >          ms->smp.cpus = cpus;
> >          ms->smp.cores = cores;
> >          ms->smp.threads = threads;
> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> > index d071da787b..fbde6b04e6 100644
> > --- a/hw/i386/pc.c
> > +++ b/hw/i386/pc.c
> > @@ -746,23 +746,15 @@ void pc_smp_parse(MachineState *ms, QemuOpts *opts)
> >              exit(1);
> >          }
> >  
> > -        if (sockets * dies * cores * threads > ms->smp.max_cpus) {
> > -            error_report("cpu topology: "
> > -                         "sockets (%u) * dies (%u) * cores (%u) * threads (%u) > "
> > -                         "maxcpus (%u)",
> > +        if (sockets * dies * cores * threads != ms->smp.max_cpus) {
> > +            error_report("Invalid CPU topology deprecated: "
> > +                         "sockets (%u) * dies (%u) * cores (%u) * threads (%u) "
> > +                         "!= maxcpus (%u)",
> >                           sockets, dies, cores, threads,
> >                           ms->smp.max_cpus);
> >              exit(1);
> >          }
> >  
> > -        if (sockets * dies * cores * threads != ms->smp.max_cpus) {
> > -            warn_report("Invalid CPU topology deprecated: "
> > -                        "sockets (%u) * dies (%u) * cores (%u) * threads (%u) "
> > -                        "!= maxcpus (%u)",
> > -                        sockets, dies, cores, threads,
> > -                        ms->smp.max_cpus);
> > -        }
> > -
> >          ms->smp.cpus = cpus;
> >          ms->smp.cores = cores;
> >          ms->smp.threads = threads;
> > -- 
> > 2.27.0  
> 


Re: [PATCH] smp: drop support for deprecated (invalid topologies)

Posted by Eduardo Habkost 4 days ago
On Mon, Sep 14, 2020 at 09:37:20AM +0200, Igor Mammedov wrote:
> On Fri, 11 Sep 2020 11:04:47 -0400
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Fri, Sep 11, 2020 at 09:32:02AM -0400, Igor Mammedov wrote:
> > > it's was deprecated since 3.1
> > > 
> > > Support for invalid topologies is removed, the user must ensure
> > > that topologies described with -smp include all possible cpus,
> > > i.e. (sockets * cores * threads) == maxcpus or QEMU will
> > > exit with error.
> > > 
> > > Signed-off-by: Igor Mammedov <imammedo@redhat.com>  
> > 
> > Acked-by: 
> > 
> > memory tree I guess?
> 
> It would be better for Paolo to take it since he has
> queued numa deprecations, due to context confilict in
> deprecated.rst.
> 
> Paolo,
> can you queue this patch as well?

I'm queueing this in machine-next (sorry for not noticing it
before I sent today's pull request).

-- 
Eduardo

Re: [PATCH] smp: drop support for deprecated (invalid topologies)

Posted by Paolo Bonzini 2 days ago
On 18/09/20 22:57, Eduardo Habkost wrote:
> On Mon, Sep 14, 2020 at 09:37:20AM +0200, Igor Mammedov wrote:
>> On Fri, 11 Sep 2020 11:04:47 -0400
>> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>>
>>> On Fri, Sep 11, 2020 at 09:32:02AM -0400, Igor Mammedov wrote:
>>>> it's was deprecated since 3.1
>>>>
>>>> Support for invalid topologies is removed, the user must ensure
>>>> that topologies described with -smp include all possible cpus,
>>>> i.e. (sockets * cores * threads) == maxcpus or QEMU will
>>>> exit with error.
>>>>
>>>> Signed-off-by: Igor Mammedov <imammedo@redhat.com>  
>>>
>>> Acked-by: 
>>>
>>> memory tree I guess?
>>
>> It would be better for Paolo to take it since he has
>> queued numa deprecations, due to context confilict in
>> deprecated.rst.
>>
>> Paolo,
>> can you queue this patch as well?
> 
> I'm queueing this in machine-next (sorry for not noticing it
> before I sent today's pull request).

It's not part of my pending pull request, so whoever comes second will
drop the patch.

Paolo