Re: [Qemu-devel] [PATCH v8 6/7] s390x/cpu: Add error handling to cpu creation

[David Hildenbrand]

> On Fri, Mar 04, 2016 at 12:07:28PM +0100, David Hildenbrand wrote:
> >   
> > > >      cpu_exec_init(cs, &err);
> > > >      if (err != NULL) {
> > > >          error_propagate(errp, err);
> > > >          return;
> > > >      }
> > > > +    scc->next_cpu_id = cs->cpu_index + 1;    
> > > 
> > > It appears that scc->next_cpu_id (and hence cpu->id) is some sort of 
> > > arch_id
> > > for you. If it is just going to be monotonically increasing like 
> > > cs->cpu_index,
> > > couldn't you just use cs->cpu_index instead of introducing additional IDs 
> > > ?
> > > 
> > > Note that cpu_exec_init(cs, &err) returns with the next available 
> > > cpu_index
> > > which can be compared against max_cpus directly.
> > > 
> > > Regards,
> > > Bharata.  
> > 
> > I don't think that we should mix the id setting and cpu_index for now.
> > 
> > We can't simply set cpu_index before the device is realized. That logic
> > belongs to cpu_exec_init().  
> 
> Yes, I see that, but apart from the following obvious uses of the id
> property from realizefn, are there other uses ?
> 
> 1 Checking against max_cpus
>   cpu_index can be used for this.
> 
> 2 Checking if cpu with such an id exists
>   cpu_exec_init() would never return with an in-use index. Hence cpu_index
>   can be used here too given that you don't define ->get_arch_id()
> 
> 3 Checking if the id is the next expected one
>   cpu_exec_init/cpu_exec_exit take care of deletion too, so I guess you will
>   always have the next available id to be used in cpu_index.
> 
> Did I miss any other use other than these which makes it necessary to have
> an explicit id property here ?

After all the discussions about
-device-add s390-cpu,id=XX

As substitute/addition in the future for hotplug it is the straightforward
approach to allow setting the id as property. Nobody knows what crazy new
hotplug method we will come up with. But doing it the device way with properties
cannot be wrong. And the id is a fundamental concept of a vcpu (cpu-add id=XX).

So I'd like to avoid reworking everything again, to realize later that we
want it as a property and rewriting it once again.

David