[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/3] PPC: Clean up misuse of qdev_init() in kvm-
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 1/3] PPC: Clean up misuse of qdev_init() in kvm-openpic creation |
Date: |
Wed, 18 Feb 2015 15:43:09 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) |
Scott, can you review?
Markus Armbruster <address@hidden> writes:
> We call ppce500_init_mpic_kvm() to create a "kvm-openpic". If it
> fails, we call ppce500_init_mpic_qemu() to fall back to plain
> "openpic".
>
> ppce500_init_mpic_kvm() uses qdev_init(). qdev_init()'s error
> handling has an unwanted side effect: it calls qerror_report_err(),
> which prints to stderr. Looks like an error, but isn't.
>
> In QMP context, it would stash the error in the monitor instead,
> making the QMP command fail. Fortunately, it's only called from board
> initialization, never in QMP context.
>
> Clean up by cutting out the qdev_init() middle-man: set property
> "realized" directly.
>
> While there, improve the error message when we can't satisfy an
> explicit user request for "kvm-openpic", and exit(1) instead of
> abort().
>
> Cc: Alexander Graf <address@hidden>
> Cc: Scott Wood <address@hidden>
> Cc: address@hidden
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
> hw/ppc/e500.c | 20 +++++++++++---------
> 1 file changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
> index 7e17d18..fd0d138 100644
> --- a/hw/ppc/e500.c
> +++ b/hw/ppc/e500.c
> @@ -706,17 +706,19 @@ static DeviceState
> *ppce500_init_mpic_qemu(PPCE500Params *params,
> }
>
> static DeviceState *ppce500_init_mpic_kvm(PPCE500Params *params,
> - qemu_irq **irqs)
> + qemu_irq **irqs, Error **errp)
> {
> + Error *err = NULL;
> DeviceState *dev;
> CPUState *cs;
> - int r;
>
> dev = qdev_create(NULL, TYPE_KVM_OPENPIC);
> qdev_prop_set_uint32(dev, "model", params->mpic_version);
>
> - r = qdev_init(dev);
> - if (r) {
> + object_property_set_bool(OBJECT(dev), true, "realized", &err);
> + if (err) {
> + error_propagate(errp, err);
> + object_unparent(OBJECT(dev));
> return NULL;
> }
>
> @@ -747,15 +749,15 @@ static qemu_irq *ppce500_init_mpic(PPCE500Params
> *params, MemoryRegion *ccsr,
> "kernel_irqchip", true);
> bool irqchip_required = qemu_opt_get_bool(machine_opts,
> "kernel_irqchip", false);
> + Error *err = NULL;
>
> if (irqchip_allowed) {
> - dev = ppce500_init_mpic_kvm(params, irqs);
> + dev = ppce500_init_mpic_kvm(params, irqs, &err);
> }
> -
> if (irqchip_required && !dev) {
> - fprintf(stderr, "%s: irqchip requested but unavailable\n",
> - __func__);
> - abort();
> + error_report("kernel_irqchip requested but unavailable: %s",
> + error_get_pretty(err));
> + exit(1);
> }
> }