[Qemu-devel] [PATCH qmp v4 0/6] Add error_abort and associated cleanups

[Qemu-devel] [PATCH qmp v4 0/6] Add error_abort and associated cleanups

[Peter Crosthwaite]

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

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

-- 
1.8.5.2

[Qemu-devel] [PATCH qmp v4 1/6] error: Add error_abort

[Peter Crosthwaite]

Add a special Error * that can be passed to error handling APIs to
signal that any errors are fatal and should abort QEMU. There are two
advantages to this:

- allows for brevity when wishing to assert success of Error **
  accepting APIs. No need for this pattern:
        Error * local_err = NULL;
        api_call(foo, bar, &local_err);
        assert_no_error(local_err);
  This also removes the need for _nofail variants of APIs with
  asserting call sites now reduced to 1LOC.
- SIGABRT happens from within the offending API. When a fatal error
  occurs in an API call (when the caller is asserting sucess) failure
  often means the API itself is broken. With the abort happening in the
  API call now, the stack frames into the call are available at debug
  time. In the assert_no_error scheme the abort happens after the fact.

The exact semantic is that when an error is raised, if the argument
Error ** matches &error_abort, then the abort occurs immediately. The
error messaged is reported.

For error_propagate, if the destination error is &error_abort, then
the abort happens at propagation time.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
changed since v2:
Reverted to v1 (no delayed assertion) (Markus review)
changed since v1:
Delayed assertions that *errp == NULL.

 include/qapi/error.h |  6 ++++++
 util/error.c         | 22 +++++++++++++++++++++-
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/include/qapi/error.h b/include/qapi/error.h
index 7d4c696..c0f0c3b 100644
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -95,4 +95,10 @@ void error_propagate(Error **dst_err, Error *local_err);
  */
 void error_free(Error *err);
 
+/**
+ * If passed to error_set and friends, abort().
+ */
+
+extern Error *error_abort;
+
 #endif
diff --git a/util/error.c b/util/error.c
index 3ee362a..f11f1d5 100644
--- a/util/error.c
+++ b/util/error.c
@@ -23,6 +23,8 @@ struct Error
     ErrorClass err_class;
 };
 
