diff options
| -rw-r--r-- | HACKING | 55 | ||||
| -rw-r--r-- | hw/arm/imx25_pdk.c | 7 | ||||
| -rw-r--r-- | hw/arm/kzm.c | 7 | ||||
| -rw-r--r-- | hw/arm/netduino2.c | 7 | ||||
| -rw-r--r-- | hw/arm/xlnx-ep108.c | 7 | ||||
| -rw-r--r-- | include/qapi/error.h | 15 | ||||
| -rw-r--r-- | vl.c | 7 |
7 files changed, 71 insertions, 34 deletions
@@ -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(). diff --git a/hw/arm/imx25_pdk.c b/hw/arm/imx25_pdk.c index b167e19f5a..e3cffd190c 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 55fb36021c..2eeb9d8973 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 86065d360a..49da6e360d 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 2cd69b5faf..0de132abd2 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/include/qapi/error.h b/include/qapi/error.h index 45d6c72dee..e64fe54cf9 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; @@ -4557,12 +4557,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)); } |