All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bharata B Rao <bharata@linux.vnet.ibm.com>
To: David Hildenbrand <dahi@linux.vnet.ibm.com>
Cc: Matthew Rosato <mjrosato@linux.vnet.ibm.com>,
	qemu-devel@nongnu.org, agraf@suse.de, borntraeger@de.ibm.com,
	imammedo@redhat.com, cornelia.huck@de.ibm.com,
	pbonzini@redhat.com, afaerber@suse.de, rth@twiddle.net
Subject: Re: [Qemu-devel] [PATCH v8 6/7] s390x/cpu: Add error handling to cpu creation
Date: Fri, 4 Mar 2016 17:01:29 +0530	[thread overview]
Message-ID: <20160304113129.GC5054@in.ibm.com> (raw)
In-Reply-To: <20160304120728.0df50c1e@thinkpad-w530>

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 ?

Regards,
Bharata.

  reply	other threads:[~2016-03-04 11:32 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-03 21:30 [Qemu-devel] [PATCH v8 0/7] Allow hotplug of s390 CPUs Matthew Rosato
2016-03-03 21:30 ` [Qemu-devel] [PATCH v8 1/7] s390x/cpu: Cleanup init in preparation for hotplug Matthew Rosato
2016-03-03 21:30 ` [Qemu-devel] [PATCH v8 2/7] s390x/cpu: Set initial CPU state in common routine Matthew Rosato
2016-03-03 21:30 ` [Qemu-devel] [PATCH v8 3/7] s390x/cpu: Get rid of side effects when creating a vcpu Matthew Rosato
2016-03-04  7:45   ` David Hildenbrand
2016-03-03 21:30 ` [Qemu-devel] [PATCH v8 4/7] s390x/cpu: Tolerate max_cpus Matthew Rosato
2016-03-04  8:05   ` David Hildenbrand
2016-03-04 14:09     ` Matthew Rosato
2016-03-03 21:30 ` [Qemu-devel] [PATCH v8 5/7] s390x/cpu: Add CPU property links Matthew Rosato
2016-03-04  8:07   ` David Hildenbrand
2016-03-03 21:30 ` [Qemu-devel] [PATCH v8 6/7] s390x/cpu: Add error handling to cpu creation Matthew Rosato
2016-03-04  8:01   ` David Hildenbrand
2016-03-04 10:16   ` Bharata B Rao
2016-03-04 11:07     ` David Hildenbrand
2016-03-04 11:31       ` Bharata B Rao [this message]
2016-03-04 11:44         ` Bharata B Rao
2016-03-04 11:50         ` David Hildenbrand
2016-03-04 18:03           ` Igor Mammedov
2016-03-07 10:02             ` David Hildenbrand
2016-03-07 10:12               ` Igor Mammedov
2016-03-07 11:49                 ` Cornelia Huck
2016-03-07 14:50                   ` Igor Mammedov
2016-03-03 21:30 ` [Qemu-devel] [PATCH v8 7/7] s390x/cpu: Allow hotplug of CPUs Matthew Rosato
2016-03-04  7:46   ` David Hildenbrand
2016-03-04 13:03 ` [Qemu-devel] [PATCH v8 0/7] Allow hotplug of s390 CPUs Cornelia Huck

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160304113129.GC5054@in.ibm.com \
    --to=bharata@linux.vnet.ibm.com \
    --cc=afaerber@suse.de \
    --cc=agraf@suse.de \
    --cc=borntraeger@de.ibm.com \
    --cc=cornelia.huck@de.ibm.com \
    --cc=dahi@linux.vnet.ibm.com \
    --cc=imammedo@redhat.com \
    --cc=mjrosato@linux.vnet.ibm.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=rth@twiddle.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.