Re: [Qemu-devel] [PATCHv4 2/8] pseries: Cleanup error handling of spapr_cpu_init()

From: Alexey Kardashevskiy <aik@ozlabs.ru>
To: David Gibson <david@gibson.dropbear.id.au>,
	armbru@redhat.com, mdroth@linux.vnet.ibm.com
Cc: lvivier@redhat.com, thuth@redhat.com, qemu-ppc@nongnu.org,
	qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCHv4 2/8] pseries: Cleanup error handling of spapr_cpu_init()
Date: Tue, 19 Jan 2016 15:30:55 +1100	[thread overview]
Message-ID: <569DBBFF.8030104@ozlabs.ru> (raw)
In-Reply-To: <1453174759-22837-3-git-send-email-david@gibson.dropbear.id.au>

On 01/19/2016 02:39 PM, David Gibson wrote:
> Currently spapr_cpu_init() is hardcoded to handle any errors as fatal.
> That works for now, since it's only called from initial setup where an
> error here means we really can't proceed.
>
> However, we'll want to handle this more flexibly for cpu hotplug in future
> so generalize this using the error reporting infrastructure.  While we're
> at it make a small cleanup in a related part of ppc_spapr_init() to use
> error_report() instead of an old-style explicit fprintf().
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> Reviewed-by: Bharata B Rao <bharata@linux.vnet.ibm.com>

Reviewed-by: Alexey Kardashevskiy <aik@ozlabs.ru>


> ---
>   hw/ppc/spapr.c | 15 +++++++++++----
>   1 file changed, 11 insertions(+), 4 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index fa7a7f4..b7fd09a 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1617,7 +1617,8 @@ static void spapr_boot_set(void *opaque, const char *boot_device,
>       machine->boot_order = g_strdup(boot_device);
>   }
>
> -static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu)
> +static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu,
> +                           Error **errp)
>   {
>       CPUPPCState *env = &cpu->env;
>
> @@ -1635,7 +1636,13 @@ static void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu)
>       }
>
>       if (cpu->max_compat) {
> -        ppc_set_compat(cpu, cpu->max_compat, &error_fatal);
> +        Error *local_err = NULL;
> +
> +        ppc_set_compat(cpu, cpu->max_compat, &local_err);
> +        if (local_err) {
> +            error_propagate(errp, local_err);
> +            return;
> +        }
>       }
>
>       xics_cpu_setup(spapr->icp, cpu);
> @@ -1804,10 +1811,10 @@ static void ppc_spapr_init(MachineState *machine)
>       for (i = 0; i < smp_cpus; i++) {
>           cpu = cpu_ppc_init(machine->cpu_model);
>           if (cpu == NULL) {
> -            fprintf(stderr, "Unable to find PowerPC CPU definition\n");
> +            error_report("Unable to find PowerPC CPU definition");
>               exit(1);
>           }
> -        spapr_cpu_init(spapr, cpu);
> +        spapr_cpu_init(spapr, cpu, &error_fatal);
>       }
>
>       if (kvm_enabled()) {
>


-- 
Alexey