[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 02/11] pseries: Cleanup error handling of spapr_
From: |
David Gibson |
Subject: |
Re: [Qemu-devel] [PATCH 02/11] pseries: Cleanup error handling of spapr_cpu_init() |
Date: |
Mon, 14 Dec 2015 12:04:46 +1100 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
On Fri, Dec 11, 2015 at 07:54:48AM -0700, Eric Blake wrote:
> On 12/10/2015 05:11 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
> > the error infrastructure instead of an old-style explicit fprintf / exit.
> >
> > Signed-off-by: David Gibson <address@hidden>
> > ---
> > hw/ppc/spapr.c | 10 +++++-----
> > 1 file changed, 5 insertions(+), 5 deletions(-)
>
> > @@ -1633,7 +1634,7 @@ static void spapr_cpu_init(sPAPRMachineState *spapr,
> > PowerPCCPU *cpu)
> > }
> >
> > if (cpu->max_compat) {
> > - ppc_set_compat(cpu, cpu->max_compat, &error_fatal);
> > + ppc_set_compat(cpu, cpu->max_compat, errp);
> > }
> >
> > xics_cpu_setup(spapr->icp, cpu);
>
> Pre-patch: you can't reach the xics_cpu_setup() call on error.
>
> Post-patch: depending on what the caller passed in, you can fall through
> to xics_cpu_setup() with a potentially incomplete cpu.
>
> I think a more robust solution is probably along the lines of:
>
> Error *err = NULL;
> if (cpu->max_compat) {
> ppc_set_compat(cpu, cpu->max_compat, &err);
> if (err) {
> error_propagate(errp, err);
> return;
> }
> }
> xics_cpu_setup(spapr_icp, cpu);
Yes, good point. I _think_ xics_cpu_setup() would be safe to call
even if ppc_set_compat() fails, but checking for the error immediately
is safer indeed. I'll adjust in the next spin.
--
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
- [Qemu-devel] [PATCH 00/11] Error handling cleanups for pseries machine type, David Gibson, 2015/12/10
- [Qemu-devel] [PATCH 04/11] pseries: Clean up error handling in spapr_validate_node_memory(), David Gibson, 2015/12/10
- [Qemu-devel] [PATCH 05/11] pseries: Cleanup error handling in spapr_vga_init(), David Gibson, 2015/12/10
- [Qemu-devel] [PATCH 02/11] pseries: Cleanup error handling of spapr_cpu_init(), David Gibson, 2015/12/10
- [Qemu-devel] [PATCH 11/11] pseries: Clean up error reporting in htab migration functions, David Gibson, 2015/12/10
- [Qemu-devel] [PATCH 08/11] pseries: Clean up error handling in spapr_rtas_register(), David Gibson, 2015/12/10
- [Qemu-devel] [PATCH 09/11] pseries: Clean up error handling in xics_system_init(), David Gibson, 2015/12/10
- [Qemu-devel] [PATCH 10/11] pseries: Clean up error handling in ppc_spapr_init(), David Gibson, 2015/12/10