Re: [Qemu-devel] [PATCH 07/11] pseries: Cleanup error handling in spapr_kvm_type()

[David Gibson]

On Fri, Dec 11, 2015 at 11:01:37AM +0100, Thomas Huth wrote:
> On 11/12/15 01:11, David Gibson wrote:
> > Use error_setg() and &error_fatal instead of an explicit exit().
> > 
> > Signed-off-by: David Gibson <address@hidden>
> > ---
> >  hw/ppc/spapr.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index fd16db4..546d2f5 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -2035,8 +2035,8 @@ static int spapr_kvm_type(const char *vm_type)
> >          return 2;
> >      }
> >  
> > -    error_report("Unknown kvm-type specified '%s'", vm_type);
> > -    exit(1);
> > +    error_setg(&error_fatal, "Unknown kvm-type specified '%s'", vm_type);
> > +    return 0;
> >  }
> 
> Honestly, I'd rather prefer the original code here. error_setg() should
> IMHO be used to set an error in an "flexible" error variable. Using it
> with an "hard-coded" error_fatal sounds ugly to me. And as far as I can
> see, no other code in QEMU uses error_setg(&error_fatal, ...) - so we
> should maybe not start with this in the spapr code as well.

Whereas I was thinking that if we have a standard way of tripping a
fatal error, we should use that, rather than using an explicit exit().


> If you still would like to get rid of the exit() here ... maybe you
> could introduce some kind of error_report_fatal() function instead that
> exits after reporting the error with error_report() ?

I don't see that there's any point adding an error_report_fatal() that
would be semantically equivalent to error_setg(&error_fatal, ...).

Markus, an opnion on the right way to do this?

-- 
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