+Error *error_abort;
+
 void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...)
 {
     Error *err;
@@ -41,6 +43,11 @@ void error_set(Error **errp, ErrorClass err_class, const char *fmt, ...)
     va_end(ap);
     err->err_class = err_class;
 
+    if (errp == &error_abort) {
+        error_report("%s", error_get_pretty(err));
+        abort();
+    }
+
     *errp = err;
 
     errno = saved_errno;
@@ -72,6 +79,11 @@ void error_set_errno(Error **errp, int os_errno, ErrorClass err_class,
     va_end(ap);
     err->err_class = err_class;
 
+    if (errp == &error_abort) {
+        error_report("%s", error_get_pretty(err));
+        abort();
+    }
+
     *errp = err;
 
     errno = saved_errno;
@@ -112,6 +124,11 @@ void error_set_win32(Error **errp, int win32_err, ErrorClass err_class,
     va_end(ap);
     err->err_class = err_class;
 
+    if (errp == &error_abort) {
+        error_report("%s", error_get_pretty(err));
+        abort();
+    }
+
     *errp = err;
 }
 
@@ -153,7 +170,10 @@ void error_free(Error *err)
 
 void error_propagate(Error **dst_err, Error *local_err)
 {
-    if (dst_err && !*dst_err) {
+    if (local_err && dst_err == &error_abort) {
+        error_report("%s", error_get_pretty(local_err));
+        abort();
+    } else if (dst_err && !*dst_err) {
         *dst_err = local_err;
     } else if (local_err) {
         error_free(local_err);
-- 
1.8.5.2

[Qemu-devel] [PATCH qmp v4 2/6] hw/core/qdev: Delete dead code

[Peter Crosthwaite]

This is unreachable code, as it's already asserted that no errors have
occurred. Delete.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---

 hw/core/qdev.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index e374a93..adbff18 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -746,11 +746,6 @@ static void device_initfn(Object *obj)
         }
         class = object_class_get_parent(class);
     } while (class != object_class_by_name(TYPE_DEVICE));
-    if (err != NULL) {
-        qerror_report_err(err);
-        error_free(err);
-        exit(1);
-    }
 
     object_property_add_link(OBJECT(dev), "parent_bus", TYPE_BUS,
                              (Object **)&dev->parent_bus, &err);
-- 
1.8.5.2

[Qemu-devel] [PATCH qmp v4 3/6] hw: Remove assert_no_error usages

[Peter Crosthwaite]

Replace assert_no_error() usages with the error_abort system.
&error_abort is passed into API calls to signal to the Error sub-system
that any errors are fatal. Removes need for caller assertions.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
changed since v2:
Applied change pattern to target-arm/cpu.c
changed since v1: Split unrelated dead code removal to sep patch
Remove stray " " (Markus Review).

 hw/core/qdev-properties-system.c |  8 ++------
 hw/core/qdev-properties.c        | 40 ++++++++++------------------------------
 hw/core/qdev.c                   | 23 +++++++----------------
 hw/dma/xilinx_axidma.c           | 13 ++++---------
 hw/net/xilinx_axienet.c          | 13 ++++---------
 include/hw/xilinx.h              | 14 ++++----------
 target-arm/cpu.c                 |  7 ++-----
 7 files changed, 33 insertions(+), 85 deletions(-)

diff --git a/hw/core/qdev-properties-system.c b/hw/core/qdev-properties-system.c
index 729efa8..3f29b49 100644
--- a/hw/core/qdev-properties-system.c
+++ b/hw/core/qdev-properties-system.c
@@ -352,21 +352,17 @@ void qdev_prop_set_drive_nofail(DeviceState *dev, const char *name,
 void qdev_prop_set_chr(DeviceState *dev, const char *name,
                        CharDriverState *value)
 {
-    Error *errp = NULL;
     assert(!value || value->label);
     object_property_set_str(OBJECT(dev),
-                            value ? value->label : "", name, &errp);
-    assert_no_error(errp);
+                            value ? value->label : "", name, &error_abort);
 }
 
 void qdev_prop_set_netdev(DeviceState *dev, const char *name,
                           NetClientState *value)
 {
-    Error *errp = NULL;
     assert(!value || value->name);
     object_property_set_str(OBJECT(dev),
-                            value ? value->name : "", name, &errp);
-    assert_no_error(errp);
+                            value ? value->name : "", name, &error_abort);
 }
 
 void qdev_set_nic_properties(DeviceState *dev, NICInfo *nd)
diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index dc8ae69..b949f0e 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -1003,73 +1003,55 @@ void qdev_prop_parse(DeviceState *dev, const char *name, const char *value,
 
 void qdev_prop_set_bit(DeviceState *dev, const char *name, bool value)
 {
-    Error *errp = NULL;
-    object_property_set_bool(OBJECT(dev), value, name, &errp);
-    assert_no_error(errp);
+    object_property_set_bool(OBJECT(dev), value, name, &error_abort);
 }
 
 void qdev_prop_set_uint8(DeviceState *dev, const char *name, uint8_t value)
 {
-    Error *errp = NULL;
-    object_property_set_int(OBJECT(dev), value, name, &errp);
-    assert_no_error(errp);
+    object_property_set_int(OBJECT(dev), value, name, &error_abort);
 }
 
 void qdev_prop_set_uint16(DeviceState *dev, const char *name, uint16_t value)
 {
-    Error *errp = NULL;
-    object_property_set_int(OBJECT(dev), value, name, &errp);
-    assert_no_error(errp);
+    object_property_set_int(OBJECT(dev), value, name, &error_abort);
 }
 
 void qdev_prop_set_uint32(DeviceState *dev, const char *name, uint32_t value)
 {
-    Error *errp = NULL;
-    object_property_set_int(OBJECT(dev), value, name, &errp);
-    assert_no_error(errp);
+    object_property_set_int(OBJECT(dev), value, name, &error_abort);
 }
 
 void qdev_prop_set_int32(DeviceState *dev, const char *name, int32_t value)
 {
-    Error *errp = NULL;
-    object_property_set_int(OBJECT(dev), value, name, &errp);
-    assert_no_error(errp);
+    object_property_set_int(OBJECT(dev), value, name, &error_abort);
 }
 
 void qdev_prop_set_uint64(DeviceState *dev, const char *name, uint64_t value)
 {
-    Error *errp = NULL;
-    object_property_set_int(OBJECT(dev), value, name, &errp);
-    assert_no_error(errp);
+    object_property_set_int(OBJECT(dev), value, name, &error_abort);
 }
 
 void qdev_prop_set_string(DeviceState *dev, const char *name, const char *value)
 {
-    Error *errp = NULL;
-    object_property_set_str(OBJECT(dev), value, name, &errp);
-    assert_no_error(errp);
+    object_property_set_str(OBJECT(dev), value, name, &error_abort);
 }
 
 void qdev_prop_set_macaddr(DeviceState *dev, const char *name, uint8_t *value)
 {
-    Error *errp = NULL;
     char str[2 * 6 + 5 + 1];
     snprintf(str, sizeof(str), "%02x:%02x:%02x:%02x:%02x:%02x",
              value[0], value[1], value[2], value[3], value[4], value[5]);
 
-    object_property_set_str(OBJECT(dev), str, name, &errp);
-    assert_no_error(errp);
+    object_property_set_str(OBJECT(dev), str, name, &error_abort);
 }
 
 void qdev_prop_set_enum(DeviceState *dev, const char *name, int value)
 {
     Property *prop;
-    Error *errp = NULL;
 
     prop = qdev_prop_find(dev, name);
     object_property_set_str(OBJECT(dev), prop->info->enum_table[value],
-                            name, &errp);
-    assert_no_error(errp);
+                            name, &error_abort);
 }
 
 void qdev_prop_set_ptr(DeviceState *dev, const char *name, void *value)
@@ -1161,12 +1143,10 @@ static void set_size(Object *obj, Visitor *v, void *opaque,
 static int parse_size(DeviceState *dev, Property *prop, const char *str)
 {
     uint64_t *ptr = qdev_get_prop_ptr(dev, prop);
-    Error *errp = NULL;
 
     if (str != NULL) {
-        parse_option_size(prop->name, str, ptr, &errp);
+        parse_option_size(prop->name, str, ptr, &error_abort);
     }
-    assert_no_error(errp);
     return 0;
 }
 
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index adbff18..7d869fc 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -656,14 +656,13 @@ void qdev_property_add_static(DeviceState *dev, Property *prop,
     }
 
     if (prop->qtype == QTYPE_QBOOL) {
-        object_property_set_bool(obj, prop->defval, prop->name, &local_err);
+        object_property_set_bool(obj, prop->defval, prop->name, &error_abort);
     } else if (prop->info->enum_table) {
         object_property_set_str(obj, prop->info->enum_table[prop->defval],
-                                prop->name, &local_err);
+                                prop->name, &error_abort);
     } else if (prop->qtype == QTYPE_QINT) {
-        object_property_set_int(obj, prop->defval, prop->name, &local_err);
+        object_property_set_int(obj, prop->defval, prop->name, &error_abort);
     }
-    assert_no_error(local_err);
 }
 
 static bool device_get_realized(Object *obj, Error **err)
@@ -723,7 +722,6 @@ static void device_initfn(Object *obj)
     DeviceState *dev = DEVICE(obj);
     ObjectClass *class;
     Property *prop;
-    Error *err = NULL;
 
     if (qdev_hotplug) {
         dev->hotplugged = 1;
@@ -739,26 +737,19 @@ static void device_initfn(Object *obj)
     class = object_get_class(OBJECT(dev));
     do {
         for (prop = DEVICE_CLASS(class)->props; prop && prop->name; prop++) {
-            qdev_property_add_legacy(dev, prop, &err);
-            assert_no_error(err);
-            qdev_property_add_static(dev, prop, &err);
-            assert_no_error(err);
+            qdev_property_add_legacy(dev, prop, &error_abort);
+            qdev_property_add_static(dev, prop, &error_abort);
         }
         class = object_class_get_parent(class);
     } while (class != object_class_by_name(TYPE_DEVICE));
 
     object_property_add_link(OBJECT(dev), "parent_bus", TYPE_BUS,
-                             (Object **)&dev->parent_bus, &err);
-    assert_no_error(err);
+                             (Object **)&dev->parent_bus, &error_abort);
 }
 
 static void device_post_init(Object *obj)
 {
-    DeviceState *dev = DEVICE(obj);
-    Error *err = NULL;
-
-    qdev_prop_set_globals(dev, &err);
-    assert_no_error(err);
+    qdev_prop_set_globals(DEVICE(obj), &error_abort);
 }
 
 /* Unlink device from bus and free the structure.  */
diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c
index d67c5f1..19f07b3 100644
--- a/hw/dma/xilinx_axidma.c
+++ b/hw/dma/xilinx_axidma.c
@@ -569,26 +569,21 @@ static void xilinx_axidma_init(Object *obj)
 {
     XilinxAXIDMA *s = XILINX_AXI_DMA(obj);
     SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
-    Error *errp = NULL;
 
     object_property_add_link(obj, "axistream-connected", TYPE_STREAM_SLAVE,
-                             (Object **) &s->tx_data_dev, &errp);
-    assert_no_error(errp);
+                             (Object **)&s->tx_data_dev, &error_abort);
     object_property_add_link(obj, "axistream-control-connected",
                              TYPE_STREAM_SLAVE,
-                             (Object **) &s->tx_control_dev, &errp);
-    assert_no_error(errp);
+                             (Object **)&s->tx_control_dev, &error_abort);
 
     object_initialize(&s->rx_data_dev, sizeof(s->rx_data_dev),
                       TYPE_XILINX_AXI_DMA_DATA_STREAM);
     object_initialize(&s->rx_control_dev, sizeof(s->rx_control_dev),
                       TYPE_XILINX_AXI_DMA_CONTROL_STREAM);
     object_property_add_child(OBJECT(s), "axistream-connected-target",
-                              (Object *)&s->rx_data_dev, &errp);
-    assert_no_error(errp);
+                              (Object *)&s->rx_data_dev, &error_abort);
     object_property_add_child(OBJECT(s), "axistream-control-connected-target",
-                              (Object *)&s->rx_control_dev, &errp);
-    assert_no_error(errp);
+                              (Object *)&s->rx_control_dev, &error_abort);
 
     sysbus_init_irq(sbd, &s->streams[0].irq);
     sysbus_init_irq(sbd, &s->streams[1].irq);
diff --git a/hw/net/xilinx_axienet.c b/hw/net/xilinx_axienet.c
index 3eb7715..0bd5eda 100644
--- a/hw/net/xilinx_axienet.c
+++ b/hw/net/xilinx_axienet.c
@@ -980,26 +980,21 @@ static void xilinx_enet_init(Object *obj)
 {
     XilinxAXIEnet *s = XILINX_AXI_ENET(obj);
     SysBusDevice *sbd = SYS_BUS_DEVICE(obj);
-    Error *errp = NULL;
 
     object_property_add_link(obj, "axistream-connected", TYPE_STREAM_SLAVE,
-                             (Object **) &s->tx_data_dev, &errp);
-    assert_no_error(errp);
+                             (Object **) &s->tx_data_dev, &error_abort);
     object_property_add_link(obj, "axistream-control-connected",
                              TYPE_STREAM_SLAVE,
-                             (Object **) &s->tx_control_dev, &errp);
-    assert_no_error(errp);
+                             (Object **) &s->tx_control_dev, &error_abort);
 
     object_initialize(&s->rx_data_dev, sizeof(s->rx_data_dev),
                       TYPE_XILINX_AXI_ENET_DATA_STREAM);
     object_initialize(&s->rx_control_dev, sizeof(s->rx_control_dev),
                       TYPE_XILINX_AXI_ENET_CONTROL_STREAM);
     object_property_add_child(OBJECT(s), "axistream-connected-target",
-                              (Object *)&s->rx_data_dev, &errp);
-    assert_no_error(errp);
+                              (Object *)&s->rx_data_dev, &error_abort);
     object_property_add_child(OBJECT(s), "axistream-control-connected-target",
-                              (Object *)&s->rx_control_dev, &errp);
-    assert_no_error(errp);
+                              (Object *)&s->rx_control_dev, &error_abort);
 
     sysbus_init_irq(sbd, &s->irq);
 
