1. Patchew
  2. Libvirt
  3. View series

[libvirt] [PATCH] vl.c deprecate incorrect CPUs topology

Igor Mammedov posted 1 patch 5 years, 7 months ago
Failed in applying to current master (apply log)
There is a newer version of this series
qemu-deprecated.texi | 11 +++++++++++
vl.c                 |  6 ++++++
2 files changed, 17 insertions(+)
[libvirt] [PATCH] vl.c deprecate incorrect CPUs topology
Posted by Igor Mammedov 5 years, 7 months ago 
-smp [cpus],sockets/cores/threads[,maxcpus] should describe topology
so that total number of logical CPUs [sockets * cores * threads]
would be equal to [maxcpus], however historically we didn't have
such check in QEMU and it is possible to start VM with an invalid
topology.
Deprecate invalid options combination so we can make sure that
the topology VM started with is always correct in the future.
Users with an invalid sockets/cores/threads/maxcpus values should
fix their CLI to make sure that
   [sockets * cores * threads] == [maxcpus]

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 qemu-deprecated.texi | 11 +++++++++++
 vl.c                 |  6 ++++++
 2 files changed, 17 insertions(+)

diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
index 87212b6..d5d9ce6 100644
--- a/qemu-deprecated.texi
+++ b/qemu-deprecated.texi
@@ -159,6 +159,17 @@ 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.
 
+@subsection -smp X,[socket=a,core=b,thread=c],maxcpus=Y (since 3.1)
+
+CPU topology properties should describe whole machine topology including
+possible CPUs. but historically it was possible to start QEMU with
+an incorrect topology where
+  socket * core * thread >= X && < maxcpus
+which could lead to incorrect topology enumeration by the guest.
+Support for invalid topology will be removed, end user must ensure
+that topology described with -smp includes all possible cpus, i.e.:
+  socket * core * thread == maxcpus
+
 @section QEMU Machine Protocol (QMP) commands
 
 @subsection block-dirty-bitmap-add "autoload" parameter (since 2.12.0)
diff --git a/vl.c b/vl.c
index 5ba06ad..bc53828 100644
--- a/vl.c
+++ b/vl.c
@@ -1238,6 +1238,12 @@ static void smp_parse(QemuOpts *opts)
             exit(1);
         }
 
