* [Qemu-devel] [RFC PATCH v1 01/25] exec: convert error_report to error_report_err
2015-09-11 5:33 [Qemu-devel] [RFC PATCH v1 00/25] error: Automatic error concatenation and prefixing Peter Crosthwaite
@ 2015-09-11 5:33 ` Peter Crosthwaite
2015-09-11 15:28 ` Eric Blake
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 02/25] s390x: virtio-ccw: Remove un-needed if guard Peter Crosthwaite
` (25 subsequent siblings)
26 siblings, 1 reply; 36+ messages in thread
From: Peter Crosthwaite @ 2015-09-11 5:33 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, armbru
This is a standard error_report_err. Convert it.
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
exec.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/exec.c b/exec.c
index 31d2dc7..1e7fad8 100644
--- a/exec.c
+++ b/exec.c
@@ -1241,7 +1241,7 @@ static void *file_ram_alloc(RAMBlock *block,
error:
if (mem_prealloc) {
- error_report("%s", error_get_pretty(*errp));
+ error_report_err(*errp);
exit(1);
}
return NULL;
--
1.9.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [RFC PATCH v1 02/25] s390x: virtio-ccw: Remove un-needed if guard
2015-09-11 5:33 [Qemu-devel] [RFC PATCH v1 00/25] error: Automatic error concatenation and prefixing Peter Crosthwaite
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 01/25] exec: convert error_report to error_report_err Peter Crosthwaite
@ 2015-09-11 5:33 ` Peter Crosthwaite
2015-09-11 15:28 ` Eric Blake
2015-09-14 7:09 ` Cornelia Huck
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 03/25] error: Factor out common error setter logic Peter Crosthwaite
` (24 subsequent siblings)
26 siblings, 2 replies; 36+ messages in thread
From: Peter Crosthwaite @ 2015-09-11 5:33 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, armbru
error_propagate is already NULL safe. Remove these un-needed if
guards.
Cc: Richard Henderson <rth@twiddle.net>
Cc: Alexander Graf <agraf@suse.de>
Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
hw/s390x/virtio-ccw.c | 28 +++++++---------------------
1 file changed, 7 insertions(+), 21 deletions(-)
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index d36373e..db085bc 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -957,9 +957,7 @@ static void virtio_ccw_net_realize(VirtioCcwDevice *ccw_dev, Error **errp)
object_get_typename(OBJECT(qdev)));
qdev_set_parent_bus(vdev, BUS(&ccw_dev->bus));
object_property_set_bool(OBJECT(vdev), true, "realized", &err);
- if (err) {
- error_propagate(errp, err);
- }
+ error_propagate(errp, err);
}
static void virtio_ccw_net_instance_init(Object *obj)
@@ -980,9 +978,7 @@ static void virtio_ccw_blk_realize(VirtioCcwDevice *ccw_dev, Error **errp)
qdev_set_parent_bus(vdev, BUS(&ccw_dev->bus));
object_property_set_bool(OBJECT(vdev), true, "realized", &err);
- if (err) {
- error_propagate(errp, err);
- }
+ error_propagate(errp, err);
}
static void virtio_ccw_blk_instance_init(Object *obj)
@@ -1017,9 +1013,7 @@ static void virtio_ccw_serial_realize(VirtioCcwDevice *ccw_dev, Error **errp)
qdev_set_parent_bus(vdev, BUS(&ccw_dev->bus));
object_property_set_bool(OBJECT(vdev), true, "realized", &err);
- if (err) {
- error_propagate(errp, err);
- }
+ error_propagate(errp, err);
}
@@ -1039,9 +1033,7 @@ static void virtio_ccw_balloon_realize(VirtioCcwDevice *ccw_dev, Error **errp)
qdev_set_parent_bus(vdev, BUS(&ccw_dev->bus));
object_property_set_bool(OBJECT(vdev), true, "realized", &err);
- if (err) {
- error_propagate(errp, err);
- }
+ error_propagate(errp, err);
}
static void virtio_ccw_balloon_instance_init(Object *obj)
@@ -1077,9 +1069,7 @@ static void virtio_ccw_scsi_realize(VirtioCcwDevice *ccw_dev, Error **errp)
qdev_set_parent_bus(vdev, BUS(&ccw_dev->bus));
object_property_set_bool(OBJECT(vdev), true, "realized", &err);
- if (err) {
- error_propagate(errp, err);
- }
+ error_propagate(errp, err);
}
static void virtio_ccw_scsi_instance_init(Object *obj)
@@ -1101,9 +1091,7 @@ static void vhost_ccw_scsi_realize(VirtioCcwDevice *ccw_dev, Error **errp)
qdev_set_parent_bus(vdev, BUS(&ccw_dev->bus));
object_property_set_bool(OBJECT(vdev), true, "realized", &err);
- if (err) {
- error_propagate(errp, err);
- }
+ error_propagate(errp, err);
}
static void vhost_ccw_scsi_instance_init(Object *obj)
@@ -1891,9 +1879,7 @@ static void virtio_ccw_9p_realize(VirtioCcwDevice *ccw_dev, Error **errp)
qdev_set_parent_bus(vdev, BUS(&ccw_dev->bus));
object_property_set_bool(OBJECT(vdev), true, "realized", &err);
- if (err) {
- error_propagate(errp, err);
- }
+ error_propagate(errp, err);
}
static void virtio_ccw_9p_class_init(ObjectClass *klass, void *data)
--
1.9.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [RFC PATCH v1 02/25] s390x: virtio-ccw: Remove un-needed if guard
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 02/25] s390x: virtio-ccw: Remove un-needed if guard Peter Crosthwaite
@ 2015-09-11 15:28 ` Eric Blake
2015-09-14 7:09 ` Cornelia Huck
1 sibling, 0 replies; 36+ messages in thread
From: Eric Blake @ 2015-09-11 15:28 UTC (permalink / raw)
To: Peter Crosthwaite, qemu-devel; +Cc: peter.maydell, armbru
[-- Attachment #1: Type: text/plain, Size: 633 bytes --]
On 09/10/2015 11:33 PM, Peter Crosthwaite wrote:
> error_propagate is already NULL safe. Remove these un-needed if
> guards.
>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Alexander Graf <agraf@suse.de>
> Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
> ---
>
> hw/s390x/virtio-ccw.c | 28 +++++++---------------------
> 1 file changed, 7 insertions(+), 21 deletions(-)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
We could probably do a Coccinelle patch to catch more offenders.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [RFC PATCH v1 02/25] s390x: virtio-ccw: Remove un-needed if guard
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 02/25] s390x: virtio-ccw: Remove un-needed if guard Peter Crosthwaite
2015-09-11 15:28 ` Eric Blake
@ 2015-09-14 7:09 ` Cornelia Huck
1 sibling, 0 replies; 36+ messages in thread
From: Cornelia Huck @ 2015-09-14 7:09 UTC (permalink / raw)
To: Peter Crosthwaite; +Cc: peter.maydell, qemu-devel, armbru
On Thu, 10 Sep 2015 22:33:12 -0700
Peter Crosthwaite <crosthwaitepeter@gmail.com> wrote:
> error_propagate is already NULL safe. Remove these un-needed if
> guards.
>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Alexander Graf <agraf@suse.de>
> Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
> ---
>
> hw/s390x/virtio-ccw.c | 28 +++++++---------------------
> 1 file changed, 7 insertions(+), 21 deletions(-)
Shorter code is good :)
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [RFC PATCH v1 03/25] error: Factor out common error setter logic
2015-09-11 5:33 [Qemu-devel] [RFC PATCH v1 00/25] error: Automatic error concatenation and prefixing Peter Crosthwaite
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 01/25] exec: convert error_report to error_report_err Peter Crosthwaite
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 02/25] s390x: virtio-ccw: Remove un-needed if guard Peter Crosthwaite
@ 2015-09-11 5:33 ` Peter Crosthwaite
2015-09-11 15:30 ` Eric Blake
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 04/25] error: Add support for multiple errors Peter Crosthwaite
` (23 subsequent siblings)
26 siblings, 1 reply; 36+ messages in thread
From: Peter Crosthwaite @ 2015-09-11 5:33 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, armbru
Currently set_error, set_error_errno, set_error_win32 do pretty much
the same thing with only a little bit of string manipulation in the
error code cases. Coreify that string manipulation as a callback and
factor out the bulk of the error_set fns to a common helper that calls
the callback at the right time.
Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
util/error.c | 81 +++++++++++++++++++++++++-----------------------------------
1 file changed, 33 insertions(+), 48 deletions(-)
diff --git a/util/error.c b/util/error.c
index 14f4351..b93e5c8 100644
--- a/util/error.c
+++ b/util/error.c
@@ -22,7 +22,9 @@ struct Error
Error *error_abort;
-void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...)
+static void do_error_set(Error **errp, ErrorClass err_class,
+ void (*mod)(Error *, void *), void *mod_opaque,
+ const char *fmt, ...)
{
Error *err;
va_list ap;
@@ -38,6 +40,9 @@ void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...)
va_start(ap, fmt);
err->msg = g_strdup_vprintf(fmt, ap);
va_end(ap);
+ if (mod) {
+ mod(err, mod_opaque);
+ }
err->err_class = err_class;
if (errp == &error_abort) {
@@ -50,40 +55,33 @@ void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...)
errno = saved_errno;
}
-void error_set_errno(Error **errp, int os_errno, ErrorClass err_class,
- const char *fmt, ...)
+void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...)
{
- Error *err;
- char *msg1;
va_list ap;
- int saved_errno = errno;
- if (errp == NULL) {
- return;
- }
- assert(*errp == NULL);
+ va_start(ap, fmt);
+ do_error_set(errp, err_class, NULL, NULL, fmt, ap);
+ va_end(ap);
+}
- err = g_malloc0(sizeof(*err));
+static void error_set_errno_mod(Error *err, void *opaque) {
+ int os_errno = *(int *)opaque;
+ char *msg1 = err->msg;
- va_start(ap, fmt);
- msg1 = g_strdup_vprintf(fmt, ap);
if (os_errno != 0) {
err->msg = g_strdup_printf("%s: %s", msg1, strerror(os_errno));
g_free(msg1);
- } else {
- err->msg = msg1;
- }
- va_end(ap);
- err->err_class = err_class;
-
- if (errp == &error_abort) {
- error_report_err(err);
- abort();
}
+}
- *errp = err;
+void error_set_errno(Error **errp, int os_errno, ErrorClass err_class,
+ const char *fmt, ...)
+{
+ va_list ap;
- errno = saved_errno;
+ va_start(ap, fmt);
+ do_error_set(errp, err_class, error_set_errno_mod, &os_errno, fmt, ap);
+ va_end(ap);
}
void error_setg_file_open(Error **errp, int os_errno, const char *filename)
@@ -93,40 +91,27 @@ void error_setg_file_open(Error **errp, int os_errno, const char *filename)
#ifdef _WIN32
-void error_set_win32(Error **errp, int win32_err, ErrorClass err_class,
- const char *fmt, ...)
-{
- Error *err;
- char *msg1;
- va_list ap;
+static void error_set_win32_mod(Error *err, void *opaque) {
+ int win32_err = *(int *)opaque;
+ char *msg1 = err->msg;
- if (errp == NULL) {
- return;
- }
- assert(*errp == NULL);
-
- err = g_malloc0(sizeof(*err));
-
- va_start(ap, fmt);
- msg1 = g_strdup_vprintf(fmt, ap);
if (win32_err != 0) {
char *msg2 = g_win32_error_message(win32_err);
err->msg = g_strdup_printf("%s: %s (error: %x)", msg1, msg2,
(unsigned)win32_err);
g_free(msg2);
g_free(msg1);
- } else {
- err->msg = msg1;
}
- va_end(ap);
- err->err_class = err_class;
+}
- if (errp == &error_abort) {
- error_report_err(err);
- abort();
- }
+void error_set_win32(Error **errp, int win32_err, ErrorClass err_class,
+ const char *fmt, ...)
+{
+ va_list ap;
- *errp = err;
+ va_start(ap, fmt);
+ do_error_set(errp, err_class, error_set_win32_mod, &win32_err, fmt, ap);
+ va_end(ap);
}
#endif
--
1.9.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [RFC PATCH v1 03/25] error: Factor out common error setter logic
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 03/25] error: Factor out common error setter logic Peter Crosthwaite
@ 2015-09-11 15:30 ` Eric Blake
0 siblings, 0 replies; 36+ messages in thread
From: Eric Blake @ 2015-09-11 15:30 UTC (permalink / raw)
To: Peter Crosthwaite, qemu-devel; +Cc: peter.maydell, armbru
[-- Attachment #1: Type: text/plain, Size: 823 bytes --]
On 09/10/2015 11:33 PM, Peter Crosthwaite wrote:
> Currently set_error, set_error_errno, set_error_win32 do pretty much
> the same thing with only a little bit of string manipulation in the
> error code cases. Coreify that string manipulation as a callback and
> factor out the bulk of the error_set fns to a common helper that calls
> the callback at the right time.
>
Commit 5523750 pretty much already does this. This patch can be dropped
when rebasing the series to latest.
> Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
> ---
>
> util/error.c | 81 +++++++++++++++++++++++++-----------------------------------
> 1 file changed, 33 insertions(+), 48 deletions(-)
>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [RFC PATCH v1 04/25] error: Add support for multiple errors
2015-09-11 5:33 [Qemu-devel] [RFC PATCH v1 00/25] error: Automatic error concatenation and prefixing Peter Crosthwaite
` (2 preceding siblings ...)
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 03/25] error: Factor out common error setter logic Peter Crosthwaite
@ 2015-09-11 5:33 ` Peter Crosthwaite
2015-09-11 15:49 ` Eric Blake
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 05/25] error: Add error prefix API Peter Crosthwaite
` (22 subsequent siblings)
26 siblings, 1 reply; 36+ messages in thread
From: Peter Crosthwaite @ 2015-09-11 5:33 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, armbru
Allow errors to stack. If an error is already set, don't assert,
instead, form a linked list. Recent errors are at the front of the
list, older ones at the back.
The assertion against the destination erro already being set is
removed.
copy/free are all to call their functionality recursively.
Propagation is implemented as concatenation of two error lists.
Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
qapi/common.json | 5 ++++-
util/error.c | 27 +++++++++++++++++++++------
2 files changed, 25 insertions(+), 7 deletions(-)
diff --git a/qapi/common.json b/qapi/common.json
index bad56bf..04175db 100644
--- a/qapi/common.json
+++ b/qapi/common.json
@@ -22,11 +22,14 @@
# @KVMMissingCap: the requested operation can't be fulfilled because a
# required KVM capability is missing
#
+# @MultipleErrors: Multiple errors have occured
+#
# Since: 1.2
##
{ 'enum': 'ErrorClass',
'data': [ 'GenericError', 'CommandNotFound', 'DeviceEncrypted',
- 'DeviceNotActive', 'DeviceNotFound', 'KVMMissingCap' ] }
+ 'DeviceNotActive', 'DeviceNotFound', 'KVMMissingCap',
+ 'MultipleErrors' ] }
##
# @VersionTriple
diff --git a/util/error.c b/util/error.c
index b93e5c8..890ce58 100644
--- a/util/error.c
+++ b/util/error.c
@@ -18,28 +18,25 @@ struct Error
{
char *msg;
ErrorClass err_class;
+ struct Error *next;
};
Error *error_abort;
static void do_error_set(Error **errp, ErrorClass err_class,
void (*mod)(Error *, void *), void *mod_opaque,
- const char *fmt, ...)
+ const char *fmt, va_list ap)
{
Error *err;
- va_list ap;
int saved_errno = errno;
if (errp == NULL) {
return;
}
- assert(*errp == NULL);
err = g_malloc0(sizeof(*err));
- va_start(ap, fmt);
err->msg = g_strdup_vprintf(fmt, ap);
- va_end(ap);
if (mod) {
mod(err, mod_opaque);
}
@@ -50,6 +47,7 @@ static void do_error_set(Error **errp, ErrorClass err_class,
abort();
}
+ err->next = *errp;
*errp = err;
errno = saved_errno;
@@ -120,15 +118,22 @@ Error *error_copy(const Error *err)
{
Error *err_new;
+ if (!err) {
+ return NULL;
+ }
err_new = g_malloc0(sizeof(*err));
err_new->msg = g_strdup(err->msg);
err_new->err_class = err->err_class;
+ err_new->next = error_copy(err->next);
return err_new;
}
ErrorClass error_get_class(const Error *err)
{
+ if (err->next) {
+ return ERROR_CLASS_MULTIPLE_ERRORS;
+ }
return err->err_class;
}
@@ -145,6 +150,9 @@ void error_report_err(Error *err)
void error_free(Error *err)
{
+ if (err->next) {
+ error_free(err->next);
+ }
if (err) {
g_free(err->msg);
g_free(err);
@@ -156,8 +164,15 @@ void error_propagate(Error **dst_errp, Error *local_err)
if (local_err && dst_errp == &error_abort) {
error_report_err(local_err);
abort();
- } else if (dst_errp && !*dst_errp) {
+ } else if (dst_errp) {
+ Error *i;
+ Error *old_dst_err = *dst_errp;
+
*dst_errp = local_err;
+ for (i = local_err; i; i = i->next) {
+ dst_errp = &i->next;
+ }
+ *dst_errp = old_dst_err;
} else if (local_err) {
error_free(local_err);
}
--
1.9.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [RFC PATCH v1 04/25] error: Add support for multiple errors
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 04/25] error: Add support for multiple errors Peter Crosthwaite
@ 2015-09-11 15:49 ` Eric Blake
0 siblings, 0 replies; 36+ messages in thread
From: Eric Blake @ 2015-09-11 15:49 UTC (permalink / raw)
To: Peter Crosthwaite, qemu-devel; +Cc: peter.maydell, armbru
[-- Attachment #1: Type: text/plain, Size: 4061 bytes --]
On 09/10/2015 11:33 PM, Peter Crosthwaite wrote:
> Allow errors to stack. If an error is already set, don't assert,
> instead, form a linked list. Recent errors are at the front of the
> list, older ones at the back.
>
> The assertion against the destination erro already being set is
s/erro/error/
> removed.
Do we want to do that blindly, or do we want a design where users must
explicitly ask for nested errors?
I'm wondering aloud here: (haven't actually thought too hard, but typing
as I go)
Since your goal was reducing boilerplate, is there some way we could have:
myfunc1(..., error_add(errp));
myfunc2(..., error_add(errp));
be some way to mark errp as allowing error concatenation? That is,
error_add() would return errp; if *errp was NULL it does nothing
further, but *errp is non-NULL it then sets a flag in errp that it is
okay for further errors to be concatenated. Then when setting or
propagating an error, we can use the flag within errp to determine if
the caller is okay with us appending to the existing error, or whether
there may be a programming error in that we are detecting a followup
error to an errp that wasn't properly cleared earlier.
Or maybe:
error_start_chaining(errp);
myfunc1(..., errp);
myfunc2(..., errp);
error_stop_chaining(errp);
where we use a counter of how many requests (since myfunc1() may itself
call nested start/stop, so chaining is okay if the counter is non-zero).
>
> copy/free are all to call their functionality recursively.
>
> Propagation is implemented as concatenation of two error lists.
>
> Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
> ---
>
> qapi/common.json | 5 ++++-
> util/error.c | 27 +++++++++++++++++++++------
> 2 files changed, 25 insertions(+), 7 deletions(-)
>
> diff --git a/qapi/common.json b/qapi/common.json
> index bad56bf..04175db 100644
> --- a/qapi/common.json
> +++ b/qapi/common.json
> @@ -22,11 +22,14 @@
> # @KVMMissingCap: the requested operation can't be fulfilled because a
> # required KVM capability is missing
> #
> +# @MultipleErrors: Multiple errors have occured
s/occured/occurred/
Missing a (since 2.5) designation.
Do we want to change the QMP wire format when multiple errors have been
chained together, to return a linked list or array of those errors, for
easier machine parsing of the individual errors? If so, it requires
some documentation updates. If not, packing the chained error
information into a single string is okay for humans, but not as nice for
computers.
> +#
> # Since: 1.2
> ##
> { 'enum': 'ErrorClass',
> 'data': [ 'GenericError', 'CommandNotFound', 'DeviceEncrypted',
> - 'DeviceNotActive', 'DeviceNotFound', 'KVMMissingCap' ] }
> + 'DeviceNotActive', 'DeviceNotFound', 'KVMMissingCap',
> + 'MultipleErrors' ] }
>
> ##
> # @VersionTriple
> diff --git a/util/error.c b/util/error.c
> index b93e5c8..890ce58 100644
> --- a/util/error.c
> +++ b/util/error.c
> @@ -18,28 +18,25 @@ struct Error
> {
> char *msg;
> ErrorClass err_class;
> + struct Error *next;
> };
Merge conflicts in this area; but doesn't hold up review of the concept.
>
> Error *error_abort;
>
> static void do_error_set(Error **errp, ErrorClass err_class,
> void (*mod)(Error *, void *), void *mod_opaque,
> - const char *fmt, ...)
> + const char *fmt, va_list ap)
> {
> Error *err;
> - va_list ap;
> int saved_errno = errno;
>
> if (errp == NULL) {
> return;
> }
> - assert(*errp == NULL);
Here's where I'm wondering if we can have some sort of flag to say
whether the caller was okay with error concatentation, in which case
this would be something like:
assert(!*errp || errp->chaining_okay);
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [RFC PATCH v1 05/25] error: Add error prefix API
2015-09-11 5:33 [Qemu-devel] [RFC PATCH v1 00/25] error: Automatic error concatenation and prefixing Peter Crosthwaite
` (3 preceding siblings ...)
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 04/25] error: Add support for multiple errors Peter Crosthwaite
@ 2015-09-11 5:33 ` Peter Crosthwaite
2015-09-11 16:04 ` Eric Blake
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 06/25] error: Add error_printf_fn() Peter Crosthwaite
` (21 subsequent siblings)
26 siblings, 1 reply; 36+ messages in thread
From: Peter Crosthwaite @ 2015-09-11 5:33 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, armbru
Add an API to prefix an already set error with a caller-centric
message.
If multiple errors are set, all are prefixed individually.
Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
include/qapi/error.h | 6 ++++++
util/error.c | 26 ++++++++++++++++++++++++++
2 files changed, 32 insertions(+)
diff --git a/include/qapi/error.h b/include/qapi/error.h
index f44c451..b25c72f 100644
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -78,6 +78,12 @@ ErrorClass error_get_class(const Error *err);
Error *error_copy(const Error *err);
/**
+ * Prefix an error message with a formatted string.
+ */
+
+void error_prefix(Error *err, const char *fmt, ...);
+
+/**
* Get a human readable representation of an error object.
*/
const char *error_get_pretty(Error *err);
diff --git a/util/error.c b/util/error.c
index 890ce58..e9c23ce 100644
--- a/util/error.c
+++ b/util/error.c
@@ -19,6 +19,7 @@ struct Error
char *msg;
ErrorClass err_class;
struct Error *next;
+ bool prefixed;
};
Error *error_abort;
@@ -142,6 +143,26 @@ const char *error_get_pretty(Error *err)
return err->msg;
}
+void error_prefix(Error *err, const char *fmt, ...) {
+ char *msg;
+ char *fmt_full;
+ va_list ap;
+
+ if (!err || err->prefixed) {
+ return;
+ }
+ err->prefixed = true;
+
+ msg = err->msg;
+ fmt_full = g_strdup_printf("%s%%s", fmt);
+
+ va_start(ap, fmt);
+ err->msg = g_strdup_printf(fmt_full, ap, msg);
+ va_end(ap);
+ g_free(fmt_full);
+ g_free(msg);
+}
+
void error_report_err(Error *err)
{
error_report("%s", error_get_pretty(err));
@@ -170,6 +191,11 @@ void error_propagate(Error **dst_errp, Error *local_err)
*dst_errp = local_err;
for (i = local_err; i; i = i->next) {
+ /* Propagation implies that the caller is no longer the owner of the
+ * error. Therefore reset prefixes, so higher level handlers can
+ * prefix again.
+ */
+ i->prefixed = false;
dst_errp = &i->next;
}
*dst_errp = old_dst_err;
--
1.9.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [RFC PATCH v1 05/25] error: Add error prefix API
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 05/25] error: Add error prefix API Peter Crosthwaite
@ 2015-09-11 16:04 ` Eric Blake
0 siblings, 0 replies; 36+ messages in thread
From: Eric Blake @ 2015-09-11 16:04 UTC (permalink / raw)
To: Peter Crosthwaite, qemu-devel; +Cc: peter.maydell, armbru
[-- Attachment #1: Type: text/plain, Size: 3458 bytes --]
On 09/10/2015 11:33 PM, Peter Crosthwaite wrote:
> Add an API to prefix an already set error with a caller-centric
> message.
>
> If multiple errors are set, all are prefixed individually.
>
Might be nice to rebase your series to add this first, prior to
multi-error support. (That is, while prefixing multiple errors requires
multi-error support, adding caller-centric prefix might be useful even
now without multi-error).
> Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
> ---
>
> include/qapi/error.h | 6 ++++++
> util/error.c | 26 ++++++++++++++++++++++++++
> 2 files changed, 32 insertions(+)
>
> diff --git a/include/qapi/error.h b/include/qapi/error.h
> index f44c451..b25c72f 100644
> --- a/include/qapi/error.h
> +++ b/include/qapi/error.h
> @@ -78,6 +78,12 @@ ErrorClass error_get_class(const Error *err);
> Error *error_copy(const Error *err);
>
> /**
> + * Prefix an error message with a formatted string.
> + */
> +
> +void error_prefix(Error *err, const char *fmt, ...);
The string is basically only adding details for human consumption.
> +++ b/util/error.c
> @@ -19,6 +19,7 @@ struct Error
> char *msg;
> ErrorClass err_class;
> struct Error *next;
> + bool prefixed;
> };
I'm not sure I follow why this field is needed.
>
> Error *error_abort;
> @@ -142,6 +143,26 @@ const char *error_get_pretty(Error *err)
> return err->msg;
> }
>
> +void error_prefix(Error *err, const char *fmt, ...) {
Brace on its own line.
> + char *msg;
> + char *fmt_full;
> + va_list ap;
> +
> + if (!err || err->prefixed) {
> + return;
> + }
> + err->prefixed = true;
> +
> + msg = err->msg;
> + fmt_full = g_strdup_printf("%s%%s", fmt);
> +
> + va_start(ap, fmt);
> + err->msg = g_strdup_printf(fmt_full, ap, msg);
I don't think that is portable. Passing va_list as an argument to plain
printf just isn't going to do what you think.
(There has been a request to add recursive printf via %r,
http://austingroupbugs.net/view.php?id=800, but glibc does not support
the extension yet)
It's not to say that you can't chain, just that you can't chain like
this. I don't know if using GString internally to error would make the
matter any easier (but one of the patches that will probably land before
yours, and thus affect your need to rebase, is mine which adds use of
GString for adding a human-readable SUFFIX rather than prefix:
https://lists.gnu.org/archive/html/qemu-devel/2015-09/msg02818.html)
> + va_end(ap);
> + g_free(fmt_full);
> + g_free(msg);
> +}
> +
> void error_report_err(Error *err)
> {
> error_report("%s", error_get_pretty(err));
> @@ -170,6 +191,11 @@ void error_propagate(Error **dst_errp, Error *local_err)
>
> *dst_errp = local_err;
> for (i = local_err; i; i = i->next) {
> + /* Propagation implies that the caller is no longer the owner of the
> + * error. Therefore reset prefixes, so higher level handlers can
> + * prefix again.
> + */
> + i->prefixed = false;
I'm still not quite sure I buy the semantics of this flag.
> dst_errp = &i->next;
> }
> *dst_errp = old_dst_err;
>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [RFC PATCH v1 06/25] error: Add error_printf_fn()
2015-09-11 5:33 [Qemu-devel] [RFC PATCH v1 00/25] error: Automatic error concatenation and prefixing Peter Crosthwaite
` (4 preceding siblings ...)
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 05/25] error: Add error prefix API Peter Crosthwaite
@ 2015-09-11 5:33 ` Peter Crosthwaite
2015-09-11 16:10 ` Eric Blake
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 07/25] sysbus: mmio_map+mmio_get_region: ignore range OOB errors Peter Crosthwaite
` (20 subsequent siblings)
26 siblings, 1 reply; 36+ messages in thread
From: Peter Crosthwaite @ 2015-09-11 5:33 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, armbru
Add an API to report an error with a custom printf function. Use
this for the implementation of error_report_err().
Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
include/qapi/error.h | 7 +++++++
util/error.c | 22 ++++++++++++++++++++--
2 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/include/qapi/error.h b/include/qapi/error.h
index b25c72f..0e5c311 100644
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -94,6 +94,13 @@ const char *error_get_pretty(Error *err);
void error_report_err(Error *);
/**
+ * Report an and free an error object using a custom printf implementation.
+ */
+
+void error_printf_fn(Error *err, void (*printf_fn)(void *, const char *, ...),
+ void *printf_opaque);
+
+/**
* Propagate an error to an indirect pointer to an error. This function will
* always transfer ownership of the error reference and handles the case where
* dst_err is NULL correctly. Errors after the first are discarded.
diff --git a/util/error.c b/util/error.c
index e9c23ce..c4656af 100644
--- a/util/error.c
+++ b/util/error.c
@@ -163,10 +163,18 @@ void error_prefix(Error *err, const char *fmt, ...) {
g_free(msg);
}
+static void error_report_err_printf(void *opaque, const char *fmt, ...)
+{
+ va_list ap;
+
+ va_start(ap, fmt);
+ error_vreport(fmt, ap);
+ va_end(ap);
+}
+
void error_report_err(Error *err)
{
- error_report("%s", error_get_pretty(err));
- error_free(err);
+ error_printf_fn(err, error_report_err_printf, NULL);
}
void error_free(Error *err)
@@ -180,6 +188,16 @@ void error_free(Error *err)
}
}
+void error_printf_fn(Error *err, void (*printf_fn)(void *, const char *, ...),
+ void *printf_opaque)
+{
+ if (err->next) {
+ error_printf_fn(err->next, printf_fn, printf_opaque);
+ }
+ printf_fn(printf_opaque, "%s\n", error_get_pretty(err));
+ error_free(err);
+}
+
void error_propagate(Error **dst_errp, Error *local_err)
{
if (local_err && dst_errp == &error_abort) {
--
1.9.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [RFC PATCH v1 06/25] error: Add error_printf_fn()
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 06/25] error: Add error_printf_fn() Peter Crosthwaite
@ 2015-09-11 16:10 ` Eric Blake
0 siblings, 0 replies; 36+ messages in thread
From: Eric Blake @ 2015-09-11 16:10 UTC (permalink / raw)
To: Peter Crosthwaite, qemu-devel; +Cc: peter.maydell, armbru
[-- Attachment #1: Type: text/plain, Size: 1885 bytes --]
On 09/10/2015 11:33 PM, Peter Crosthwaite wrote:
> Add an API to report an error with a custom printf function. Use
> this for the implementation of error_report_err().
>
> Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
> ---
>
> include/qapi/error.h | 7 +++++++
> util/error.c | 22 ++++++++++++++++++++--
> 2 files changed, 27 insertions(+), 2 deletions(-)
Sounds independently useful.
>
> diff --git a/include/qapi/error.h b/include/qapi/error.h
> index b25c72f..0e5c311 100644
> --- a/include/qapi/error.h
> +++ b/include/qapi/error.h
> @@ -94,6 +94,13 @@ const char *error_get_pretty(Error *err);
> void error_report_err(Error *);
>
> /**
> + * Report an and free an error object using a custom printf implementation.
s/an and/and/
> + */
> +
> +void error_printf_fn(Error *err, void (*printf_fn)(void *, const char *, ...),
> + void *printf_opaque);
> +
> +/**
> * Propagate an error to an indirect pointer to an error. This function will
> * always transfer ownership of the error reference and handles the case where
> * dst_err is NULL correctly. Errors after the first are discarded.
> diff --git a/util/error.c b/util/error.c
> index e9c23ce..c4656af 100644
> --- a/util/error.c
> +void error_printf_fn(Error *err, void (*printf_fn)(void *, const char *, ...),
> + void *printf_opaque)
> +{
> + if (err->next) {
> + error_printf_fn(err->next, printf_fn, printf_opaque);
> + }
> + printf_fn(printf_opaque, "%s\n", error_get_pretty(err));
> + error_free(err);
> +}
Of course, if you rebase this to come independent of multi-error, it
won't need the first 'if'. But it looks like a reasonable factorization.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* [Qemu-devel] [RFC PATCH v1 07/25] sysbus: mmio_map+mmio_get_region: ignore range OOB errors
2015-09-11 5:33 [Qemu-devel] [RFC PATCH v1 00/25] error: Automatic error concatenation and prefixing Peter Crosthwaite
` (5 preceding siblings ...)
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 06/25] error: Add error_printf_fn() Peter Crosthwaite
@ 2015-09-11 5:33 ` Peter Crosthwaite
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 08/25] memory: nop APIs when they have NULL arguments Peter Crosthwaite
` (19 subsequent siblings)
26 siblings, 0 replies; 36+ messages in thread
From: Peter Crosthwaite @ 2015-09-11 5:33 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, armbru
Ignore these errors as they may be a follow on effect of device
realisation failure. Ideally we should have an error ** to populate
in own right, but that requires an API change. Mark FIXME.
Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
hw/core/sysbus.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/hw/core/sysbus.c b/hw/core/sysbus.c
index 3c58629..6fde10e 100644
--- a/hw/core/sysbus.c
+++ b/hw/core/sysbus.c
@@ -127,7 +127,12 @@ bool sysbus_has_mmio(SysBusDevice *dev, unsigned int n)
static void sysbus_mmio_map_common(SysBusDevice *dev, int n, hwaddr addr,
bool may_overlap, int priority)
{
- assert(n >= 0 && n < dev->num_mmio);
+ assert(n >= 0);
+
+ if (n < dev->num_mmio) {
+ /* FIXME: Add an Error ** to this function for this condition */
+ return;
+ }
if (dev->mmio[n].addr == addr) {
/* ??? region already mapped here. */
@@ -186,6 +191,10 @@ void sysbus_init_mmio(SysBusDevice *dev, MemoryRegion *memory)
MemoryRegion *sysbus_mmio_get_region(SysBusDevice *dev, int n)
{
+ if (n >= dev->num_mmio) {
+ /* FIXME: Add an Error ** to this function for this condition */
+ return NULL;
+ }
return dev->mmio[n].memory;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [RFC PATCH v1 08/25] memory: nop APIs when they have NULL arguments
2015-09-11 5:33 [Qemu-devel] [RFC PATCH v1 00/25] error: Automatic error concatenation and prefixing Peter Crosthwaite
` (6 preceding siblings ...)
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 07/25] sysbus: mmio_map+mmio_get_region: ignore range OOB errors Peter Crosthwaite
@ 2015-09-11 5:33 ` Peter Crosthwaite
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 09/25] qdev: gpio: Ignore unconnectable GPIOs Peter Crosthwaite
` (18 subsequent siblings)
26 siblings, 0 replies; 36+ messages in thread
From: Peter Crosthwaite @ 2015-09-11 5:33 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, armbru
When doing a subregion or alias and the SR/Aliased region is NULL,
perform no action. This makes the memory API tolerant of API calls
following an earlier failure with setting up these dependencies.
This in turn allows removal of some of the constant error-checking
boiler-plate from machine models.
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
memory.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/memory.c b/memory.c
index 0d8b2d9..2d03cb7 100644
--- a/memory.c
+++ b/memory.c
@@ -1266,6 +1266,9 @@ void memory_region_init_alias(MemoryRegion *mr,
hwaddr offset,
uint64_t size)
{
+ if (!orig) {
+ return;
+ }
memory_region_init(mr, owner, name, size);
mr->alias = orig;
mr->alias_offset = offset;
@@ -1759,6 +1762,9 @@ void memory_region_add_subregion(MemoryRegion *mr,
hwaddr offset,
MemoryRegion *subregion)
{
+ if (!subregion) {
+ return;
+ }
subregion->may_overlap = false;
subregion->priority = 0;
memory_region_add_subregion_common(mr, offset, subregion);
@@ -1769,6 +1775,9 @@ void memory_region_add_subregion_overlap(MemoryRegion *mr,
MemoryRegion *subregion,
int priority)
{
+ if (!subregion) {
+ return;
+ }
subregion->may_overlap = true;
subregion->priority = priority;
memory_region_add_subregion_common(mr, offset, subregion);
--
1.9.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [RFC PATCH v1 09/25] qdev: gpio: Ignore unconnectable GPIOs
2015-09-11 5:33 [Qemu-devel] [RFC PATCH v1 00/25] error: Automatic error concatenation and prefixing Peter Crosthwaite
` (7 preceding siblings ...)
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 08/25] memory: nop APIs when they have NULL arguments Peter Crosthwaite
@ 2015-09-11 5:33 ` Peter Crosthwaite
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 10/25] arm: xlnx-zynqmp: Update error API usages Peter Crosthwaite
` (17 subsequent siblings)
26 siblings, 0 replies; 36+ messages in thread
From: Peter Crosthwaite @ 2015-09-11 5:33 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, armbru
Nop rather than assert if a GPIO connection cannot be made. This
allows for continuation of machine init after an error which allows
removal of error detection boilerplate and detection of subsequent
errors.
The actual error (i.e. a GPIO endpoint was not inited) will have
already been raised by other APIs.
Ideally this API should raise an error in its own right. Add a FIXME.
Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
hw/core/qdev.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index b2f404a..52140ba 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -467,7 +467,11 @@ qemu_irq qdev_get_gpio_in_named(DeviceState *dev, const char *name, int n)
{
NamedGPIOList *gpio_list = qdev_get_named_gpio_list(dev, name);
- assert(n >= 0 && n < gpio_list->num_in);
+ assert(n >= 0);
+ if (n >= gpio_list->num_in) {
+ /* FIXME: add error ** to this API */
+ return NULL;
+ }
return gpio_list->in[n];
}
@@ -491,7 +495,8 @@ void qdev_connect_gpio_out_named(DeviceState *dev, const char *name, int n,
"/unattached"),
"non-qdev-gpio[*]", OBJECT(pin), NULL);
}
- object_property_set_link(OBJECT(dev), OBJECT(pin), propname, &error_abort);
+ /* FIXME: add error ** to this API */
+ object_property_set_link(OBJECT(dev), OBJECT(pin), propname, NULL);
g_free(propname);
}
--
1.9.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [RFC PATCH v1 10/25] arm: xlnx-zynqmp: Update error API usages
2015-09-11 5:33 [Qemu-devel] [RFC PATCH v1 00/25] error: Automatic error concatenation and prefixing Peter Crosthwaite
` (8 preceding siblings ...)
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 09/25] qdev: gpio: Ignore unconnectable GPIOs Peter Crosthwaite
@ 2015-09-11 5:33 ` Peter Crosthwaite
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 11/25] arm: fsl-imx*: " Peter Crosthwaite
` (16 subsequent siblings)
26 siblings, 0 replies; 36+ messages in thread
From: Peter Crosthwaite @ 2015-09-11 5:33 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, armbru
Collect all errors and remove constant checking for realize failures.
Use error_report_err as appropriate.
Cc: Alistair Francis <alistair.francis@xilinx.com>
Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
hw/arm/xilinx_zynq.c | 14 +-------------
hw/arm/xlnx-ep108.c | 2 +-
hw/arm/xlnx-zynqmp.c | 29 +----------------------------
3 files changed, 3 insertions(+), 42 deletions(-)
diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index a4e7b5c..de2be6a 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -132,24 +132,12 @@ static void zynq_init(MachineState *machine)
*/
if (object_property_find(OBJECT(cpu), "has_el3", NULL)) {
object_property_set_bool(OBJECT(cpu), false, "has_el3", &err);
- if (err) {
- error_report_err(err);
- exit(1);
- }
}
object_property_set_int(OBJECT(cpu), ZYNQ_BOARD_MIDR, "midr", &err);
- if (err) {
- error_report_err(err);
- exit(1);
- }
-
object_property_set_int(OBJECT(cpu), MPCORE_PERIPHBASE, "reset-cbar", &err);
- if (err) {
- error_report_err(err);
- exit(1);
- }
object_property_set_bool(OBJECT(cpu), true, "realized", &err);
+
if (err) {
error_report_err(err);
exit(1);
diff --git a/hw/arm/xlnx-ep108.c b/hw/arm/xlnx-ep108.c
index f94da86..29948b2 100644
--- a/hw/arm/xlnx-ep108.c
+++ b/hw/arm/xlnx-ep108.c
@@ -41,7 +41,7 @@ static void xlnx_ep108_init(MachineState *machine)
object_property_set_bool(OBJECT(&s->soc), true, "realized", &err);
if (err) {
- error_report("%s", error_get_pretty(err));
+ error_report_err(err);
exit(1);
}
diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 388baef..89624f9 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -120,10 +120,6 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
qdev_prop_set_uint32(DEVICE(&s->gic), "revision", 2);
qdev_prop_set_uint32(DEVICE(&s->gic), "num-cpu", XLNX_ZYNQMP_NUM_APU_CPUS);
object_property_set_bool(OBJECT(&s->gic), true, "realized", &err);
- if (err) {
- error_propagate((errp), (err));
- return;
- }
assert(ARRAY_SIZE(xlnx_zynqmp_gic_regions) == XLNX_ZYNQMP_GIC_REGIONS);
for (i = 0; i < XLNX_ZYNQMP_GIC_REGIONS; i++) {
SysBusDevice *gic = SYS_BUS_DEVICE(&s->gic);
@@ -163,17 +159,9 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
object_property_set_int(OBJECT(&s->apu_cpu[i]), GIC_BASE_ADDR,
"reset-cbar", &err);
- if (err) {
- error_propagate((errp), (err));
- return;
- }
object_property_set_bool(OBJECT(&s->apu_cpu[i]), true, "realized",
&err);
- if (err) {
- error_propagate((errp), (err));
- return;
- }
sysbus_connect_irq(SYS_BUS_DEVICE(&s->gic), i,
qdev_get_gpio_in(DEVICE(&s->apu_cpu[i]),
@@ -201,17 +189,9 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
object_property_set_bool(OBJECT(&s->rpu_cpu[i]), true, "reset-hivecs",
&err);
- if (err != NULL) {
- error_propagate(errp, err);
- return;
- }
object_property_set_bool(OBJECT(&s->rpu_cpu[i]), true, "realized",
&err);
- if (err) {
- error_propagate((errp), (err));
- return;
- }
}
if (!s->boot_cpu_ptr) {
@@ -231,10 +211,6 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
qdev_set_nic_properties(DEVICE(&s->gem[i]), nd);
}
object_property_set_bool(OBJECT(&s->gem[i]), true, "realized", &err);
- if (err) {
- error_propagate((errp), (err));
- return;
- }
sysbus_mmio_map(SYS_BUS_DEVICE(&s->gem[i]), 0, gem_addr[i]);
sysbus_connect_irq(SYS_BUS_DEVICE(&s->gem[i]), 0,
gic_spi[gem_intr[i]]);
@@ -242,14 +218,11 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
for (i = 0; i < XLNX_ZYNQMP_NUM_UARTS; i++) {
object_property_set_bool(OBJECT(&s->uart[i]), true, "realized", &err);
- if (err) {
- error_propagate((errp), (err));
- return;
- }
sysbus_mmio_map(SYS_BUS_DEVICE(&s->uart[i]), 0, uart_addr[i]);
sysbus_connect_irq(SYS_BUS_DEVICE(&s->uart[i]), 0,
gic_spi[uart_intr[i]]);
}
+ error_propagate((errp), (err));
}
static Property xlnx_zynqmp_props[] = {
--
1.9.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [RFC PATCH v1 11/25] arm: fsl-imx*: Update error API usages
2015-09-11 5:33 [Qemu-devel] [RFC PATCH v1 00/25] error: Automatic error concatenation and prefixing Peter Crosthwaite
` (9 preceding siblings ...)
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 10/25] arm: xlnx-zynqmp: Update error API usages Peter Crosthwaite
@ 2015-09-11 5:33 ` Peter Crosthwaite
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 12/25] arm: netduino: " Peter Crosthwaite
` (15 subsequent siblings)
26 siblings, 0 replies; 36+ messages in thread
From: Peter Crosthwaite @ 2015-09-11 5:33 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, armbru
Collect all errors and remove constant checking for realize failures.
Cc: Jean-Christophe Dubois <jcd@tribudubois.net>
Cc: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
hw/arm/fsl-imx25.c | 46 ++--------------------------------------------
hw/arm/fsl-imx31.c | 42 ++----------------------------------------
2 files changed, 4 insertions(+), 84 deletions(-)
diff --git a/hw/arm/fsl-imx25.c b/hw/arm/fsl-imx25.c
index 6d157c9..f510698 100644
--- a/hw/arm/fsl-imx25.c
+++ b/hw/arm/fsl-imx25.c
@@ -72,16 +72,8 @@ static void fsl_imx25_realize(DeviceState *dev, Error **errp)
Error *err = NULL;
object_property_set_bool(OBJECT(&s->cpu), true, "realized", &err);
- if (err) {
- error_propagate(errp, err);
- return;
- }
object_property_set_bool(OBJECT(&s->avic), true, "realized", &err);
- if (err) {
- error_propagate(errp, err);
- return;
- }
sysbus_mmio_map(SYS_BUS_DEVICE(&s->avic), 0, FSL_IMX25_AVIC_ADDR);
sysbus_connect_irq(SYS_BUS_DEVICE(&s->avic), 0,
qdev_get_gpio_in(DEVICE(&s->cpu), ARM_CPU_IRQ));
@@ -89,10 +81,6 @@ static void fsl_imx25_realize(DeviceState *dev, Error **errp)
qdev_get_gpio_in(DEVICE(&s->cpu), ARM_CPU_FIQ));
object_property_set_bool(OBJECT(&s->ccm), true, "realized", &err);
- if (err) {
- error_propagate(errp, err);
- return;
- }
sysbus_mmio_map(SYS_BUS_DEVICE(&s->ccm), 0, FSL_IMX25_CCM_ADDR);
/* Initialize all UARTs */
@@ -123,10 +111,6 @@ static void fsl_imx25_realize(DeviceState *dev, Error **errp)
}
object_property_set_bool(OBJECT(&s->uart[i]), true, "realized", &err);
- if (err) {
- error_propagate(errp, err);
- return;
- }
sysbus_mmio_map(SYS_BUS_DEVICE(&s->uart[i]), 0, serial_table[i].addr);
sysbus_connect_irq(SYS_BUS_DEVICE(&s->uart[i]), 0,
qdev_get_gpio_in(DEVICE(&s->avic),
@@ -148,10 +132,6 @@ static void fsl_imx25_realize(DeviceState *dev, Error **errp)
s->gpt[i].ccm = DEVICE(&s->ccm);
object_property_set_bool(OBJECT(&s->gpt[i]), true, "realized", &err);
- if (err) {
- error_propagate(errp, err);
- return;
- }
sysbus_mmio_map(SYS_BUS_DEVICE(&s->gpt[i]), 0, gpt_table[i].addr);
sysbus_connect_irq(SYS_BUS_DEVICE(&s->gpt[i]), 0,
qdev_get_gpio_in(DEVICE(&s->avic),
@@ -171,10 +151,6 @@ static void fsl_imx25_realize(DeviceState *dev, Error **errp)
s->epit[i].ccm = DEVICE(&s->ccm);
object_property_set_bool(OBJECT(&s->epit[i]), true, "realized", &err);
- if (err) {
- error_propagate(errp, err);
- return;
- }
sysbus_mmio_map(SYS_BUS_DEVICE(&s->epit[i]), 0, epit_table[i].addr);
sysbus_connect_irq(SYS_BUS_DEVICE(&s->epit[i]), 0,
qdev_get_gpio_in(DEVICE(&s->avic),
@@ -183,10 +159,6 @@ static void fsl_imx25_realize(DeviceState *dev, Error **errp)
qdev_set_nic_properties(DEVICE(&s->fec), &nd_table[0]);
object_property_set_bool(OBJECT(&s->fec), true, "realized", &err);
- if (err) {
- error_propagate(errp, err);
- return;
- }
sysbus_mmio_map(SYS_BUS_DEVICE(&s->fec), 0, FSL_IMX25_FEC_ADDR);
sysbus_connect_irq(SYS_BUS_DEVICE(&s->fec), 0,
qdev_get_gpio_in(DEVICE(&s->avic), FSL_IMX25_FEC_IRQ));
@@ -204,10 +176,6 @@ static void fsl_imx25_realize(DeviceState *dev, Error **errp)
};
object_property_set_bool(OBJECT(&s->i2c[i]), true, "realized", &err);
- if (err) {
- error_propagate(errp, err);
- return;
- }
sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c[i]), 0, i2c_table[i].addr);
sysbus_connect_irq(SYS_BUS_DEVICE(&s->i2c[i]), 0,
qdev_get_gpio_in(DEVICE(&s->avic),
@@ -217,28 +185,16 @@ static void fsl_imx25_realize(DeviceState *dev, Error **errp)
/* initialize 2 x 16 KB ROM */
memory_region_init_rom_device(&s->rom[0], NULL, NULL, NULL,
"imx25.rom0", FSL_IMX25_ROM0_SIZE, &err);
- if (err) {
- error_propagate(errp, err);
- return;
- }
memory_region_add_subregion(get_system_memory(), FSL_IMX25_ROM0_ADDR,
&s->rom[0]);
memory_region_init_rom_device(&s->rom[1], NULL, NULL, NULL,
"imx25.rom1", FSL_IMX25_ROM1_SIZE, &err);
- if (err) {
- error_propagate(errp, err);
- return;
- }
memory_region_add_subregion(get_system_memory(), FSL_IMX25_ROM1_ADDR,
&s->rom[1]);
/* initialize internal RAM (128 KB) */
memory_region_init_ram(&s->iram, NULL, "imx25.iram", FSL_IMX25_IRAM_SIZE,
&err);
- if (err) {
- error_propagate(errp, err);
- return;
- }
memory_region_add_subregion(get_system_memory(), FSL_IMX25_IRAM_ADDR,
&s->iram);
vmstate_register_ram_global(&s->iram);
@@ -248,6 +204,8 @@ static void fsl_imx25_realize(DeviceState *dev, Error **errp)
&s->iram, 0, FSL_IMX25_IRAM_ALIAS_SIZE);
memory_region_add_subregion(get_system_memory(), FSL_IMX25_IRAM_ALIAS_ADDR,
&s->iram_alias);
+
+ error_propagate(errp, err);
}
static void fsl_imx25_class_init(ObjectClass *oc, void *data)
diff --git a/hw/arm/fsl-imx31.c b/hw/arm/fsl-imx31.c
index 87548c8..eed6c7c 100644
--- a/hw/arm/fsl-imx31.c
+++ b/hw/arm/fsl-imx31.c
@@ -64,16 +64,8 @@ static void fsl_imx31_realize(DeviceState *dev, Error **errp)
Error *err = NULL;
object_property_set_bool(OBJECT(&s->cpu), true, "realized", &err);
- if (err) {
- error_propagate(errp, err);
- return;
- }
object_property_set_bool(OBJECT(&s->avic), true, "realized", &err);
- if (err) {
- error_propagate(errp, err);
- return;
- }
sysbus_mmio_map(SYS_BUS_DEVICE(&s->avic), 0, FSL_IMX31_AVIC_ADDR);
sysbus_connect_irq(SYS_BUS_DEVICE(&s->avic), 0,
qdev_get_gpio_in(DEVICE(&s->cpu), ARM_CPU_IRQ));
@@ -81,10 +73,6 @@ static void fsl_imx31_realize(DeviceState *dev, Error **errp)
qdev_get_gpio_in(DEVICE(&s->cpu), ARM_CPU_FIQ));
object_property_set_bool(OBJECT(&s->ccm), true, "realized", &err);
- if (err) {
- error_propagate(errp, err);
- return;
- }
sysbus_mmio_map(SYS_BUS_DEVICE(&s->ccm), 0, FSL_IMX31_CCM_ADDR);
/* Initialize all UARTS */
@@ -112,10 +100,6 @@ static void fsl_imx31_realize(DeviceState *dev, Error **errp)
}
object_property_set_bool(OBJECT(&s->uart[i]), true, "realized", &err);
- if (err) {
- error_propagate(errp, err);
- return;
- }
sysbus_mmio_map(SYS_BUS_DEVICE(&s->uart[i]), 0, serial_table[i].addr);
sysbus_connect_irq(SYS_BUS_DEVICE(&s->uart[i]), 0,
@@ -126,10 +110,6 @@ static void fsl_imx31_realize(DeviceState *dev, Error **errp)
s->gpt.ccm = DEVICE(&s->ccm);
object_property_set_bool(OBJECT(&s->gpt), true, "realized", &err);
- if (err) {
- error_propagate(errp, err);
- return;
- }
sysbus_mmio_map(SYS_BUS_DEVICE(&s->gpt), 0, FSL_IMX31_GPT_ADDR);
sysbus_connect_irq(SYS_BUS_DEVICE(&s->gpt), 0,
@@ -148,10 +128,6 @@ static void fsl_imx31_realize(DeviceState *dev, Error **errp)
s->epit[i].ccm = DEVICE(&s->ccm);
object_property_set_bool(OBJECT(&s->epit[i]), true, "realized", &err);
- if (err) {
- error_propagate(errp, err);
- return;
- }
sysbus_mmio_map(SYS_BUS_DEVICE(&s->epit[i]), 0, epit_table[i].addr);
sysbus_connect_irq(SYS_BUS_DEVICE(&s->epit[i]), 0,
@@ -172,10 +148,6 @@ static void fsl_imx31_realize(DeviceState *dev, Error **errp)
/* Initialize the I2C */
object_property_set_bool(OBJECT(&s->i2c[i]), true, "realized", &err);
- if (err) {
- error_propagate(errp, err);
- return;
- }
/* Map I2C memory */
sysbus_mmio_map(SYS_BUS_DEVICE(&s->i2c[i]), 0, i2c_table[i].addr);
/* Connect I2C IRQ to PIC */
@@ -188,30 +160,18 @@ static void fsl_imx31_realize(DeviceState *dev, Error **errp)
memory_region_init_rom_device(&s->secure_rom, NULL, NULL, NULL,
"imx31.secure_rom",
FSL_IMX31_SECURE_ROM_SIZE, &err);
- if (err) {
- error_propagate(errp, err);
- return;
- }
memory_region_add_subregion(get_system_memory(), FSL_IMX31_SECURE_ROM_ADDR,
&s->secure_rom);
/* There is also a 16k ROM */
memory_region_init_rom_device(&s->rom, NULL, NULL, NULL, "imx31.rom",
FSL_IMX31_ROM_SIZE, &err);
- if (err) {
- error_propagate(errp, err);
- return;
- }
memory_region_add_subregion(get_system_memory(), FSL_IMX31_ROM_ADDR,
&s->rom);
/* initialize internal RAM (16 KB) */
memory_region_init_ram(&s->iram, NULL, "imx31.iram", FSL_IMX31_IRAM_SIZE,
&err);
- if (err) {
- error_propagate(errp, err);
- return;
- }
memory_region_add_subregion(get_system_memory(), FSL_IMX31_IRAM_ADDR,
&s->iram);
vmstate_register_ram_global(&s->iram);
@@ -221,6 +181,8 @@ static void fsl_imx31_realize(DeviceState *dev, Error **errp)
&s->iram, 0, FSL_IMX31_IRAM_ALIAS_SIZE);
memory_region_add_subregion(get_system_memory(), FSL_IMX31_IRAM_ALIAS_ADDR,
&s->iram_alias);
+
+ error_propagate(errp, err);
}
static void fsl_imx31_class_init(ObjectClass *oc, void *data)
--
1.9.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [RFC PATCH v1 12/25] arm: netduino: Update error API usages
2015-09-11 5:33 [Qemu-devel] [RFC PATCH v1 00/25] error: Automatic error concatenation and prefixing Peter Crosthwaite
` (10 preceding siblings ...)
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 11/25] arm: fsl-imx*: " Peter Crosthwaite
@ 2015-09-11 5:33 ` Peter Crosthwaite
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 13/25] arm: allwinner: " Peter Crosthwaite
` (14 subsequent siblings)
26 siblings, 0 replies; 36+ messages in thread
From: Peter Crosthwaite @ 2015-09-11 5:33 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, armbru
Collect all errors and remove constant checking for realize failures.
Use error_report_err as appropriate.
Cc: Alistair Francis <alistair.francis@xilinx.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
hw/arm/netduino2.c | 2 +-
hw/arm/stm32f205_soc.c | 14 ++------------
2 files changed, 3 insertions(+), 13 deletions(-)
diff --git a/hw/arm/netduino2.c b/hw/arm/netduino2.c
index 8f26780..9a694bb 100644
--- a/hw/arm/netduino2.c
+++ b/hw/arm/netduino2.c
@@ -38,7 +38,7 @@ static void netduino2_init(MachineState *machine)
qdev_prop_set_string(dev, "cpu-model", "cortex-m3");
object_property_set_bool(OBJECT(dev), true, "realized", &err);
if (err != NULL) {
- error_report("%s", error_get_pretty(err));
+ error_report_err(err);
exit(1);
}
}
diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
index 0f3bdc7..015deb2 100644
--- a/hw/arm/stm32f205_soc.c
+++ b/hw/arm/stm32f205_soc.c
@@ -94,10 +94,6 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
/* System configuration controller */
syscfgdev = DEVICE(&s->syscfg);
object_property_set_bool(OBJECT(&s->syscfg), true, "realized", &err);
- if (err != NULL) {
- error_propagate(errp, err);
- return;
- }
syscfgbusdev = SYS_BUS_DEVICE(syscfgdev);
sysbus_mmio_map(syscfgbusdev, 0, 0x40013800);
sysbus_connect_irq(syscfgbusdev, 0, pic[71]);
@@ -106,10 +102,6 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
for (i = 0; i < STM_NUM_USARTS; i++) {
usartdev = DEVICE(&(s->usart[i]));
object_property_set_bool(OBJECT(&s->usart[i]), true, "realized", &err);
- if (err != NULL) {
- error_propagate(errp, err);
- return;
- }
usartbusdev = SYS_BUS_DEVICE(usartdev);
sysbus_mmio_map(usartbusdev, 0, usart_addr[i]);
sysbus_connect_irq(usartbusdev, 0, pic[usart_irq[i]]);
@@ -120,14 +112,12 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
timerdev = DEVICE(&(s->timer[i]));
qdev_prop_set_uint64(timerdev, "clock-frequency", 1000000000);
object_property_set_bool(OBJECT(&s->timer[i]), true, "realized", &err);
- if (err != NULL) {
- error_propagate(errp, err);
- return;
- }
timerbusdev = SYS_BUS_DEVICE(timerdev);
sysbus_mmio_map(timerbusdev, 0, timer_addr[i]);
sysbus_connect_irq(timerbusdev, 0, pic[timer_irq[i]]);
}
+
+ error_propagate(errp, err);
}
static Property stm32f205_soc_properties[] = {
--
1.9.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [RFC PATCH v1 13/25] arm: allwinner: Update error API usages
2015-09-11 5:33 [Qemu-devel] [RFC PATCH v1 00/25] error: Automatic error concatenation and prefixing Peter Crosthwaite
` (11 preceding siblings ...)
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 12/25] arm: netduino: " Peter Crosthwaite
@ 2015-09-11 5:33 ` Peter Crosthwaite
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 14/25] arm: digic: " Peter Crosthwaite
` (13 subsequent siblings)
26 siblings, 0 replies; 36+ messages in thread
From: Peter Crosthwaite @ 2015-09-11 5:33 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, armbru
Collect all errors and remove constant checking for realize failures.
Prefix errors as appropriate and raise all errors at once at the end
of machine init.
Cc: Li Guang <lig.fnst@cn.fujitsu.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
hw/arm/allwinner-a10.c | 18 ++----------------
hw/arm/cubieboard.c | 28 ++++++++++------------------
2 files changed, 12 insertions(+), 34 deletions(-)
diff --git a/hw/arm/allwinner-a10.c b/hw/arm/allwinner-a10.c
index ff249af..2da6cf9 100644
--- a/hw/arm/allwinner-a10.c
+++ b/hw/arm/allwinner-a10.c
@@ -50,18 +50,10 @@ static void aw_a10_realize(DeviceState *dev, Error **errp)
Error *err = NULL;
object_property_set_bool(OBJECT(&s->cpu), true, "realized", &err);
- if (err != NULL) {
- error_propagate(errp, err);
- return;
- }
irq = qdev_get_gpio_in(DEVICE(&s->cpu), ARM_CPU_IRQ);
fiq = qdev_get_gpio_in(DEVICE(&s->cpu), ARM_CPU_FIQ);
object_property_set_bool(OBJECT(&s->intc), true, "realized", &err);
- if (err != NULL) {
- error_propagate(errp, err);
- return;
- }
sysbusdev = SYS_BUS_DEVICE(&s->intc);
sysbus_mmio_map(sysbusdev, 0, AW_A10_PIC_REG_BASE);
sysbus_connect_irq(sysbusdev, 0, irq);
@@ -71,10 +63,6 @@ static void aw_a10_realize(DeviceState *dev, Error **errp)
}
object_property_set_bool(OBJECT(&s->timer), true, "realized", &err);
- if (err != NULL) {
- error_propagate(errp, err);
- return;
- }
sysbusdev = SYS_BUS_DEVICE(&s->timer);
sysbus_mmio_map(sysbusdev, 0, AW_A10_PIT_REG_BASE);
sysbus_connect_irq(sysbusdev, 0, s->irq[22]);
@@ -85,10 +73,6 @@ static void aw_a10_realize(DeviceState *dev, Error **errp)
sysbus_connect_irq(sysbusdev, 5, s->irq[68]);
object_property_set_bool(OBJECT(&s->emac), true, "realized", &err);
- if (err != NULL) {
- error_propagate(errp, err);
- return;
- }
sysbusdev = SYS_BUS_DEVICE(&s->emac);
sysbus_mmio_map(sysbusdev, 0, AW_A10_EMAC_BASE);
sysbus_connect_irq(sysbusdev, 0, s->irq[55]);
@@ -96,6 +80,8 @@ static void aw_a10_realize(DeviceState *dev, Error **errp)
/* FIXME use a qdev chardev prop instead of serial_hds[] */
serial_mm_init(get_system_memory(), AW_A10_UART0_REG_BASE, 2, s->irq[1],
115200, serial_hds[0], DEVICE_NATIVE_ENDIAN);
+
+ error_propagate(errp, err);
}
static void aw_a10_class_init(ObjectClass *oc, void *data)
diff --git a/hw/arm/cubieboard.c b/hw/arm/cubieboard.c
index 1582250..91d9026 100644
--- a/hw/arm/cubieboard.c
+++ b/hw/arm/cubieboard.c
@@ -37,29 +37,21 @@ static void cubieboard_init(MachineState *machine)
s->a10 = AW_A10(object_new(TYPE_AW_A10));
- object_property_set_int(OBJECT(&s->a10->emac), 1, "phy-addr", &err);
- if (err != NULL) {
- error_report("Couldn't set phy address: %s", error_get_pretty(err));
- exit(1);
- }
+ object_property_set_int(OBJECT(&s->a10->emac), 1, "ph-addr", &err);
+ error_prefix(err, "Couldn't set phy address: ");
- object_property_set_int(OBJECT(&s->a10->timer), 32768, "clk0-freq", &err);
- if (err != NULL) {
- error_report("Couldn't set clk0 frequency: %s", error_get_pretty(err));
- exit(1);
- }
+ object_property_set_int(OBJECT(&s->a10->timer), 32768, "ck0-freq", &err);
+ error_prefix(err, "Couldn't set clk0 frequency: ");
- object_property_set_int(OBJECT(&s->a10->timer), 24000000, "clk1-freq",
+ object_property_set_int(OBJECT(&s->a10->timer), 24000000, "ck1-freq",
&err);
- if (err != NULL) {
- error_report("Couldn't set clk1 frequency: %s", error_get_pretty(err));
- exit(1);
- }
+ error_prefix(err, "Couldn't set clk1 frequency:");
object_property_set_bool(OBJECT(s->a10), true, "realized", &err);
- if (err != NULL) {
- error_report("Couldn't realize Allwinner A10: %s",
- error_get_pretty(err));
+ error_prefix(err, "Couldn't realize Allwinner A10: ");
+
+ if (err) {
+ error_report_err(err);
exit(1);
}
--
1.9.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [RFC PATCH v1 14/25] arm: digic: Update error API usages
2015-09-11 5:33 [Qemu-devel] [RFC PATCH v1 00/25] error: Automatic error concatenation and prefixing Peter Crosthwaite
` (12 preceding siblings ...)
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 13/25] arm: allwinner: " Peter Crosthwaite
@ 2015-09-11 5:33 ` Peter Crosthwaite
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 15/25] cpu: arm: Remove un-needed error checking Peter Crosthwaite
` (12 subsequent siblings)
26 siblings, 0 replies; 36+ messages in thread
From: Peter Crosthwaite @ 2015-09-11 5:33 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, armbru
Collect all errors and remove constant checking for realize failures.
Prefix errors and use error_report_err() as appropriate.
Cc: Antony Pavlov <antonynpavlov@gmail.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
hw/arm/digic.c | 19 ++-----------------
hw/arm/digic_boards.c | 4 ++--
2 files changed, 4 insertions(+), 19 deletions(-)
diff --git a/hw/arm/digic.c b/hw/arm/digic.c
index ec8c330..2632517 100644
--- a/hw/arm/digic.c
+++ b/hw/arm/digic.c
@@ -60,36 +60,21 @@ static void digic_realize(DeviceState *dev, Error **errp)
int i;
object_property_set_bool(OBJECT(&s->cpu), true, "reset-hivecs", &err);
- if (err != NULL) {
- error_propagate(errp, err);
- return;
- }
-
object_property_set_bool(OBJECT(&s->cpu), true, "realized", &err);
- if (err != NULL) {
- error_propagate(errp, err);
- return;
- }
for (i = 0; i < DIGIC4_NB_TIMERS; i++) {
object_property_set_bool(OBJECT(&s->timer[i]), true, "realized", &err);
- if (err != NULL) {
- error_propagate(errp, err);
- return;
- }
sbd = SYS_BUS_DEVICE(&s->timer[i]);
sysbus_mmio_map(sbd, 0, DIGIC4_TIMER_BASE(i));
}
object_property_set_bool(OBJECT(&s->uart), true, "realized", &err);
- if (err != NULL) {
- error_propagate(errp, err);
- return;
- }
sbd = SYS_BUS_DEVICE(&s->uart);
sysbus_mmio_map(sbd, 0, DIGIC_UART_BASE);
+
+ error_propagate(errp, err);
}
static void digic_class_init(ObjectClass *oc, void *data)
diff --git a/hw/arm/digic_boards.c b/hw/arm/digic_boards.c
index f8ba9e5..4256ece 100644
--- a/hw/arm/digic_boards.c
+++ b/hw/arm/digic_boards.c
@@ -64,8 +64,8 @@ static void digic4_board_init(DigicBoard *board)
s->digic = DIGIC(object_new(TYPE_DIGIC));
object_property_set_bool(OBJECT(s->digic), true, "realized", &err);
if (err != NULL) {
- error_report("Couldn't realize DIGIC SoC: %s",
- error_get_pretty(err));
+ error_prefix(err, "Couldn't realize DIGIC SoC: ");
+ error_report_err(err);
exit(1);
}
--
1.9.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [RFC PATCH v1 15/25] cpu: arm: Remove un-needed error checking
2015-09-11 5:33 [Qemu-devel] [RFC PATCH v1 00/25] error: Automatic error concatenation and prefixing Peter Crosthwaite
` (13 preceding siblings ...)
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 14/25] arm: digic: " Peter Crosthwaite
@ 2015-09-11 5:33 ` Peter Crosthwaite
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 16/25] ppc: Update error API usages Peter Crosthwaite
` (11 subsequent siblings)
26 siblings, 0 replies; 36+ messages in thread
From: Peter Crosthwaite @ 2015-09-11 5:33 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, armbru
Collect all errors and remove constant checking for realize failures.
Cc: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
hw/cpu/a15mpcore.c | 6 ++----
hw/cpu/a9mpcore.c | 22 ++--------------------
hw/cpu/arm11mpcore.c | 18 ++----------------
hw/cpu/realview_mpcore.c | 9 +--------
4 files changed, 7 insertions(+), 48 deletions(-)
diff --git a/hw/cpu/a15mpcore.c b/hw/cpu/a15mpcore.c
index 58ac02e..b7da299 100644
--- a/hw/cpu/a15mpcore.c
+++ b/hw/cpu/a15mpcore.c
@@ -57,10 +57,6 @@ static void a15mp_priv_realize(DeviceState *dev, Error **errp)
qdev_prop_set_uint32(gicdev, "num-cpu", s->num_cpu);
qdev_prop_set_uint32(gicdev, "num-irq", s->num_irq);
object_property_set_bool(OBJECT(&s->gic), true, "realized", &err);
- if (err != NULL) {
- error_propagate(errp, err);
- return;
- }
busdev = SYS_BUS_DEVICE(&s->gic);
/* Pass through outbound IRQ lines from the GIC */
@@ -104,6 +100,8 @@ static void a15mp_priv_realize(DeviceState *dev, Error **errp)
sysbus_mmio_get_region(busdev, 0));
memory_region_add_subregion(&s->container, 0x2000,
sysbus_mmio_get_region(busdev, 1));
+
+ error_propagate(errp, err);
}
static Property a15mp_priv_properties[] = {
diff --git a/hw/cpu/a9mpcore.c b/hw/cpu/a9mpcore.c
index c09358c..41f5a96 100644
--- a/hw/cpu/a9mpcore.c
+++ b/hw/cpu/a9mpcore.c
@@ -53,20 +53,12 @@ static void a9mp_priv_realize(DeviceState *dev, Error **errp)
scudev = DEVICE(&s->scu);
qdev_prop_set_uint32(scudev, "num-cpu", s->num_cpu);
object_property_set_bool(OBJECT(&s->scu), true, "realized", &err);
- if (err != NULL) {
- error_propagate(errp, err);
- return;
- }
scubusdev = SYS_BUS_DEVICE(&s->scu);
gicdev = DEVICE(&s->gic);
qdev_prop_set_uint32(gicdev, "num-cpu", s->num_cpu);
qdev_prop_set_uint32(gicdev, "num-irq", s->num_irq);
object_property_set_bool(OBJECT(&s->gic), true, "realized", &err);
- if (err != NULL) {
- error_propagate(errp, err);
- return;
- }
gicbusdev = SYS_BUS_DEVICE(&s->gic);
/* Pass through outbound IRQ lines from the GIC */
@@ -78,28 +70,16 @@ static void a9mp_priv_realize(DeviceState *dev, Error **errp)
gtimerdev = DEVICE(&s->gtimer);
qdev_prop_set_uint32(gtimerdev, "num-cpu", s->num_cpu);
object_property_set_bool(OBJECT(&s->gtimer), true, "realized", &err);
- if (err != NULL) {
- error_propagate(errp, err);
- return;
- }
gtimerbusdev = SYS_BUS_DEVICE(&s->gtimer);
mptimerdev = DEVICE(&s->mptimer);
qdev_prop_set_uint32(mptimerdev, "num-cpu", s->num_cpu);
object_property_set_bool(OBJECT(&s->mptimer), true, "realized", &err);
- if (err != NULL) {
- error_propagate(errp, err);
- return;
- }
mptimerbusdev = SYS_BUS_DEVICE(&s->mptimer);
wdtdev = DEVICE(&s->wdt);
qdev_prop_set_uint32(wdtdev, "num-cpu", s->num_cpu);
object_property_set_bool(OBJECT(&s->wdt), true, "realized", &err);
- if (err != NULL) {
- error_propagate(errp, err);
- return;
- }
wdtbusdev = SYS_BUS_DEVICE(&s->wdt);
/* Memory map (addresses are offsets from PERIPHBASE):
@@ -141,6 +121,8 @@ static void a9mp_priv_realize(DeviceState *dev, Error **errp)
sysbus_connect_irq(wdtbusdev, i,
qdev_get_gpio_in(gicdev, ppibase + 30));
}
+
+ error_propagate(errp, err);
}
static Property a9mp_priv_properties[] = {
diff --git a/hw/cpu/arm11mpcore.c b/hw/cpu/arm11mpcore.c
index 717d3e4..5ba8977 100644
--- a/hw/cpu/arm11mpcore.c
+++ b/hw/cpu/arm11mpcore.c
@@ -74,18 +74,10 @@ static void mpcore_priv_realize(DeviceState *dev, Error **errp)
qdev_prop_set_uint32(scudev, "num-cpu", s->num_cpu);
object_property_set_bool(OBJECT(&s->scu), true, "realized", &err);
- if (err != NULL) {
- error_propagate(errp, err);
- return;
- }
qdev_prop_set_uint32(gicdev, "num-cpu", s->num_cpu);
qdev_prop_set_uint32(gicdev, "num-irq", s->num_irq);
object_property_set_bool(OBJECT(&s->gic), true, "realized", &err);
- if (err != NULL) {
- error_propagate(errp, err);
- return;
- }
/* Pass through outbound IRQ lines from the GIC */
sysbus_pass_irq(sbd, SYS_BUS_DEVICE(&s->gic));
@@ -95,19 +87,13 @@ static void mpcore_priv_realize(DeviceState *dev, Error **errp)
qdev_prop_set_uint32(mptimerdev, "num-cpu", s->num_cpu);
object_property_set_bool(OBJECT(&s->mptimer), true, "realized", &err);
- if (err != NULL) {
- error_propagate(errp, err);
- return;
- }
qdev_prop_set_uint32(wdtimerdev, "num-cpu", s->num_cpu);
object_property_set_bool(OBJECT(&s->wdtimer), true, "realized", &err);
- if (err != NULL) {
- error_propagate(errp, err);
- return;
- }
mpcore_priv_map_setup(s);
+
+ error_propagate(errp, err);
}
static void mpcore_priv_initfn(Object *obj)
diff --git a/hw/cpu/realview_mpcore.c b/hw/cpu/realview_mpcore.c
index c39a2da..f6c3b89 100644
--- a/hw/cpu/realview_mpcore.c
+++ b/hw/cpu/realview_mpcore.c
@@ -66,10 +66,6 @@ static void realview_mpcore_realize(DeviceState *dev, Error **errp)
qdev_prop_set_uint32(priv, "num-cpu", s->num_cpu);
object_property_set_bool(OBJECT(&s->priv), true, "realized", &err);
- if (err != NULL) {
- error_propagate(errp, err);
- return;
- }
sysbus_pass_irq(sbd, SYS_BUS_DEVICE(&s->priv));
for (i = 0; i < 32; i++) {
s->cpuic[i] = qdev_get_gpio_in(priv, i);
@@ -77,10 +73,6 @@ static void realview_mpcore_realize(DeviceState *dev, Error **errp)
/* ??? IRQ routing is hardcoded to "normal" mode. */
for (n = 0; n < 4; n++) {
object_property_set_bool(OBJECT(&s->gic[n]), true, "realized", &err);
- if (err != NULL) {
- error_propagate(errp, err);
- return;
- }
gic = DEVICE(&s->gic[n]);
gicbusdev = SYS_BUS_DEVICE(&s->gic[n]);
sysbus_mmio_map(gicbusdev, 0, 0x10040000 + n * 0x10000);
@@ -90,6 +82,7 @@ static void realview_mpcore_realize(DeviceState *dev, Error **errp)
}
}
qdev_init_gpio_in(dev, mpcore_rirq_set_irq, 64);
+ error_propagate(errp, err);
}
static void mpcore_rirq_init(Object *obj)
--
1.9.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [RFC PATCH v1 16/25] ppc: Update error API usages
2015-09-11 5:33 [Qemu-devel] [RFC PATCH v1 00/25] error: Automatic error concatenation and prefixing Peter Crosthwaite
` (14 preceding siblings ...)
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 15/25] cpu: arm: Remove un-needed error checking Peter Crosthwaite
@ 2015-09-11 5:33 ` Peter Crosthwaite
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 17/25] i386: pc: " Peter Crosthwaite
` (10 subsequent siblings)
26 siblings, 0 replies; 36+ messages in thread
From: Peter Crosthwaite @ 2015-09-11 5:33 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, armbru
Use error_prefix() and error_report_err() as appropriate.
Cc: Alexander Graf <agraf@suse.de>
Cc: qemu-ppc@nongnu.org
Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
hw/ppc/e500.c | 4 ++--
hw/ppc/spapr.c | 4 ++--
hw/ppc/spapr_drc.c | 6 ++----
3 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index d300846..798e03d 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -751,8 +751,8 @@ static qemu_irq *ppce500_init_mpic(MachineState *machine, PPCE500Params *params,
dev = ppce500_init_mpic_kvm(params, irqs, &err);
}
if (machine_kernel_irqchip_required(machine) && !dev) {
- error_report("kernel_irqchip requested but unavailable: %s",
- error_get_pretty(err));
+ error_prefix(err, "kernel_irqchip requested but unavailable: ");
+ error_report_err(err);
exit(1);
}
}
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index bf0c64f..0cbca9f 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -121,8 +121,8 @@ static XICSState *xics_system_init(MachineState *machine,
icp = try_create_xics(TYPE_KVM_XICS, nr_servers, nr_irqs, &err);
}
if (machine_kernel_irqchip_required(machine) && !icp) {
- error_report("kernel_irqchip requested but unavailable: %s",
- error_get_pretty(err));
+ error_prefix(err, "kernel_irqchip requested but unavailable: ");
+ error_report_err(err);
}
}
diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index ee87432..9837ef2 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -418,8 +418,7 @@ static void realize(DeviceState *d, Error **errp)
object_property_add_alias(root_container, link_name,
drc->owner, child_name, &err);
if (err) {
- error_report("%s", error_get_pretty(err));
- error_free(err);
+ error_report_err(err);
object_unref(OBJECT(drc));
}
g_free(child_name);
@@ -439,8 +438,7 @@ static void unrealize(DeviceState *d, Error **errp)
snprintf(name, sizeof(name), "%x", drck->get_index(drc));
object_property_del(root_container, name, &err);
if (err) {
- error_report("%s", error_get_pretty(err));
- error_free(err);
+ error_report_err(err);
object_unref(OBJECT(drc));
}
}
--
1.9.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [RFC PATCH v1 17/25] i386: pc: Update error API usages
2015-09-11 5:33 [Qemu-devel] [RFC PATCH v1 00/25] error: Automatic error concatenation and prefixing Peter Crosthwaite
` (15 preceding siblings ...)
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 16/25] ppc: Update error API usages Peter Crosthwaite
@ 2015-09-11 5:33 ` Peter Crosthwaite
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 18/25] monitor: update " Peter Crosthwaite
` (9 subsequent siblings)
26 siblings, 0 replies; 36+ messages in thread
From: Peter Crosthwaite @ 2015-09-11 5:33 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, armbru
Use error_prefix() and error_report_err() as appropriate.
Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
hw/i386/pc.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 9f2924e..1edee5b 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1276,9 +1276,8 @@ void pc_acpi_init(const char *default_dsdt)
acpi_table_add_builtin(opts, &err);
if (err) {
- error_report("WARNING: failed to load %s: %s", filename,
- error_get_pretty(err));
- error_free(err);
+ error_prefix(err, "WARNING: failed to load %s: ", filename);
+ error_report_err(err);
}
g_free(filename);
}
--
1.9.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [RFC PATCH v1 18/25] monitor: update error API usages
2015-09-11 5:33 [Qemu-devel] [RFC PATCH v1 00/25] error: Automatic error concatenation and prefixing Peter Crosthwaite
` (16 preceding siblings ...)
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 17/25] i386: pc: " Peter Crosthwaite
@ 2015-09-11 5:33 ` Peter Crosthwaite
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 19/25] qdev: Update " Peter Crosthwaite
` (8 subsequent siblings)
26 siblings, 0 replies; 36+ messages in thread
From: Peter Crosthwaite @ 2015-09-11 5:33 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, armbru
Convert monitor_printf() to a error API printfer and use
error_printf_fn().
Cc: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
hmp.c | 33 +++++++++++++--------------------
include/monitor/monitor.h | 2 +-
monitor.c | 11 +++++------
stubs/mon-printf.c | 2 +-
4 files changed, 20 insertions(+), 28 deletions(-)
diff --git a/hmp.c b/hmp.c
index 3f807b7..82e1309 100644
--- a/hmp.c
+++ b/hmp.c
@@ -40,8 +40,7 @@ static void hmp_handle_error(Monitor *mon, Error **errp)
{
assert(errp);
if (*errp) {
- monitor_printf(mon, "%s\n", error_get_pretty(*errp));
- error_free(*errp);
+ error_printf_fn(*errp, monitor_printf, mon);
}
}
@@ -534,8 +533,7 @@ void hmp_info_vnc(Monitor *mon, const QDict *qdict)
info = qmp_query_vnc(&err);
if (err) {
- monitor_printf(mon, "%s\n", error_get_pretty(err));
- error_free(err);
+ error_printf_fn(err, monitor_printf, mon);
return;
}
@@ -657,8 +655,7 @@ void hmp_info_balloon(Monitor *mon, const QDict *qdict)
info = qmp_query_balloon(&err);
if (err) {
- monitor_printf(mon, "%s\n", error_get_pretty(err));
- error_free(err);
+ error_printf_fn(err, monitor_printf, mon);
return;
}
@@ -926,8 +923,7 @@ void hmp_ringbuf_read(Monitor *mon, const QDict *qdict)
data = qmp_ringbuf_read(chardev, size, false, 0, &err);
if (err) {
- monitor_printf(mon, "%s\n", error_get_pretty(err));
- error_free(err);
+ error_printf_fn(err, monitor_printf, mon);
return;
}
@@ -1020,8 +1016,8 @@ void hmp_balloon(Monitor *mon, const QDict *qdict)
qmp_balloon(value, &err);
if (err) {
- monitor_printf(mon, "balloon: %s\n", error_get_pretty(err));
- error_free(err);
+ error_prefix(err, "balloon: ");
+ error_printf_fn(err, monitor_printf, mon);
}
}
@@ -1169,8 +1165,7 @@ void hmp_migrate_set_cache_size(Monitor *mon, const QDict *qdict)
qmp_migrate_set_cache_size(value, &err);
if (err) {
- monitor_printf(mon, "%s\n", error_get_pretty(err));
- error_free(err);
+ error_printf_fn(err, monitor_printf, mon);
return;
}
}
@@ -1207,9 +1202,8 @@ void hmp_migrate_set_capability(Monitor *mon, const QDict *qdict)
qapi_free_MigrationCapabilityStatusList(caps);
if (err) {
- monitor_printf(mon, "migrate_set_capability: %s\n",
- error_get_pretty(err));
- error_free(err);
+ error_prefix(err, "migrate_set_capability: ");
+ error_printf_fn(err, monitor_printf, mon);
}
}
@@ -1249,9 +1243,8 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
}
if (err) {
- monitor_printf(mon, "migrate_set_parameter: %s\n",
- error_get_pretty(err));
- error_free(err);
+ error_prefix(err, "migrate_set_parameter: ");
+ error_printf_fn(err, monitor_printf, mon);
}
}
@@ -1483,8 +1476,8 @@ void hmp_migrate(Monitor *mon, const QDict *qdict)
qmp_migrate(uri, !!blk, blk, !!inc, inc, false, false, &err);
if (err) {
- monitor_printf(mon, "migrate: %s\n", error_get_pretty(err));
- error_free(err);
+ error_prefix(err, "migrate: ");
+ error_printf_fn(err, monitor_printf, mon);
return;
}
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 9aff47e..2900a5c 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -33,7 +33,7 @@ int monitor_fd_param(Monitor *mon, const char *fdname, Error **errp);
void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
GCC_FMT_ATTR(2, 0);
-void monitor_printf(Monitor *mon, const char *fmt, ...) GCC_FMT_ATTR(2, 3);
+void monitor_printf(void *mon, const char *fmt, ...) GCC_FMT_ATTR(2, 3);
void monitor_flush(Monitor *mon);
int monitor_set_cpu(int cpu_index);
int monitor_get_cpu_index(void);
diff --git a/monitor.c b/monitor.c
index 5455ab9..b0747f4 100644
--- a/monitor.c
+++ b/monitor.c
@@ -363,11 +363,12 @@ void monitor_vprintf(Monitor *mon, const char *fmt, va_list ap)
g_free(buf);
}
-void monitor_printf(Monitor *mon, const char *fmt, ...)
+void monitor_printf(void *mon, const char *fmt, ...)
{
+ Monitor *monitor = mon;
va_list ap;
va_start(ap, fmt);
- monitor_vprintf(mon, fmt, ap);
+ monitor_vprintf(monitor, fmt, ap);
va_end(ap);
}
@@ -1417,8 +1418,7 @@ static void hmp_boot_set(Monitor *mon, const QDict *qdict)
qemu_boot_set(bootdevice, &local_err);
if (local_err) {
- monitor_printf(mon, "%s\n", error_get_pretty(local_err));
- error_free(local_err);
+ error_printf_fn(local_err, monitor_printf, mon);
} else {
monitor_printf(mon, "boot device list now set to %s\n", bootdevice);
}
@@ -5318,8 +5318,7 @@ static void bdrv_password_cb(void *opaque, const char *password,
bdrv_add_key(bs, password, &local_err);
if (local_err) {
- monitor_printf(mon, "%s\n", error_get_pretty(local_err));
- error_free(local_err);
+ error_printf_fn(local_err, monitor_printf, mon);
ret = -EPERM;
}
if (mon->password_completion_cb)
diff --git a/stubs/mon-printf.c b/stubs/mon-printf.c
index 0ce2ca6..97a0c2a 100644
--- a/stubs/mon-printf.c
+++ b/stubs/mon-printf.c
@@ -1,7 +1,7 @@
#include "qemu-common.h"
#include "monitor/monitor.h"
-void monitor_printf(Monitor *mon, const char *fmt, ...)
+void monitor_printf(void *mon, const char *fmt, ...)
{
}
--
1.9.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [RFC PATCH v1 19/25] qdev: Update error API usages
2015-09-11 5:33 [Qemu-devel] [RFC PATCH v1 00/25] error: Automatic error concatenation and prefixing Peter Crosthwaite
` (17 preceding siblings ...)
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 18/25] monitor: update " Peter Crosthwaite
@ 2015-09-11 5:33 ` Peter Crosthwaite
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 20/25] block: " Peter Crosthwaite
` (7 subsequent siblings)
26 siblings, 0 replies; 36+ messages in thread
From: Peter Crosthwaite @ 2015-09-11 5:33 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, armbru
Use error_prefix() and error_report_err() as appropriate.
Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
hw/core/qdev-properties.c | 7 +++----
hw/core/qdev.c | 6 +++---
2 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 04fd80a..6494185 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -1065,10 +1065,9 @@ static void qdev_prop_set_globals_for_type(DeviceState *dev,
object_property_parse(OBJECT(dev), prop->value, prop->property, &err);
if (err != NULL) {
assert(prop->user_provided);
- error_report("Warning: global %s.%s=%s ignored (%s)",
- prop->driver, prop->property, prop->value,
- error_get_pretty(err));
- error_free(err);
+ error_prefix(err, "Warning: global %s.%s=%s ignored (%s)",
+ prop->driver, prop->property, prop->value);
+ error_report_err(err);
return;
}
}
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 52140ba..708f5fd 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -365,9 +365,9 @@ void qdev_init_nofail(DeviceState *dev)
object_property_set_bool(OBJECT(dev), true, "realized", &err);
if (err) {
- error_report("Initialization of device %s failed: %s",
- object_get_typename(OBJECT(dev)),
- error_get_pretty(err));
+ error_prefix(err, "Initialization of device %s failed: ",
+ object_get_typename(OBJECT(dev)));
+ error_report_err(err);
exit(1);
}
}
--
1.9.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [RFC PATCH v1 20/25] block: Update error API usages
2015-09-11 5:33 [Qemu-devel] [RFC PATCH v1 00/25] error: Automatic error concatenation and prefixing Peter Crosthwaite
` (18 preceding siblings ...)
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 19/25] qdev: Update " Peter Crosthwaite
@ 2015-09-11 5:33 ` Peter Crosthwaite
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 21/25] tests: " Peter Crosthwaite
` (6 subsequent siblings)
26 siblings, 0 replies; 36+ messages in thread
From: Peter Crosthwaite @ 2015-09-11 5:33 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, armbru
Use error_prefix() and error_report_err() as appropriate. Use the
prefix + propagate combo rather than manual prefix + freeing.
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: qemu-block@nongnu.org
Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
block.c | 5 ++---
block/qcow2.c | 5 ++---
block/qed.c | 5 ++---
block/sheepdog.c | 8 +++-----
blockdev.c | 10 +++++-----
hw/block/dataplane/virtio-blk.c | 5 ++---
qemu-img.c | 38 ++++++++++++++++++--------------------
7 files changed, 34 insertions(+), 42 deletions(-)
diff --git a/block.c b/block.c
index 090923c..f743f1e 100644
--- a/block.c
+++ b/block.c
@@ -1233,9 +1233,8 @@ int bdrv_open_backing_file(BlockDriverState *bs, QDict *options, Error **errp)
bdrv_unref(backing_hd);
backing_hd = NULL;
bs->open_flags |= BDRV_O_NO_BACKING;
- error_setg(errp, "Could not open backing file: %s",
- error_get_pretty(local_err));
- error_free(local_err);
+ error_prefix(local_err, "Could not open backing file: ");
+ error_propagate(errp, local_err);
goto free_exit;
}
diff --git a/block/qcow2.c b/block/qcow2.c
index ea34ae2..b5c45e1 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1581,9 +1581,8 @@ static void qcow2_invalidate_cache(BlockDriverState *bs, Error **errp)
ret = qcow2_open(bs, options, flags, &local_err);
QDECREF(options);
if (local_err) {
- error_setg(errp, "Could not reopen qcow2 layer: %s",
- error_get_pretty(local_err));
- error_free(local_err);
+ error_prefix(local_err, "Could not reopen qcow2 layer: ");
+ error_propagate(errp, local_err);
return;
} else if (ret < 0) {
error_setg_errno(errp, -ret, "Could not reopen qcow2 layer");
diff --git a/block/qed.c b/block/qed.c
index 954ed00..0c4c5b6 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -1605,9 +1605,8 @@ static void bdrv_qed_invalidate_cache(BlockDriverState *bs, Error **errp)
memset(s, 0, sizeof(BDRVQEDState));
ret = bdrv_qed_open(bs, NULL, bs->open_flags, &local_err);
if (local_err) {
- error_setg(errp, "Could not reopen qed layer: %s",
- error_get_pretty(local_err));
- error_free(local_err);
+ error_prefix(local_err, "Could not reopen qed layer: ");
+ error_propagate(errp, local_err);
return;
} else if (ret < 0) {
error_setg_errno(errp, -ret, "Could not reopen qed layer");
diff --git a/block/sheepdog.c b/block/sheepdog.c
index 9585beb..4712da0 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1776,8 +1776,7 @@ static int sd_create(const char *filename, QemuOpts *opts,
fd = connect_to_sdog(s, &local_err);
if (fd < 0) {
- error_report("%s", error_get_pretty(local_err));
- error_free(local_err);
+ error_report_err(local_err);
ret = -EIO;
goto out;
}
@@ -2318,9 +2317,8 @@ static int sd_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)
ret = do_sd_create(s, &new_vid, 1, &local_err);
if (ret < 0) {
- error_report("failed to create inode for snapshot: %s",
- error_get_pretty(local_err));
- error_free(local_err);
+ error_prefix(local_err, "failed to create inode for snapshot: ");
+ error_report_err(local_err);
goto cleanup;
}
diff --git a/blockdev.c b/blockdev.c
index 6b48be6..78921fb 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1391,13 +1391,13 @@ static void internal_snapshot_abort(BlkTransactionState *common)
}
if (bdrv_snapshot_delete(bs, sn->id_str, sn->name, &local_error) < 0) {
- error_report("Failed to delete snapshot with id '%s' and name '%s' on "
- "device '%s' in abort: %s",
+ error_prefix(local_error,
+ "Failed to delete snapshot with id '%s' and name '%s' on "
+ "device '%s' in abort:",
sn->id_str,
sn->name,
- bdrv_get_device_name(bs),
- error_get_pretty(local_error));
- error_free(local_error);
+ bdrv_get_device_name(bs));
+ error_report_err(local_error);
}
}
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 6106e46..ead5585 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -166,9 +166,8 @@ void virtio_blk_data_plane_create(VirtIODevice *vdev, VirtIOBlkConf *conf,
*/
if (blk_op_is_blocked(conf->conf.blk, BLOCK_OP_TYPE_DATAPLANE,
&local_err)) {
- error_setg(errp, "cannot start dataplane thread: %s",
- error_get_pretty(local_err));
- error_free(local_err);
+ error_prefix(local_err, "cannot start dataplane thread: ");
+ error_propagate(errp, local_err);
return;
}
diff --git a/qemu-img.c b/qemu-img.c
index 6ff4e85..22fc1d0 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -213,9 +213,8 @@ static BlockBackend *img_open(const char *id, const char *filename,
blk = blk_new_open(id, filename, NULL, options, flags, &local_err);
if (!blk) {
- error_report("Could not open '%s': %s", filename,
- error_get_pretty(local_err));
- error_free(local_err);
+ error_prefix(local_err, "Could not open '%s': ", filename);
+ error_report_err(local_err);
goto fail;
}
@@ -359,8 +358,8 @@ static int img_create(int argc, char **argv)
bdrv_img_create(filename, fmt, base_filename, base_fmt,
options, img_size, BDRV_O_FLAGS, &local_err, quiet);
if (local_err) {
- error_report("%s: %s", filename, error_get_pretty(local_err));
- error_free(local_err);
+ error_prefix(local_err, "%s: ", filename);
+ error_report_err(local_err);
goto fail;
}
@@ -1712,9 +1711,8 @@ static int img_convert(int argc, char **argv)
bdrv_snapshot_load_tmp_by_id_or_name(bs[0], snapshot_name, &local_err);
}
if (local_err) {
- error_report("Failed to load snapshot: %s",
- error_get_pretty(local_err));
- error_free(local_err);
+ error_prefix(local_err, "Failed to load snapshot: ");
+ error_report_err(local_err);
ret = -1;
goto out;
}
@@ -1810,9 +1808,9 @@ static int img_convert(int argc, char **argv)
/* Create the new image */
ret = bdrv_create(drv, out_filename, opts, &local_err);
if (ret < 0) {
- error_report("%s: error while converting %s: %s",
- out_filename, out_fmt, error_get_pretty(local_err));
- error_free(local_err);
+ error_prefix(local_err, "%s: error while converting %s: ",
+ out_filename, out_fmt);
+ error_report_err(local_err);
goto out;
}
}
@@ -2437,9 +2435,9 @@ static int img_snapshot(int argc, char **argv)
case SNAPSHOT_DELETE:
bdrv_snapshot_delete_by_id_or_name(bs, snapshot_name, &err);
if (err) {
- error_report("Could not delete snapshot '%s': (%s)",
- snapshot_name, error_get_pretty(err));
- error_free(err);
+ error_prefix(err, "Could not delete snapshot '%s': ",
+ snapshot_name);
+ error_report_err(err);
ret = 1;
}
break;
@@ -2572,9 +2570,9 @@ static int img_rebase(int argc, char **argv)
blk_old_backing = blk_new_open("old_backing", backing_name, NULL,
options, src_flags, &local_err);
if (!blk_old_backing) {
- error_report("Could not open old backing file '%s': %s",
- backing_name, error_get_pretty(local_err));
- error_free(local_err);
+ error_prefix(local_err, "Could not open old backing file '%s': ",
+ backing_name);
+ error_report_err(local_err);
goto out;
}
@@ -2589,9 +2587,9 @@ static int img_rebase(int argc, char **argv)
blk_new_backing = blk_new_open("new_backing", out_baseimg, NULL,
options, src_flags, &local_err);
if (!blk_new_backing) {
- error_report("Could not open new backing file '%s': %s",
- out_baseimg, error_get_pretty(local_err));
- error_free(local_err);
+ error_prefix(local_err, "Could not open new backing file '%s':",
+ out_baseimg);
+ error_report_err(local_err);
goto out;
}
}
--
1.9.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [RFC PATCH v1 21/25] tests: Update error API usages
2015-09-11 5:33 [Qemu-devel] [RFC PATCH v1 00/25] error: Automatic error concatenation and prefixing Peter Crosthwaite
` (19 preceding siblings ...)
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 20/25] block: " Peter Crosthwaite
@ 2015-09-11 5:33 ` Peter Crosthwaite
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 22/25] usb: bus: " Peter Crosthwaite
` (5 subsequent siblings)
26 siblings, 0 replies; 36+ messages in thread
From: Peter Crosthwaite @ 2015-09-11 5:33 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, armbru
Use error_prefix() and error_report_err() as appropriate.
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
tests/test-aio.c | 5 ++---
tests/test-thread-pool.c | 5 ++---
tests/test-throttle.c | 9 ++++-----
3 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/tests/test-aio.c b/tests/test-aio.c
index 217e337..daeeb47 100644
--- a/tests/test-aio.c
+++ b/tests/test-aio.c
@@ -802,9 +802,8 @@ int main(int argc, char **argv)
ctx = aio_context_new(&local_error);
if (!ctx) {
- error_report("Failed to create AIO Context: '%s'",
- error_get_pretty(local_error));
- error_free(local_error);
+ error_prefix(local_err, "Failed to create AIO Context: ");
+ error_report_err(local_err);
exit(1);
}
src = aio_get_g_source(ctx);
diff --git a/tests/test-thread-pool.c b/tests/test-thread-pool.c
index 6a0b981..231c789 100644
--- a/tests/test-thread-pool.c
+++ b/tests/test-thread-pool.c
@@ -229,9 +229,8 @@ int main(int argc, char **argv)
ctx = aio_context_new(&local_error);
if (!ctx) {
- error_report("Failed to create AIO Context: '%s'",
- error_get_pretty(local_error));
- error_free(local_error);
+ error_prefix(local_error, "Failed to create AIO Context:");
+ error_report_err(local_error);
exit(1);
}
pool = aio_get_thread_pool(ctx);
diff --git a/tests/test-throttle.c b/tests/test-throttle.c
index 85c9b6c..4996445 100644
--- a/tests/test-throttle.c
+++ b/tests/test-throttle.c
@@ -587,12 +587,11 @@ int main(int argc, char **argv)
ctx = qemu_get_aio_context();
if (!ctx) {
- error_report("Failed to create AIO Context: '%s'",
- local_error ? error_get_pretty(local_error) :
- "Failed to initialize the QEMU main loop");
- if (local_error) {
- error_free(local_error);
+ if (!local_error) {
+ error_setg(&local_error, "Failed to initialize the QEMU main loop");
}
+ error_prefix(local_error, "Failed to create AIO Context: ");
+ error_report_err(local_error);
exit(1);
}
--
1.9.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [RFC PATCH v1 22/25] usb: bus: Update error API usages
2015-09-11 5:33 [Qemu-devel] [RFC PATCH v1 00/25] error: Automatic error concatenation and prefixing Peter Crosthwaite
` (20 preceding siblings ...)
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 21/25] tests: " Peter Crosthwaite
@ 2015-09-11 5:33 ` Peter Crosthwaite
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 23/25] scsi: " Peter Crosthwaite
` (4 subsequent siblings)
26 siblings, 0 replies; 36+ messages in thread
From: Peter Crosthwaite @ 2015-09-11 5:33 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, armbru
Use error_prefix() and error_report_err() as appropriate. Use the
prefix + propagate combo rather than manual prefix + freeing.
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
hw/usb/bus.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index 5f39e1e..44264ee 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -329,9 +329,8 @@ static USBDevice *usb_try_create_simple(USBBus *bus, const char *name,
}
object_property_set_bool(OBJECT(dev), true, "realized", &err);
if (err) {
- error_setg(errp, "Failed to initialize USB device '%s': %s",
- name, error_get_pretty(err));
- error_free(err);
+ error_prefix(err, "Failed to initialize USB device '%s': ", name);
+ error_propagate(errp, err);
object_unparent(OBJECT(dev));
return NULL;
}
@@ -722,9 +721,8 @@ USBDevice *usbdevice_create(const char *cmdline)
}
object_property_set_bool(OBJECT(dev), true, "realized", &err);
if (err) {
- error_report("Failed to initialize USB device '%s': %s",
- f->name, error_get_pretty(err));
- error_free(err);
+ error_prefix(err, "Failed to initialize USB device '%s': ", f->name);
+ error_report_err(err);
object_unparent(OBJECT(dev));
return NULL;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [RFC PATCH v1 23/25] scsi: Update error API usages
2015-09-11 5:33 [Qemu-devel] [RFC PATCH v1 00/25] error: Automatic error concatenation and prefixing Peter Crosthwaite
` (21 preceding siblings ...)
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 22/25] usb: bus: " Peter Crosthwaite
@ 2015-09-11 5:33 ` Peter Crosthwaite
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 24/25] migration: savevm: " Peter Crosthwaite
` (3 subsequent siblings)
26 siblings, 0 replies; 36+ messages in thread
From: Peter Crosthwaite @ 2015-09-11 5:33 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, armbru
Use the prefix + propagate combo rather than manual prefix + freeing.
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
hw/scsi/vhost-scsi.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/hw/scsi/vhost-scsi.c b/hw/scsi/vhost-scsi.c
index 7eacca9..bd55ebc 100644
--- a/hw/scsi/vhost-scsi.c
+++ b/hw/scsi/vhost-scsi.c
@@ -219,9 +219,8 @@ static void vhost_scsi_realize(DeviceState *dev, Error **errp)
if (vs->conf.vhostfd) {
vhostfd = monitor_fd_param(cur_mon, vs->conf.vhostfd, &err);
if (vhostfd == -1) {
- error_setg(errp, "vhost-scsi: unable to parse vhostfd: %s",
- error_get_pretty(err));
- error_free(err);
+ error_prefix(err, "vhost-scsi: unable to parse vhostfd: ");
+ error_propagate(errp, err);
return;
}
} else {
--
1.9.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [RFC PATCH v1 24/25] migration: savevm: Update error API usages
2015-09-11 5:33 [Qemu-devel] [RFC PATCH v1 00/25] error: Automatic error concatenation and prefixing Peter Crosthwaite
` (22 preceding siblings ...)
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 23/25] scsi: " Peter Crosthwaite
@ 2015-09-11 5:33 ` Peter Crosthwaite
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 25/25] core: " Peter Crosthwaite
` (2 subsequent siblings)
26 siblings, 0 replies; 36+ messages in thread
From: Peter Crosthwaite @ 2015-09-11 5:33 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, armbru
Use error_prefix() and convert monitor_printf to error_printf_fn.
Cc: Juan Quintela <quintela@redhat.com>
Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
migration/savevm.c | 22 +++++++++-------------
1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/migration/savevm.c b/migration/savevm.c
index 6071215..683201d 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1266,12 +1266,10 @@ static int del_existing_snapshots(Monitor *mon, const char *name)
bdrv_snapshot_find(bs, snapshot, name) >= 0) {
bdrv_snapshot_delete_by_id_or_name(bs, name, &err);
if (err) {
- monitor_printf(mon,
- "Error while deleting snapshot on device '%s':"
- " %s\n",
- bdrv_get_device_name(bs),
- error_get_pretty(err));
- error_free(err);
+ error_prefix(err,
+ "Error while deleting snapshot on device '%s':",
+ bdrv_get_device_name(bs));
+ error_printf_fn(err, monitor_printf, mon);
return -1;
}
}
@@ -1360,7 +1358,7 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
vm_state_size = qemu_ftell(f);
qemu_fclose(f);
if (ret < 0) {
- monitor_printf(mon, "%s\n", error_get_pretty(local_err));
+ error_printf_fn(local_err, monitor_printf, mon);
error_free(local_err);
goto the_end;
}
@@ -1512,12 +1510,10 @@ void hmp_delvm(Monitor *mon, const QDict *qdict)
err = NULL;
bdrv_snapshot_delete_by_id_or_name(bs, name, &err);
if (err) {
- monitor_printf(mon,
- "Error while deleting snapshot on device '%s':"
- " %s\n",
- bdrv_get_device_name(bs),
- error_get_pretty(err));
- error_free(err);
+ error_prefix(err,
+ "Error while deleting snapshot on device '%s':",
+ bdrv_get_device_name(bs));
+ error_printf_fn(err, monitor_printf, mon);
}
}
}
--
1.9.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [RFC PATCH v1 25/25] core: Update error API usages
2015-09-11 5:33 [Qemu-devel] [RFC PATCH v1 00/25] error: Automatic error concatenation and prefixing Peter Crosthwaite
` (23 preceding siblings ...)
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 24/25] migration: savevm: " Peter Crosthwaite
@ 2015-09-11 5:33 ` Peter Crosthwaite
2015-09-11 6:42 ` [Qemu-devel] [RFC PATCH v1 00/25] error: Automatic error concatenation and prefixing Markus Armbruster
2015-09-11 15:27 ` Eric Blake
26 siblings, 0 replies; 36+ messages in thread
From: Peter Crosthwaite @ 2015-09-11 5:33 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.maydell, armbru
Use error_prefix() and error_report_err() as appropriate.
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
---
arch_init.c | 5 ++---
ui/vnc.c | 5 ++---
vl.c | 7 +++----
3 files changed, 7 insertions(+), 10 deletions(-)
diff --git a/arch_init.c b/arch_init.c
index 38f5fb9..bb8db89 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -258,9 +258,8 @@ void do_acpitable_option(const QemuOpts *opts)
acpi_table_add(opts, &err);
if (err) {
- error_report("Wrong acpi table provided: %s",
- error_get_pretty(err));
- error_free(err);
+ error_prefix(err, "Wrong acpi table provided: ");
+ error_report_err(err);
exit(1);
}
#endif
diff --git a/ui/vnc.c b/ui/vnc.c
index caf82f5..84324bf 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3775,9 +3775,8 @@ int vnc_init_func(void *opaque, QemuOpts *opts, Error **errp)
vnc_display_init(id);
vnc_display_open(id, &local_err);
if (local_err != NULL) {
- error_report("Failed to start VNC server: %s",
- error_get_pretty(local_err));
- error_free(local_err);
+ error_prefix(local_err, "Failed to start VNC server: ");
+ error_report_err(local_err);
exit(1);
}
return 0;
diff --git a/vl.c b/vl.c
index 584ca88..17dcffd 100644
--- a/vl.c
+++ b/vl.c
@@ -4543,7 +4543,7 @@ int main(int argc, char **argv, char **envp)
Error *local_err = NULL;
qemu_boot_set(boot_once, &local_err);
if (local_err) {
- error_report("%s", error_get_pretty(local_err));
+ error_report_err(local_err);
exit(1);
}
qemu_register_reset(restore_boot_order, g_strdup(boot_order));
@@ -4633,9 +4633,8 @@ int main(int argc, char **argv, char **envp)
Error *local_err = NULL;
qemu_start_incoming_migration(incoming, &local_err);
if (local_err) {
- error_report("-incoming %s: %s", incoming,
- error_get_pretty(local_err));
- error_free(local_err);
+ error_prefix(local_err, "-incoming %s: ", incoming);
+ error_report_err(local_err);
exit(1);
}
} else if (autostart) {
--
1.9.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [RFC PATCH v1 00/25] error: Automatic error concatenation and prefixing
2015-09-11 5:33 [Qemu-devel] [RFC PATCH v1 00/25] error: Automatic error concatenation and prefixing Peter Crosthwaite
` (24 preceding siblings ...)
2015-09-11 5:33 ` [Qemu-devel] [RFC PATCH v1 25/25] core: " Peter Crosthwaite
@ 2015-09-11 6:42 ` Markus Armbruster
2015-09-11 15:53 ` Eric Blake
2015-09-11 15:27 ` Eric Blake
26 siblings, 1 reply; 36+ messages in thread
From: Markus Armbruster @ 2015-09-11 6:42 UTC (permalink / raw)
To: Peter Crosthwaite; +Cc: peter.maydell, qemu-devel
Quick initial high-level feedback, since I'm afraid real review will
take a while (series is long, and I'm still swamped).
Peter Crosthwaite <crosthwaitepeter@gmail.com> writes:
> Hi Markus and all,
>
> This patch series adds support for automatically concatenating multiple
> errors to the one Error *.
>
> I'll start with what I am actually trying to do, which is get rid of the
> boilerplate:
>
> some_api_call(... , &local_err);
> if (local_err) {
> error_propagate(errp, local_err);
> return;
> }
> some_similar_api_call(... , &local_err);
> if (local_err) {
> error_propagate(errp, local_err);
> return;
> }
> some_similar_api_call(... , &local_err);
> if (local_err) {
> error_propagate(errp, local_err);
> return;
> }
> ...
>
> It is usually 1 LOC for the API and 4 LOC for the error handling boiler
> plate making our code hard reading. This is particularly bad in hw/arm
> where we have a good number of fully QOMified SoCs and machine models,
> each of which need these checks on recursive realize calls and friends.
The problem is real. A way to avoid or even reduce the boiler plate
would be nice, provided it's reasonably hard to misuse, and doesn't mess
with the current usage patterns of the Error interface.
> The removed LOC just in ARM pays for the extra core code needed to
> implement this. And the number of ARM machines is only going to grow.
>
> So the plan is:
>
> 1: Allow an Error * to contain more that one actual error from API
> calls.
Can you explain *why* you want to support multiple errors?
> 2: Refactor key APIs (some_similar_api_call() in the above example)
> to not fatal when previous errors have occured in dependencies.
I'm afraid this is going to be non-trivial :)
For every place that now continues after errors, you need to show that
continuing is safe.
> Point 1 kind of got big on me. Patch 4 is the main event, listifying
> errors. The follow up issue however, is it tricky to get a sane
> definition of error_get_pretty for a multi-error. So instead the
> strategy is to remove error_get_pretty() and replace with some error
> API helper with well defined behaviour for multi-error. The two leading
> uses of error get pretty are prefixing an error, and reporting an error
> via a custom printf handler. So two new APIs are added for that (P5-6).
> There aren't many error_get_pretty's left after that, and they
> shouldn't be in the path of any multi-errors.
>
> I think the error_prefix is valuable it its own right, as it now means
> the code for report or propagating a prefixed error is now consistent
> with the non-prefixed variants.
>
> That is, we used to have:
>
> /* If we are prefixing we have to do it this way: */
> error_setg(errp, "some prefix %s", error_get_pretty(local_err));
> error_free(local_err);
>
> vs:
>
> /* but if not prefixing it is like this: */
> error_propagate(errp, local_err);
>
> Now with this patch series the two are much more recognisable as the same
> with:
>
> /* This code is almost the same as the above non-prefixed propagation */
> error_prefix(local_err, "some prefix"):
> error_propagate(errp, local_err);
Less flexible, but that might be a good thing. Need to see the actual
uses to tell.
> Point 2 is less about error API and more about machine generation.
> Sysbus, Memory and Qdev all have APIs that depend on successful device-
> init and realize calls for argument devices. As we are trying to remove
> the error detection for those argument devs, those APIs need to tweaked
> to handle realize failure. This actually wasn't as bad as I thought it
> would be. See patches (7-9).
>
> All patches after that walk the various major subsystems converting
> error APIs to this new scheme and deleting now-unneeded boiler plate.
> ARM is first (P10-15) seeing good clean up of propagate handers.
>
> So the net result for these ARM machines, is error behaviour that is
> something like a compiler. If any one thing fails, then machine-init
> (compilation) fails. But an early fail does not stop machine-init
> (compilation), instead it proceeds to the end collecting subsequent
> errors as it goes.
Simple compilers stop on first error. Not as bad as it may sound when
your machine gets from running "make" to compiler dying on the first
error real fast.
More ambitious compilers continue to diagnose more errors. This isn't
trivial. The compiler has to satisfy post conditions even after an
error, typically by synthesizing suitable error values. It has to take
pains to avoid error cascades. Experienced users recognize when that
effort fails, and typically ignore the remaining errors wholesale then.
In QEMU, error cascades might be less of a problem than with compilers.
To tell for sure, we'd have to try.
However, satisfying post conditions is at least as much of a problem.
More so since they're generally unstated. Can you explain your strategy
for solving this one?
> Some other interesting food for thought is the qemu_fdt APIs which I
> have been wanting to convert to error API but the local_err propagation
> is going to be brutal in heavy users like e500.c. This would solve that
> as fdt API could be easily made multi-error safe and clients like e500
> can just collect multi-errors and single-fail at the end.
>
> Long term, we can use this to catch cases of multiple genuine machine
> init errors in the one boot but that is a secondary goal to simply
> cutting down on code boilerplate. The best feature of this series is
> the diffstat.
>
> Patches 1-3 are cleanup that can be taken independent of the series.
>
> I think P3 may be obsolete from a recent merge, but i'll wait
> for architectural feedback before rebasing.
Sorry, only very high level feedback so far. Best I can do right now.
Hope it's useful anyway.
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [RFC PATCH v1 00/25] error: Automatic error concatenation and prefixing
2015-09-11 6:42 ` [Qemu-devel] [RFC PATCH v1 00/25] error: Automatic error concatenation and prefixing Markus Armbruster
@ 2015-09-11 15:53 ` Eric Blake
0 siblings, 0 replies; 36+ messages in thread
From: Eric Blake @ 2015-09-11 15:53 UTC (permalink / raw)
To: Markus Armbruster, Peter Crosthwaite; +Cc: peter.maydell, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1825 bytes --]
On 09/11/2015 12:42 AM, Markus Armbruster wrote:
> Quick initial high-level feedback, since I'm afraid real review will
> take a while (series is long, and I'm still swamped).
>
>>
>> So the net result for these ARM machines, is error behaviour that is
>> something like a compiler. If any one thing fails, then machine-init
>> (compilation) fails. But an early fail does not stop machine-init
>> (compilation), instead it proceeds to the end collecting subsequent
>> errors as it goes.
>
> Simple compilers stop on first error. Not as bad as it may sound when
> your machine gets from running "make" to compiler dying on the first
> error real fast.
>
> More ambitious compilers continue to diagnose more errors. This isn't
> trivial. The compiler has to satisfy post conditions even after an
> error, typically by synthesizing suitable error values. It has to take
> pains to avoid error cascades. Experienced users recognize when that
> effort fails, and typically ignore the remaining errors wholesale then.
>
> In QEMU, error cascades might be less of a problem than with compilers.
> To tell for sure, we'd have to try.
>
> However, satisfying post conditions is at least as much of a problem.
> More so since they're generally unstated. Can you explain your strategy
> for solving this one?
And that's why I'm arguing that error chaining has to be an explicit
opt-in, and not the default. Code that has been audited to be safe
against cascading errors can use error chaining to reduce boilerplate,
but the default behavior should continue to be treating the first error
as fatal rather than getting the system into a worse state due to failed
postconditions.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [RFC PATCH v1 00/25] error: Automatic error concatenation and prefixing
2015-09-11 5:33 [Qemu-devel] [RFC PATCH v1 00/25] error: Automatic error concatenation and prefixing Peter Crosthwaite
` (25 preceding siblings ...)
2015-09-11 6:42 ` [Qemu-devel] [RFC PATCH v1 00/25] error: Automatic error concatenation and prefixing Markus Armbruster
@ 2015-09-11 15:27 ` Eric Blake
26 siblings, 0 replies; 36+ messages in thread
From: Eric Blake @ 2015-09-11 15:27 UTC (permalink / raw)
To: Peter Crosthwaite, qemu-devel; +Cc: peter.maydell, armbru
[-- Attachment #1: Type: text/plain, Size: 4673 bytes --]
On 09/10/2015 11:33 PM, Peter Crosthwaite wrote:
> Hi Markus and all,
>
> This patch series adds support for automatically concatenating multiple
> errors to the one Error *.
Sounds interesting!
> So the plan is:
>
> 1: Allow an Error * to contain more that one actual error from API
> calls.
Is this for all errors, or do you have to make a special call to make it
obvious that a particular error can be concatenated to while the default
is still to report programming error if a second error is attempted atop
a regular error that hasn't requested concatenation?
> 2: Refactor key APIs (some_similar_api_call() in the above example)
> to not fatal when previous errors have occured in dependencies.
>
> Point 1 kind of got big on me. Patch 4 is the main event, listifying
> errors. The follow up issue however, is it tricky to get a sane
> definition of error_get_pretty for a multi-error. So instead the
> strategy is to remove error_get_pretty() and replace with some error
> API helper with well defined behaviour for multi-error. The two leading
> uses of error get pretty are prefixing an error, and reporting an error
> via a custom printf handler. So two new APIs are added for that (P5-6).
> There aren't many error_get_pretty's left after that, and they
> shouldn't be in the path of any multi-errors.
>
> I think the error_prefix is valuable it its own right, as it now means
> the code for report or propagating a prefixed error is now consistent
> with the non-prefixed variants.
>
> That is, we used to have:
>
> /* If we are prefixing we have to do it this way: */
> error_setg(errp, "some prefix %s", error_get_pretty(local_err));
> error_free(local_err);
>
> vs:
>
> /* but if not prefixing it is like this: */
> error_propagate(errp, local_err);
>
> Now with this patch series the two are much more recognisable as the same
> with:
>
> /* This code is almost the same as the above non-prefixed propagation */
> error_prefix(local_err, "some prefix"):
> error_propagate(errp, local_err);
Seems nice in its own right.
>
> Point 2 is less about error API and more about machine generation.
> Sysbus, Memory and Qdev all have APIs that depend on successful device-
> init and realize calls for argument devices. As we are trying to remove
> the error detection for those argument devs, those APIs need to tweaked
> to handle realize failure. This actually wasn't as bad as I thought it
> would be. See patches (7-9).
>
> All patches after that walk the various major subsystems converting
> error APIs to this new scheme and deleting now-unneeded boiler plate.
> ARM is first (P10-15) seeing good clean up of propagate handers.
>
> So the net result for these ARM machines, is error behaviour that is
> something like a compiler. If any one thing fails, then machine-init
> (compilation) fails. But an early fail does not stop machine-init
> (compilation), instead it proceeds to the end collecting subsequent
> errors as it goes.
Sometimes that causes more problems (ignoring an error and proceeding on
can cause confusing followup errors), but usually it manages to work out.
>
> Some other interesting food for thought is the qemu_fdt APIs which I
> have been wanting to convert to error API but the local_err propagation
> is going to be brutal in heavy users like e500.c. This would solve that
> as fdt API could be easily made multi-error safe and clients like e500
> can just collect multi-errors and single-fail at the end.
>
> Long term, we can use this to catch cases of multiple genuine machine
> init errors in the one boot but that is a secondary goal to simply
> cutting down on code boilerplate. The best feature of this series is
> the diffstat.
>
> Patches 1-3 are cleanup that can be taken independent of the series.
>
> I think P3 may be obsolete from a recent merge, but i'll wait
> for architectural feedback before rebasing.
Yeah, both Markus and I have been touching error.c lately, so a rebase
will probably be needed.
>
> Regards,
> Peter
> --END---
>
> Signed-off-by: Peter Crosthwaite <crosthwaite.peter@gmail.com>
> ---
> __HAS_COVER__ | 0
> 1 file changed, 0 insertions(+), 0 deletions(-)
> create mode 100644 __HAS_COVER__
>
> diff --git a/__HAS_COVER__ b/__HAS_COVER__
> new file mode 100644
> index 0000000..e69de29
>
Huh - whatever you are using to version your cover letter made the real
diffstat be part of the signature rather than the main body of the email.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 36+ messages in thread