diff --git a/include/hw/xilinx.h b/include/hw/xilinx.h
index 0c0251a..9d6debe 100644
--- a/include/hw/xilinx.h
+++ b/include/hw/xilinx.h
@@ -59,16 +59,13 @@ xilinx_axiethernet_init(DeviceState *dev, NICInfo *nd, StreamSlave *ds,
                         StreamSlave *cs, hwaddr base, qemu_irq irq, int txmem,
                         int rxmem)
 {
-    Error *errp = NULL;
-
     qdev_set_nic_properties(dev, nd);
     qdev_prop_set_uint32(dev, "rxmem", rxmem);
     qdev_prop_set_uint32(dev, "txmem", txmem);
     object_property_set_link(OBJECT(dev), OBJECT(ds),
-                             "axistream-connected", &errp);
+                             "axistream-connected", &error_abort);
     object_property_set_link(OBJECT(dev), OBJECT(cs),
-                             "axistream-control-connected", &errp);
-    assert_no_error(errp);
+                             "axistream-control-connected", &error_abort);
     qdev_init_nofail(dev);
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
     sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq);
@@ -78,14 +75,11 @@ static inline void
 xilinx_axidma_init(DeviceState *dev, StreamSlave *ds, StreamSlave *cs,
                    hwaddr base, qemu_irq irq, qemu_irq irq2, int freqhz)
 {
-    Error *errp = NULL;
-
     qdev_prop_set_uint32(dev, "freqhz", freqhz);
     object_property_set_link(OBJECT(dev), OBJECT(ds),
-                             "axistream-connected", &errp);
+                             "axistream-connected", &error_abort);
     object_property_set_link(OBJECT(dev), OBJECT(cs),
-                             "axistream-control-connected", &errp);
-    assert_no_error(errp);
+                             "axistream-control-connected", &error_abort);
     qdev_init_nofail(dev);
 
     sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, base);
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index 408d207..e4801a8 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -252,18 +252,15 @@ static Property arm_cpu_reset_hivecs_property =
 static void arm_cpu_post_init(Object *obj)
 {
     ARMCPU *cpu = ARM_CPU(obj);
-    Error *err = NULL;
 
     if (arm_feature(&cpu->env, ARM_FEATURE_CBAR)) {
         qdev_property_add_static(DEVICE(obj), &arm_cpu_reset_cbar_property,
-                                 &err);
-        assert_no_error(err);
+                                 &error_abort);
     }
 
     if (!arm_feature(&cpu->env, ARM_FEATURE_M)) {
         qdev_property_add_static(DEVICE(obj), &arm_cpu_reset_hivecs_property,
-                                 &err);
-        assert_no_error(err);
+                                 &error_abort);
     }
 }
 
-- 
1.8.5.2

[Qemu-devel] [PATCH qmp v4 4/6] target-i386: Remove assert_no_error usage

[Peter Crosthwaite]

