Re: [Qemu-devel] [PATCH] Add another sanity check to smp_parse() function

[Thomas Huth <thuth@redhat.com>]

On 23/07/15 14:07, Igor Mammedov wrote:
> On Wed, 22 Jul 2015 15:59:50 +0200
> Thomas Huth <thuth@redhat.com> 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",
> just a nit, maybe s/maxcpus/[max]cpus/

I think it should be pretty obvious for the user that maxcpus = cpus
when the "maxcpus" option has not been specified but only the "cpus"
option. So I'd prefer to keep it the way without the square brackets,
it's IMHO easier to read.

 Thomas