David, your commit 11ad93f suggests you're highly qualified to review.
No good deed shall go unpunished ;)
Markus Armbruster <arm...@redhat.com> writes:
> We call try_create_xics() to create a "xics-kvm". If it fails, we
> call it again to fall back to plain "xics".
>
> try_create_xics() 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 "xics-kvm", and exit(1) instead of abort().
> Simplify the abort when we can't create "xics".
>
> Cc: Alexander Graf <ag...@suse.de>
> Cc: qemu-...@nongnu.org
> Signed-off-by: Markus Armbruster <arm...@redhat.com>
> ---
> hw/ppc/spapr.c | 25 ++++++++++++-------------
> 1 file changed, 12 insertions(+), 13 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index b560459..de6d0bb 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -109,17 +109,20 @@ struct sPAPRMachineState {
> sPAPREnvironment *spapr;
>
> static XICSState *try_create_xics(const char *type, int nr_servers,
> - int nr_irqs)
> + int nr_irqs, Error **errp)
> {
> + Error *err;
> DeviceState *dev;
>
> dev = qdev_create(NULL, type);
> qdev_prop_set_uint32(dev, "nr_servers", nr_servers);
> qdev_prop_set_uint32(dev, "nr_irqs", nr_irqs);
> - if (qdev_init(dev) < 0) {
> + object_property_set_bool(OBJECT(dev), true, "realized", &err);
> + if (err) {
> + error_propagate(errp, err);
> + object_unparent(OBJECT(dev));
> return NULL;
> }
> -
> return XICS_COMMON(dev);
> }
>
> @@ -133,23 +136,19 @@ static XICSState *xics_system_init(int nr_servers, int
> nr_irqs)
> "kernel_irqchip", true);
> bool irqchip_required = qemu_opt_get_bool(machine_opts,
> "kernel_irqchip", false);
> + Error *err = NULL;
> +
> if (irqchip_allowed) {
> - icp = try_create_xics(TYPE_KVM_XICS, nr_servers, nr_irqs);
> + icp = try_create_xics(TYPE_KVM_XICS, nr_servers, nr_irqs, &err);
> }
> -
> if (irqchip_required && !icp) {
> - perror("Failed to create in-kernel XICS\n");
> - abort();
> + error_report("kernel_irqchip requested but unavailable: %s",
> + error_get_pretty(err));
> }
> }
>
> if (!icp) {
> - icp = try_create_xics(TYPE_XICS, nr_servers, nr_irqs);
> - }
> -
> - if (!icp) {
> - perror("Failed to create XICS\n");
> - abort();
> + icp = try_create_xics(TYPE_XICS, nr_servers, nr_irqs, &error_abort);
> }
>
> return icp;
