On Wed, 1 Jan 2014 18:46:24 -0800
Peter Crosthwaite <peter.crosthwa...@xilinx.com> wrote:
> Following our discussion RE self asserting API calls, here is a spin of
> my proposal. This series obsoletes the need for _nofail variants for
> Error ** accepting APIs. Is also greatly reduces the verbosity of calls
> sites that are currently asserting against errors.
>
> Patch 1 is the main event - addition of error_abort. The following
> patches then cleanup uses of _nofail and assert_no_error().
Applied to the qmp branch, thanks.
>
> To give it a smoke test, I introduce a (critical) bug into QOM:
>
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -797,7 +797,7 @@ void object_property_set(Object *obj, Visitor *v,
> const char *name,
> return;
> }
>
> - if (!prop->set) {
> + if (prop->set) {
> error_set(errp, QERR_PERMISSION_DENIED);
> } else {
> prop->set(obj, v, prop->opaque, name, errp);
>
> And run QEMU:
>
> $ gdb --args ./microblazeel-softmmu/qemu-system-microblazeel -M
> petalogix-ml605 -nographic
>
> Without application of this series, the bug manifests as:
>
> qemu-system-microblazeel: Insufficient permission to perform this operation
>
> Program received signal SIGABRT, Aborted.
> (gdb) bt
> 0 0x00007ffff55f7425 in __GI_raise (sig=<optimized out>) at
> ../nptl/sysdeps/unix/sysv/linux/raise.c:64
> 1 0x00007ffff55fab8b in __GI_abort () at abort.c:91
> 2 0x00005555557073da in assert_no_error (err=<optimized out>) at
> qobject/qerror.c:128
> 3 0x0000555555615159 in qdev_property_add_static (dev=0x555555e9e6a0,
> prop=0x5555559ea4c0, errp=0x7fffffffe3c0) at hw/core/qdev.c:666
> 4 0x0000555555615355 in device_initfn (obj=<optimized out>) at
> hw/core/qdev.c:744
>
> With this series, we now get a the full backtrace into the QOM API when
> the assertion occurs (note stack frames 2-4 giving visibility into the
> broken QOM API):
>
> qemu-system-microblazeel: Insufficient permission to perform this operation
>
> Program received signal SIGABRT, Aborted.
> (gdb) bt
> 0 0x00007ffff55f7425 in __GI_raise (sig=<optimized out>) at
> ../nptl/sysdeps/unix/sysv/linux/raise.c:64
> 1 0x00007ffff55fab8b in __GI_abort () at abort.c:91
> 2 0x000055555570c5a4 in error_set (errp=0x555555e1b5f0,
> err_class=ERROR_CLASS_GENERIC_ERROR, fmt=0x55555571f5c0 "Insufficient
> permission to perform this operation") at util/error.c:47
> 3 0x00005555556778e5 in object_property_set_qobject (obj=0x555555e9e6a0,
> value=<optimized out>, name=0x55555574e7a6 "xlnx.base-vectors",
> errp=0x555555e1b5f0) at qom/qom-qobject.c:24
> 4 0x000055555567674d in object_property_set_int (obj=0x555555e9e6a0,
> value=<optimized out>, name=0x55555574e7a6 "xlnx.base-vectors",
> errp=0x555555e1b5f0) at qom/object.c:898
> 5 0x0000555555615192 in qdev_property_add_static (dev=0x555555e9e6a0,
> prop=0x5555559ea4c0, errp=0x555555e1b5f0) at hw/core/qdev.c:664
> 6 0x000055555561534f in device_initfn (obj=<optimized out>) at
> hw/core/qdev.c:741
>
> Changed since v2:
> Removed new assert_no_error usages in target-arm/cpu.c
> Changed since v1:
> Addressed Markus review.
> Guarded against erroneous setting of error_abort != NULL.
>
>
> Peter Crosthwaite (6):
> error: Add error_abort
> hw/core/qdev: Delete dead code
> hw: Remove assert_no_error usages
> target-i386: Remove assert_no_error usage
> qemu-option: Remove qemu_opts_create_nofail
> qerror: Remove assert_no_error()
>
> block/blkdebug.c | 2 +-
> block/blkverify.c | 2 +-
> block/curl.c | 2 +-
> block/gluster.c | 2 +-
> block/iscsi.c | 2 +-
> block/nbd.c | 3 ++-
> block/qcow2.c | 2 +-
> block/raw-posix.c | 2 +-
> block/raw-win32.c | 5 +++--
> block/rbd.c | 2 +-
> block/sheepdog.c | 2 +-
> block/vvfat.c | 2 +-
> blockdev.c | 6 ++++--
> hw/core/qdev-properties-system.c | 8 ++------
> hw/core/qdev-properties.c | 40
> ++++++++++------------------------------
> hw/core/qdev.c | 28 +++++++---------------------
> hw/dma/xilinx_axidma.c | 13 ++++---------
> hw/net/xilinx_axienet.c | 13 ++++---------
> hw/watchdog/watchdog.c | 3 ++-
> include/hw/xilinx.h | 14 ++++----------
> include/qapi/error.h | 6 ++++++
> include/qapi/qmp/qerror.h | 1 -
> include/qemu/option.h | 1 -
> qdev-monitor.c | 2 +-
> qemu-img.c | 2 +-
> qobject/qerror.c | 8 --------
> target-arm/cpu.c | 7 ++-----
> target-i386/cpu.c | 4 +---
> util/error.c | 22 +++++++++++++++++++++-
> util/qemu-config.c | 2 +-
> util/qemu-option.c | 9 ---------
> util/qemu-sockets.c | 18 +++++++++---------
> vl.c | 15 +++++++++------
> 33 files changed, 103 insertions(+), 147 deletions(-)
>
