From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54154) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZUEE2-0008E8-Db for qemu-devel@nongnu.org; Tue, 25 Aug 2015 09:25:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZUEDz-0002up-4X for qemu-devel@nongnu.org; Tue, 25 Aug 2015 09:25:18 -0400 Received: from mx1.redhat.com ([209.132.183.28]:51366) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZUEDy-0002uV-WF for qemu-devel@nongnu.org; Tue, 25 Aug 2015 09:25:15 -0400 Message-ID: <55DC6CAC.20107@redhat.com> Date: Tue, 25 Aug 2015 15:25:00 +0200 From: Thomas Huth MIME-Version: 1.0 References: <1437573590-2801-1-git-send-email-thuth@redhat.com> <20150819155813.GA32203@thinpad.lan.raisama.net> In-Reply-To: <20150819155813.GA32203@thinpad.lan.raisama.net> Content-Type: text/plain; charset=utf-8 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: Eduardo Habkost Cc: Peter Maydell , James Hogan , Jia Liu , Max Filippov , Bastian Koppelmann , 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 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