From: Thomas Huth <thuth@redhat.com>
To: Eduardo Habkost <ehabkost@redhat.com>
Cc: Peter Maydell <peter.maydell@linaro.org>,
James Hogan <james.hogan@imgtec.com>, Jia Liu <proljc@gmail.com>,
Max Filippov <jcmvbkbc@gmail.com>,
Bastian Koppelmann <kbastian@mail.uni-paderborn.de>,
Anthony Green <green@moxielogic.com>,
Stefano Stabellini <stefano.stabellini@eu.citrix.com>,
Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>,
Alexander Graf <agraf@suse.de>,
qemu-devel@nongnu.org, Blue Swirl <blauwirbel@gmail.com>,
Christian Borntraeger <borntraeger@de.ibm.com>,
Michael Walle <michael@walle.cc>,
"Edgar E. Iglesias" <edgar.iglesias@gmail.com>,
Cornelia Huck <cornelia.huck@de.ibm.com>,
Paolo Bonzini <pbonzini@redhat.com>,
Guan Xuetao <gxt@mprc.pku.edu.cn>,
Aurelien Jarno <aurelien@aurel32.net>,
Richard Henderson <rth@twiddle.net>
Subject: Re: [Qemu-devel] [PATCH] Add another sanity check to smp_parse() function
Date: Tue, 25 Aug 2015 15:25:00 +0200 [thread overview]
Message-ID: <55DC6CAC.20107@redhat.com> (raw)
In-Reply-To: <20150819155813.GA32203@thinpad.lan.raisama.net>
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 <thuth@redhat.com>
>> ---
>> 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 <ehabkost@redhat.com>
>
> 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
next prev parent reply other threads:[~2015-08-25 13:25 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-22 13:59 [Qemu-devel] [PATCH] Add another sanity check to smp_parse() function Thomas Huth
2015-07-23 12:07 ` Igor Mammedov
2015-07-24 11:53 ` Thomas Huth
2015-08-18 23:39 ` Thomas Huth
2015-08-19 15:58 ` Eduardo Habkost
2015-08-25 13:25 ` Thomas Huth [this message]
2015-08-26 11:36 ` Cornelia Huck
2015-08-26 12:11 ` Bastian Koppelmann
2015-08-26 16:02 ` Eduardo Habkost
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=55DC6CAC.20107@redhat.com \
--to=thuth@redhat.com \
--cc=agraf@suse.de \
--cc=aurelien@aurel32.net \
--cc=blauwirbel@gmail.com \
--cc=borntraeger@de.ibm.com \
--cc=cornelia.huck@de.ibm.com \
--cc=edgar.iglesias@gmail.com \
--cc=ehabkost@redhat.com \
--cc=green@moxielogic.com \
--cc=gxt@mprc.pku.edu.cn \
--cc=james.hogan@imgtec.com \
--cc=jcmvbkbc@gmail.com \
--cc=kbastian@mail.uni-paderborn.de \
--cc=mark.cave-ayland@ilande.co.uk \
--cc=michael@walle.cc \
--cc=pbonzini@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=proljc@gmail.com \
--cc=qemu-devel@nongnu.org \
--cc=rth@twiddle.net \
--cc=stefano.stabellini@eu.citrix.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).