Re: [Qemu-ppc] [PATCH] hw/pci-host/prep: Do not use hw_error() in realize function

[David Gibson]

On Thu, Feb 09, 2017 at 12:14:41PM +0100, Thomas Huth wrote:
> hw_error() is for CPU related errors only (it prints out a
> register dump and calls abort()), so we should not use it
> if we just failed to load the bios image. Apart from that,
> realize() functions should not exit directly but always set
> the errp with error_setg() in case of errors instead.
> Additionally, move some code around and delete the bios memory
> subregion again in case of such an error, so that we leave a
> clean state when returning to the caller.
> 
> Signed-off-by: Thomas Huth <address@hidden>

Applied to ppc-for-2.9.

> ---
>  hw/pci-host/prep.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/pci-host/prep.c b/hw/pci-host/prep.c
> index 5580293..260a119 100644
> --- a/hw/pci-host/prep.c
> +++ b/hw/pci-host/prep.c
> @@ -309,7 +309,6 @@ static void raven_realize(PCIDevice *d, Error **errp)
>      memory_region_set_readonly(&s->bios, true);
>      memory_region_add_subregion(get_system_memory(), (uint32_t)(-BIOS_SIZE),
>                                  &s->bios);
> -    vmstate_register_ram_global(&s->bios);
>      if (s->bios_name) {
>          filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, s->bios_name);
>          if (filename) {
> @@ -328,12 +327,15 @@ static void raven_realize(PCIDevice *d, Error **errp)
>                  }
>              }
>          }
> +        g_free(filename);
>          if (bios_size < 0 || bios_size > BIOS_SIZE) {
> -            /* FIXME should error_setg() */
> -            hw_error("qemu: could not load bios image '%s'\n", s->bios_name);
> +            memory_region_del_subregion(get_system_memory(), &s->bios);
> +            error_setg(errp, "Could not load bios image '%s'", s->bios_name);
> +            return;
>          }
> -        g_free(filename);
>      }
> +
> +    vmstate_register_ram_global(&s->bios);
>  }
>  
>  static const VMStateDescription vmstate_raven = {
> @@ -361,7 +363,6 @@ static void raven_class_init(ObjectClass *klass, void 
> *data)
>      /*
>       * Reason: PCI-facing part of the host bridge, not usable without
>       * the host-facing part, which can't be device_add'ed, yet.
> -     * Reason: realize() method uses hw_error().
>       */
>      dc->cannot_instantiate_with_device_add_yet = true;
>  }

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature