Re: [Qemu-devel] [RFC PATCH v1 05/10] cpu: Abstract CPU core type

[David Gibson]

On Wed, Mar 09, 2016 at 11:40:53AM +0100, Igor Mammedov wrote:
> On Tue, 8 Mar 2016 15:26:27 +1100
> David Gibson <address@hidden> wrote:
> > On Mon, Mar 07, 2016 at 11:29:29AM +0100, Igor Mammedov wrote:
> > > On Mon, 7 Mar 2016 14:36:55 +1100
> > > David Gibson <address@hidden> wrote:
[snip]
> > > > > > > on top of that I'd add numeric 'threads' property to base class so
> > > > > > > all derived cores would inherit it.
> > > > > > > 
> > > > > > > Then as easy integration with -smp threads=x, a machine could push
> > > > > > > a global variable 'cpu-core.threads=[smp_threads]' which would
> > > > > > > make every created cpu-core object to have threads set
> > > > > > > at instance_init() time (device_init).
> > > > > > > 
> > > > > > > That way user won't have to specify 'threads=y' for every
> > > > > > >   device_add spapr-core,core=x
> > > > > > > as it will be taken from global property 'cpu-core.threads'
> > > > > > > but if user wishes he/she still could override global by 
> > > > > > > explicitly
> > > > > > > providing thread property at device_add time:
> > > > > > >   device_add spapr-core,core=x,threads=y
> > > > > > > 
> > > > > > > wrt this series it would mean, instead of creating threads in 
> > > > > > > property
> > > > > > > setter, delaying threads creation to core.realize() time,
> > > > > > > but since realize is allowed to fail it should be fine do so.     
> > > > > > >  
> > > > > > 
> > > > > > Ok that would suit us as there are two properties on which thread 
> > > > > > creation
> > > > > > is dependent upon: nr_threads and cpu_model. If thread objects can 
> > > > > > be
> > > > > > created at core realize time, then we don't have to resort to the 
> > > > > > ugliness
> > > > > > of creating the threads from either of the property setters. I 
> > > > > > always
> > > > > > assumed that we shouldn't be creating objects from realize, but if 
> > > > > > that
> > > > > > is fine, it is good.    
> > > > > since realize is allowed to fail, it should be safe from hotplug pov
> > > > > to create internal objects there, as far as proper cleanups are done
> > > > > for failure path.    
> > > >   
> > > [...]  
> > > > I'm not clear from the above if you're also intending to move at least
> > > > the adding of the threads as child properties is supposed to go into
> > > > the base type,  
> > > I'm not sure that I've got question, could you please rephrase?  
> > 
> > So, it seems like we're agreed that moving the nr_threads property to
> > the base type is a good idea.
> > 
> > My question is, do we also move the object_property_add_child() calls
> > for each thread to the base type (possibly via a helper function or
> > method the base type provides to derived types)?
> I can't think of a reason to do so,
> why can't subtype-core.realize() do it?

It can, but I'm always suspicious of boilerplate stuff that every
subtype *has* to do in order to work properly.

> What would one gain creating callbacks and calling them from base class?

Enforcing - or at least making as easy as possible - consistency in
the child object naming.

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature