From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59115) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1V6dXb-0000sM-OQ for qemu-devel@nongnu.org; Tue, 06 Aug 2013 05:27:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1V6dXV-00022D-6f for qemu-devel@nongnu.org; Tue, 06 Aug 2013 05:26:55 -0400 Message-ID: <5200C154.8010108@suse.de> Date: Tue, 06 Aug 2013 11:26:44 +0200 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1375777673-20274-1-git-send-email-aik@ozlabs.ru> <1375777673-20274-5-git-send-email-aik@ozlabs.ru> In-Reply-To: <1375777673-20274-5-git-send-email-aik@ozlabs.ru> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 4/6] xics: minor changes and cleanups List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexey Kardashevskiy Cc: Anthony Liguori , Alexander Graf , qemu-devel@nongnu.org, qemu-ppc@nongnu.org, Paul Mackerras , David Gibson Am 06.08.2013 10:27, schrieb Alexey Kardashevskiy: > Before XICS-KVM comes, it makes sense to clean up the emulated XICS a b= it. >=20 > This does: > 1. add assert in ics_realize() > 2. change variable names from "k" to more informative ones > 3. add "const" to every TypeInfo > 4. replace fprintf(stderr, ..."\n") with error_report > 5. replace old style qdev_init_nofail() with new style > object_property_set_bool(). >=20 > Signed-off-by: Alexey Kardashevskiy > --- > hw/intc/xics.c | 32 ++++++++++++++++++++++---------- > 1 file changed, 22 insertions(+), 10 deletions(-) >=20 > diff --git a/hw/intc/xics.c b/hw/intc/xics.c > index 436c788..20840e3 100644 > --- a/hw/intc/xics.c > +++ b/hw/intc/xics.c > @@ -29,6 +29,7 @@ > #include "trace.h" > #include "hw/ppc/spapr.h" > #include "hw/ppc/xics.h" > +#include "qemu/error-report.h" > =20 > /* > * ICP: Presentation layer > @@ -211,7 +212,7 @@ static void icp_class_init(ObjectClass *klass, void= *data) > dc->vmsd =3D &vmstate_icp_server; > } > =20 > -static TypeInfo icp_info =3D { > +static const TypeInfo icp_info =3D { > .name =3D TYPE_ICP, > .parent =3D TYPE_DEVICE, > .instance_size =3D sizeof(ICPState), > @@ -446,6 +447,7 @@ static int ics_realize(DeviceState *dev) > { > ICSState *ics =3D ICS(dev); > =20 > + assert(ics->nr_irqs); Why assert here? As pointed out, this function is bogus, it should have an Error **errp argument, so you should just do something like this: if (ics->nr_irqs =3D=3D 0) { error_setg(errp, "Number of interrupts needs to be greater 0"); return; } which propagates the error to the caller and lets him decide how to handle it, such as actually specifying a different property value and trying realized =3D true again. > ics->irqs =3D g_malloc0(ics->nr_irqs * sizeof(ICSIRQState)); > ics->islsi =3D g_malloc0(ics->nr_irqs * sizeof(bool)); > ics->qirqs =3D qemu_allocate_irqs(ics_set_irq, ics, ics->nr_irqs); > @@ -456,15 +458,15 @@ static int ics_realize(DeviceState *dev) > static void ics_class_init(ObjectClass *klass, void *data) > { > DeviceClass *dc =3D DEVICE_CLASS(klass); > - ICSStateClass *k =3D ICS_CLASS(klass); > + ICSStateClass *icsc =3D ICS_CLASS(klass); > =20 > dc->init =3D ics_realize; > dc->vmsd =3D &vmstate_ics; > dc->reset =3D ics_reset; > - k->post_load =3D ics_post_load; > + icsc->post_load =3D ics_post_load; > } > =20 > -static TypeInfo ics_info =3D { > +static const TypeInfo ics_info =3D { > .name =3D TYPE_ICS, > .parent =3D TYPE_DEVICE, > .instance_size =3D sizeof(ICSState), > @@ -680,8 +682,8 @@ static void xics_cpu_setup(XICSState *icp, PowerPCC= PU *cpu) > break; > =20 > default: > - fprintf(stderr, "XICS interrupt controller does not support th= is CPU " > - "bus model\n"); > + error_report("XICS interrupt controller does not support this = CPU " > + "bus model"); > abort(); > } > } > @@ -690,6 +692,7 @@ static void xics_realize(DeviceState *dev, Error **= errp) > { > XICSState *icp =3D XICS(dev); > ICSState *ics =3D icp->ics; > + Error *error =3D NULL; > int i; > =20 > /* Registration of global state belongs into realize */ > @@ -706,15 +709,24 @@ static void xics_realize(DeviceState *dev, Error = **errp) > ics->nr_irqs =3D icp->nr_irqs; > ics->offset =3D XICS_IRQ_BASE; > ics->icp =3D icp; > - qdev_init_nofail(DEVICE(ics)); > + object_property_set_bool(OBJECT(icp->ics), true, "realized", &erro= r); > + if (error) { > + error_propagate(errp, error); > + return; > + } > =20 > + assert(icp->nr_servers); Ditto here - this one already has errp. Otherwise good, thanks. Andreas > icp->ss =3D g_malloc0(icp->nr_servers*sizeof(ICPState)); > for (i =3D 0; i < icp->nr_servers; i++) { > char buffer[32]; > object_initialize(&icp->ss[i], TYPE_ICP); > snprintf(buffer, sizeof(buffer), "icp[%d]", i); > object_property_add_child(OBJECT(icp), buffer, OBJECT(&icp->ss= [i]), NULL); > - qdev_init_nofail(DEVICE(&icp->ss[i])); > + object_property_set_bool(OBJECT(&icp->ss[i]), true, "realized"= , &error); > + if (error) { > + error_propagate(errp, error); > + return; > + } > } > } > =20 > @@ -735,12 +747,12 @@ static Property xics_properties[] =3D { > static void xics_class_init(ObjectClass *oc, void *data) > { > DeviceClass *dc =3D DEVICE_CLASS(oc); > - XICSStateClass *k =3D XICS_CLASS(oc); > + XICSStateClass *xsc =3D XICS_CLASS(oc); > =20 > dc->realize =3D xics_realize; > dc->props =3D xics_properties; > dc->reset =3D xics_reset; > - k->cpu_setup =3D xics_cpu_setup; > + xsc->cpu_setup =3D xics_cpu_setup; > } > =20 > static const TypeInfo xics_info =3D { --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg