[PATCHv3,2/7] pnv: Fix some error handling cpu realize()

Message ID 20180614044129.13606-3-david@gibson.dropbear.id.au
State New
Headers show
Series
  • Better handling of machine specific per-cpu information
Related show

Commit Message

David Gibson June 14, 2018, 4:41 a.m.
In pnv_core_realize() we call two functions with an Error * parameter in
succession, which will go badly if they both cause errors.  In fact, a
failure in either of them indicates a qemu internal error, so we can just
use &error_abort in both cases.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/pnv_core.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

Comments

Cédric Le Goater June 14, 2018, 5:26 a.m. | #1
On 06/14/2018 06:41 AM, David Gibson wrote:
> In pnv_core_realize() we call two functions with an Error * parameter in
> succession, which will go badly if they both cause errors.  In fact, a
> failure in either of them indicates a qemu internal error, so we can just
> use &error_abort in both cases.

Yes. I think that most of the object_property_add_child() could use
&error_abort, object_property_add_const_link() also.

> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>


Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C. 


> ---
>  hw/ppc/pnv_core.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
> index 13ad7d9e04..01f47c8037 100644
> --- a/hw/ppc/pnv_core.c
> +++ b/hw/ppc/pnv_core.c
> @@ -172,12 +172,9 @@ static void pnv_core_realize(DeviceState *dev, Error **errp)
>          object_initialize(obj, size, typename);
>  
>          snprintf(name, sizeof(name), "thread[%d]", i);
> -        object_property_add_child(OBJECT(pc), name, obj, &local_err);
> +        object_property_add_child(OBJECT(pc), name, obj, &error_abort);
>          object_property_add_alias(obj, "core-pir", OBJECT(pc),
> -                                  "pir", &local_err);
> -        if (local_err) {
> -            goto err;
> -        }
> +                                  "pir", &error_abort);
>          object_unref(obj);
>      }
>  
>
Greg Kurz June 14, 2018, 6:35 a.m. | #2
On Thu, 14 Jun 2018 14:41:24 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:

> In pnv_core_realize() we call two functions with an Error * parameter in
> succession, which will go badly if they both cause errors.  In fact, a
> failure in either of them indicates a qemu internal error, so we can just
> use &error_abort in both cases.
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

>  hw/ppc/pnv_core.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
> index 13ad7d9e04..01f47c8037 100644
> --- a/hw/ppc/pnv_core.c
> +++ b/hw/ppc/pnv_core.c
> @@ -172,12 +172,9 @@ static void pnv_core_realize(DeviceState *dev, Error **errp)
>          object_initialize(obj, size, typename);
>  
>          snprintf(name, sizeof(name), "thread[%d]", i);
> -        object_property_add_child(OBJECT(pc), name, obj, &local_err);
> +        object_property_add_child(OBJECT(pc), name, obj, &error_abort);
>          object_property_add_alias(obj, "core-pir", OBJECT(pc),
> -                                  "pir", &local_err);
> -        if (local_err) {
> -            goto err;
> -        }
> +                                  "pir", &error_abort);
>          object_unref(obj);
>      }
>

Patch

diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
index 13ad7d9e04..01f47c8037 100644
--- a/hw/ppc/pnv_core.c
+++ b/hw/ppc/pnv_core.c
@@ -172,12 +172,9 @@  static void pnv_core_realize(DeviceState *dev, Error **errp)
         object_initialize(obj, size, typename);
 
         snprintf(name, sizeof(name), "thread[%d]", i);
-        object_property_add_child(OBJECT(pc), name, obj, &local_err);
+        object_property_add_child(OBJECT(pc), name, obj, &error_abort);
         object_property_add_alias(obj, "core-pir", OBJECT(pc),
-                                  "pir", &local_err);
-        if (local_err) {
-            goto err;
-        }
+                                  "pir", &error_abort);
         object_unref(obj);
     }