Re: [Qemu-devel] [PATCH v3 0/6] Add error_abort and associated cleanups

[Luiz Capitulino <lcapitulino@redhat.com>]

On Tue, 10 Dec 2013 20:09:25 -0800
Peter Crosthwaite <peter.crosthwaite@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:
> Addressed Markus review.
> 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-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 +++++++++------
>  32 files changed, 101 insertions(+), 142 deletions(-)
>