Re: [Qemu-ppc] [PATCH 2/7] pnv: Add missing error check during cpu realize()

[Greg Kurz]

On Wed, 13 Jun 2018 11:14:57 +0200
Cédric Le Goater <address@hidden> wrote:

> >> index 13ad7d9e04..efb68226bb 100644
> >> --- a/hw/ppc/pnv_core.c
> >> +++ b/hw/ppc/pnv_core.c
> >> @@ -173,6 +173,9 @@ static void pnv_core_realize(DeviceState *dev, Error 
> >> **errp)
> >>  
> >>          snprintf(name, sizeof(name), "thread[%d]", i);
> >>          object_property_add_child(OBJECT(pc), name, obj, &local_err);
> >> +        if (local_err) {
> >> +            goto err;
> >> +        }
> >>          object_property_add_alias(obj, "core-pir", OBJECT(pc),
> >>                                    "pir", &local_err);
> >>          if (local_err) {  
> > 
> > Hmm... the current error path seems to assume failures to be
> > caused by object_property_add_child(). It hence unparents the
> > previously parented CPUs, but not the current one. So we'll
> > miss one call to object_unparent() if object_property_add_alias()
> > fails.  
> 
> yes, let's just put NULL or &error_abort instead.
> 

NULL means we really don't care if the call fails or succeeds.

&error_abort means we consider a failure to be a unrecoverable bug.

So I would rather pass &error_abort here.

But if the guest is already running and functional, and we hit
the error during hotplug, does the guest really deserve to be
aborted or should we just fail the hotplug ?

> C.