* [Qemu-devel] [PULL 0/3] Error reporting patches for 2016-02-09
@ 2016-02-09 13:25 Markus Armbruster
2016-02-09 13:25 ` [Qemu-devel] [PULL 1/3] Use error_fatal to simplify obvious fatal errors (again) Markus Armbruster
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Markus Armbruster @ 2016-02-09 13:25 UTC (permalink / raw)
To: qemu-devel
The following changes since commit e4a096b1cd4350eeca5dcdc391ab333d2083d7fd:
ui/cocoa.m: Include qemu/osdep.h (2016-02-08 13:14:40 +0000)
are available in the git repository at:
git://repo.or.cz/qemu/armbru.git tags/pull-error-2016-02-09
for you to fetch changes up to d76a3bf5c46d9e981faf09e6194c795ee54e5ae4:
HACKING: Add a section on error handling and reporting (2016-02-09 13:19:49 +0100)
----------------------------------------------------------------
Error reporting patches for 2016-02-09
----------------------------------------------------------------
Markus Armbruster (3):
Use error_fatal to simplify obvious fatal errors (again)
error: Improve documentation some more
HACKING: Add a section on error handling and reporting
HACKING | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++
hw/arm/imx25_pdk.c | 7 +------
hw/arm/kzm.c | 7 +------
hw/arm/netduino2.c | 7 +------
hw/arm/xlnx-ep108.c | 7 +------
include/qapi/error.h | 15 ++++++++++----
vl.c | 7 +------
7 files changed, 71 insertions(+), 34 deletions(-)
--
2.4.3
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Qemu-devel] [PULL 1/3] Use error_fatal to simplify obvious fatal errors (again)
2016-02-09 13:25 [Qemu-devel] [PULL 0/3] Error reporting patches for 2016-02-09 Markus Armbruster
@ 2016-02-09 13:25 ` Markus Armbruster
2016-02-09 13:25 ` [Qemu-devel] [PULL 2/3] error: Improve documentation some more Markus Armbruster
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Markus Armbruster @ 2016-02-09 13:25 UTC (permalink / raw)
To: qemu-devel
Done with the Coccinelle semantic patch from commit 007b065, plus
manual clean up of dead variables.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1452783732-6581-1-git-send-email-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
hw/arm/imx25_pdk.c | 7 +------
hw/arm/kzm.c | 7 +------
hw/arm/netduino2.c | 7 +------
hw/arm/xlnx-ep108.c | 7 +------
vl.c | 7 +------
5 files changed, 5 insertions(+), 30 deletions(-)
diff --git a/hw/arm/imx25_pdk.c b/hw/arm/imx25_pdk.c
index b167e19..e3cffd1 100644
--- a/hw/arm/imx25_pdk.c
+++ b/hw/arm/imx25_pdk.c
@@ -65,7 +65,6 @@ static struct arm_boot_info imx25_pdk_binfo;
static void imx25_pdk_init(MachineState *machine)
{
IMX25PDK *s = g_new0(IMX25PDK, 1);
- Error *err = NULL;
unsigned int ram_size;
unsigned int alias_offset;
int i;
@@ -74,11 +73,7 @@ static void imx25_pdk_init(MachineState *machine)
object_property_add_child(OBJECT(machine), "soc", OBJECT(&s->soc),
&error_abort);
- object_property_set_bool(OBJECT(&s->soc), true, "realized", &err);
- if (err != NULL) {
- error_report_err(err);
- exit(1);
- }
+ object_property_set_bool(OBJECT(&s->soc), true, "realized", &error_fatal);
/* We need to initialize our memory */
if (machine->ram_size > (FSL_IMX25_SDRAM0_SIZE + FSL_IMX25_SDRAM1_SIZE)) {
diff --git a/hw/arm/kzm.c b/hw/arm/kzm.c
index 55fb360..2eeb9d8 100644
--- a/hw/arm/kzm.c
+++ b/hw/arm/kzm.c
@@ -64,7 +64,6 @@ static struct arm_boot_info kzm_binfo = {
static void kzm_init(MachineState *machine)
{
IMX31KZM *s = g_new0(IMX31KZM, 1);
- Error *err = NULL;
unsigned int ram_size;
unsigned int alias_offset;
unsigned int i;
@@ -73,11 +72,7 @@ static void kzm_init(MachineState *machine)
object_property_add_child(OBJECT(machine), "soc", OBJECT(&s->soc),
&error_abort);
- object_property_set_bool(OBJECT(&s->soc), true, "realized", &err);
- if (err != NULL) {
- error_report_err(err);
- exit(1);
- }
+ object_property_set_bool(OBJECT(&s->soc), true, "realized", &error_fatal);
/* Check the amount of memory is compatible with the SOC */
if (machine->ram_size > (FSL_IMX31_SDRAM0_SIZE + FSL_IMX31_SDRAM1_SIZE)) {
diff --git a/hw/arm/netduino2.c b/hw/arm/netduino2.c
index 86065d3..49da6e3 100644
--- a/hw/arm/netduino2.c
+++ b/hw/arm/netduino2.c
@@ -30,18 +30,13 @@
static void netduino2_init(MachineState *machine)
{
DeviceState *dev;
- Error *err = NULL;
dev = qdev_create(NULL, TYPE_STM32F205_SOC);
if (machine->kernel_filename) {
qdev_prop_set_string(dev, "kernel-filename", machine->kernel_filename);
}
qdev_prop_set_string(dev, "cpu-model", "cortex-m3");
- object_property_set_bool(OBJECT(dev), true, "realized", &err);
- if (err != NULL) {
- error_report_err(err);
- exit(1);
- }
+ object_property_set_bool(OBJECT(dev), true, "realized", &error_fatal);
}
static void netduino2_machine_init(MachineClass *mc)
diff --git a/hw/arm/xlnx-ep108.c b/hw/arm/xlnx-ep108.c
index 2cd69b5..0de132a 100644
--- a/hw/arm/xlnx-ep108.c
+++ b/hw/arm/xlnx-ep108.c
@@ -32,7 +32,6 @@ static void xlnx_ep108_init(MachineState *machine)
{
XlnxEP108 *s = g_new0(XlnxEP108, 1);
int i;
- Error *err = NULL;
uint64_t ram_size = machine->ram_size;
/* Create the memory region to pass to the SoC */
@@ -58,11 +57,7 @@ static void xlnx_ep108_init(MachineState *machine)
object_property_set_link(OBJECT(&s->soc), OBJECT(&s->ddr_ram),
"ddr-ram", &error_abort);
- object_property_set_bool(OBJECT(&s->soc), true, "realized", &err);
- if (err) {
- error_report_err(err);
- exit(1);
- }
+ object_property_set_bool(OBJECT(&s->soc), true, "realized", &error_fatal);
for (i = 0; i < XLNX_ZYNQMP_NUM_SPIS; i++) {
SSIBus *spi_bus;
diff --git a/vl.c b/vl.c
index c581e39..a5502ed 100644
--- a/vl.c
+++ b/vl.c
@@ -4555,12 +4555,7 @@ int main(int argc, char **argv, char **envp)
net_check_clients();
if (boot_once) {
- Error *local_err = NULL;
- qemu_boot_set(boot_once, &local_err);
- if (local_err) {
- error_report_err(local_err);
- exit(1);
- }
+ qemu_boot_set(boot_once, &error_fatal);
qemu_register_reset(restore_boot_order, g_strdup(boot_order));
}
--
2.4.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Qemu-devel] [PULL 2/3] error: Improve documentation some more
2016-02-09 13:25 [Qemu-devel] [PULL 0/3] Error reporting patches for 2016-02-09 Markus Armbruster
2016-02-09 13:25 ` [Qemu-devel] [PULL 1/3] Use error_fatal to simplify obvious fatal errors (again) Markus Armbruster
@ 2016-02-09 13:25 ` Markus Armbruster
2016-02-09 13:25 ` [Qemu-devel] [PULL 3/3] HACKING: Add a section on error handling and reporting Markus Armbruster
2016-02-09 17:44 ` [Qemu-devel] [PULL 0/3] Error reporting patches for 2016-02-09 Peter Maydell
3 siblings, 0 replies; 5+ messages in thread
From: Markus Armbruster @ 2016-02-09 13:25 UTC (permalink / raw)
To: qemu-devel
Don't claim error_report_err() always reports to stderr. It actually
reports to the current monitor when we have one.
Clarify intended use of error_abort and error_fatal.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1454522628-28294-2-git-send-email-armbru@redhat.com>
Reviewed-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
include/qapi/error.h | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/include/qapi/error.h b/include/qapi/error.h
index 45d6c72..e64fe54 100644
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -27,11 +27,11 @@
* error_setg(&err, "invalid quark\n"
* "Valid quarks are up, down, strange, charm, top, bottom.");
*
- * Report an error to stderr:
+ * Report an error to the current monitor if we have one, else stderr:
* error_report_err(err);
* This frees the error object.
*
- * Report an error to stderr with additional text prepended:
+ * Likewise, but with additional text prepended:
* error_reportf_err(err, "Could not frobnicate '%s': ", name);
*
* Report an error somewhere else:
@@ -162,6 +162,9 @@ ErrorClass error_get_class(const Error *err);
* human-readable error message is made from printf-style @fmt, ...
* The resulting message should be a single phrase, with no newline or
* trailing punctuation.
+ * Please don't error_setg(&error_fatal, ...), use error_report() and
+ * exit(), because that's more obvious.
+ * Likewise, don't error_setg(&error_abort, ...), use assert().
*/
#define error_setg(errp, fmt, ...) \
error_setg_internal((errp), __FILE__, __LINE__, __func__, \
@@ -213,6 +216,8 @@ void error_setg_win32_internal(Error **errp,
* the error object.
* Else, move the error object from @local_err to *@dst_errp.
* On return, @local_err is invalid.
+ * Please don't error_propagate(&error_fatal, ...), use
+ * error_report_err() and exit(), because that's more obvious.
*/
void error_propagate(Error **dst_errp, Error *local_err);
@@ -291,12 +296,14 @@ void error_set_internal(Error **errp,
GCC_FMT_ATTR(6, 7);
/*
- * Pass to error_setg() & friends to abort() on error.
+ * Special error destination to abort on error.
+ * See error_setg() and error_propagate() for details.
*/
extern Error *error_abort;
/*
- * Pass to error_setg() & friends to exit(1) on error.
+ * Special error destination to exit(1) on error.
+ * See error_setg() and error_propagate() for details.
*/
extern Error *error_fatal;
--
2.4.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [Qemu-devel] [PULL 3/3] HACKING: Add a section on error handling and reporting
2016-02-09 13:25 [Qemu-devel] [PULL 0/3] Error reporting patches for 2016-02-09 Markus Armbruster
2016-02-09 13:25 ` [Qemu-devel] [PULL 1/3] Use error_fatal to simplify obvious fatal errors (again) Markus Armbruster
2016-02-09 13:25 ` [Qemu-devel] [PULL 2/3] error: Improve documentation some more Markus Armbruster
@ 2016-02-09 13:25 ` Markus Armbruster
2016-02-09 17:44 ` [Qemu-devel] [PULL 0/3] Error reporting patches for 2016-02-09 Peter Maydell
3 siblings, 0 replies; 5+ messages in thread
From: Markus Armbruster @ 2016-02-09 13:25 UTC (permalink / raw)
To: qemu-devel
Inspired by an RFC PATCH from Lluís Vilanova.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1454522628-28294-3-git-send-email-armbru@redhat.com>
Reviewed-by: Lluís Vilanova <vilanova@ac.upc.edu>
---
HACKING | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 55 insertions(+)
diff --git a/HACKING b/HACKING
index 12fbc8a..058aa8f 100644
--- a/HACKING
+++ b/HACKING
@@ -157,3 +157,58 @@ painful. These are:
* you may assume that integers are 2s complement representation
* you may assume that right shift of a signed integer duplicates
the sign bit (ie it is an arithmetic shift, not a logical shift)
+
+7. Error handling and reporting
+
+7.1 Reporting errors to the human user
+
+Do not use printf(), fprintf() or monitor_printf(). Instead, use
+error_report() or error_vreport() from error-report.h. This ensures the
+error is reported in the right place (current monitor or stderr), and in
+a uniform format.
+
+Use error_printf() & friends to print additional information.
+
+error_report() prints the current location. In certain common cases
+like command line parsing, the current location is tracked
+automatically. To manipulate it manually, use the loc_*() from
+error-report.h.
+
+7.2 Propagating errors
+
+An error can't always be reported to the user right where it's detected,
+but often needs to be propagated up the call chain to a place that can
+handle it. This can be done in various ways.
+
+The most flexible one is Error objects. See error.h for usage
+information.
+
+Use the simplest suitable method to communicate success / failure to
+callers. Stick to common methods: non-negative on success / -1 on
+error, non-negative / -errno, non-null / null, or Error objects.
+
+Example: when a function returns a non-null pointer on success, and it
+can fail only in one way (as far as the caller is concerned), returning
+null on failure is just fine, and certainly simpler and a lot easier on
+the eyes than propagating an Error object through an Error ** parameter.
+
+Example: when a function's callers need to report details on failure
+only the function really knows, use Error **, and set suitable errors.
+
+Do not report an error to the user when you're also returning an error
+for somebody else to handle. Leave the reporting to the place that
+consumes the error returned.
+
+7.3 Handling errors
+
+Calling exit() is fine when handling configuration errors during
+startup. It's problematic during normal operation. In particular,
+monitor commands should never exit().
+
+Do not call exit() or abort() to handle an error that can be triggered
+by the guest (e.g., some unimplemented corner case in guest code
+translation or device emulation). Guests should not be able to
+terminate QEMU.
+
+Note that &error_fatal is just another way to exit(1), and &error_abort
+is just another way to abort().
--
2.4.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PULL 0/3] Error reporting patches for 2016-02-09
2016-02-09 13:25 [Qemu-devel] [PULL 0/3] Error reporting patches for 2016-02-09 Markus Armbruster
` (2 preceding siblings ...)
2016-02-09 13:25 ` [Qemu-devel] [PULL 3/3] HACKING: Add a section on error handling and reporting Markus Armbruster
@ 2016-02-09 17:44 ` Peter Maydell
3 siblings, 0 replies; 5+ messages in thread
From: Peter Maydell @ 2016-02-09 17:44 UTC (permalink / raw)
To: Markus Armbruster; +Cc: QEMU Developers
On 9 February 2016 at 13:25, Markus Armbruster <armbru@redhat.com> wrote:
> The following changes since commit e4a096b1cd4350eeca5dcdc391ab333d2083d7fd:
>
> ui/cocoa.m: Include qemu/osdep.h (2016-02-08 13:14:40 +0000)
>
> are available in the git repository at:
>
> git://repo.or.cz/qemu/armbru.git tags/pull-error-2016-02-09
>
> for you to fetch changes up to d76a3bf5c46d9e981faf09e6194c795ee54e5ae4:
>
> HACKING: Add a section on error handling and reporting (2016-02-09 13:19:49 +0100)
>
> ----------------------------------------------------------------
> Error reporting patches for 2016-02-09
>
> ----------------------------------------------------------------
> Markus Armbruster (3):
> Use error_fatal to simplify obvious fatal errors (again)
> error: Improve documentation some more
> HACKING: Add a section on error handling and reporting
>
Applied, thanks.
-- PMM
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-02-09 17:45 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-09 13:25 [Qemu-devel] [PULL 0/3] Error reporting patches for 2016-02-09 Markus Armbruster
2016-02-09 13:25 ` [Qemu-devel] [PULL 1/3] Use error_fatal to simplify obvious fatal errors (again) Markus Armbruster
2016-02-09 13:25 ` [Qemu-devel] [PULL 2/3] error: Improve documentation some more Markus Armbruster
2016-02-09 13:25 ` [Qemu-devel] [PULL 3/3] HACKING: Add a section on error handling and reporting Markus Armbruster
2016-02-09 17:44 ` [Qemu-devel] [PULL 0/3] Error reporting patches for 2016-02-09 Peter Maydell
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.