Re: [RFC PATCH v2 2/4] hw/arm/virt: Parse -smp cluster parameter in virt_smp_parse

["wangyanan (Y)" <wangyanan55@huawei.com>]

On 2021/4/30 15:01, Andrew Jones wrote:
> On Fri, Apr 30, 2021 at 08:41:25AM +0200, Andrew Jones wrote:
>> On Fri, Apr 30, 2021 at 01:09:00PM +0800, wangyanan (Y) wrote:
>>> But I think the requirement for ARM "if even one parameter other than cpus
>>> or maxcpus
>>> is provided then all parameters must be provided" will be better. This can
>>> ensure the
>>> whole accurate users-specified topology. As you mentioned, if anybody who
>>> bothers
>>> to specify one, why not also specify the others.
>>>
>>> I can add the requirement for ARM in the documentation, and also check the
>>> parameters
>>> in virt_smp_parse. Will this be fine?
>> We sort of have to support command lines that are missing 'maxcpus' and
>> 'clusters', unless we work together with libvirt to make the change.
>> Currently libvirt will generate '-smp 16,sockets=16,cores=1,threads=1'
>> from '<vcpu placement='static'>16</vcpu>'. That's sufficient for our
>> stricter, but not completely strict requirements. And, I still think
>> 'threads' could be optional, because there's a good chance the user
>> doesn't want to describe them, so a default of 1 is good enough. Also,
>> given maxcpus, but not cpus, it's pretty obvious that cpus should equal
>> maxcpus.
>>
> We also still need just 'cpus' or just 'maxcpus' to work, since that
> already works now. So, at least these should work
>
>   -smp N
>   -smp maxcpus=N
>   -smp N,maxcpus=M
>   -smp N,sockets=N,cores=1,threads=1
>   -smp N,maxcpus=M,sockets=M,cores=1,threads=1
>
> since they work today, even though no topology is described.
Yes. I forgot this point that we should consider the compat.
> If we want to
> describe a topology for the first three, then we'll have to pick one,
> which brings us back to the sockets over cores stuff. Or, we could choose
> to just not generate topology descriptions when none is provided.
I find that both preferring cores over sockets and not generating 
topology descriptions
when none is provided can't solve everything. We can only ensure guest 
will get one
socket with multiple cores with qemu cmdline "-smp N".

But if we specify N cpus without any other parameters in libvirt xml, a 
qemu cmdline like
"-smp N, sockets=N, cores=1,threads=1" will be generated, and as a 
result guest will get
N sockets. In this case, we still can't avoid the silent change.

So maybe we should just prefer sockets over cores like the general 
smp_parse() and libvirt,
and let users use "-smp N, sockets=1, cores=N,threads=1" to get what 
they want if they
use version 6.0 or later.

Thanks,
Yanan
> Thanks,
> drew
>
> .