Re: [Qemu-devel] [PATCH v7 01/13] machine: Don't allow CPU toplogies with partially filled cores

[Igor Mammedov]

On Wed, 3 Feb 2016 15:38:09 -0200
Eduardo Habkost <address@hidden> wrote:

> On Mon, Feb 01, 2016 at 10:41:58AM +0100, Igor Mammedov wrote:
> [...]
> > > 
> > > How exactly would you migrate a machine today, if you do the
> > > following?
> > > 
> > >   $ qemu-system-x86_64 -smp 8,sockets=2,cores=2,threads=2,maxcpus=16
> > >   (QMP) cpu-add id=15  
> > isn't it the same broken topology? 
> >   sockets*cores*threads != maxcpus
> > But if you ask if it's possible to migrate machine with non-sequentially
> > hotplugged CPUs than answer is no it's not possible with cpu-add.  
> 
> I was asking about migration with non-sequential hotplugged CPUs
> (e.g. with -smp 8,sockets=4,cores=2,threads=2,maxcpus=16).
machine with non-sequential hotplugged CPUs can't be migrated currently
since there is not any way to say which CPUs should be created on target.

That should be though possible to do so with -device/device_add
interface once we enable it for x86 CPU threads.
Stumble block there is where to get APIC ID for a CPU (i.e. address
thing which tells where to plug CPU at).
Adding socket,core,thread properties to CPU would help to solve it,
and they look generic enough to put them in CPUClass.

> 
> 
> [...]
> > > > > > 
> > > > > > Perhaps this check should be enforced per target/machine if
> > > > > > arch requires it.      
> > > > > 
> > > > > It is. Please see the patch. It introduces a validate_smp_config
> > > > > method.
> > > > > 
> > > > > But we need your input to clarify if
> > > > > validate_smp_config_generic() is safe for pc-2.6 too.    
> > > > it breaks migration as it could prevent target from starting if
> > > > there is hotplugged CPUs on source side.    
> > > 
> > > It looks like this is a problem only if the machine allows
> > > hotplug of individual threads. What if we just add this to the
> > > beginning of validate_smp_config_generic():
> > > 
> > >     if (mc->hot_add_cpu && max_cpus > smp_cpus) {  
> > It would break migration after hotplugging CPUs upto max_cpus
> > and then trying to migrate.
> > 
> > Also when we move x86 to device_add that will regress since
> > hot_add_cpu() won't be used in that case.
> > 
> > I think there is not much point enforcing restrictions
> > in this patch as generic. We should leave hook empty and allow
> > target to override it. Then SPAPR could enforce it's own limit
> > on partial cores.  
> 
> Agreed. It looks like we won't get rid of thread-based CPU
> hotplug in x86 in the foreseable future.
>