Re: [Qemu-devel] [PATCH 06/10] pseries: Improve error handling in find_unknown_sysbus_device()

[Markus Armbruster <armbru@redhat.com>]

David Gibson <david@gibson.dropbear.id.au> writes:

> Use error_setg() to return an error instead of using an explicit exit().
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
>  hw/ppc/spapr.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index bb5eaa5..ddca6e6 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1106,6 +1106,7 @@ static void spapr_reset_htab(sPAPRMachineState *spapr, Error **errp)
>  
>  static int find_unknown_sysbus_device(SysBusDevice *sbdev, void *opaque)
>  {
> +    Error **errp = opaque;
>      bool matched = false;
>  
>      if (object_dynamic_cast(OBJECT(sbdev), TYPE_SPAPR_PCI_HOST_BRIDGE)) {
> @@ -1113,9 +1114,10 @@ static int find_unknown_sysbus_device(SysBusDevice *sbdev, void *opaque)
>      }
>  
>      if (!matched) {
> -        error_report("Device %s is not supported by this machine yet.",
> -                     qdev_fw_name(DEVICE(sbdev)));
> -        exit(1);
> +        error_setg(errp,
> +                   "Device %s is not supported by this machine yet",
> +                   qdev_fw_name(DEVICE(sbdev)));
> +        return 1; /* Don't continue scanning devices */

Re the comment: really?

find_unknown_sysbus_device() gets passed to
foreach_dynamic_sysbus_device(), which passes it on to
find_sysbus_device().

find_sysbus_device() calls it directly for non-containers, ignoring the
function value.

For containers, it iterates over the container's contents with
object_child_foreach().  That function indeed stops when the callback
returns non-zero.  However, the callback is find_sysbus_device(), not
find_unknown_sysbus_device().

Am I confused?

>      }
>  
>      return 0;
> @@ -1150,7 +1152,7 @@ static void ppc_spapr_reset(void)
>      uint32_t rtas_limit;
>  
>      /* Check for unknown sysbus devices */
> -    foreach_dynamic_sysbus_device(find_unknown_sysbus_device, NULL);
> +    foreach_dynamic_sysbus_device(find_unknown_sysbus_device, &error_fatal);
>  
>      /* Reset the hash table & recalc the RMA */
>      spapr_reset_htab(spapr, &error_fatal);