From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42291) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZUZYV-00076A-JB for qemu-devel@nongnu.org; Wed, 26 Aug 2015 08:11:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZUZYR-0003Zv-Ir for qemu-devel@nongnu.org; Wed, 26 Aug 2015 08:11:51 -0400 Received: from mail.uni-paderborn.de ([131.234.142.9]:57029) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZUZYR-0003ZV-BT for qemu-devel@nongnu.org; Wed, 26 Aug 2015 08:11:47 -0400 Message-ID: <55DDACD9.8000404@mail.uni-paderborn.de> Date: Wed, 26 Aug 2015 14:11:05 +0200 From: Bastian Koppelmann MIME-Version: 1.0 References: <1437573590-2801-1-git-send-email-thuth@redhat.com> <20150819155813.GA32203@thinpad.lan.raisama.net> <55DC6CAC.20107@redhat.com> In-Reply-To: <55DC6CAC.20107@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] Add another sanity check to smp_parse() function List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Thomas Huth , Eduardo Habkost Cc: Peter Maydell , James Hogan , Jia Liu , Max Filippov , Anthony Green , Stefano Stabellini , Mark Cave-Ayland , Alexander Graf , qemu-devel@nongnu.org, Blue Swirl , Christian Borntraeger , Michael Walle , "Edgar E. Iglesias" , Cornelia Huck , Paolo Bonzini , Guan Xuetao , Aurelien Jarno , Richard Henderson Am 25.08.2015 um 15:25 schrieb Thomas Huth: > On 19/08/15 17:58, Eduardo Habkost wrote: >> On Wed, Jul 22, 2015 at 03:59:50PM +0200, Thomas Huth wrote: >>> The code in smp_parse already checks the topology information for >>> sockets * cores * threads < cpus and bails out with an error in >>> that case. However, it is still possible to supply a bad configuration >>> the other way round, e.g. with: >>> >>> qemu-system-xxx -smp 4,sockets=1,cores=4,threads=2 >>> >>> QEMU then still starts the guest, with topology configuration that >>> is rather incomprehensible and likely not what the user wanted. >>> So let's add another check to refuse such wrong configurations. >>> >>> Signed-off-by: Thomas Huth >>> --- >>> vl.c | 8 +++++++- >>> 1 file changed, 7 insertions(+), 1 deletion(-) >>> >>> diff --git a/vl.c b/vl.c >>> index 5856396..c8d24b1 100644 >>> --- a/vl.c >>> +++ b/vl.c >>> @@ -1224,7 +1224,13 @@ static void smp_parse(QemuOpts *opts) >>> exit(1); >>> } >>> >>> - max_cpus = qemu_opt_get_number(opts, "maxcpus", 0); >>> + max_cpus = qemu_opt_get_number(opts, "maxcpus", cpus); >>> + if (sockets * cores * threads > max_cpus) { >>> + fprintf(stderr, "cpu topology: error: " >>> + "sockets (%u) * cores (%u) * threads (%u) > maxcpus (%u)\n", >>> + sockets, cores, threads, max_cpus); >>> + exit(1); >>> + } >> I am always afraid of breaking existing setups when we do that, because >> there may be existing VMs running with these weird configurations, and >> people won't be able to live-migrate them to a newer QEMU. >> >> But I think we really have to start refusing to run obviously broken >> configurations one day, or we will never fix this mess, so: >> >> Reviewed-by: Eduardo Habkost >> >> I want to apply this through the x86 tree, but I would like to get some >> Acked-by from other maintainers first. > Ok, thanks! > > So *ping* to the other CPU core maintainers here ... could I get some > more ACKs, please? > > Thomas > > Acked-by: Bastian Koppelmann