Re: [Qemu-devel] [PATCHv3 8/9] pseries: Clean up error reporting in ppc_spapr_init()
On Mon, Jan 18, 2016 at 10:31:42AM +0100, Thomas Huth wrote:
> On 18.01.2016 05:24, David Gibson wrote:
> > This function includes a number of explicit fprintf()s for errors.
> > Change these to use error_report() instead.
> >
> > Also replace the single exit(EXIT_FAILURE) with an explicit exit(1), since
> > the latter is the more usual idiom in qemu by a large margin.
> >
> > Signed-off-by: David Gibson
> > ---
> > hw/ppc/spapr.c | 25 +
> > 1 file changed, 13 insertions(+), 12 deletions(-)
> >
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 148ca5a..58f26cd 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -1789,8 +1789,8 @@ static void ppc_spapr_init(MachineState *machine)
> > }
> >
> > if (spapr->rma_size > node0_size) {
> > -fprintf(stderr, "Error: Numa node 0 has to span the RMA
> > (%#08"HWADDR_PRIx")\n",
> > -spapr->rma_size);
> > +error_report("Numa node 0 has to span the RMA (%#08"HWADDR_PRIx")",
> > + spapr->rma_size);
> > exit(1);
> > }
> >
> > @@ -1856,10 +1856,10 @@ static void ppc_spapr_init(MachineState *machine)
> > ram_addr_t hotplug_mem_size = machine->maxram_size -
> > machine->ram_size;
> >
> > if (machine->ram_slots > SPAPR_MAX_RAM_SLOTS) {
> > -error_report("Specified number of memory slots %" PRIu64
> > - " exceeds max supported %d",
> > - machine->ram_slots, SPAPR_MAX_RAM_SLOTS);
> > -exit(EXIT_FAILURE);
> > +error_report("Specified number of memory slots %"
> > + PRIu64" exceeds max supported %d",
> > +machine->ram_slots, SPAPR_MAX_RAM_SLOTS);
>
> Why did you change the indentation of the "machine->ram_slots, ..." line
> here? The original looked better to me.
I don't know. Probably just a mistake on my part, I'll fix it.
> > +exit(1);
>
> EXIT_FAILURE still seems to be used quite often in the QEMU sources...
> All in all, this hunk does not really change anything from a functional
> point of view, so I'd like to suggest to omit this hunk completely
> instead to avoid code churn here.
This I'll keep, as Markus says for local consistency.
--
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
Re: [Qemu-devel] [PATCHv3 8/9] pseries: Clean up error reporting in ppc_spapr_init()
Thomas Huth writes:
> On 18.01.2016 05:24, David Gibson wrote:
>> This function includes a number of explicit fprintf()s for errors.
>> Change these to use error_report() instead.
>>
>> Also replace the single exit(EXIT_FAILURE) with an explicit exit(1), since
>> the latter is the more usual idiom in qemu by a large margin.
>>
>> Signed-off-by: David Gibson
>> ---
>> hw/ppc/spapr.c | 25 +
>> 1 file changed, 13 insertions(+), 12 deletions(-)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 148ca5a..58f26cd 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -1789,8 +1789,8 @@ static void ppc_spapr_init(MachineState *machine)
>> }
>>
>> if (spapr->rma_size > node0_size) {
>> -fprintf(stderr, "Error: Numa node 0 has to span the RMA
>> (%#08"HWADDR_PRIx")\n",
>> -spapr->rma_size);
>> +error_report("Numa node 0 has to span the RMA (%#08"HWADDR_PRIx")",
>> + spapr->rma_size);
>> exit(1);
>> }
>>
>> @@ -1856,10 +1856,10 @@ static void ppc_spapr_init(MachineState *machine)
>> ram_addr_t hotplug_mem_size = machine->maxram_size -
>> machine->ram_size;
>>
>> if (machine->ram_slots > SPAPR_MAX_RAM_SLOTS) {
>> -error_report("Specified number of memory slots %" PRIu64
>> - " exceeds max supported %d",
>> - machine->ram_slots, SPAPR_MAX_RAM_SLOTS);
>> -exit(EXIT_FAILURE);
>> +error_report("Specified number of memory slots %"
>> + PRIu64" exceeds max supported %d",
>> +machine->ram_slots, SPAPR_MAX_RAM_SLOTS);
>
> Why did you change the indentation of the "machine->ram_slots, ..." line
> here? The original looked better to me.
Agreed.
>> +exit(1);
>
> EXIT_FAILURE still seems to be used quite often in the QEMU sources...
> All in all, this hunk does not really change anything from a functional
> point of view, so I'd like to suggest to omit this hunk completely
> instead to avoid code churn here.
It makes the code locally consistent, so I'd keep it.
Re: [Qemu-devel] [PATCHv3 8/9] pseries: Clean up error reporting in ppc_spapr_init()
On 18.01.2016 05:24, David Gibson wrote:
> This function includes a number of explicit fprintf()s for errors.
> Change these to use error_report() instead.
>
> Also replace the single exit(EXIT_FAILURE) with an explicit exit(1), since
> the latter is the more usual idiom in qemu by a large margin.
>
> Signed-off-by: David Gibson
> ---
> hw/ppc/spapr.c | 25 +
> 1 file changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 148ca5a..58f26cd 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1789,8 +1789,8 @@ static void ppc_spapr_init(MachineState *machine)
> }
>
> if (spapr->rma_size > node0_size) {
> -fprintf(stderr, "Error: Numa node 0 has to span the RMA
> (%#08"HWADDR_PRIx")\n",
> -spapr->rma_size);
> +error_report("Numa node 0 has to span the RMA (%#08"HWADDR_PRIx")",
> + spapr->rma_size);
> exit(1);
> }
>
> @@ -1856,10 +1856,10 @@ static void ppc_spapr_init(MachineState *machine)
> ram_addr_t hotplug_mem_size = machine->maxram_size -
> machine->ram_size;
>
> if (machine->ram_slots > SPAPR_MAX_RAM_SLOTS) {
> -error_report("Specified number of memory slots %" PRIu64
> - " exceeds max supported %d",
> - machine->ram_slots, SPAPR_MAX_RAM_SLOTS);
> -exit(EXIT_FAILURE);
> +error_report("Specified number of memory slots %"
> + PRIu64" exceeds max supported %d",
> +machine->ram_slots, SPAPR_MAX_RAM_SLOTS);
Why did you change the indentation of the "machine->ram_slots, ..." line
here? The original looked better to me.
> +exit(1);
EXIT_FAILURE still seems to be used quite often in the QEMU sources...
All in all, this hunk does not really change anything from a functional
point of view, so I'd like to suggest to omit this hunk completely
instead to avoid code churn here.
Thomas
[Qemu-devel] [PATCHv3 8/9] pseries: Clean up error reporting in ppc_spapr_init()
This function includes a number of explicit fprintf()s for errors.
Change these to use error_report() instead.
Also replace the single exit(EXIT_FAILURE) with an explicit exit(1), since
the latter is the more usual idiom in qemu by a large margin.
Signed-off-by: David Gibson
---
hw/ppc/spapr.c | 25 +
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 148ca5a..58f26cd 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1789,8 +1789,8 @@ static void ppc_spapr_init(MachineState *machine)
}
if (spapr->rma_size > node0_size) {
-fprintf(stderr, "Error: Numa node 0 has to span the RMA
(%#08"HWADDR_PRIx")\n",
-spapr->rma_size);
+error_report("Numa node 0 has to span the RMA (%#08"HWADDR_PRIx")",
+ spapr->rma_size);
exit(1);
}
@@ -1856,10 +1856,10 @@ static void ppc_spapr_init(MachineState *machine)
ram_addr_t hotplug_mem_size = machine->maxram_size - machine->ram_size;
if (machine->ram_slots > SPAPR_MAX_RAM_SLOTS) {
-error_report("Specified number of memory slots %" PRIu64
- " exceeds max supported %d",
- machine->ram_slots, SPAPR_MAX_RAM_SLOTS);
-exit(EXIT_FAILURE);
+error_report("Specified number of memory slots %"
+ PRIu64" exceeds max supported %d",
+machine->ram_slots, SPAPR_MAX_RAM_SLOTS);
+exit(1);
}
spapr->hotplug_memory.base = ROUND_UP(machine->ram_size,
@@ -1955,8 +1955,9 @@ static void ppc_spapr_init(MachineState *machine)
}
if (spapr->rma_size < (MIN_RMA_SLOF << 20)) {
-fprintf(stderr, "qemu: pSeries SLOF firmware requires >= "
-"%ldM guest RMA (Real Mode Area memory)\n", MIN_RMA_SLOF);
+error_report(
+"pSeries SLOF firmware requires >= %ldM guest RMA (Real Mode Area
memory)",
+MIN_RMA_SLOF);
exit(1);
}
@@ -1972,8 +1973,8 @@ static void ppc_spapr_init(MachineState *machine)
kernel_le = kernel_size > 0;
}
if (kernel_size < 0) {
-fprintf(stderr, "qemu: error loading %s: %s\n",
-kernel_filename, load_elf_strerror(kernel_size));
+error_report("error loading %s: %s",
+ kernel_filename, load_elf_strerror(kernel_size));
exit(1);
}
@@ -1986,8 +1987,8 @@ static void ppc_spapr_init(MachineState *machine)
initrd_size = load_image_targphys(initrd_filename, initrd_base,
load_limit - initrd_base);
if (initrd_size < 0) {
-fprintf(stderr, "qemu: could not load initial ram disk '%s'\n",
-initrd_filename);
+error_report("could not load initial ram disk '%s'",
+ initrd_filename);
exit(1);
}
} else {
--
2.5.0
