[v5,1/4] qdev: using error_abort instead of using local_err

From: Gonglei <arei.gonglei@huawei.com>

This error can not happen normally. If it happens indicates
something very wrong, we should abort QEMU. moreover, The
user can only refer to /machine/peripheral, not
/machine/unattached.

Meanwhile remove superfluous check about local_err.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
---
 hw/core/qdev.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Am 02.09.2014 14:03, schrieb arei.gonglei@huawei.com:
> From: Gonglei <arei.gonglei@huawei.com>
> 
> This error can not happen normally. If it happens indicates
> something very wrong, we should abort QEMU. moreover, The
> user can only refer to /machine/peripheral, not
> /machine/unattached.
> 
> Meanwhile remove superfluous check about local_err.
> 
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> ---
>  hw/core/qdev.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index da1ba48..4a1ac5b 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -820,13 +820,13 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
>      }
>  
>      if (value && !dev->realized) {
> -        if (!obj->parent && local_err == NULL) {
> +        if (!obj->parent) {
>              static int unattached_count;
>              gchar *name = g_strdup_printf("device[%d]", unattached_count++);
>  
>              object_property_add_child(container_get(qdev_get_machine(),
>                                                      "/unattached"),
> -                                      name, obj, &local_err);
> +                                      name, obj, &error_abort);
>              g_free(name);
>          }
>  

One way to trigger this abort would be for the user to add a property of
that format via -object or similar mechanisms, when allowing a custom
path. Today, objects are always under /objects though.

In fact the only way to get an Error** back from
object_property_add[_child]() is if a duplicate property exists. Any
other error conditions would already abort in GLib code.

Anyway, this is not strictly an error fix and it's a question of how
much we want to protect the user from his own stupidity, so I'm leaning
towards applying this patch. I'd prefer to decouple it from the other
cleanup though.

Regards,
Andreas

Hi,

> Subject: Re: [Qemu-devel] [PATCH v5 1/4] qdev: using error_abort instead of
> using local_err
> 
> Am 02.09.2014 14:03, schrieb arei.gonglei@huawei.com:
> > From: Gonglei <arei.gonglei@huawei.com>
> >
> > This error can not happen normally. If it happens indicates
> > something very wrong, we should abort QEMU. moreover, The
> > user can only refer to /machine/peripheral, not
> > /machine/unattached.
> >
> > Meanwhile remove superfluous check about local_err.
> >
> > Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> > Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
> > ---
> >  hw/core/qdev.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> > index da1ba48..4a1ac5b 100644
> > --- a/hw/core/qdev.c
> > +++ b/hw/core/qdev.c
> > @@ -820,13 +820,13 @@ static void device_set_realized(Object *obj, bool
> value, Error **errp)
> >      }
> >
> >      if (value && !dev->realized) {
> > -        if (!obj->parent && local_err == NULL) {
> > +        if (!obj->parent) {
> >              static int unattached_count;
> >              gchar *name = g_strdup_printf("device[%d]",
> unattached_count++);
> >
> >
> object_property_add_child(container_get(qdev_get_machine(),
> >
> "/unattached"),
> > -                                      name, obj, &local_err);
> > +                                      name, obj, &error_abort);
> >              g_free(name);
> >          }
> >
> 
> One way to trigger this abort would be for the user to add a property of
> that format via -object or similar mechanisms, when allowing a custom
> path. Today, objects are always under /objects though.
> 
> In fact the only way to get an Error** back from
> object_property_add[_child]() is if a duplicate property exists. Any
> other error conditions would already abort in GLib code.
> 
> Anyway, this is not strictly an error fix and it's a question of how
> much we want to protect the user from his own stupidity, so I'm leaning
> towards applying this patch. I'd prefer to decouple it from the other
> cleanup though.
> 
I'll post v6, rework patch 2/4 by Peter's suggestion, add 'Reviewed-by'
and remove patch 4/4 by Michael's idea. :)

Please consider to apply v6. Thanks!

Best regards,
-Gonglei

> Regards,
> Andreas
> 
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg