Re: [Qemu-devel] [PATCHv3 5/9] pseries: Cleanup error handling in spapr_vga_init()

[Thomas Huth]

On 18.01.2016 05:24, David Gibson wrote:
> Use error_setg() to return an error rather than an explicit exit().
> Previously it was an exit(0) instead of a non-zero exit code, which was
> simply a bug.  Also improve the error message.
> 
> While we're at it change the type of spapr_vga_init() to bool since that's
> how we're using it anyway.
> 
> Signed-off-by: David Gibson 
> ---
>  hw/ppc/spapr.c | 10 +-
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 87097bc..bb5eaa5 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1246,7 +1246,7 @@ static void spapr_rtc_create(sPAPRMachineState *spapr)
>  }
>  
>  /* Returns whether we want to use VGA or not */
> -static int spapr_vga_init(PCIBus *pci_bus)
> +static bool spapr_vga_init(PCIBus *pci_bus, Error **errp)
>  {
>  switch (vga_interface_type) {
>  case VGA_NONE:
> @@ -1257,9 +1257,9 @@ static int spapr_vga_init(PCIBus *pci_bus)
>  case VGA_VIRTIO:
>  return pci_vga_init(pci_bus) != NULL;
>  default:
> -fprintf(stderr, "This vga model is not supported,"
> -"currently it only supports -vga std\n");
> -exit(0);
> +error_setg(errp,
> +   "Unsupported VGA mode, only -vga std or -vga virtio is 
> supported");
> +return false;
>  }
>  }
>  
> @@ -1934,7 +1934,7 @@ static void ppc_spapr_init(MachineState *machine)
>  }
>  
>  /* Graphics */
> -if (spapr_vga_init(phb->bus)) {
> +if (spapr_vga_init(phb->bus, _fatal)) {
>  spapr->has_graphics = true;
>  machine->usb |= defaults_enabled() && !machine->usb_disabled;
>  }

Reviewed-by: Thomas Huth 

[Qemu-devel] [PATCHv3 5/9] pseries: Cleanup error handling in spapr_vga_init()

[David Gibson]

Use error_setg() to return an error rather than an explicit exit().
Previously it was an exit(0) instead of a non-zero exit code, which was
simply a bug.  Also improve the error message.

While we're at it change the type of spapr_vga_init() to bool since that's
how we're using it anyway.

Signed-off-by: David Gibson 
---
 hw/ppc/spapr.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 87097bc..bb5eaa5 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1246,7 +1246,7 @@ static void spapr_rtc_create(sPAPRMachineState *spapr)
 }
 
 /* Returns whether we want to use VGA or not */
-static int spapr_vga_init(PCIBus *pci_bus)
+static bool spapr_vga_init(PCIBus *pci_bus, Error **errp)
 {
 switch (vga_interface_type) {
 case VGA_NONE:
@@ -1257,9 +1257,9 @@ static int spapr_vga_init(PCIBus *pci_bus)
 case VGA_VIRTIO:
 return pci_vga_init(pci_bus) != NULL;
 default:
-fprintf(stderr, "This vga model is not supported,"
-"currently it only supports -vga std\n");
-exit(0);
+error_setg(errp,
+   "Unsupported VGA mode, only -vga std or -vga virtio is 
supported");
+return false;
 }
 }
 
@@ -1934,7 +1934,7 @@ static void ppc_spapr_init(MachineState *machine)
 }
 
 /* Graphics */
-if (spapr_vga_init(phb->bus)) {
+if (spapr_vga_init(phb->bus, _fatal)) {
 spapr->has_graphics = true;
 machine->usb |= defaults_enabled() && !machine->usb_disabled;
 }
-- 
2.5.0