From 8d780f43921feb7fd8d0b58f779a22d1265f2378 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 17 Nov 2015 17:05:49 +0100 Subject: error: Document how to accumulate multiple errors Suggested-by: Eric Blake Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-Id: <1447776349-2344-1-git-send-email-armbru@redhat.com> --- include/qapi/error.h | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) (limited to 'include') diff --git a/include/qapi/error.h b/include/qapi/error.h index 6285cf5..1480f59 100644 --- a/include/qapi/error.h +++ b/include/qapi/error.h @@ -76,6 +76,23 @@ * But when all you do with the error is pass it on, please use * foo(arg, errp); * for readability. + * + * Receive and accumulate multiple errors (first one wins): + * Error *err = NULL, *local_err = NULL; + * foo(arg, &err); + * bar(arg, &local_err); + * error_propagate(&err, local_err); + * if (err) { + * handle the error... + * } + * + * Do *not* "optimize" this to + * foo(arg, &err); + * bar(arg, &err); // WRONG! + * if (err) { + * handle the error... + * } + * because this may pass a non-null err to bar(). */ #ifndef ERROR_H -- cgit v1.1 From 6231a6da9f40c3a33589402c9c57557e3a4f05ad Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 10 Dec 2015 17:29:15 +0100 Subject: hw: Inline the qdev_prop_set_drive_nofail() wrapper Signed-off-by: Markus Armbruster Message-Id: <1449764955-10741-3-git-send-email-armbru@redhat.com> Reviewed-by: Peter Maydell --- include/hw/qdev-properties.h | 2 -- 1 file changed, 2 deletions(-) (limited to 'include') diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h index 77538a8..254afd8 100644 --- a/include/hw/qdev-properties.h +++ b/include/hw/qdev-properties.h @@ -180,8 +180,6 @@ void qdev_prop_set_chr(DeviceState *dev, const char *name, CharDriverState *valu void qdev_prop_set_netdev(DeviceState *dev, const char *name, NetClientState *value); void qdev_prop_set_drive(DeviceState *dev, const char *name, BlockBackend *value, Error **errp); -void qdev_prop_set_drive_nofail(DeviceState *dev, const char *name, - BlockBackend *value); void qdev_prop_set_macaddr(DeviceState *dev, const char *name, uint8_t *value); void qdev_prop_set_enum(DeviceState *dev, const char *name, int value); /* FIXME: Remove opaque pointer properties. */ -- cgit v1.1 From d10e54329bbfe6bfacf75eb33caead39eef9f3c8 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Thu, 17 Dec 2015 17:35:18 +0100 Subject: isa: Clean up error handling around isa_bus_new() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We can have at most one ISA bus. If you try to create another one, isa_bus_new() complains to stderr and returns null. isa_bus_new() is called in two contexts, machine's init() and device's realize() methods. Since complaining to stderr is not proper in the latter context, convert isa_bus_new() to Error. Machine's init(): * mips_jazz_init(), called from the init() methods of machines "magnum" and "pica" * mips_r4k_init(), the init() method of machine "mips" * pc_init1() called from the init() methods of non-q35 PC machines * typhoon_init(), called from clipper_init(), the init() method of machine "clipper" These callers always create the first ISA bus, hence isa_bus_new() can't fail. Simply pass &error_abort. Device's realize(): * i82378_realize(), of PCI device "i82378" * ich9_lpc_realize(), of PCI device "ICH9-LPC" * pci_ebus_realize(), of PCI device "ebus" * piix3_realize(), of PCI device "pci-piix3", abstract parent of "PIIX3" and "PIIX3-xen" * piix4_realize(), of PCI device "PIIX4" * vt82c686b_realize(), of PCI device "VT82C686B" Propagate the error. Note that these devices are typically created only by machine init() methods with qdev_init_nofail() or similar. If we screwed up and created an ISA bus before that call, we now give up right away. Before, we'd hobble on, and typically die in isa_bus_irqs(). Similar if someone finds a way to hot-plug one of these critters. Cc: Richard Henderson Cc: "Michael S. Tsirkin" Cc: "Hervé Poussineau" Cc: Aurelien Jarno Cc: Mark Cave-Ayland Signed-off-by: Markus Armbruster Reviewed-by: Marcel Apfelbaum Reviewed-by: Hervé Poussineau Reviewed-by: Michael S. Tsirkin Message-Id: <1450370121-5768-11-git-send-email-armbru@redhat.com> --- include/hw/isa/isa.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'include') diff --git a/include/hw/isa/isa.h b/include/hw/isa/isa.h index d758b39..de3cd3d 100644 --- a/include/hw/isa/isa.h +++ b/include/hw/isa/isa.h @@ -59,7 +59,7 @@ struct ISADevice { }; ISABus *isa_bus_new(DeviceState *dev, MemoryRegion *address_space, - MemoryRegion *address_space_io); + MemoryRegion *address_space_io, Error **errp); void isa_bus_irqs(ISABus *bus, qemu_irq *irqs); qemu_irq isa_get_irq(ISADevice *dev, int isairq); void isa_init_irq(ISADevice *dev, qemu_irq *p, int isairq); -- cgit v1.1 From f4d0064afcff4c38b379800674938cde8f069dcd Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 18 Dec 2015 16:35:08 +0100 Subject: error: Improve documentation While there, tighten error_append_hint()'s assertion. Signed-off-by: Markus Armbruster Message-Id: <1450452927-8346-6-git-send-email-armbru@redhat.com> Reviewed-by: Eric Blake --- include/qapi/error.h | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) (limited to 'include') diff --git a/include/qapi/error.h b/include/qapi/error.h index 1480f59..b18a608 100644 --- a/include/qapi/error.h +++ b/include/qapi/error.h @@ -18,6 +18,15 @@ * Create an error: * error_setg(&err, "situation normal, all fouled up"); * + * Create an error and add additional explanation: + * error_setg(&err, "invalid quark"); + * error_append_hint(&err, "Valid quarks are up, down, strange, " + * "charm, top, bottom.\n"); + * + * Do *not* contract this to + * error_setg(&err, "invalid quark\n" + * "Valid quarks are up, down, strange, charm, top, bottom."); + * * Report an error to stderr: * error_report_err(err); * This frees the error object. @@ -26,6 +35,7 @@ * const char *msg = error_get_pretty(err); * do with msg what needs to be done... * error_free(err); + * Note that this loses hints added with error_append_hint(). * * Handle an error without reporting it (just for completeness): * error_free(err); @@ -142,6 +152,8 @@ ErrorClass error_get_class(const Error *err); * If @errp is anything else, *@errp must be NULL. * The new error's class is ERROR_CLASS_GENERIC_ERROR, and its * human-readable error message is made from printf-style @fmt, ... + * The resulting message should be a single phrase, with no newline or + * trailing punctuation. */ #define error_setg(errp, fmt, ...) \ error_setg_internal((errp), __FILE__, __LINE__, __func__, \ @@ -198,7 +210,11 @@ void error_propagate(Error **dst_errp, Error *local_err); /** * Append a printf-style human-readable explanation to an existing error. - * May be called multiple times, and safe if @errp is NULL. + * @errp may be NULL, but not &error_fatal or &error_abort. + * Trivially the case if you call it only after error_setg() or + * error_propagate(). + * May be called multiple times. The resulting hint should end with a + * newline. */ void error_append_hint(Error **errp, const char *fmt, ...) GCC_FMT_ATTR(2, 3); @@ -232,7 +248,7 @@ void error_free_or_abort(Error **errp); /* * Convenience function to error_report() and free @err. */ -void error_report_err(Error *); +void error_report_err(Error *err); /* * Just like error_setg(), except you get to specify the error class. -- cgit v1.1 From 8277d2aa58fe4f8f3ee394ea647ea652faf822a4 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 18 Dec 2015 16:35:12 +0100 Subject: error: New error_prepend(), error_reportf_err() Instead of simply propagating an error verbatim, we sometimes want to add to its message, like this: frobnicate(arg, &err); error_setg(errp, "Can't frobnicate %s: %s", arg, error_get_pretty(err)); error_free(err); This is suboptimal, because it loses err's hint (if any). Moreover, when errp is &error_abort or is subsequently propagated to &error_abort, the abort message points to the place where we last added to the error, not to the place where it originated. To avoid these issues, provide means to add to an error's message in place: frobnicate(arg, errp); error_prepend(errp, "Can't frobnicate %s: ", arg); Likewise, reporting an error like frobnicate(arg, &err); error_report("Can't frobnicate %s: %s", arg, error_get_pretty(err)); can lose err's hint. To avoid: error_reportf_err(err, "Can't frobnicate %s: ", arg); The next commits will put these functions to use. Signed-off-by: Markus Armbruster Reviewed-by: Eric Blake Message-Id: <1450452927-8346-10-git-send-email-armbru@redhat.com> --- include/qapi/error.h | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) (limited to 'include') diff --git a/include/qapi/error.h b/include/qapi/error.h index b18a608..45d6c72 100644 --- a/include/qapi/error.h +++ b/include/qapi/error.h @@ -31,6 +31,9 @@ * error_report_err(err); * This frees the error object. * + * Report an error to stderr with additional text prepended: + * error_reportf_err(err, "Could not frobnicate '%s': ", name); + * * Report an error somewhere else: * const char *msg = error_get_pretty(err); * do with msg what needs to be done... @@ -48,6 +51,10 @@ * error_propagate(errp, err); * where Error **errp is a parameter, by convention the last one. * + * Pass an existing error to the caller with the message modified: + * error_propagate(errp, err); + * error_prepend(errp, "Could not frobnicate '%s': ", name); + * * Create a new error and pass it to the caller: * error_setg(errp, "situation normal, all fouled up"); * @@ -108,9 +115,10 @@ #ifndef ERROR_H #define ERROR_H +#include +#include #include "qemu/compiler.h" #include "qapi-types.h" -#include /* * Opaque error object. @@ -208,7 +216,20 @@ void error_setg_win32_internal(Error **errp, */ void error_propagate(Error **dst_errp, Error *local_err); -/** +/* + * Prepend some text to @errp's human-readable error message. + * The text is made by formatting @fmt, @ap like vprintf(). + */ +void error_vprepend(Error **errp, const char *fmt, va_list ap); + +/* + * Prepend some text to @errp's human-readable error message. + * The text is made by formatting @fmt, ... like printf(). + */ +void error_prepend(Error **errp, const char *fmt, ...) + GCC_FMT_ATTR(2, 3); + +/* * Append a printf-style human-readable explanation to an existing error. * @errp may be NULL, but not &error_fatal or &error_abort. * Trivially the case if you call it only after error_setg() or @@ -251,6 +272,12 @@ void error_free_or_abort(Error **errp); void error_report_err(Error *err); /* + * Convenience function to error_prepend(), error_report() and free @err. + */ +void error_reportf_err(Error *err, const char *fmt, ...) + GCC_FMT_ATTR(2, 3); + +/* * Just like error_setg(), except you get to specify the error class. * Note: use of error classes other than ERROR_CLASS_GENERIC_ERROR is * strongly discouraged. -- cgit v1.1 From 533fdaedeb7f7ae90866312f57249ebbb7e3c97f Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 18 Dec 2015 16:35:27 +0100 Subject: error: Consistently name Error * objects err, and not errp Signed-off-by: Markus Armbruster Message-Id: <1450452927-8346-25-git-send-email-armbru@redhat.com> Reviewed-by: Eric Blake --- include/qemu/sockets.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'include') diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h index 74c692d..2e7f985 100644 --- a/include/qemu/sockets.h +++ b/include/qemu/sockets.h @@ -53,7 +53,7 @@ int recv_all(int fd, void *buf, int len1, bool single_read); /* callback function for nonblocking connect * valid fd on success, negative error code on failure */ -typedef void NonBlockingConnectHandler(int fd, Error *errp, void *opaque); +typedef void NonBlockingConnectHandler(int fd, Error *err, void *opaque); InetSocketAddress *inet_parse(const char *str, Error **errp); int inet_listen_opts(QemuOpts *opts, int port_offset, Error **errp); -- cgit v1.1