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

[Andrew Jones]

On Fri, Apr 30, 2021 at 05:33:42PM +0800, wangyanan (Y) wrote:
> 
> 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.

Rather than doing the preference of sockets over cores thing, let's
require that sockets, cores, and one of maxcpus or cpus are provided,
as we discussed in the other mail.

Thanks,
drew