Re: [libvirt] [Qemu-devel] [PATCH] vl.c deprecate incorrect CPUs topology

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(a)redhat.com&gt; > --- > 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