| Message ID | 157403285265.409804.8683093665795248192.stgit@bahia.lan |
|---|---|
| State | New |
| Headers | show |
| Series | ppc: Some more QOM cleanup for XICS | expand |
On 18/11/2019 00:20, Greg Kurz wrote: > Failing to set any of the ICS property should really never happen: > - object_property_add_child() always succeed unless the child object > already has a parent, which isn't the case here obviously since the > ICS has just been created with object_new() > - the ICS has an "nr-irqs" property than can be set as long as the ICS > isn't realized > > In both cases, an error indicates there is a bug in QEMU. Propagating the > error, ie. exiting QEMU since spapr_irq_init() is called with &error_fatal > doesn't make much sense. Abort instead. This is consistent with what is > done with XIVE : both qdev_create() and qdev_prop_set_uint32() abort QEMU > on error. > > Signed-off-by: Greg Kurz <groug@kaod.org> Reviewed-by: Cédric Le Goater <clg@kaod.org> Thanks, C. > --- > hw/ppc/spapr_irq.c | 13 ++----------- > 1 file changed, 2 insertions(+), 11 deletions(-) > > diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c > index 487c8ceb35a3..37f65dac9ca8 100644 > --- a/hw/ppc/spapr_irq.c > +++ b/hw/ppc/spapr_irq.c > @@ -313,20 +313,11 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp) > Object *obj; > > obj = object_new(TYPE_ICS_SPAPR); > - object_property_add_child(OBJECT(spapr), "ics", obj, &local_err); > - if (local_err) { > - error_propagate(errp, local_err); > - return; > - } > > + object_property_add_child(OBJECT(spapr), "ics", obj, &error_abort); > object_property_set_link(obj, OBJECT(spapr), ICS_PROP_XICS, > &error_abort); > - object_property_set_int(obj, smc->nr_xirqs, "nr-irqs", &local_err); > - if (local_err) { > - error_propagate(errp, local_err); > - return; > - } > - > + object_property_set_int(obj, smc->nr_xirqs, "nr-irqs", &error_abort); > object_property_set_bool(obj, true, "realized", &local_err); > if (local_err) { > error_propagate(errp, local_err); >
diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c index 487c8ceb35a3..37f65dac9ca8 100644 --- a/hw/ppc/spapr_irq.c +++ b/hw/ppc/spapr_irq.c @@ -313,20 +313,11 @@ void spapr_irq_init(SpaprMachineState *spapr, Error **errp) Object *obj; obj = object_new(TYPE_ICS_SPAPR); - object_property_add_child(OBJECT(spapr), "ics", obj, &local_err); - if (local_err) { - error_propagate(errp, local_err); - return; - } + object_property_add_child(OBJECT(spapr), "ics", obj, &error_abort); object_property_set_link(obj, OBJECT(spapr), ICS_PROP_XICS, &error_abort); - object_property_set_int(obj, smc->nr_xirqs, "nr-irqs", &local_err); - if (local_err) { - error_propagate(errp, local_err); - return; - } - + object_property_set_int(obj, smc->nr_xirqs, "nr-irqs", &error_abort); object_property_set_bool(obj, true, "realized", &local_err); if (local_err) { error_propagate(errp, local_err);
Failing to set any of the ICS property should really never happen: - object_property_add_child() always succeed unless the child object already has a parent, which isn't the case here obviously since the ICS has just been created with object_new() - the ICS has an "nr-irqs" property than can be set as long as the ICS isn't realized In both cases, an error indicates there is a bug in QEMU. Propagating the error, ie. exiting QEMU since spapr_irq_init() is called with &error_fatal doesn't make much sense. Abort instead. This is consistent with what is done with XIVE : both qdev_create() and qdev_prop_set_uint32() abort QEMU on error. Signed-off-by: Greg Kurz <groug@kaod.org> --- hw/ppc/spapr_irq.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-)