[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] vl.c deprecate incorrect CPUs topology
From: |
Eduardo Habkost |
Subject: |
Re: [Qemu-devel] [PATCH v2] vl.c deprecate incorrect CPUs topology |
Date: |
Mon, 27 Aug 2018 23:52:36 -0300 |
User-agent: |
Mutt/1.9.2 (2017-12-15) |
On Mon, Aug 27, 2018 at 03:53:26PM +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 <address@hidden>
> ---
> v2:
> - spelling&&co fixes (Andrew Jones <address@hidden>)
> ---
> qemu-deprecated.texi | 11 +++++++++++
> vl.c | 6 ++++++
> 2 files changed, 17 insertions(+)
>
> diff --git a/qemu-deprecated.texi b/qemu-deprecated.texi
> index 87212b6..b649b2e 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.
>
> address@hidden -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
> + sockets * cores * threads >= X && X < maxcpus
> +which could lead to incorrect topology enumeration by the guest.
> +Support for invalid topology will be removed, the user must ensure
> +topologies described with -smp includes all possible cpus, i.e.:
> + sockets * cores * threads == 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..238f856 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 CPU topology deprecated: "
> + "sockets (%u) * cores (%u) * threads (%u) "
> + "!= maxcpus (%u)",
> + sockets, cores, threads, max_cpus);
> + }
We don't need to print the warning if
sockets * cores * threads > max_cpus,
because it already triggers error_report()/exit() below. I
suggest reordering the checks:
if (sockets * cores * threads > max_cpus) {
error_report(...);
exit(1);
}
if (sockets * cores * threads != max_cpus) {
warn_report(...)
}
> if (sockets * cores * threads > max_cpus) {
> error_report("cpu topology: "
> "sockets (%u) * cores (%u) * threads (%u) > "
> --
> 2.7.4
>
--
Eduardo