Re: [Qemu-devel] [PATCH v5 01/10] vl: Don't allow CPU toplogies with partially filled cores

[Bharata B Rao]

On Wed, Dec 02, 2015 at 12:14:59PM -0200, Eduardo Habkost wrote:
> On Wed, Dec 02, 2015 at 02:52:39PM +0100, Igor Mammedov wrote:
> > On Fri, 20 Nov 2015 18:24:30 +0530
> > Bharata B Rao <address@hidden> wrote:
> > 
> > > Prevent guests from booting with CPU topologies that have partially
> > > filled CPU cores or can result in partially filled CPU cores after
> > > CPU hotplug like
> > > 
> > > -smp 15,sockets=1,cores=4,threads=4,maxcpus=16 or
> > > -smp 15,sockets=1,cores=4,threads=4,maxcpus=17.
> > 
> > CCing Eduardo who might tell if it's ok wrt x86
> 
> I am always afraid of introducing fatal errors for configurations
> that are obviously wrong but may already exist in production (so
> it would prevent users from migrating existing VMs). But I am
> becoming more inclined to be a bit more aggressive and stop
> allowing these broken configurations.
> 
> But I would rewrite the error message, and just say that "cpus"
> and "maxcpus" must be multiples of "threads". It's easier to
> understand and explain than what "partially filled cores" mean.
> You don't need to mention "sockets" and "cores" in the error
> message, either.
> 
> Also, please print different error messages to indicate if
> "maxcpus" or "cpus" is the culprit.

Sure, makes it easier for the user.

> 
> What about (cpus % (cores * threads) != 0)? It would result in
> sockets with different numbers of cores. Should we allow that?

I had planned to prevent that and also

(max_cpus % (cores * threads) != 0) in the next iteration. Would be
good to know if enforcing such a condition would be acceptable.

> 
> The patch makes QEMU crash with the following command-line:
> 
>   $ qemu-system-x86_64 -smp 2,cores=2,sockets=2
>   Floating point exception (core dumped)
>   $
> 
> It can probably be solved by moving the check after
> smp_{cpus,cores,threads} are already set.

Oh yes, will fix in the next iteration.

Regards,
Bharata.