All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
To: qemu-devel@nongnu.org
Cc: imammedo@redhat.com, afaerber@suse.de, armbru@redhat.com,
	pbonzini@redhat.com
Subject: [Qemu-devel] [RFC PATCH v1 0/5] Add error_abort and associated cleanups
Date: Mon,  2 Dec 2013 21:49:20 -0800	[thread overview]
Message-ID: <cover.1386049636.git.peter.crosthwaite@xilinx.com> (raw)

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 greately 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().

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


Peter Crosthwaite (5):
  error: Add error_abort
  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                      |  2 +-
 block/qcow2.c                    |  2 +-
 block/raw-posix.c                |  2 +-
 block/raw-win32.c                |  4 ++--
 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                             | 17 ++++++++++-------
 32 files changed, 100 insertions(+), 143 deletions(-)

-- 
1.8.4.4

             reply	other threads:[~2013-12-03  5:50 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-12-03  5:49 Peter Crosthwaite [this message]
2013-12-03  5:49 ` [Qemu-devel] [RFC PATCH v1 1/5] error: Add error_abort Peter Crosthwaite
2013-12-03  5:50 ` [Qemu-devel] [RFC PATCH v1 2/5] hw: Remove assert_no_error usages Peter Crosthwaite
2013-12-03  9:35   ` Markus Armbruster
2013-12-03 10:04     ` Peter Crosthwaite
2013-12-03  5:51 ` [Qemu-devel] [RFC PATCH v1 3/5] target-i386: Remove assert_no_error usage Peter Crosthwaite
2013-12-03  5:51 ` [Qemu-devel] [RFC PATCH v1 4/5] qemu-option: Remove qemu_opts_create_nofail Peter Crosthwaite
2013-12-03  9:42   ` Markus Armbruster
2013-12-03 10:17     ` Peter Crosthwaite
2013-12-03 10:44       ` Markus Armbruster
2013-12-04  6:45     ` Peter Crosthwaite
2013-12-03  5:52 ` [Qemu-devel] [RFC PATCH v1 5/5] qerror: Remove assert_no_error() Peter Crosthwaite
2013-12-03  9:44 ` [Qemu-devel] [RFC PATCH v1 0/5] Add error_abort and associated cleanups Markus Armbruster
2013-12-03 11:49   ` Igor Mammedov
2013-12-03 11:57   ` Paolo Bonzini
2013-12-03 12:03     ` Peter Crosthwaite
2013-12-03 12:58   ` Eric Blake
2013-12-03 13:53     ` Markus Armbruster
2013-12-03 20:33       ` Igor Mammedov
2013-12-03 20:43         ` Eric Blake
2013-12-04  9:11           ` Markus Armbruster
2013-12-04 14:46             ` Eric Blake
2013-12-05 10:37         ` Paolo Bonzini
2013-12-05 15:32           ` Igor Mammedov
2013-12-05 15:59             ` Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cover.1386049636.git.peter.crosthwaite@xilinx.com \
    --to=peter.crosthwaite@xilinx.com \
    --cc=afaerber@suse.de \
    --cc=armbru@redhat.com \
    --cc=imammedo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.