Replace an assert_no_error() usage with the error_abort system.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---

 target-i386/cpu.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index bb98f6d..6b7b1a9 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1600,7 +1600,6 @@ static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def,
                                 const char *name)
 {
     x86_def_t *def;
-    Error *err = NULL;
     int i;
 
     if (name == NULL) {
@@ -1608,8 +1607,7 @@ static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def,
     }
     if (kvm_enabled() && strcmp(name, "host") == 0) {
         kvm_cpu_fill_host(x86_cpu_def);
-        object_property_set_bool(OBJECT(cpu), true, "pmu", &err);
-        assert_no_error(err);
+        object_property_set_bool(OBJECT(cpu), true, "pmu", &error_abort);
         return 0;
     }
 
-- 
1.8.5.2

[Qemu-devel] [PATCH qmp v4 5/6] qemu-option: Remove qemu_opts_create_nofail

[Peter Crosthwaite]

This is a boiler-plate _nofail variant of qemu_opts_create. Remove and
use error_abort in call sites.

null/0 arguments needs to be added for the id and fail_if_exists fields
in affected callsites due to argument inconsistency between the normal and
no_fail variants.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
changed since v2:
Flip fail_if_exists to 0 that were missed in v2 (Markus review)
changed since v1:
Wrap some long lines (Markus review)
Flip fail_if_exists to 0 in call sites (Markus review)
Remove spurious whitespace fix (Markus review)
Mention fail_if_exists in commit msg (Markus review)

 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/watchdog/watchdog.c |  3 ++-
 include/qemu/option.h  |  1 -
 qdev-monitor.c         |  2 +-
 qemu-img.c             |  2 +-
 util/qemu-config.c     |  2 +-
 util/qemu-option.c     |  9 ---------
 util/qemu-sockets.c    | 18 +++++++++---------
 vl.c                   | 15 +++++++++------
 21 files changed, 42 insertions(+), 44 deletions(-)

diff --git a/block/blkdebug.c b/block/blkdebug.c
index 957be2c..ebc5f13 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -359,7 +359,7 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags,
     const char *filename, *config;
     int ret;
 
-    opts = qemu_opts_create_nofail(&runtime_opts);
+    opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
     qemu_opts_absorb_qdict(opts, options, &local_err);
     if (error_is_set(&local_err)) {
         error_propagate(errp, local_err);
diff --git a/block/blkverify.c b/block/blkverify.c
index 3c63528..1c1637f 100644
--- a/block/blkverify.c
+++ b/block/blkverify.c
@@ -125,7 +125,7 @@ static int blkverify_open(BlockDriverState *bs, QDict *options, int flags,
     const char *filename, *raw;
     int ret;
 
-    opts = qemu_opts_create_nofail(&runtime_opts);
+    opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
     qemu_opts_absorb_qdict(opts, options, &local_err);
     if (error_is_set(&local_err)) {
         error_propagate(errp, local_err);
diff --git a/block/curl.c b/block/curl.c
index 5a46f97..a603936 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -413,7 +413,7 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags,
         return -EROFS;
     }
 
-    opts = qemu_opts_create_nofail(&runtime_opts);
+    opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
     qemu_opts_absorb_qdict(opts, options, &local_err);
     if (error_is_set(&local_err)) {
         qerror_report_err(local_err);
diff --git a/block/gluster.c b/block/gluster.c
index 877686a..563d497 100644
--- a/block/gluster.c
+++ b/block/gluster.c
@@ -298,7 +298,7 @@ static int qemu_gluster_open(BlockDriverState *bs,  QDict *options,
     Error *local_err = NULL;
     const char *filename;
 
-    opts = qemu_opts_create_nofail(&runtime_opts);
+    opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
     qemu_opts_absorb_qdict(opts, options, &local_err);
     if (error_is_set(&local_err)) {
         qerror_report_err(local_err);
diff --git a/block/iscsi.c b/block/iscsi.c
index fa69408..02eba5d 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -1109,7 +1109,7 @@ static int iscsi_open(BlockDriverState *bs, QDict *options, int flags,
         return -EINVAL;
     }
 
-    opts = qemu_opts_create_nofail(&runtime_opts);
+    opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
     qemu_opts_absorb_qdict(opts, options, &local_err);
     if (error_is_set(&local_err)) {
         qerror_report_err(local_err);
diff --git a/block/nbd.c b/block/nbd.c
index 4455a13..327e913 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -205,7 +205,8 @@ static int nbd_config(BDRVNBDState *s, QDict *options, char **export)
         return -EINVAL;
     }
 
-    s->socket_opts = qemu_opts_create_nofail(&socket_optslist);
+    s->socket_opts = qemu_opts_create(&socket_optslist, NULL, 0,
+                                      &error_abort);
 
     qemu_opts_absorb_qdict(s->socket_opts, options, &local_err);
     if (error_is_set(&local_err)) {
diff --git a/block/qcow2.c b/block/qcow2.c
index f29aa88..8ec9db1 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -669,7 +669,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
     }
 
     /* Enable lazy_refcounts according to image and command line options */
-    opts = qemu_opts_create_nofail(&qcow2_runtime_opts);
+    opts = qemu_opts_create(&qcow2_runtime_opts, NULL, 0, &error_abort);
     qemu_opts_absorb_qdict(opts, options, &local_err);
     if (error_is_set(&local_err)) {
         error_propagate(errp, local_err);
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 10c6b34..0676037 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -287,7 +287,7 @@ static int raw_open_common(BlockDriverState *bs, QDict *options,
     int fd, ret;
     struct stat st;
 
-    opts = qemu_opts_create_nofail(&raw_runtime_opts);
+    opts = qemu_opts_create(&raw_runtime_opts, NULL, 0, &error_abort);
     qemu_opts_absorb_qdict(opts, options, &local_err);
     if (error_is_set(&local_err)) {
         error_propagate(errp, local_err);
diff --git a/block/raw-win32.c b/block/raw-win32.c
index 2bad5a3..ce314fd 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -248,7 +248,7 @@ static int raw_open(BlockDriverState *bs, QDict *options, int flags,
 
     s->type = FTYPE_FILE;
 
-    opts = qemu_opts_create_nofail(&raw_runtime_opts);
+    opts = qemu_opts_create(&raw_runtime_opts, NULL, 0, &error_abort);
     qemu_opts_absorb_qdict(opts, options, &local_err);
     if (error_is_set(&local_err)) {
         error_propagate(errp, local_err);
@@ -550,7 +550,8 @@ static int hdev_open(BlockDriverState *bs, QDict *options, int flags,
     Error *local_err = NULL;
     const char *filename;
 
-    QemuOpts *opts = qemu_opts_create_nofail(&raw_runtime_opts);
+    QemuOpts *opts = qemu_opts_create(&raw_runtime_opts, NULL, 0,
+                                      &error_abort);
     qemu_opts_absorb_qdict(opts, options, &local_err);
     if (error_is_set(&local_err)) {
         error_propagate(errp, local_err);
diff --git a/block/rbd.c b/block/rbd.c
index 4a1ea5b..f453f04 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -461,7 +461,7 @@ static int qemu_rbd_open(BlockDriverState *bs, QDict *options, int flags,
     const char *filename;
     int r;
 
-    opts = qemu_opts_create_nofail(&runtime_opts);
+    opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
     qemu_opts_absorb_qdict(opts, options, &local_err);
     if (error_is_set(&local_err)) {
         qerror_report_err(local_err);
diff --git a/block/sheepdog.c b/block/sheepdog.c
index d1c812d..5ce0658 100644
--- a/block/sheepdog.c
+++ b/block/sheepdog.c
@@ -1383,7 +1383,7 @@ static int sd_open(BlockDriverState *bs, QDict *options, int flags,
 
     s->bs = bs;
 
-    opts = qemu_opts_create_nofail(&runtime_opts);
+    opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
     qemu_opts_absorb_qdict(opts, options, &local_err);
     if (error_is_set(&local_err)) {
         qerror_report_err(local_err);
diff --git a/block/vvfat.c b/block/vvfat.c
index 1abb8ad..664941c 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1083,7 +1083,7 @@ DLOG(if (stderr == NULL) {
     setbuf(stderr, NULL);
 })
 
-    opts = qemu_opts_create_nofail(&runtime_opts);
+    opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
     qemu_opts_absorb_qdict(opts, options, &local_err);
     if (error_is_set(&local_err)) {
         qerror_report_err(local_err);
diff --git a/blockdev.c b/blockdev.c
index 6a85961..1cb6f4c 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -682,7 +682,8 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
     bs_opts = qdict_new();
     qemu_opts_to_qdict(all_opts, bs_opts);
 
-    legacy_opts = qemu_opts_create_nofail(&qemu_legacy_drive_opts);
+    legacy_opts = qemu_opts_create(&qemu_legacy_drive_opts, NULL, 0,
+                                   &error_abort);
     qemu_opts_absorb_qdict(legacy_opts, bs_opts, &local_err);
     if (error_is_set(&local_err)) {
         qerror_report_err(local_err);
@@ -853,7 +854,8 @@ DriveInfo *drive_init(QemuOpts *all_opts, BlockInterfaceType block_default_type)
 
     if (type == IF_VIRTIO) {
         QemuOpts *devopts;
-        devopts = qemu_opts_create_nofail(qemu_find_opts("device"));
+        devopts = qemu_opts_create(qemu_find_opts("device"), NULL, 0,
+                                   &error_abort);
         if (arch_type == QEMU_ARCH_S390X) {
             qemu_opt_set(devopts, "driver", "virtio-blk-s390");
         } else {
diff --git a/hw/watchdog/watchdog.c b/hw/watchdog/watchdog.c
index 387962e..f28161b 100644
--- a/hw/watchdog/watchdog.c
+++ b/hw/watchdog/watchdog.c
@@ -66,7 +66,8 @@ int select_watchdog(const char *p)
     QLIST_FOREACH(model, &watchdog_list, entry) {
         if (strcasecmp(model->wdt_name, p) == 0) {
             /* add the device */
-            opts = qemu_opts_create_nofail(qemu_find_opts("device"));
+            opts = qemu_opts_create(qemu_find_opts("device"), NULL, 0,
+                                    &error_abort);
             qemu_opt_set(opts, "driver", p);
             return 0;
         }
diff --git a/include/qemu/option.h b/include/qemu/option.h
index 5c0c6dd..3ea871a 100644
--- a/include/qemu/option.h
+++ b/include/qemu/option.h
@@ -136,7 +136,6 @@ int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque,
 QemuOpts *qemu_opts_find(QemuOptsList *list, const char *id);
 QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id,
                            int fail_if_exists, Error **errp);
-QemuOpts *qemu_opts_create_nofail(QemuOptsList *list);
 void qemu_opts_reset(QemuOptsList *list);
 void qemu_opts_loc_restore(QemuOpts *opts);
 int qemu_opts_set(QemuOptsList *list, const char *id,
diff --git a/qdev-monitor.c b/qdev-monitor.c
index dc37a43..6280771 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -730,7 +730,7 @@ int qemu_global_option(const char *str)
         return -1;
     }
 
-    opts = qemu_opts_create_nofail(&qemu_global_opts);
+    opts = qemu_opts_create(&qemu_global_opts, NULL, 0, &error_abort);
     qemu_opt_set(opts, "driver", driver);
     qemu_opt_set(opts, "property", property);
     qemu_opt_set(opts, "value", str+offset+1);
diff --git a/qemu-img.c b/qemu-img.c
index a4b3931..c989850 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -2564,7 +2564,7 @@ static int img_resize(int argc, char **argv)
     }
 
     /* Parse size */
-    param = qemu_opts_create_nofail(&resize_options);
+    param = qemu_opts_create(&resize_options, NULL, 0, &error_abort);
     if (qemu_opt_set(param, BLOCK_OPT_SIZE, size)) {
         /* Error message already printed when size parsing fails */
         ret = -1;
diff --git a/util/qemu-config.c b/util/qemu-config.c
index 04da942..7973659 100644
--- a/util/qemu-config.c
+++ b/util/qemu-config.c
@@ -311,7 +311,7 @@ int qemu_config_parse(FILE *fp, QemuOptsList **lists, const char *fname)
                 error_free(local_err);
                 goto out;
             }
-            opts = qemu_opts_create_nofail(list);
+            opts = qemu_opts_create(list, NULL, 0, &error_abort);
             continue;
         }
         if (sscanf(line, " %63s = \"%1023[^\"]\"", arg, value) == 2) {
diff --git a/util/qemu-option.c b/util/qemu-option.c
index efcb5dc..668e5d9 100644
--- a/util/qemu-option.c
+++ b/util/qemu-option.c
@@ -791,15 +791,6 @@ QemuOpts *qemu_opts_create(QemuOptsList *list, const char *id,
     return opts;
 }
 
-QemuOpts *qemu_opts_create_nofail(QemuOptsList *list)
-{
-    QemuOpts *opts;
-    Error *errp = NULL;
-    opts = qemu_opts_create(list, NULL, 0, &errp);
-    assert_no_error(errp);
-    return opts;
-}
-
 void qemu_opts_reset(QemuOptsList *list)
 {
     QemuOpts *opts, *next_opts;
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 6b97dc1..8818d7c 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -578,7 +578,7 @@ int inet_listen(const char *str, char *ostr, int olen,
 
     addr = inet_parse(str, errp);
     if (addr != NULL) {
-        opts = qemu_opts_create_nofail(&socket_optslist);
+        opts = qemu_opts_create(&socket_optslist, NULL, 0, &error_abort);
         inet_addr_to_opts(opts, addr);
         qapi_free_InetSocketAddress(addr);
         sock = inet_listen_opts(opts, port_offset, errp);
@@ -617,7 +617,7 @@ int inet_connect(const char *str, Error **errp)
 
     addr = inet_parse(str, errp);
     if (addr != NULL) {
-        opts = qemu_opts_create_nofail(&socket_optslist);
+        opts = qemu_opts_create(&socket_optslist, NULL, 0, &error_abort);
         inet_addr_to_opts(opts, addr);
         qapi_free_InetSocketAddress(addr);
         sock = inet_connect_opts(opts, errp, NULL, NULL);
@@ -651,7 +651,7 @@ int inet_nonblocking_connect(const char *str,
 
     addr = inet_parse(str, errp);
     if (addr != NULL) {
-        opts = qemu_opts_create_nofail(&socket_optslist);
+        opts = qemu_opts_create(&socket_optslist, NULL, 0, &error_abort);
         inet_addr_to_opts(opts, addr);
         qapi_free_InetSocketAddress(addr);
         sock = inet_connect_opts(opts, errp, callback, opaque);
@@ -794,7 +794,7 @@ int unix_listen(const char *str, char *ostr, int olen, Error **errp)
     char *path, *optstr;
     int sock, len;
 
-    opts = qemu_opts_create_nofail(&socket_optslist);
+    opts = qemu_opts_create(&socket_optslist, NULL, 0, &error_abort);
 
     optstr = strchr(str, ',');
     if (optstr) {
@@ -822,7 +822,7 @@ int unix_connect(const char *path, Error **errp)
     QemuOpts *opts;
     int sock;
 
-    opts = qemu_opts_create_nofail(&socket_optslist);
+    opts = qemu_opts_create(&socket_optslist, NULL, 0, &error_abort);
     qemu_opt_set(opts, "path", path);
     sock = unix_connect_opts(opts, errp, NULL, NULL);
     qemu_opts_del(opts);
@@ -839,7 +839,7 @@ int unix_nonblocking_connect(const char *path,
 
     g_assert(callback != NULL);
 
-    opts = qemu_opts_create_nofail(&socket_optslist);
+    opts = qemu_opts_create(&socket_optslist, NULL, 0, &error_abort);
     qemu_opt_set(opts, "path", path);
     sock = unix_connect_opts(opts, errp, callback, opaque);
     qemu_opts_del(opts);
@@ -889,7 +889,7 @@ int socket_connect(SocketAddress *addr, Error **errp,
     QemuOpts *opts;
     int fd;
 
-    opts = qemu_opts_create_nofail(&socket_optslist);
+    opts = qemu_opts_create(&socket_optslist, NULL, 0, &error_abort);
     switch (addr->kind) {
     case SOCKET_ADDRESS_KIND_INET:
         inet_addr_to_opts(opts, addr->inet);
@@ -921,7 +921,7 @@ int socket_listen(SocketAddress *addr, Error **errp)
     QemuOpts *opts;
     int fd;
 
-    opts = qemu_opts_create_nofail(&socket_optslist);
+    opts = qemu_opts_create(&socket_optslist, NULL, 0, &error_abort);
     switch (addr->kind) {
     case SOCKET_ADDRESS_KIND_INET:
         inet_addr_to_opts(opts, addr->inet);
@@ -949,7 +949,7 @@ int socket_dgram(SocketAddress *remote, SocketAddress *local, Error **errp)
     QemuOpts *opts;
     int fd;
 
-    opts = qemu_opts_create_nofail(&socket_optslist);
+    opts = qemu_opts_create(&socket_optslist, NULL, 0, &error_abort);
     switch (remote->kind) {
     case SOCKET_ADDRESS_KIND_INET:
         qemu_opt_set(opts, "host", remote->inet->host);
diff --git a/vl.c b/vl.c
index 7511e70..2082c23 100644
--- a/vl.c
+++ b/vl.c
@@ -545,7 +545,7 @@ QemuOpts *qemu_get_machine_opts(void)
     assert(list);
     opts = qemu_opts_find(list, NULL);
     if (!opts) {
-        opts = qemu_opts_create_nofail(list);
+        opts = qemu_opts_create(list, NULL, 0, &error_abort);
     }
     return opts;
 }
@@ -2254,7 +2254,8 @@ static int balloon_parse(const char *arg)
                 return  -1;
         } else {
             /* create empty opts */
-            opts = qemu_opts_create_nofail(qemu_find_opts("device"));
+            opts = qemu_opts_create(qemu_find_opts("device"), NULL, 0,
+                                    &error_abort);
         }
         qemu_opt_set(opts, "driver", "virtio-balloon");
         return 0;
@@ -2514,14 +2515,14 @@ static int virtcon_parse(const char *devname)
         exit(1);
     }
 
-    bus_opts = qemu_opts_create_nofail(device);
+    bus_opts = qemu_opts_create(device, NULL, 0, &error_abort);
     if (arch_type == QEMU_ARCH_S390X) {
         qemu_opt_set(bus_opts, "driver", "virtio-serial-s390");
     } else {
         qemu_opt_set(bus_opts, "driver", "virtio-serial-pci");
     }
 
-    dev_opts = qemu_opts_create_nofail(device);
+    dev_opts = qemu_opts_create(device, NULL, 0, &error_abort);
     qemu_opt_set(dev_opts, "driver", "virtconsole");
 
     snprintf(label, sizeof(label), "virtcon%d", index);
@@ -3380,7 +3381,8 @@ int main(int argc, char **argv, char **envp)
 
                 qemu_opt_set_bool(fsdev, "readonly",
                                 qemu_opt_get_bool(opts, "readonly", 0));
-                device = qemu_opts_create_nofail(qemu_find_opts("device"));
+                device = qemu_opts_create(qemu_find_opts("device"), NULL, 0,
+                                          &error_abort);
                 qemu_opt_set(device, "driver", "virtio-9p-pci");
                 qemu_opt_set(device, "fsdev",
                              qemu_opt_get(opts, "mount_tag"));
@@ -3400,7 +3402,8 @@ int main(int argc, char **argv, char **envp)
                 }
                 qemu_opt_set(fsdev, "fsdriver", "synth");
 
-                device = qemu_opts_create_nofail(qemu_find_opts("device"));
+                device = qemu_opts_create(qemu_find_opts("device"), NULL, 0,
+                                          &error_abort);
                 qemu_opt_set(device, "driver", "virtio-9p-pci");
                 qemu_opt_set(device, "fsdev", "v_synth");
                 qemu_opt_set(device, "mount_tag", "v_synth");
-- 
1.8.5.2

[Qemu-devel] [PATCH qmp v4 6/6] qerror: Remove assert_no_error()

[Peter Crosthwaite]

This is no longer needed, and is obsoleted by error_abort. Remove.

Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---

 include/qapi/qmp/qerror.h | 1 -
 qobject/qerror.c          | 8 --------
 2 files changed, 9 deletions(-)

diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index c30c2f6..73c67b7 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -29,7 +29,6 @@ typedef struct QError {
 QString *qerror_human(const QError *qerror);
 void qerror_report(ErrorClass err_class, const char *fmt, ...) GCC_FMT_ATTR(2, 3);
 void qerror_report_err(Error *err);
-void assert_no_error(Error *err);
 
 /*
  * QError class list
diff --git a/qobject/qerror.c b/qobject/qerror.c
index fc8331a..e3608e2 100644
--- a/qobject/qerror.c
+++ b/qobject/qerror.c
@@ -121,14 +121,6 @@ void qerror_report_err(Error *err)
     }
 }
 
-void assert_no_error(Error *err)
-{
-    if (err) {
-        qerror_report_err(err);
-        abort();
-    }
-}
-
 /**
  * qobject_to_qerror(): Convert a QObject into a QError
  */
-- 
1.8.5.2

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

[Luiz Capitulino]

On Wed,  1 Jan 2014 18:46:24 -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:
> 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(-)
> 

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

[Andreas Färber]

Am 06.01.2014 20:13, schrieb Luiz Capitulino:
> On Wed,  1 Jan 2014 18:46:24 -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.

Please normalize "hw/core/qdev:" to just "qdev:", thanks.

Andreas

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

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

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

[Luiz Capitulino]

On Mon, 06 Jan 2014 20:18:45 +0100
Andreas Färber <afaerber@suse.de> wrote:

> Am 06.01.2014 20:13, schrieb Luiz Capitulino:
> > On Wed,  1 Jan 2014 18:46:24 -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.
> 
> Please normalize "hw/core/qdev:" to just "qdev:", thanks.

Done. You still have time for a Reviewed-by :)