Re: [Qemu-devel] [PATCH RFC 06/16] vl: move smp parsing to machine pre_init

[Paolo Bonzini]

On 14/06/2016 13:39, Andrew Jones wrote:
> On Tue, Jun 14, 2016 at 10:17:49AM +0200, Paolo Bonzini wrote:
>> On 13/06/2016 22:35, Andrew Jones wrote:
>>> On Mon, Jun 13, 2016 at 07:04:01PM +0200, Paolo Bonzini wrote:
>>>> On 10/06/2016 19:40, Andrew Jones wrote:
>> They should just not specify it and get a default of 1. ;)
> 
> Yeah, threads, the only one we should never calculate, could be
> optional. If not specified, defaulting to 1 makes perfect sense.
> But, threads=0, which is weird, but in a way specifying that it's
> not specified, also makes some sense.

If it's weird, let's make it invalid. :)

>>>> - cpus % (cores * threads) != 0
>>>
>>> Hmm. This makes sense where cpus is the number of cpu packages,
>>
>> I'm not sure I understand what you mean here.  The point is that the
>> machine starts with an integral number of sockets.
> 
> OK, s/cpus/maxcpus/ then. By using the currently online number, I
> thought you were starting to prepare for cpu packages, which are
> indivisible sets of cores and threads.

Yes, that's what I meant to do.  Isn't cpus what you call
"total-online-threads" below?

> But now that I think about
> it, cpus % (cores * threads) isn't right for that either. It should
> be total-online-threads % (cores * threads) != 0
> 
>> [...] you could just specify -machine cpus=16,maxcpus=32 and expect 1
>> core/socket, 1 thread/core, and 16 to 32 sockets.
> 
> Or 32 cores/socket (only 16 populated), 1 thread/core

Does that make sense?  How do you "populate" parts of a socket lazily? I
know it's all virtual, but... ;)

>> Though I think that we can at least agree on defaults for threads,
>> maxcpus and cpus.  The only sticky point is sockets vs. cores.  We can
>> make them both mandatory for now.  That is: cores and sockets are
>> mandatory until we decide which one to default to 1; threads is
>> optional; cpus is mandatory if you want hotplug, otherwise it's
>> redundant; maxcpus is optional and always redundant.
> 
> Agreed. I'll do the following for the next round of this series
> 
>  threads: 1
>  cores:   required
>  sockets: required
>  maxcpus: maxcpus ? maxcpus : sockets * cores * threads
>  cpus:    cpus ? cpus : maxcpus
> 
> If maxcpus is input, it's redundant, but should be sanity checked.
> Maybe the user wants to use the QEMU cmdline to check their math...

Yes, all this is fine.  As to the checks, here's my list:

>>> - any argument < 1
>>> - any argument > some compile-time value (1024?) to avoid overflows
>>> - cpus % (cores * threads) != 0
>>> - cpus > sockets * cores * threads
>>> - maxcpus != cores * threads * sockets

?  The second, fourth and fifth are fine, and IMO the first too.  What
about the "integral package" check?   It needs to be disabled with some
ugly global when using -smp, but apart from that I believe it's better
to have it.

Paolo