+        if (sockets * cores * threads != max_cpus) {
+            warn_report("Invalid CPUs topology deprecated. "
+                        "sockets (%u) * cores (%u) * threads (%u) "
+                        "!= maxcpus (%u)",
+                        sockets, cores, threads, max_cpus);
+        }
         if (sockets * cores * threads > max_cpus) {
             error_report("cpu topology: "
                          "sockets (%u) * cores (%u) * threads (%u) > "
-- 
2.7.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] vl.c deprecate incorrect CPUs topology
Posted by Andrew Jones 5 years, 7 months ago 
On Mon, Aug 27, 2018 at 01:56:08PM +0200, Igor Mammedov wrote:
> -smp [cpus],sockets/cores/threads[,maxcpus] should describe topology
> so that total number of logical CPUs [sockets * cores * threads]
> would be equal to [maxcpus], however historically we didn't have
> such check in QEMU and it is possible to start VM with an invalid
> topology.
> Deprecate invalid options combination so we can make sure that
> the topology VM started with is always correct in the future.
> Users with an invalid sockets/cores/threads/maxcpus values should
> fix their CLI to make sure that
>    [sockets * cores * threads] == [maxcpus]
> 
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  qemu-deprecated.texi | 11 +++++++++++
>  vl.c                 |  6 ++++++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
> index 87212b6..d5d9ce6 100644
> --- a/qemu-deprecated.texi
> +++ b/qemu-deprecated.texi
> @@ -159,6 +159,17 @@ 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.
>  
> +@subsection -smp X,[socket=a,core=b,thread=c],maxcpus=Y (since 3.1)

sockets, cores, threads

> +
> +CPU topology properties should describe whole machine topology including
> +possible CPUs. but historically it was possible to start QEMU with

/./,/

> +an incorrect topology where
> +  socket * core * thread >= X && < maxcpus

sockets * cores * threads

&& X < maxcpus

> +which could lead to incorrect topology enumeration by the guest.
> +Support for invalid topology will be removed, end user must ensure
topologies
/end user/the user/

> +that topology described with -smp includes all possible cpus, i.e.:
/that/the

> +  socket * core * thread == maxcpus

sockets * cores * threads

> +
>  @section QEMU Machine Protocol (QMP) commands
>  
>  @subsection block-dirty-bitmap-add "autoload" parameter (since 2.12.0)
> diff --git a/vl.c b/vl.c
> index 5ba06ad..bc53828 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1238,6 +1238,12 @@ static void smp_parse(QemuOpts *opts)
>              exit(1);
>          }
>  
> +        if (sockets * cores * threads != max_cpus) {
> +            warn_report("Invalid CPUs topology deprecated. "

/CPUs/CPU/

Not sure we want a period after deprecated. Would ':' be more appropriate?

> +                        "sockets (%u) * cores (%u) * threads (%u) "
> +                        "!= maxcpus (%u)",
> +                        sockets, cores, threads, max_cpus);
> +        }
>          if (sockets * cores * threads > max_cpus) {
>              error_report("cpu topology: "
>                           "sockets (%u) * cores (%u) * threads (%u) > "
> -- 
> 2.7.4
>

Thanks,
drew 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH] vl.c deprecate incorrect CPUs topology
Posted by Igor Mammedov 5 years, 7 months ago 
On Mon, 27 Aug 2018 14:13:43 +0200
Andrew Jones <drjones@redhat.com> wrote:

> On Mon, Aug 27, 2018 at 01:56:08PM +0200, Igor Mammedov wrote:
> > -smp [cpus],sockets/cores/threads[,maxcpus] should describe topology
> > so that total number of logical CPUs [sockets * cores * threads]
> > would be equal to [maxcpus], however historically we didn't have
> > such check in QEMU and it is possible to start VM with an invalid
> > topology.
> > Deprecate invalid options combination so we can make sure that
> > the topology VM started with is always correct in the future.
> > Users with an invalid sockets/cores/threads/maxcpus values should
> > fix their CLI to make sure that
> >    [sockets * cores * threads] == [maxcpus]
> > 
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> >  qemu-deprecated.texi | 11 +++++++++++
> >  vl.c                 |  6 ++++++
> >  2 files changed, 17 insertions(+)
> > 
> > diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
> > index 87212b6..d5d9ce6 100644
> > --- a/qemu-deprecated.texi
> > +++ b/qemu-deprecated.texi
> > @@ -159,6 +159,17 @@ 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.
> >  
> > +@subsection -smp X,[socket=a,core=b,thread=c],maxcpus=Y (since 3.1)  
> 
> sockets, cores, threads
> 
> > +
> > +CPU topology properties should describe whole machine topology including
> > +possible CPUs. but historically it was possible to start QEMU with  
> 
> /./,/
> 
> > +an incorrect topology where
> > +  socket * core * thread >= X && < maxcpus  
> 
> sockets * cores * threads
> 
> && X < maxcpus
> 
> > +which could lead to incorrect topology enumeration by the guest.
> > +Support for invalid topology will be removed, end user must ensure  
> topologies
> /end user/the user/
> 
> > +that topology described with -smp includes all possible cpus, i.e.:  
> /that/the
> 
> > +  socket * core * thread == maxcpus  
> 
> sockets * cores * threads
> 
> > +
> >  @section QEMU Machine Protocol (QMP) commands
> >  
> >  @subsection block-dirty-bitmap-add "autoload" parameter (since 2.12.0)
> > diff --git a/vl.c b/vl.c
> > index 5ba06ad..bc53828 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -1238,6 +1238,12 @@ static void smp_parse(QemuOpts *opts)
> >              exit(1);
> >          }
> >  
> > +        if (sockets * cores * threads != max_cpus) {
> > +            warn_report("Invalid CPUs topology deprecated. "  
> 
> /CPUs/CPU/
> 
> Not sure we want a period after deprecated. Would ':' be more appropriate?
> 
> > +                        "sockets (%u) * cores (%u) * threads (%u) "
> > +                        "!= maxcpus (%u)",
> > +                        sockets, cores, threads, max_cpus);
> > +        }
> >          if (sockets * cores * threads > max_cpus) {
> >              error_report("cpu topology: "
> >                           "sockets (%u) * cores (%u) * threads (%u) > "
> > -- 
> > 2.7.4
> >  
> 
> Thanks,
> drew 
> 

Thanks for corrections, v2 is on the way

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.