From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57866) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a8QgB-00082n-22 for qemu-devel@nongnu.org; Mon, 14 Dec 2015 05:48:32 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a8QgA-0005rU-2n for qemu-devel@nongnu.org; Mon, 14 Dec 2015 05:48:31 -0500 From: Markus Armbruster References: <1449743372-17169-1-git-send-email-armbru@redhat.com> <1449743372-17169-2-git-send-email-armbru@redhat.com> <566E94CA.40202@redhat.com> Date: Mon, 14 Dec 2015 11:48:20 +0100 In-Reply-To: <566E94CA.40202@redhat.com> (Thomas Huth's message of "Mon, 14 Dec 2015 11:07:06 +0100") Message-ID: <878u4xuwzv.fsf@blackfin.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH 01/12] hw: Don't use hw_error() for machine initialization errors List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Thomas Huth Cc: Guan Xuetao , qemu-ppc@nongnu.org, qemu-devel@nongnu.org, qemu-arm@nongnu.org, Richard Henderson Thomas Huth writes: > On 10/12/15 11:29, Markus Armbruster wrote: >> Printing CPU registers is not helpful during machine initialization. >> Moreover, these are straightforward configuration or "can get >> resources" errors, so dumping core isn't appropriate either. Replace >> hw_error() by error_report(); exit(1). Matches how we report these >> errors in other machine initializations. >> >> Cc: Richard Henderson >> Cc: qemu-arm@nongnu.org >> Cc: qemu-ppc@nongnu.org >> Cc: Guan Xuetao >> Signed-off-by: Markus Armbruster >> --- >> hw/alpha/dp264.c | 11 ++++++----- >> hw/arm/highbank.c | 6 ++++-- >> hw/char/exynos4210_uart.c | 9 ++++++--- >> hw/m68k/an5206.c | 4 +++- >> hw/ppc/mac_newworld.c | 11 ++++++----- >> hw/ppc/mac_oldworld.c | 16 +++++++++------- >> hw/ppc/prep.c | 10 ++++++---- >> hw/unicore32/puv3.c | 10 +++++++--- >> 8 files changed, 47 insertions(+), 30 deletions(-) > [...] >> diff --git a/hw/ppc/prep.c b/hw/ppc/prep.c >> index 5ad28f7..8f08f07 100644 >> --- a/hw/ppc/prep.c >> +++ b/hw/ppc/prep.c >> @@ -33,6 +33,7 @@ >> #include "hw/pci/pci_host.h" >> #include "hw/ppc/ppc.h" >> #include "hw/boards.h" >> +#include "qemu/error-report.h" >> #include "qemu/log.h" >> #include "hw/ide.h" >> #include "hw/loader.h" >> @@ -532,7 +533,7 @@ static void ppc_prep_init(MachineState *machine) >> kernel_size = load_image_targphys(kernel_filename, kernel_base, >> ram_size - kernel_base); >> if (kernel_size < 0) { >> - hw_error("qemu: could not load kernel '%s'\n", kernel_filename); >> + error_report("could not load kernel '%s'", kernel_filename); >> exit(1); >> } >> /* load initrd */ >> @@ -541,8 +542,8 @@ static void ppc_prep_init(MachineState *machine) >> initrd_size = load_image_targphys(initrd_filename, initrd_base, >> ram_size - initrd_base); >> if (initrd_size < 0) { >> - hw_error("qemu: could not load initial ram disk '%s'\n", >> - initrd_filename); >> + error_report("could not load initial ram disk '%s'", >> + initrd_filename); > > Shouldn't you add an "exit(1)" here, too? Yes. I rechecked the complete series for additional instances of this mistake, and found none. >> } >> } else { >> initrd_base = 0; > > Apart from the missing exit(), the patch looks like a good idea to me. Thanks!