[07/11] pseries: Cleanup error handling in spapr_kvm_type()

Use error_setg() and &error_fatal instead of an explicit exit().

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/ppc/spapr.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

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 <david@gibson.dropbear.id.au>
> ---
>  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.

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() ?

 Thomas

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 <david@gibson.dropbear.id.au>
> > ---
> >  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?