[PULL 00/28] Error reporting patches patches for 2020-07-02

[PULL 00/28] Error reporting patches patches for 2020-07-02

[Markus Armbruster]

The following changes since commit fc1bff958998910ec8d25db86cd2f53ff125f7ab:

  hw/misc/pca9552: Add missing TypeInfo::class_size field (2020-06-29 21:16:10 +0100)

are available in the Git repository at:

  git://repo.or.cz/qemu/armbru.git tags/pull-error-2020-07-02

for you to fetch changes up to 9cde9caa04beac25cef32a8a9d0bd26d6b91a41a:

  migration/rdma: Plug memory leaks in qemu_rdma_registration_stop() (2020-07-02 11:54:48 +0200)

----------------------------------------------------------------
Error reporting patches patches for 2020-07-02

----------------------------------------------------------------
Markus Armbruster (26):
      net/virtio: Fix failover_replug_primary() return value regression
      pci: Delete useless error_propagate()
      Clean up some calls to ignore Error objects the right way
      tests: Use &error_abort where appropriate
      tests: Use error_free_or_abort() where appropriate
      usb/dev-mtp: Fix Error double free after inotify failure
      spapr: Plug minor memory leak in spapr_machine_init()
      qga: Plug unlikely memory leak in guest-set-memory-blocks
      sd/milkymist-memcard: Plug minor memory leak in realize
      test-util-filemonitor: Plug unlikely memory leak
      vnc: Plug minor memory leak in vnc_display_open()
      aspeed: Clean up roundabout error propagation
      qdev: Drop qbus_set_bus_hotplug_handler() parameter @errp
      qdev: Drop qbus_set_hotplug_handler() parameter @errp
      hw: Fix error API violation around object_property_set_link()
      hw/arm: Drop useless object_property_set_link() error handling
      riscv/sifive_u: Fix sifive_u_soc_realize() error API violations
      riscv_hart: Fix riscv_harts_realize() error API violations
      mips/cps: Fix mips_cps_realize() error API violations
      x86: Fix x86_cpu_new() error handling
      amd_iommu: Fix amdvi_realize() error API violation
      arm/stm32f205 arm/stm32f405: Fix realize error API violation
      aspeed: Fix realize error API violation
      hw/arm/armsse: Fix armsse_realize() error API violation
      arm/{bcm2835,fsl-imx25,fsl-imx6}: Fix realize error API violations
      migration/rdma: Plug memory leaks in qemu_rdma_registration_stop()

Peter Maydell (1):
      hw/virtio/virtio-iommu-pci.c: Fix typo in error message

lichun (1):
      chardev/tcp: Fix error message double free error

 include/hw/qdev-core.h             |   5 +-
 chardev/char-socket.c              |   9 ++-
 hw/9pfs/9p.c                       |   6 +-
 hw/acpi/pcihp.c                    |   3 +-
 hw/acpi/piix4.c                    |   2 +-
 hw/arm/armsse.c                    |  61 ++++++---------------
 hw/arm/armv7m.c                    |   7 +--
 hw/arm/aspeed_ast2600.c            |  35 ++++--------
 hw/arm/aspeed_soc.c                |  29 ++++------
 hw/arm/bcm2835_peripherals.c       |  12 ++--
 hw/arm/fsl-imx25.c                 |  12 ++--
 hw/arm/fsl-imx6.c                  |  12 ++--
 hw/arm/nrf51_soc.c                 |   6 +-
 hw/arm/stm32f205_soc.c             |   2 +-
 hw/arm/stm32f405_soc.c             |   2 +-
 hw/arm/virt.c                      |   4 +-
 hw/char/virtio-serial-bus.c        |   4 +-
 hw/core/bus.c                      |   8 +--
 hw/display/virtio-gpu-pci.c        |   2 +-
 hw/display/virtio-vga.c            |   2 +-
 hw/dma/sparc32_dma.c               |   6 +-
 hw/dma/xilinx_axidma.c             |  12 +---
 hw/i386/amd_iommu.c                |   6 +-
 hw/i386/x86.c                      |   8 ++-
 hw/mips/cps.c                      |  35 ++++++++----
 hw/misc/macio/macio.c              |   3 +-
 hw/net/virtio-net.c                |   2 +-
 hw/net/xilinx_axienet.c            |  12 +---
 hw/pci/pci.c                       |   3 -
 hw/pci/pcie.c                      |   2 +-
 hw/pci/shpc.c                      |   2 +-
 hw/ppc/spapr.c                     |   4 +-
 hw/ppc/spapr_drc.c                 |   4 +-
 hw/ppc/spapr_pci.c                 |   4 +-
 hw/riscv/riscv_hart.c              |  14 ++---
 hw/riscv/sifive_u.c                |  12 +++-
 hw/s390x/ap-bridge.c               |   2 +-
 hw/s390x/css-bridge.c              |   2 +-
 hw/s390x/s390-pci-bus.c            |  14 +----
 hw/scsi/scsi-bus.c                 |   2 +-
 hw/scsi/virtio-scsi.c              |   4 +-
 hw/scsi/vmw_pvscsi.c               |   2 +-
 hw/sd/milkymist-memcard.c          |   5 +-
 hw/usb/bus.c                       |   2 +-
 hw/usb/dev-mtp.c                   |   2 -
 hw/usb/dev-smartcard-reader.c      |   2 +-
 hw/virtio/virtio-iommu-pci.c       |   4 +-
 hw/xen/xen-bus.c                   |   2 +-
 hw/xen/xen-legacy-backend.c        |   2 +-
 migration/rdma.c                   |  19 +++----
 qga/commands-posix.c               |   1 +
 tests/check-block-qdict.c          |  24 ++------
 tests/check-qobject.c              |   5 +-
 tests/check-qom-proplist.c         |  14 ++---
 tests/test-base64.c                |   3 +-
 tests/test-bdrv-graph-mod.c        |   4 +-
 tests/test-block-iothread.c        |   3 +-
 tests/test-crypto-cipher.c         |   8 +--
 tests/test-io-task.c               |   4 +-
 tests/test-logging.c               |  12 +---
 tests/test-qemu-opts.c             |  22 ++------
 tests/test-replication.c           | 109 +++++++++++--------------------------
 tests/test-string-input-visitor.c  |  33 ++++-------
 tests/test-string-output-visitor.c |  16 ++----
 tests/test-util-filemonitor.c      |   1 +
 ui/vnc.c                           |   6 +-
 66 files changed, 243 insertions(+), 438 deletions(-)

-- 
2.26.2

[PULL 01/28] chardev/tcp: Fix error message double free error

[Markus Armbruster]

From: lichun <lichun@ruijie.com.cn>

Errors are already freed by error_report_err, so we only need to call
error_free when that function is not called.

Cc: qemu-stable@nongnu.org
Signed-off-by: lichun <lichun@ruijie.com.cn>
Message-Id: <20200621213017.17978-1-lichun@ruijie.com.cn>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[Commit message improved, cc: qemu-stable]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 chardev/char-socket.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index afebeec5c3..569d54c144 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -142,6 +142,8 @@ static void check_report_connect_error(Chardev *chr,
                           "Unable to connect character device %s: ",
                           chr->label);
         s->connect_err_reported = true;
+    } else {
+        error_free(err);
     }
     qemu_chr_socket_restart_timer(chr);
 }
@@ -1086,7 +1088,6 @@ static void qemu_chr_socket_connected(QIOTask *task, void *opaque)
     if (qio_task_propagate_error(task, &err)) {
         tcp_chr_change_state(s, TCP_CHARDEV_STATE_DISCONNECTED);
         check_report_connect_error(chr, err);
-        error_free(err);
         goto cleanup;
     }
 
-- 
2.26.2

[PULL 02/28] hw/virtio/virtio-iommu-pci.c: Fix typo in error message

[Markus Armbruster]

From: Peter Maydell <peter.maydell@linaro.org>

Fix a typo in an error message in virtio_iommu_pci_realize():
"Check you machine" should be "Check your machine".

Reported-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20200625100811.12690-1-peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/virtio/virtio-iommu-pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/virtio/virtio-iommu-pci.c b/hw/virtio/virtio-iommu-pci.c
index 632533abaf..32e3215d1d 100644
--- a/hw/virtio/virtio-iommu-pci.c
+++ b/hw/virtio/virtio-iommu-pci.c
@@ -48,7 +48,7 @@ static void virtio_iommu_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
                    "%s machine fails to create iommu-map device tree bindings",
                    mc->name);
         error_append_hint(errp,
-                          "Check you machine implements a hotplug handler "
+                          "Check your machine implements a hotplug handler "
                           "for the virtio-iommu-pci device\n");
         error_append_hint(errp, "Check the guest is booted without FW or with "
                           "-no-acpi\n");
-- 
2.26.2

[PULL 03/28] net/virtio: Fix failover_replug_primary() return value regression

[Markus Armbruster]

Commit 150ab54aa6 "net/virtio: fix re-plugging of primary device"
fixed failover_replug_primary() to return false on failure.  Commit
5a0948d36c "net/virtio: Fix failover error handling crash bugs" broke
it again for hotplug_handler_plug() failure.  Unbreak it.

Commit 5a0948d36c4cbc1c5534afac6fee99de55245d12

Fixes: 5a0948d36c4cbc1c5534afac6fee99de55245d12
Cc: Jens Freimann <jfreimann@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: qemu-stable@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Jens Freimann <jfreimann@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Message-Id: <20200630090351.1247703-2-armbru@redhat.com>
---
 hw/net/virtio-net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index aff67a92df..9bb5578e5d 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3129,7 +3129,7 @@ static bool failover_replug_primary(VirtIONet *n, Error **errp)
         if (err) {
             goto out;
         }
-        hotplug_handler_plug(hotplug_ctrl, n->primary_dev, errp);
+        hotplug_handler_plug(hotplug_ctrl, n->primary_dev, &err);
     }
 
 out:
-- 
2.26.2

[PULL 04/28] pci: Delete useless error_propagate()

[Markus Armbruster]

Cc: Jens Freimann <jfreimann@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Jens Freimann <jfreimann@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Message-Id: <20200630090351.1247703-3-armbru@redhat.com>
---
 hw/pci/pci.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index b22dedc88c..de0fae10ab 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2123,7 +2123,6 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp)
         if (!pci_bus_is_express(pci_get_bus(pci_dev))) {
             error_setg(errp, "failover primary device must be on "
                              "PCIExpress bus");
-            error_propagate(errp, local_err);
             pci_qdev_unrealize(DEVICE(pci_dev));
             return;
         }
@@ -2131,7 +2130,6 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp)
         if (class_id != PCI_CLASS_NETWORK_ETHERNET) {
             error_setg(errp, "failover primary device is not an "
                              "Ethernet device");
-            error_propagate(errp, local_err);
             pci_qdev_unrealize(DEVICE(pci_dev));
             return;
         }
@@ -2141,7 +2139,6 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp)
         } else {
             error_setg(errp, "failover: primary device must be in its own "
                               "PCI slot");
-            error_propagate(errp, local_err);
             pci_qdev_unrealize(DEVICE(pci_dev));
             return;
         }
-- 
2.26.2

[PULL 05/28] Clean up some calls to ignore Error objects the right way

[Markus Armbruster]

Receiving the error in a local variable only to free it is less clear
(and also less efficient) than passing NULL.  Clean up.

Cc: Daniel P. Berrange <berrange@redhat.com>
Cc: Jerome Forissier <jerome@forissier.org>
CC: Greg Kurz <groug@kaod.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <20200630090351.1247703-4-armbru@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 chardev/char-socket.c | 6 ++----
 hw/9pfs/9p.c          | 6 ++----
 hw/arm/virt.c         | 4 +---
 hw/ppc/spapr_drc.c    | 4 +---
 ui/vnc.c              | 3 +--
 5 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 569d54c144..5758d9900f 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -817,22 +817,20 @@ static void tcp_chr_tls_init(Chardev *chr)
 {
     SocketChardev *s = SOCKET_CHARDEV(chr);
     QIOChannelTLS *tioc;
-    Error *err = NULL;
     gchar *name;
 
     if (s->is_listen) {
         tioc = qio_channel_tls_new_server(
             s->ioc, s->tls_creds,
             s->tls_authz,
-            &err);
+            NULL);
     } else {
         tioc = qio_channel_tls_new_client(
             s->ioc, s->tls_creds,
             s->addr->u.inet.host,
-            &err);
+            NULL);
     }
     if (tioc == NULL) {
-        error_free(err);
         tcp_chr_disconnect(chr);
         return;
     }
diff --git a/hw/9pfs/9p.c b/hw/9pfs/9p.c
index 45a788f6e6..9755fba9a9 100644
--- a/hw/9pfs/9p.c
+++ b/hw/9pfs/9p.c
@@ -1399,7 +1399,6 @@ static void coroutine_fn v9fs_attach(void *opaque)
     size_t offset = 7;
     V9fsQID qid;
     ssize_t err;
-    Error *local_err = NULL;
 
     v9fs_string_init(&uname);
     v9fs_string_init(&aname);
@@ -1437,9 +1436,8 @@ static void coroutine_fn v9fs_attach(void *opaque)
         error_setg(&s->migration_blocker,
                    "Migration is disabled when VirtFS export path '%s' is mounted in the guest using mount_tag '%s'",
                    s->ctx.fs_root ? s->ctx.fs_root : "NULL", s->tag);
-        err = migrate_add_blocker(s->migration_blocker, &local_err);
-        if (local_err) {
-            error_free(local_err);
+        err = migrate_add_blocker(s->migration_blocker, NULL);
+        if (err < 0) {
             error_free(s->migration_blocker);
             s->migration_blocker = NULL;
             clunk_fid(s, fid);
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index cd0834ce7f..af3050bc4b 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -217,11 +217,9 @@ static bool cpu_type_valid(const char *cpu)
 
 static void create_kaslr_seed(VirtMachineState *vms, const char *node)
 {
-    Error *err = NULL;
     uint64_t seed;
 
-    if (qemu_guest_getrandom(&seed, sizeof(seed), &err)) {
-        error_free(err);
+    if (qemu_guest_getrandom(&seed, sizeof(seed), NULL)) {
         return;
     }
     qemu_fdt_setprop_u64(vms->fdt, node, "kaslr-seed", seed);
diff --git a/hw/ppc/spapr_drc.c b/hw/ppc/spapr_drc.c
index 2689104295..951bcdf2c0 100644
--- a/hw/ppc/spapr_drc.c
+++ b/hw/ppc/spapr_drc.c
@@ -1163,16 +1163,14 @@ static void rtas_ibm_configure_connector(PowerPCCPU *cpu,
     drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
 
     if (!drc->fdt) {
-        Error *local_err = NULL;
         void *fdt;
         int fdt_size;
 
         fdt = create_device_tree(&fdt_size);
 
         if (drck->dt_populate(drc, spapr, fdt, &drc->fdt_start_offset,
-                              &local_err)) {
+                              NULL)) {
             g_free(fdt);
-            error_free(local_err);
             rc = SPAPR_DR_CC_RESPONSE_ERROR;
             goto out;
         }
diff --git a/ui/vnc.c b/ui/vnc.c
index 12a12714e1..0702a76cce 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -458,9 +458,8 @@ static VncServerInfo2List *qmp_query_server_entry(QIOChannelSocket *ioc,
     Error *err = NULL;
     SocketAddress *addr;
 
-    addr = qio_channel_socket_get_local_address(ioc, &err);
+    addr = qio_channel_socket_get_local_address(ioc, NULL);
     if (!addr) {
-        error_free(err);
         return prev;
     }
 
-- 
2.26.2

[PULL 06/28] tests: Use &error_abort where appropriate

[Markus Armbruster]

Receiving the error in a local variable only to assert there is none
is less clear than passing &error_abort.  Clean up.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20200630090351.1247703-5-armbru@redhat.com>
---
 tests/check-qobject.c              |   5 +-
 tests/check-qom-proplist.c         |   7 +-
 tests/test-logging.c               |  12 +---
 tests/test-qemu-opts.c             |  22 ++----
 tests/test-replication.c           | 109 +++++++++--------------------
 tests/test-string-input-visitor.c  |  33 +++------
 tests/test-string-output-visitor.c |  16 ++---
 7 files changed, 59 insertions(+), 145 deletions(-)

diff --git a/tests/check-qobject.c b/tests/check-qobject.c
index 593c3a0618..6b6deaeb8b 100644
--- a/tests/check-qobject.c
+++ b/tests/check-qobject.c
@@ -9,6 +9,7 @@
 
 #include "qemu/osdep.h"
 #include "block/qdict.h"
+#include "qapi/error.h"
 #include "qapi/qmp/qbool.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qlist.h"
@@ -213,7 +214,6 @@ static void qobject_is_equal_list_test(void)
 
 static void qobject_is_equal_dict_test(void)
 {
-    Error *local_err = NULL;
     QDict *dict_0, *dict_1, *dict_cloned;
     QDict *dict_different_key, *dict_different_value, *dict_different_null_key;
     QDict *dict_longer, *dict_shorter, *dict_nested;
@@ -276,8 +276,7 @@ static void qobject_is_equal_dict_test(void)
                   dict_different_null_key, dict_longer, dict_shorter,
                   dict_nested);
 
-    dict_crumpled = qobject_to(QDict, qdict_crumple(dict_1, &local_err));
-    g_assert(!local_err);
+    dict_crumpled = qobject_to(QDict, qdict_crumple(dict_1, &error_abort));
     check_equal(dict_crumpled, dict_nested);
 
     qdict_flatten(dict_nested);
diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c
index 13a824cfae..8c71734e1a 100644
--- a/tests/check-qom-proplist.c
+++ b/tests/check-qom-proplist.c
@@ -419,9 +419,7 @@ static void test_dummy_createcmdl(void)
     g_assert(dobj->bv == true);
     g_assert(dobj->av == DUMMY_PLATYPUS);
 
-    user_creatable_del("dev0", &err);
-    g_assert(err == NULL);
-    error_free(err);
+    user_creatable_del("dev0", &error_abort);
 
     object_unref(OBJECT(dobj));
 
@@ -485,8 +483,7 @@ static void test_dummy_getenum(void)
     val = object_property_get_enum(OBJECT(dobj),
                                    "av",
                                    "DummyAnimal",
-                                   &err);
-    g_assert(err == NULL);
+                                   &error_abort);
     g_assert(val == DUMMY_PLATYPUS);
 
     /* A bad enum type name */
diff --git a/tests/test-logging.c b/tests/test-logging.c
index 8580b82420..8a1161de1d 100644
--- a/tests/test-logging.c
+++ b/tests/test-logging.c
@@ -113,7 +113,6 @@ static void test_logfile_write(gconstpointer data)
     QemuLogFile *logfile;
     QemuLogFile *logfile2;
     gchar const *dir = data;
-    Error *err = NULL;
     g_autofree gchar *file_path = NULL;
     g_autofree gchar *file_path1 = NULL;
     FILE *orig_fd;
@@ -132,8 +131,7 @@ static void test_logfile_write(gconstpointer data)
      * Test that even if an open file handle is changed,
      * our handle remains valid due to RCU.
      */
-    qemu_set_log_filename(file_path, &err);
-    g_assert(!err);
+    qemu_set_log_filename(file_path, &error_abort);
     rcu_read_lock();
     logfile = atomic_rcu_read(&qemu_logfile);
     orig_fd = logfile->fd;
@@ -142,8 +140,7 @@ static void test_logfile_write(gconstpointer data)
     fflush(logfile->fd);
 
     /* Change the logfile and ensure that the handle is still valid. */
-    qemu_set_log_filename(file_path1, &err);
-    g_assert(!err);
+    qemu_set_log_filename(file_path1, &error_abort);
     logfile2 = atomic_rcu_read(&qemu_logfile);
     g_assert(logfile->fd == orig_fd);
     g_assert(logfile2->fd != logfile->fd);
@@ -156,7 +153,6 @@ static void test_logfile_lock(gconstpointer data)
 {
     FILE *logfile;
     gchar const *dir = data;
-    Error *err = NULL;
     g_autofree gchar *file_path = NULL;
 
     file_path = g_build_filename(dir, "qemu_test_logfile_lock0.log", NULL);
@@ -166,7 +162,7 @@ static void test_logfile_lock(gconstpointer data)
      * that even if an open file handle is closed,
      * our handle remains valid for use due to RCU.
      */
-    qemu_set_log_filename(file_path, &err);
+    qemu_set_log_filename(file_path, &error_abort);
     logfile = qemu_log_lock();
     g_assert(logfile);
     fprintf(logfile, "%s 1st write to file\n", __func__);
@@ -180,8 +176,6 @@ static void test_logfile_lock(gconstpointer data)
     fprintf(logfile, "%s 2nd write to file\n", __func__);
     fflush(logfile);
     qemu_log_unlock(logfile);
-
-    g_assert(!err);
 }
 
 /* Remove a directory and all its entries (non-recursive). */
diff --git a/tests/test-qemu-opts.c b/tests/test-qemu-opts.c
index 2a0f42a09b..297ffe79dd 100644
--- a/tests/test-qemu-opts.c
+++ b/tests/test-qemu-opts.c
@@ -187,7 +187,6 @@ static void test_qemu_opt_get(void)
 
 static void test_qemu_opt_get_bool(void)
 {
-    Error *err = NULL;
     QemuOptsList *list;
     QemuOpts *opts;
     bool opt;
@@ -210,16 +209,14 @@ static void test_qemu_opt_get_bool(void)
     opt = qemu_opt_get_bool(opts, "bool1", false);
     g_assert(opt == false);
 
-    qemu_opt_set_bool(opts, "bool1", true, &err);
-    g_assert(!err);
+    qemu_opt_set_bool(opts, "bool1", true, &error_abort);
 
     /* now we have set bool1, should know about it */
     opt = qemu_opt_get_bool(opts, "bool1", false);
     g_assert(opt == true);
 
     /* having reset the value, opt should be the reset one not defval */
-    qemu_opt_set_bool(opts, "bool1", false, &err);
-    g_assert(!err);
+    qemu_opt_set_bool(opts, "bool1", false, &error_abort);
 
     opt = qemu_opt_get_bool(opts, "bool1", true);
     g_assert(opt == false);
@@ -233,7 +230,6 @@ static void test_qemu_opt_get_bool(void)
 
 static void test_qemu_opt_get_number(void)
 {
-    Error *err = NULL;
     QemuOptsList *list;
     QemuOpts *opts;
     uint64_t opt;
@@ -256,16 +252,14 @@ static void test_qemu_opt_get_number(void)
     opt = qemu_opt_get_number(opts, "number1", 5);
     g_assert(opt == 5);
 
-    qemu_opt_set_number(opts, "number1", 10, &err);
-    g_assert(!err);
+    qemu_opt_set_number(opts, "number1", 10, &error_abort);
 
     /* now we have set number1, should know about it */
     opt = qemu_opt_get_number(opts, "number1", 5);
     g_assert(opt == 10);
 
     /* having reset it, the returned should be the reset one not defval */
-    qemu_opt_set_number(opts, "number1", 15, &err);
-    g_assert(!err);
+    qemu_opt_set_number(opts, "number1", 15, &error_abort);
 
     opt = qemu_opt_get_number(opts, "number1", 5);
     g_assert(opt == 15);
@@ -367,7 +361,6 @@ static void test_qemu_opt_unset(void)
 
 static void test_qemu_opts_reset(void)
 {
-    Error *err = NULL;
     QemuOptsList *list;
     QemuOpts *opts;
     uint64_t opt;
@@ -390,8 +383,7 @@ static void test_qemu_opts_reset(void)
     opt = qemu_opt_get_number(opts, "number1", 5);
     g_assert(opt == 5);
 
-    qemu_opt_set_number(opts, "number1", 10, &err);
-    g_assert(!err);
+    qemu_opt_set_number(opts, "number1", 10, &error_abort);
 
     /* now we have set number1, should know about it */
     opt = qemu_opt_get_number(opts, "number1", 5);
@@ -406,7 +398,6 @@ static void test_qemu_opts_reset(void)
 
 static void test_qemu_opts_set(void)
 {
-    Error *err = NULL;
     QemuOptsList *list;
     QemuOpts *opts;
     const char *opt;
@@ -421,8 +412,7 @@ static void test_qemu_opts_set(void)
     g_assert(opts == NULL);
 
     /* implicitly create opts and set str3 value */
-    qemu_opts_set(list, NULL, "str3", "value", &err);
-    g_assert(!err);
+    qemu_opts_set(list, NULL, "str3", "value", &error_abort);
     g_assert(!QTAILQ_EMPTY(&list->head));
 
     /* get the just created opts */
diff --git a/tests/test-replication.c b/tests/test-replication.c
index cbc37db2df..e0b03dafc2 100644
--- a/tests/test-replication.c
+++ b/tests/test-replication.c
@@ -139,8 +139,6 @@ static void make_temp(char *template)
 
 static void prepare_imgs(void)
 {
-    Error *local_err = NULL;
-
     make_temp(p_local_disk);
     make_temp(s_local_disk);
     make_temp(s_active_disk);
@@ -148,19 +146,15 @@ static void prepare_imgs(void)
 
     /* Primary */
     bdrv_img_create(p_local_disk, "qcow2", NULL, NULL, NULL, IMG_SIZE,
-                    BDRV_O_RDWR, true, &local_err);
-    g_assert(!local_err);
+                    BDRV_O_RDWR, true, &error_abort);
 
     /* Secondary */
     bdrv_img_create(s_local_disk, "qcow2", NULL, NULL, NULL, IMG_SIZE,
-                    BDRV_O_RDWR, true, &local_err);
-    g_assert(!local_err);
+                    BDRV_O_RDWR, true, &error_abort);
     bdrv_img_create(s_active_disk, "qcow2", NULL, NULL, NULL, IMG_SIZE,
-                    BDRV_O_RDWR, true, &local_err);
-    g_assert(!local_err);
+                    BDRV_O_RDWR, true, &error_abort);
     bdrv_img_create(s_hidden_disk, "qcow2", NULL, NULL, NULL, IMG_SIZE,
-                    BDRV_O_RDWR, true, &local_err);
-    g_assert(!local_err);
+                    BDRV_O_RDWR, true, &error_abort);
 }
 
 static void cleanup_imgs(void)
@@ -179,7 +173,6 @@ static BlockBackend *start_primary(void)
     BlockBackend *blk;
     QemuOpts *opts;
     QDict *qdict;
-    Error *local_err = NULL;
     char *cmdline;
 
     cmdline = g_strdup_printf("driver=replication,mode=primary,node-name=xxx,"
@@ -193,12 +186,10 @@ static BlockBackend *start_primary(void)
     qdict_set_default_str(qdict, BDRV_OPT_CACHE_DIRECT, "off");
     qdict_set_default_str(qdict, BDRV_OPT_CACHE_NO_FLUSH, "off");
 
-    blk = blk_new_open(NULL, NULL, qdict, BDRV_O_RDWR, &local_err);
+    blk = blk_new_open(NULL, NULL, qdict, BDRV_O_RDWR, &error_abort);
     g_assert(blk);
-    g_assert(!local_err);
 
-    monitor_add_blk(blk, P_ID, &local_err);
-    g_assert(!local_err);
+    monitor_add_blk(blk, P_ID, &error_abort);
 
     qemu_opts_del(opts);
 
@@ -248,12 +239,10 @@ static void test_primary_write(void)
 static void test_primary_start(void)
 {
     BlockBackend *blk = NULL;
-    Error *local_err = NULL;
 
     blk = start_primary();
 
-    replication_start_all(REPLICATION_MODE_PRIMARY, &local_err);
-    g_assert(!local_err);
+    replication_start_all(REPLICATION_MODE_PRIMARY, &error_abort);
 
     /* read from 0 to IMG_SIZE */
     test_blk_read(blk, 0, 0, IMG_SIZE, 0, IMG_SIZE, true);
@@ -266,46 +255,35 @@ static void test_primary_start(void)
 
 static void test_primary_stop(void)
 {
-    Error *local_err = NULL;
     bool failover = true;
 
     start_primary();
 
-    replication_start_all(REPLICATION_MODE_PRIMARY, &local_err);
-    g_assert(!local_err);
+    replication_start_all(REPLICATION_MODE_PRIMARY, &error_abort);
 
-    replication_stop_all(failover, &local_err);
-    g_assert(!local_err);
+    replication_stop_all(failover, &error_abort);
 
     teardown_primary();
 }
 
 static void test_primary_do_checkpoint(void)
 {
-    Error *local_err = NULL;
-
     start_primary();
 
-    replication_start_all(REPLICATION_MODE_PRIMARY, &local_err);
-    g_assert(!local_err);
+    replication_start_all(REPLICATION_MODE_PRIMARY, &error_abort);
 
-    replication_do_checkpoint_all(&local_err);
-    g_assert(!local_err);
+    replication_do_checkpoint_all(&error_abort);
 
     teardown_primary();
 }
 
 static void test_primary_get_error_all(void)
 {
-    Error *local_err = NULL;
-
     start_primary();
 
-    replication_start_all(REPLICATION_MODE_PRIMARY, &local_err);
-    g_assert(!local_err);
+    replication_start_all(REPLICATION_MODE_PRIMARY, &error_abort);
 
-    replication_get_error_all(&local_err);
-    g_assert(!local_err);
+    replication_get_error_all(&error_abort);
 
     teardown_primary();
 }
@@ -316,7 +294,6 @@ static BlockBackend *start_secondary(void)
     QDict *qdict;
     BlockBackend *blk;
     char *cmdline;
-    Error *local_err = NULL;
 
     /* add s_local_disk and forge S_LOCAL_DISK_ID */
     cmdline = g_strdup_printf("file.filename=%s,driver=qcow2,"
@@ -329,10 +306,9 @@ static BlockBackend *start_secondary(void)
     qdict_set_default_str(qdict, BDRV_OPT_CACHE_DIRECT, "off");
     qdict_set_default_str(qdict, BDRV_OPT_CACHE_NO_FLUSH, "off");
 
-    blk = blk_new_open(NULL, NULL, qdict, BDRV_O_RDWR, &local_err);
+    blk = blk_new_open(NULL, NULL, qdict, BDRV_O_RDWR, &error_abort);
     assert(blk);
-    monitor_add_blk(blk, S_LOCAL_DISK_ID, &local_err);
-    g_assert(!local_err);
+    monitor_add_blk(blk, S_LOCAL_DISK_ID, &error_abort);
 
     /* format s_local_disk with pattern "0x11" */
     test_blk_write(blk, 0x11, 0, IMG_SIZE, false);
@@ -356,10 +332,9 @@ static BlockBackend *start_secondary(void)
     qdict_set_default_str(qdict, BDRV_OPT_CACHE_DIRECT, "off");
     qdict_set_default_str(qdict, BDRV_OPT_CACHE_NO_FLUSH, "off");
 
-    blk = blk_new_open(NULL, NULL, qdict, BDRV_O_RDWR, &local_err);
+    blk = blk_new_open(NULL, NULL, qdict, BDRV_O_RDWR, &error_abort);
     assert(blk);
-    monitor_add_blk(blk, S_ID, &local_err);
-    g_assert(!local_err);
+    monitor_add_blk(blk, S_ID, &error_abort);
 
     qemu_opts_del(opts);
 
@@ -420,12 +395,10 @@ static void test_secondary_write(void)
 static void test_secondary_start(void)
 {
     BlockBackend *top_blk, *local_blk;
-    Error *local_err = NULL;
     bool failover = true;
 
     top_blk = start_secondary();
-    replication_start_all(REPLICATION_MODE_SECONDARY, &local_err);
-    g_assert(!local_err);
+    replication_start_all(REPLICATION_MODE_SECONDARY, &error_abort);
 
     /* read from s_local_disk (0, IMG_SIZE) */
     test_blk_read(top_blk, 0x11, 0, IMG_SIZE, 0, IMG_SIZE, false);
@@ -446,8 +419,7 @@ static void test_secondary_start(void)
                   0, IMG_SIZE / 2, false);
 
     /* unblock top_bs */
-    replication_stop_all(failover, &local_err);
-    g_assert(!local_err);
+    replication_stop_all(failover, &error_abort);
 
     teardown_secondary();
 }
@@ -456,12 +428,10 @@ static void test_secondary_start(void)
 static void test_secondary_stop(void)
 {
     BlockBackend *top_blk, *local_blk;
-    Error *local_err = NULL;
     bool failover = true;
 
     top_blk = start_secondary();
-    replication_start_all(REPLICATION_MODE_SECONDARY, &local_err);
-    g_assert(!local_err);
+    replication_start_all(REPLICATION_MODE_SECONDARY, &error_abort);
 
     /* write 0x22 to s_local_disk (IMG_SIZE / 2, IMG_SIZE) */
     local_blk = blk_by_name(S_LOCAL_DISK_ID);
@@ -475,8 +445,7 @@ static void test_secondary_stop(void)
     test_blk_write(top_blk, 0x33, 0, IMG_SIZE / 2, false);
 
     /* do active commit */
-    replication_stop_all(failover, &local_err);
-    g_assert(!local_err);
+    replication_stop_all(failover, &error_abort);
 
     /* read from s_local_disk (0, IMG_SIZE / 2) */
     test_blk_read(top_blk, 0x33, 0, IMG_SIZE / 2,
@@ -493,11 +462,9 @@ static void test_secondary_stop(void)
 static void test_secondary_continuous_replication(void)
 {
     BlockBackend *top_blk, *local_blk;
-    Error *local_err = NULL;
 
     top_blk = start_secondary();
-    replication_start_all(REPLICATION_MODE_SECONDARY, &local_err);
-    g_assert(!local_err);
+    replication_start_all(REPLICATION_MODE_SECONDARY, &error_abort);
 
     /* write 0x22 to s_local_disk (IMG_SIZE / 2, IMG_SIZE) */
     local_blk = blk_by_name(S_LOCAL_DISK_ID);
@@ -511,22 +478,18 @@ static void test_secondary_continuous_replication(void)
     test_blk_write(top_blk, 0x33, 0, IMG_SIZE / 2, false);
 
     /* do failover (active commit) */
-    replication_stop_all(true, &local_err);
-    g_assert(!local_err);
+    replication_stop_all(true, &error_abort);
 
     /* it should ignore all requests from now on */
 
     /* start after failover */
-    replication_start_all(REPLICATION_MODE_PRIMARY, &local_err);
-    g_assert(!local_err);
+    replication_start_all(REPLICATION_MODE_PRIMARY, &error_abort);
 
     /* checkpoint */
-    replication_do_checkpoint_all(&local_err);
-    g_assert(!local_err);
+    replication_do_checkpoint_all(&error_abort);
 
     /* stop */
-    replication_stop_all(true, &local_err);
-    g_assert(!local_err);
+    replication_stop_all(true, &error_abort);
 
     /* read from s_local_disk (0, IMG_SIZE / 2) */
     test_blk_read(top_blk, 0x33, 0, IMG_SIZE / 2,
@@ -543,12 +506,10 @@ static void test_secondary_continuous_replication(void)
 static void test_secondary_do_checkpoint(void)
 {
     BlockBackend *top_blk, *local_blk;
-    Error *local_err = NULL;
     bool failover = true;
 
     top_blk = start_secondary();
-    replication_start_all(REPLICATION_MODE_SECONDARY, &local_err);
-    g_assert(!local_err);
+    replication_start_all(REPLICATION_MODE_SECONDARY, &error_abort);
 
     /* write 0x22 to s_local_disk (IMG_SIZE / 2, IMG_SIZE) */
     local_blk = blk_by_name(S_LOCAL_DISK_ID);
@@ -559,35 +520,29 @@ static void test_secondary_do_checkpoint(void)
     test_blk_read(top_blk, 0x11, IMG_SIZE / 2,
                   IMG_SIZE / 2, 0, IMG_SIZE, false);
 
-    replication_do_checkpoint_all(&local_err);
-    g_assert(!local_err);
+    replication_do_checkpoint_all(&error_abort);
 
     /* after checkpoint, read pattern 0x22 from s_local_disk */
     test_blk_read(top_blk, 0x22, IMG_SIZE / 2,
                   IMG_SIZE / 2, 0, IMG_SIZE, false);
 
     /* unblock top_bs */
-    replication_stop_all(failover, &local_err);
-    g_assert(!local_err);
+    replication_stop_all(failover, &error_abort);
 
     teardown_secondary();
 }
 
 static void test_secondary_get_error_all(void)
 {
-    Error *local_err = NULL;
     bool failover = true;
 
     start_secondary();
-    replication_start_all(REPLICATION_MODE_SECONDARY, &local_err);
-    g_assert(!local_err);
+    replication_start_all(REPLICATION_MODE_SECONDARY, &error_abort);
 
-    replication_get_error_all(&local_err);
-    g_assert(!local_err);
+    replication_get_error_all(&error_abort);
 
     /* unblock top_bs */
-    replication_stop_all(failover, &local_err);
-    g_assert(!local_err);
+    replication_stop_all(failover, &error_abort);
 
     teardown_secondary();
 }
diff --git a/tests/test-string-input-visitor.c b/tests/test-string-input-visitor.c
index 5418e085a4..249faafc9d 100644
--- a/tests/test-string-input-visitor.c
+++ b/tests/test-string-input-visitor.c
@@ -53,8 +53,7 @@ static void test_visitor_in_int(TestInputVisitorData *data,
 
     v = visitor_input_test_init(data, "-42");
 
-    visit_type_int(v, NULL, &res, &err);
-    g_assert(!err);
+    visit_type_int(v, NULL, &res, &error_abort);
     g_assert_cmpint(res, ==, value);
 
     v = visitor_input_test_init(data, "not an int");
@@ -327,44 +326,37 @@ static void test_visitor_in_uintList(TestInputVisitorData *data,
 static void test_visitor_in_bool(TestInputVisitorData *data,
                                  const void *unused)
 {
-    Error *err = NULL;
     bool res = false;
     Visitor *v;
 
     v = visitor_input_test_init(data, "true");
 
-    visit_type_bool(v, NULL, &res, &err);
-    g_assert(!err);
+    visit_type_bool(v, NULL, &res, &error_abort);
     g_assert_cmpint(res, ==, true);
 
     v = visitor_input_test_init(data, "yes");
 
-    visit_type_bool(v, NULL, &res, &err);
-    g_assert(!err);
+    visit_type_bool(v, NULL, &res, &error_abort);
     g_assert_cmpint(res, ==, true);
 
     v = visitor_input_test_init(data, "on");
 
-    visit_type_bool(v, NULL, &res, &err);
-    g_assert(!err);
+    visit_type_bool(v, NULL, &res, &error_abort);
     g_assert_cmpint(res, ==, true);
 
     v = visitor_input_test_init(data, "false");
 
-    visit_type_bool(v, NULL, &res, &err);
-    g_assert(!err);
+    visit_type_bool(v, NULL, &res, &error_abort);
     g_assert_cmpint(res, ==, false);
 
     v = visitor_input_test_init(data, "no");
 
-    visit_type_bool(v, NULL, &res, &err);
-    g_assert(!err);
+    visit_type_bool(v, NULL, &res, &error_abort);
     g_assert_cmpint(res, ==, false);
 
     v = visitor_input_test_init(data, "off");
 
-    visit_type_bool(v, NULL, &res, &err);
-    g_assert(!err);
+    visit_type_bool(v, NULL, &res, &error_abort);
     g_assert_cmpint(res, ==, false);
 }
 
@@ -377,8 +369,7 @@ static void test_visitor_in_number(TestInputVisitorData *data,
 
     v = visitor_input_test_init(data, "3.14");
 
-    visit_type_number(v, NULL, &res, &err);
-    g_assert(!err);
+    visit_type_number(v, NULL, &res, &error_abort);
     g_assert_cmpfloat(res, ==, value);
 
     /* NaN and infinity has to be rejected */
@@ -399,13 +390,11 @@ static void test_visitor_in_string(TestInputVisitorData *data,
                                    const void *unused)
 {
     char *res = NULL, *value = (char *) "Q E M U";
-    Error *err = NULL;
     Visitor *v;
 
     v = visitor_input_test_init(data, value);
 
-    visit_type_str(v, NULL, &res, &err);
-    g_assert(!err);
+    visit_type_str(v, NULL, &res, &error_abort);
     g_assert_cmpstr(res, ==, value);
 
     g_free(res);
@@ -414,7 +403,6 @@ static void test_visitor_in_string(TestInputVisitorData *data,
 static void test_visitor_in_enum(TestInputVisitorData *data,
                                  const void *unused)
 {
-    Error *err = NULL;
     Visitor *v;
     EnumOne i;
 
@@ -423,8 +411,7 @@ static void test_visitor_in_enum(TestInputVisitorData *data,
 
         v = visitor_input_test_init(data, EnumOne_str(i));
 
-        visit_type_EnumOne(v, NULL, &res, &err);
-        g_assert(!err);
+        visit_type_EnumOne(v, NULL, &res, &error_abort);
         g_assert_cmpint(i, ==, res);
     }
 }
diff --git a/tests/test-string-output-visitor.c b/tests/test-string-output-visitor.c
index 3bd732222c..9f6581439a 100644
--- a/tests/test-string-output-visitor.c
+++ b/tests/test-string-output-visitor.c
@@ -71,11 +71,9 @@ static void test_visitor_out_int(TestOutputVisitorData *data,
                                  const void *unused)
 {
     int64_t value = 42;
-    Error *err = NULL;
     char *str;
 
-    visit_type_int(data->ov, NULL, &value, &err);
-    g_assert(!err);
+    visit_type_int(data->ov, NULL, &value, &error_abort);
 
     str = visitor_get(data);
     if (data->human) {
@@ -120,12 +118,10 @@ static void test_visitor_out_intList(TestOutputVisitorData *data,
 static void test_visitor_out_bool(TestOutputVisitorData *data,
                                   const void *unused)
 {
-    Error *err = NULL;
     bool value = true;
     char *str;
 
-    visit_type_bool(data->ov, NULL, &value, &err);
-    g_assert(!err);
+    visit_type_bool(data->ov, NULL, &value, &error_abort);
 
     str = visitor_get(data);
     g_assert_cmpstr(str, ==, "true");
@@ -135,11 +131,9 @@ static void test_visitor_out_number(TestOutputVisitorData *data,
                                     const void *unused)
 {
     double value = 3.14;
-    Error *err = NULL;
     char *str;
 
-    visit_type_number(data->ov, NULL, &value, &err);
-    g_assert(!err);
+    visit_type_number(data->ov, NULL, &value, &error_abort);
 
     str = visitor_get(data);
     g_assert_cmpstr(str, ==, "3.140000");
@@ -150,11 +144,9 @@ static void test_visitor_out_string(TestOutputVisitorData *data,
 {
     char *string = (char *) "Q E M U";
     const char *string_human = "\"Q E M U\"";
-    Error *err = NULL;
     char *str;
 
-    visit_type_str(data->ov, NULL, &string, &err);
-    g_assert(!err);
+    visit_type_str(data->ov, NULL, &string, &error_abort);
 
     str = visitor_get(data);
     if (data->human) {
-- 
2.26.2

[PULL 07/28] tests: Use error_free_or_abort() where appropriate

[Markus Armbruster]

Replace

    g_assert(err != NULL);
    error_free(err);
    err = NULL;

and variations thereof by

    error_free_or_abort(&err);

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200630090351.1247703-6-armbru@redhat.com>
---
 tests/check-block-qdict.c   | 24 ++++++------------------
 tests/check-qom-proplist.c  |  7 ++-----
 tests/test-base64.c         |  3 +--
 tests/test-bdrv-graph-mod.c |  4 +---
 tests/test-block-iothread.c |  3 +--
 tests/test-crypto-cipher.c  |  8 ++------
 tests/test-io-task.c        |  4 +---
 7 files changed, 14 insertions(+), 39 deletions(-)

diff --git a/tests/check-block-qdict.c b/tests/check-block-qdict.c
index 73d3e9f574..5a25825093 100644
--- a/tests/check-block-qdict.c
+++ b/tests/check-block-qdict.c
@@ -610,9 +610,7 @@ static void qdict_rename_keys_test(void)
     copy = qdict_clone_shallow(dict);
     qdict_rename_keys(copy, renames, &local_err);
 
-    g_assert(local_err != NULL);
-    error_free(local_err);
-    local_err = NULL;
+    error_free_or_abort(&local_err);
 
     g_assert_cmpstr(qdict_get_str(copy, "abc"), ==, "foo");
     g_assert_cmpstr(qdict_get_str(copy, "abcdef"), ==, "bar");
@@ -649,9 +647,7 @@ static void qdict_crumple_test_bad_inputs(void)
     qdict_put_str(src, "rule.0.policy", "allow");
 
     g_assert(qdict_crumple(src, &error) == NULL);
-    g_assert(error != NULL);
-    error_free(error);
-    error = NULL;
+    error_free_or_abort(&error);
     qobject_unref(src);
 
     src = qdict_new();
@@ -660,9 +656,7 @@ static void qdict_crumple_test_bad_inputs(void)
     qdict_put_str(src, "rule.a", "allow");
 
     g_assert(qdict_crumple(src, &error) == NULL);
-    g_assert(error != NULL);
-    error_free(error);
-    error = NULL;
+    error_free_or_abort(&error);
     qobject_unref(src);
 
     src = qdict_new();
@@ -673,9 +667,7 @@ static void qdict_crumple_test_bad_inputs(void)
     qdict_put_str(src, "rule.b", "allow");
 
     g_assert(qdict_crumple(src, &error) == NULL);
-    g_assert(error != NULL);
-    error_free(error);
-    error = NULL;
+    error_free_or_abort(&error);
     qobject_unref(src);
 
     src = qdict_new();
@@ -684,9 +676,7 @@ static void qdict_crumple_test_bad_inputs(void)
     qdict_put_str(src, "rule.3", "allow");
 
     g_assert(qdict_crumple(src, &error) == NULL);
-    g_assert(error != NULL);
-    error_free(error);
-    error = NULL;
+    error_free_or_abort(&error);
     qobject_unref(src);
 
     src = qdict_new();
@@ -695,9 +685,7 @@ static void qdict_crumple_test_bad_inputs(void)
     qdict_put_str(src, "rule.+1", "allow");
 
     g_assert(qdict_crumple(src, &error) == NULL);
-    g_assert(error != NULL);
-    error_free(error);
-    error = NULL;
+    error_free_or_abort(&error);
     qobject_unref(src);
 }
 
diff --git a/tests/check-qom-proplist.c b/tests/check-qom-proplist.c
index 8c71734e1a..e1e0a96661 100644
--- a/tests/check-qom-proplist.c
+++ b/tests/check-qom-proplist.c
@@ -491,17 +491,14 @@ static void test_dummy_getenum(void)
                                    "av",
                                    "BadAnimal",
                                    &err);
-    g_assert(err != NULL);
-    error_free(err);
-    err = NULL;
+    error_free_or_abort(&err);
 
     /* A non-enum property name */
     val = object_property_get_enum(OBJECT(dobj),
                                    "iv",
                                    "DummyAnimal",
                                    &err);
-    g_assert(err != NULL);
-    error_free(err);
+    error_free_or_abort(&err);
 
     object_unparent(OBJECT(dobj));
 }
diff --git a/tests/test-base64.c b/tests/test-base64.c
index ec122ceba5..a7f722c459 100644
--- a/tests/test-base64.c
+++ b/tests/test-base64.c
@@ -54,10 +54,9 @@ static void test_base64_bad(const char *input,
                                      &len,
                                      &err);
 
-    g_assert(err != NULL);
+    error_free_or_abort(&err);
     g_assert(actual == NULL);
     g_assert_cmpint(len, ==, 0);
-    error_free(err);
 }
 
 
diff --git a/tests/test-bdrv-graph-mod.c b/tests/test-bdrv-graph-mod.c
index f93f3168b0..8cff13830e 100644
--- a/tests/test-bdrv-graph-mod.c
+++ b/tests/test-bdrv-graph-mod.c
@@ -115,9 +115,7 @@ static void test_update_perm_tree(void)
                       BDRV_CHILD_FILTERED | BDRV_CHILD_PRIMARY, &error_abort);
 
     bdrv_append(filter, bs, &local_err);
-
-    g_assert_nonnull(local_err);
-    error_free(local_err);
+    error_free_or_abort(&local_err);
 
     blk_unref(root);
 }
diff --git a/tests/test-block-iothread.c b/tests/test-block-iothread.c
index a953794be2..3f866a35c6 100644
--- a/tests/test-block-iothread.c
+++ b/tests/test-block-iothread.c
@@ -650,8 +650,7 @@ static void test_propagate_mirror(void)
     blk_insert_bs(blk, src, &error_abort);
 
     bdrv_try_set_aio_context(target, ctx, &local_err);
-    g_assert(local_err);
-    error_free(local_err);
+    error_free_or_abort(&local_err);
 
     g_assert(blk_get_aio_context(blk) == main_ctx);
     g_assert(bdrv_get_aio_context(src) == main_ctx);
diff --git a/tests/test-crypto-cipher.c b/tests/test-crypto-cipher.c
index 07fa2fa616..bebba1a4f4 100644
--- a/tests/test-crypto-cipher.c
+++ b/tests/test-crypto-cipher.c
@@ -761,10 +761,7 @@ static void test_cipher_short_plaintext(void)
                                  sizeof(plaintext1),
                                  &err);
     g_assert(ret == -1);
-    g_assert(err != NULL);
-
-    error_free(err);
-    err = NULL;
+    error_free_or_abort(&err);
 
     /* Should report an error as plaintext is larger than
      * block size, but not a multiple of block size
@@ -775,9 +772,8 @@ static void test_cipher_short_plaintext(void)
                                  sizeof(plaintext2),
                                  &err);
     g_assert(ret == -1);
-    g_assert(err != NULL);
+    error_free_or_abort(&err);
 
-    error_free(err);
     qcrypto_cipher_free(cipher);
 }
 
diff --git a/tests/test-io-task.c b/tests/test-io-task.c
index c8a3813d49..85e7a98da5 100644
--- a/tests/test-io-task.c
+++ b/tests/test-io-task.c
@@ -240,9 +240,7 @@ static void test_task_thread_failure(void)
     object_unref(obj);
 
     g_assert(data.source == obj);
-    g_assert(data.err != NULL);
-
-    error_free(data.err);
+    error_free_or_abort(&data.err);
 
     self = g_thread_self();
 
-- 
2.26.2

[PULL 08/28] usb/dev-mtp: Fix Error double free after inotify failure

[Markus Armbruster]

error_report_err() frees its first argument.  Freeing it again is
wrong.  Don't.

Fixes: 47287c27d0c367a89f7b2851e23a7f8b2d499dd6
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Daniel P. Berrangé <berrange@redhat.com>
Cc: qemu-stable@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200630090351.1247703-7-armbru@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 hw/usb/dev-mtp.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/usb/dev-mtp.c b/hw/usb/dev-mtp.c
index 168428156b..15a2243101 100644
--- a/hw/usb/dev-mtp.c
+++ b/hw/usb/dev-mtp.c
@@ -634,7 +634,6 @@ static void usb_mtp_object_readdir(MTPState *s, MTPObject *o)
             error_reportf_err(err,
                               "usb-mtp: failed to add watch for %s: ",
                               o->path);
-            error_free(err);
         } else {
             trace_usb_mtp_file_monitor_event(s->dev.addr, o->path,
                                              "Watch Added");
@@ -1279,7 +1278,6 @@ static void usb_mtp_command(MTPState *s, MTPControl *c)
         if (err) {
             error_reportf_err(err,
                               "usb-mtp: file monitoring init failed: ");
-            error_free(err);
         } else {
             QTAILQ_INIT(&s->events);
         }
-- 
2.26.2

[PULL 09/28] spapr: Plug minor memory leak in spapr_machine_init()

[Markus Armbruster]

spapr_machine_init() leaks an Error object when
kvmppc_check_papr_resize_hpt() fails and spapr->resize_hpt is
SPAPR_RESIZE_HPT_DISABLED, i.e. when the host doesn't support hash
page table resizing, and the user didn't ask for it.  As harmless as
memory leaks can possibly be.  Plug it.

Fixes: 30f4b05bd090564181554d0890605eb2c143e4ea
Cc: David Gibson <dgibson@redhat.com>
Cc: qemu-ppc@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
Message-Id: <20200630090351.1247703-8-armbru@redhat.com>
---
 hw/ppc/spapr.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 4c185bcc13..44fd578ea9 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2731,6 +2731,7 @@ static void spapr_machine_init(MachineState *machine)
         error_report_err(resize_hpt_err);
         exit(1);
     }
+    error_free(resize_hpt_err);
 
     spapr->rma_size = spapr_rma_size(spapr, &error_fatal);
 
-- 
2.26.2

[PULL 10/28] qga: Plug unlikely memory leak in guest-set-memory-blocks

[Markus Armbruster]

transfer_memory_block() leaks an Error object when reading file
/sys/devices/system/memory/memory<INDEX>/state fails with errno other
than ENOENT, and @sys2memblk is false, i.e. when the state file exists
but cannot be read (seems quite unlikely), and this is
guest-set-memory-blocks, not guest-get-memory-blocks.

Plug the leak.

Fixes: bd240fca42d5f072fb758a71720d9de9990ac553
Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
Cc: Hailiang Zhang <zhang.zhanghailiang@huawei.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: zhanghailiang <zhang.zhanghailiang@huawei.com>
Message-Id: <20200630090351.1247703-9-armbru@redhat.com>
---
 qga/commands-posix.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index ae1348dc8f..cdbeb59dcc 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2421,6 +2421,7 @@ static void transfer_memory_block(GuestMemoryBlock *mem_blk, bool sys2memblk,
             if (sys2memblk) {
                 error_propagate(errp, local_err);
             } else {
+                error_free(local_err);
                 result->response =
                     GUEST_MEMORY_BLOCK_RESPONSE_TYPE_OPERATION_FAILED;
             }
-- 
2.26.2

[PULL 11/28] sd/milkymist-memcard: Plug minor memory leak in realize

[Markus Armbruster]

milkymist_memcard_realize() leaks an Error object when realization of
its "sd-card" device fails.  Quite harmless, since we only ever
realize this once, in milkymist_init() via milkymist_memcard_create().

Plug the leak.

Fixes: 3d0369ba499866cc6a839f71212d97876500762d
Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
Cc: Michael Walle <michael@walle.cc>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200630090351.1247703-10-armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/milkymist-memcard.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/sd/milkymist-memcard.c b/hw/sd/milkymist-memcard.c
index 482e97191e..afdb8aa0c0 100644
--- a/hw/sd/milkymist-memcard.c
+++ b/hw/sd/milkymist-memcard.c
@@ -280,9 +280,8 @@ static void milkymist_memcard_realize(DeviceState *dev, Error **errp)
     blk = dinfo ? blk_by_legacy_dinfo(dinfo) : NULL;
     carddev = qdev_new(TYPE_SD_CARD);
     qdev_prop_set_drive(carddev, "drive", blk);
-    qdev_realize_and_unref(carddev, BUS(&s->sdbus), &err);
-    if (err) {
-        error_setg(errp, "failed to init SD card: %s", error_get_pretty(err));
+    if (!qdev_realize_and_unref(carddev, BUS(&s->sdbus), &err)) {
+        error_propagate_prepend(errp, err, "failed to init SD card: %s");
         return;
     }
     s->enabled = blk && blk_is_inserted(blk);
-- 
2.26.2

[PULL 12/28] test-util-filemonitor: Plug unlikely memory leak

[Markus Armbruster]

test_file_monitor_events() leaks an Error object when
qemu_file_monitor_add_watch() fails, which seems unlikely.  Plug it.

Cc: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200630090351.1247703-11-armbru@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tests/test-util-filemonitor.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/test-util-filemonitor.c b/tests/test-util-filemonitor.c
index 45009c69f4..8f0eff3d03 100644
--- a/tests/test-util-filemonitor.c
+++ b/tests/test-util-filemonitor.c
@@ -495,6 +495,7 @@ test_file_monitor_events(void)
             if (*op->watchid < 0) {
                 g_printerr("Unable to add watch %s",
                            error_get_pretty(local_err));
+                error_free(local_err);
                 goto cleanup;
             }
             if (debug) {
-- 
2.26.2

[PULL 13/28] vnc: Plug minor memory leak in vnc_display_open()

[Markus Armbruster]

vnc_display_print_local_addr() leaks the Error object when
qio_channel_socket_get_local_address() fails.  Seems unlikely.  Called
when we create a VNC display with vnc_display_open().  Plug the leak
by passing NULL to ignore the error.

Cc: Daniel P. Berrange <berrange@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200630090351.1247703-12-armbru@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 ui/vnc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/ui/vnc.c b/ui/vnc.c
index 0702a76cce..527ad25124 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3274,13 +3274,12 @@ int vnc_display_pw_expire(const char *id, time_t expires)
 static void vnc_display_print_local_addr(VncDisplay *vd)
 {
     SocketAddress *addr;
-    Error *err = NULL;
 
     if (!vd->listener || !vd->listener->nsioc) {
         return;
     }
 
-    addr = qio_channel_socket_get_local_address(vd->listener->sioc[0], &err);
+    addr = qio_channel_socket_get_local_address(vd->listener->sioc[0], NULL);
     if (!addr) {
         return;
     }
-- 
2.26.2

[PULL 14/28] aspeed: Clean up roundabout error propagation

[Markus Armbruster]

Replace

        sysbus_realize(SYS_BUS_DEVICE(&s->spi[i]), &local_err);
        error_propagate(&err, local_err);
        if (err) {
            error_propagate(errp, err);
            return;
	}

by

        sysbus_realize(SYS_BUS_DEVICE(&s->spi[i]), &err);
        if (err) {
            error_propagate(errp, err);
            return;
	}

Cc: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Message-Id: <20200630090351.1247703-13-armbru@redhat.com>
---
 hw/arm/aspeed_ast2600.c | 10 ++++------
 hw/arm/aspeed_soc.c     | 10 ++++------
 2 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index 6da687299f..08b3592e36 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -228,7 +228,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
     int i;
     AspeedSoCState *s = ASPEED_SOC(dev);
     AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
-    Error *err = NULL, *local_err = NULL;
+    Error *err = NULL;
     qemu_irq irq;
 
     /* IO space */
@@ -394,8 +394,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
             return;
         }
         object_property_set_int(OBJECT(&s->spi[i]), 1, "num-cs", &err);
-        sysbus_realize(SYS_BUS_DEVICE(&s->spi[i]), &local_err);
-        error_propagate(&err, local_err);
+        sysbus_realize(SYS_BUS_DEVICE(&s->spi[i]), &err);
         if (err) {
             error_propagate(errp, err);
             return;
@@ -446,11 +445,10 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
     for (i = 0; i < sc->macs_num; i++) {
         object_property_set_bool(OBJECT(&s->ftgmac100[i]), true, "aspeed",
                                  &err);
-        sysbus_realize(SYS_BUS_DEVICE(&s->ftgmac100[i]), &local_err);
-        error_propagate(&err, local_err);
+        sysbus_realize(SYS_BUS_DEVICE(&s->ftgmac100[i]), &err);
         if (err) {
             error_propagate(errp, err);
-           return;
+            return;
         }
         sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100[i]), 0,
                         sc->memmap[ASPEED_ETH1 + i]);
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index 810cf9b6cc..ec21de50ce 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -218,7 +218,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
     int i;
     AspeedSoCState *s = ASPEED_SOC(dev);
     AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s);
-    Error *err = NULL, *local_err = NULL;
+    Error *err = NULL;
 
     /* IO space */
     create_unimplemented_device("aspeed_soc.io", sc->memmap[ASPEED_IOMEM],
@@ -340,8 +340,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
     /* SPI */
     for (i = 0; i < sc->spis_num; i++) {
         object_property_set_int(OBJECT(&s->spi[i]), 1, "num-cs", &err);
-        sysbus_realize(SYS_BUS_DEVICE(&s->spi[i]), &local_err);
-        error_propagate(&err, local_err);
+        sysbus_realize(SYS_BUS_DEVICE(&s->spi[i]), &err);
         if (err) {
             error_propagate(errp, err);
             return;
@@ -392,11 +391,10 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
     for (i = 0; i < sc->macs_num; i++) {
         object_property_set_bool(OBJECT(&s->ftgmac100[i]), true, "aspeed",
                                  &err);
-        sysbus_realize(SYS_BUS_DEVICE(&s->ftgmac100[i]), &local_err);
-        error_propagate(&err, local_err);
+        sysbus_realize(SYS_BUS_DEVICE(&s->ftgmac100[i]), &err);
         if (err) {
             error_propagate(errp, err);
-           return;
+            return;
         }
         sysbus_mmio_map(SYS_BUS_DEVICE(&s->ftgmac100[i]), 0,
                         sc->memmap[ASPEED_ETH1 + i]);
-- 
2.26.2

[PULL 15/28] qdev: Drop qbus_set_bus_hotplug_handler() parameter @errp

[Markus Armbruster]

All callers pass &error_abort.  Drop the parameter.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200630090351.1247703-14-armbru@redhat.com>
---
 include/hw/qdev-core.h      | 2 +-
 hw/core/bus.c               | 4 ++--
 hw/scsi/scsi-bus.c          | 2 +-
 hw/usb/bus.c                | 2 +-
 hw/xen/xen-bus.c            | 2 +-
 hw/xen/xen-legacy-backend.c | 2 +-
 6 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 7dc10be46f..78acdeaed6 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -537,7 +537,7 @@ char *qdev_get_dev_path(DeviceState *dev);
 
 void qbus_set_hotplug_handler(BusState *bus, Object *handler, Error **errp);
 
-void qbus_set_bus_hotplug_handler(BusState *bus, Error **errp);
+void qbus_set_bus_hotplug_handler(BusState *bus);
 
 static inline bool qbus_is_hotpluggable(BusState *bus)
 {
diff --git a/hw/core/bus.c b/hw/core/bus.c
index 6cc28b334e..8d4e810d7f 100644
--- a/hw/core/bus.c
+++ b/hw/core/bus.c
@@ -29,9 +29,9 @@ void qbus_set_hotplug_handler(BusState *bus, Object *handler, Error **errp)
                              QDEV_HOTPLUG_HANDLER_PROPERTY, errp);
 }
 
-void qbus_set_bus_hotplug_handler(BusState *bus, Error **errp)
+void qbus_set_bus_hotplug_handler(BusState *bus)
 {
-    qbus_set_hotplug_handler(bus, OBJECT(bus), errp);
+    qbus_set_hotplug_handler(bus, OBJECT(bus), &error_abort);
 }
 
 int qbus_walk_children(BusState *bus,
diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 27843bb04b..b878a08080 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -107,7 +107,7 @@ void scsi_bus_new(SCSIBus *bus, size_t bus_size, DeviceState *host,
     qbus_create_inplace(bus, bus_size, TYPE_SCSI_BUS, host, bus_name);
     bus->busnr = next_scsi_bus++;
     bus->info = info;
-    qbus_set_bus_hotplug_handler(BUS(bus), &error_abort);
+    qbus_set_bus_hotplug_handler(BUS(bus));
 }
 
 static void scsi_dma_restart_bh(void *opaque)
diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index a81aee2051..957559b18d 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -84,7 +84,7 @@ void usb_bus_new(USBBus *bus, size_t bus_size,
                  USBBusOps *ops, DeviceState *host)
 {
     qbus_create_inplace(bus, bus_size, TYPE_USB_BUS, host, NULL);
-    qbus_set_bus_hotplug_handler(BUS(bus), &error_abort);
+    qbus_set_bus_hotplug_handler(BUS(bus));
     bus->ops = ops;
     bus->busnr = next_usb_bus++;
     QTAILQ_INIT(&bus->free);
diff --git a/hw/xen/xen-bus.c b/hw/xen/xen-bus.c
index 4b00320f1c..c4e2162ae9 100644
--- a/hw/xen/xen-bus.c
+++ b/hw/xen/xen-bus.c
@@ -1391,5 +1391,5 @@ void xen_bus_init(void)
     BusState *bus = qbus_create(TYPE_XEN_BUS, dev, NULL);
 
     sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
-    qbus_set_bus_hotplug_handler(bus, &error_abort);
+    qbus_set_bus_hotplug_handler(bus);
 }
diff --git a/hw/xen/xen-legacy-backend.c b/hw/xen/xen-legacy-backend.c
index 2335ee2e65..7d4b13351e 100644
--- a/hw/xen/xen-legacy-backend.c
+++ b/hw/xen/xen-legacy-backend.c
@@ -705,7 +705,7 @@ int xen_be_init(void)
     xen_sysdev = qdev_new(TYPE_XENSYSDEV);
     sysbus_realize_and_unref(SYS_BUS_DEVICE(xen_sysdev), &error_fatal);
     xen_sysbus = qbus_create(TYPE_XENSYSBUS, xen_sysdev, "xen-sysbus");
-    qbus_set_bus_hotplug_handler(xen_sysbus, &error_abort);
+    qbus_set_bus_hotplug_handler(xen_sysbus);
 
     return 0;
 
-- 
2.26.2

[PULL 16/28] qdev: Drop qbus_set_hotplug_handler() parameter @errp

[Markus Armbruster]

qbus_set_hotplug_handler() is a simple wrapper around
object_property_set_link().

object_property_set_link() fails when the property doesn't exist, is
not settable, or its .check() method fails.  These are all programming
errors here, so passing &error_abort to qbus_set_hotplug_handler() is
appropriate.

Most of its callers do.  Exceptions:

* pcie_cap_slot_init(), shpc_init(), spapr_phb_realize() pass NULL,
  i.e. they ignore errors.

* spapr_machine_init() passes &error_fatal.

* s390_pcihost_realize(), virtio_serial_device_realize(),
  s390_pcihost_plug() pass the error to their callers.  The latter two
  keep going after the error, which looks wrong.

Drop the @errp parameter, and instead pass &error_abort to
object_property_set_link().

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200630090351.1247703-15-armbru@redhat.com>
---
 include/hw/qdev-core.h        |  3 +--
 hw/acpi/pcihp.c               |  3 +--
 hw/acpi/piix4.c               |  2 +-
 hw/char/virtio-serial-bus.c   |  4 ++--
 hw/core/bus.c                 |  6 +++---
 hw/pci/pcie.c                 |  2 +-
 hw/pci/shpc.c                 |  2 +-
 hw/ppc/spapr.c                |  3 +--
 hw/ppc/spapr_pci.c            |  4 ++--
 hw/s390x/ap-bridge.c          |  2 +-
 hw/s390x/css-bridge.c         |  2 +-
 hw/s390x/s390-pci-bus.c       | 14 +++-----------
 hw/scsi/virtio-scsi.c         |  4 ++--
 hw/scsi/vmw_pvscsi.c          |  2 +-
 hw/usb/dev-smartcard-reader.c |  2 +-
 15 files changed, 22 insertions(+), 33 deletions(-)

diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 78acdeaed6..fe78073c70 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -535,8 +535,7 @@ extern bool qdev_hot_removed;
 
 char *qdev_get_dev_path(DeviceState *dev);
 
-void qbus_set_hotplug_handler(BusState *bus, Object *handler, Error **errp);
-
+void qbus_set_hotplug_handler(BusState *bus, Object *handler);
 void qbus_set_bus_hotplug_handler(BusState *bus);
 
 static inline bool qbus_is_hotpluggable(BusState *bus)
diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
index 33ea2b76ae..9e31ab2da4 100644
--- a/hw/acpi/pcihp.c
+++ b/hw/acpi/pcihp.c
@@ -246,8 +246,7 @@ void acpi_pcihp_device_plug_cb(HotplugHandler *hotplug_dev, AcpiPciHpState *s,
             object_dynamic_cast(OBJECT(dev), TYPE_PCI_BRIDGE)) {
             PCIBus *sec = pci_bridge_get_sec_bus(PCI_BRIDGE(pdev));
 
-            qbus_set_hotplug_handler(BUS(sec), OBJECT(hotplug_dev),
-                                     &error_abort);
+            qbus_set_hotplug_handler(BUS(sec), OBJECT(hotplug_dev));
             /* We don't have to overwrite any other hotplug handler yet */
             assert(QLIST_EMPTY(&sec->child));
         }
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 630ca6ec87..283422e0d3 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -506,7 +506,7 @@ static void piix4_pm_realize(PCIDevice *dev, Error **errp)
 
     piix4_acpi_system_hot_add_init(pci_address_space_io(dev),
                                    pci_get_bus(dev), s);
-    qbus_set_hotplug_handler(BUS(pci_get_bus(dev)), OBJECT(s), &error_abort);
+    qbus_set_hotplug_handler(BUS(pci_get_bus(dev)), OBJECT(s));
 
     piix4_pm_add_propeties(s);
 }
diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index 262089c0c9..f9a4428bd6 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -1056,7 +1056,7 @@ static void virtio_serial_device_realize(DeviceState *dev, Error **errp)
     /* Spawn a new virtio-serial bus on which the ports will ride as devices */
     qbus_create_inplace(&vser->bus, sizeof(vser->bus), TYPE_VIRTIO_SERIAL_BUS,
                         dev, vdev->bus_name);
-    qbus_set_hotplug_handler(BUS(&vser->bus), OBJECT(vser), errp);
+    qbus_set_hotplug_handler(BUS(&vser->bus), OBJECT(vser));
     vser->bus.vser = vser;
     QTAILQ_INIT(&vser->ports);
 
@@ -1147,7 +1147,7 @@ static void virtio_serial_device_unrealize(DeviceState *dev)
         g_free(vser->post_load);
     }
 
-    qbus_set_hotplug_handler(BUS(&vser->bus), NULL, &error_abort);
+    qbus_set_hotplug_handler(BUS(&vser->bus), NULL);
 
     virtio_cleanup(vdev);
 }
diff --git a/hw/core/bus.c b/hw/core/bus.c
index 8d4e810d7f..544dd8a6fa 100644
--- a/hw/core/bus.c
+++ b/hw/core/bus.c
@@ -23,15 +23,15 @@
 #include "qemu/module.h"
 #include "qapi/error.h"
 
-void qbus_set_hotplug_handler(BusState *bus, Object *handler, Error **errp)
+void qbus_set_hotplug_handler(BusState *bus, Object *handler)
 {
     object_property_set_link(OBJECT(bus), handler,
-                             QDEV_HOTPLUG_HANDLER_PROPERTY, errp);
+                             QDEV_HOTPLUG_HANDLER_PROPERTY, &error_abort);
 }
 
 void qbus_set_bus_hotplug_handler(BusState *bus)
 {
-    qbus_set_hotplug_handler(bus, OBJECT(bus), &error_abort);
+    qbus_set_hotplug_handler(bus, OBJECT(bus));
 }
 
 int qbus_walk_children(BusState *bus,
diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 086d0dfceb..5b48bae0f6 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -574,7 +574,7 @@ void pcie_cap_slot_init(PCIDevice *dev, PCIESlot *s)
     dev->exp.hpev_notified = false;
 
     qbus_set_hotplug_handler(BUS(pci_bridge_get_sec_bus(PCI_BRIDGE(dev))),
-                             OBJECT(dev), NULL);
+                             OBJECT(dev));
 }
 
 void pcie_cap_slot_reset(PCIDevice *dev)
diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c
index 99d65d5c4c..b00dce629c 100644
--- a/hw/pci/shpc.c
+++ b/hw/pci/shpc.c
@@ -649,7 +649,7 @@ int shpc_init(PCIDevice *d, PCIBus *sec_bus, MemoryRegion *bar,
     shpc_cap_update_dword(d);
     memory_region_add_subregion(bar, offset, &shpc->mmio);
 
-    qbus_set_hotplug_handler(BUS(sec_bus), OBJECT(d), NULL);
+    qbus_set_hotplug_handler(BUS(sec_bus), OBJECT(d));
 
     d->cap_present |= QEMU_PCI_CAP_SHPC;
     return 0;
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 44fd578ea9..f6f034d039 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3033,8 +3033,7 @@ static void spapr_machine_init(MachineState *machine)
     register_savevm_live("spapr/htab", VMSTATE_INSTANCE_ID_ANY, 1,
                          &savevm_htab_handlers, spapr);
 
-    qbus_set_hotplug_handler(sysbus_get_default(), OBJECT(machine),
-                             &error_fatal);
+    qbus_set_hotplug_handler(sysbus_get_default(), OBJECT(machine));
 
     qemu_register_boot_set(spapr_boot_set, spapr);
 
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 329002ac04..0f00e2421f 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1719,7 +1719,7 @@ static void spapr_phb_unrealize(DeviceState *dev)
     address_space_remove_listeners(&sphb->iommu_as);
     address_space_destroy(&sphb->iommu_as);
 
-    qbus_set_hotplug_handler(BUS(phb->bus), NULL, &error_abort);
+    qbus_set_hotplug_handler(BUS(phb->bus), NULL);
     pci_unregister_root_bus(phb->bus);
 
     memory_region_del_subregion(get_system_memory(), &sphb->iowindow);
@@ -1868,7 +1868,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
         bus->flags |= PCI_BUS_EXTENDED_CONFIG_SPACE;
     }
     phb->bus = bus;
-    qbus_set_hotplug_handler(BUS(phb->bus), OBJECT(sphb), NULL);
+    qbus_set_hotplug_handler(BUS(phb->bus), OBJECT(sphb));
 
     /*
      * Initialize PHB address space.
diff --git a/hw/s390x/ap-bridge.c b/hw/s390x/ap-bridge.c
index c4e3188ad6..8bcf8ece9d 100644
--- a/hw/s390x/ap-bridge.c
+++ b/hw/s390x/ap-bridge.c
@@ -58,7 +58,7 @@ void s390_init_ap(void)
     bus = qbus_create(TYPE_AP_BUS, dev, TYPE_AP_BUS);
 
     /* Enable hotplugging */
-    qbus_set_hotplug_handler(bus, OBJECT(dev), &error_abort);
+    qbus_set_hotplug_handler(bus, OBJECT(dev));
  }
 
 static void ap_bridge_class_init(ObjectClass *oc, void *data)
diff --git a/hw/s390x/css-bridge.c b/hw/s390x/css-bridge.c
index e37a54d3f2..9d793d671e 100644
--- a/hw/s390x/css-bridge.c
+++ b/hw/s390x/css-bridge.c
@@ -111,7 +111,7 @@ VirtualCssBus *virtual_css_bus_init(void)
     cbus = VIRTUAL_CSS_BUS(bus);
 
     /* Enable hotplugging */
-    qbus_set_hotplug_handler(bus, OBJECT(dev), &error_abort);
+    qbus_set_hotplug_handler(bus, OBJECT(dev));
 
     css_register_io_adapters(CSS_IO_ADAPTER_VIRTIO, true, false,
                              0, &error_abort);
diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index a13978bb37..142e52a8ff 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -751,19 +751,11 @@ static void s390_pcihost_realize(DeviceState *dev, Error **errp)
     pci_setup_iommu(b, s390_pci_dma_iommu, s);
 
     bus = BUS(b);
-    qbus_set_hotplug_handler(bus, OBJECT(dev), &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
-        return;
-    }
+    qbus_set_hotplug_handler(bus, OBJECT(dev));
     phb->bus = b;
 
     s->bus = S390_PCI_BUS(qbus_create(TYPE_S390_PCI_BUS, dev, NULL));
-    qbus_set_hotplug_handler(BUS(s->bus), OBJECT(dev), &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
-        return;
-    }
+    qbus_set_hotplug_handler(BUS(s->bus), OBJECT(dev));
 
     s->iommu_table = g_hash_table_new_full(g_int64_hash, g_int64_equal,
                                            NULL, g_free);
@@ -921,7 +913,7 @@ static void s390_pcihost_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
         pci_bridge_map_irq(pb, dev->id, s390_pci_map_irq);
         pci_setup_iommu(&pb->sec_bus, s390_pci_dma_iommu, s);
 
-        qbus_set_hotplug_handler(BUS(&pb->sec_bus), OBJECT(s), errp);
+        qbus_set_hotplug_handler(BUS(&pb->sec_bus), OBJECT(s));
 
         if (dev->hotplugged) {
             pci_default_write_config(pdev, PCI_PRIMARY_BUS,
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 9b72094a61..b49775269e 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -934,7 +934,7 @@ static void virtio_scsi_device_realize(DeviceState *dev, Error **errp)
     scsi_bus_new(&s->bus, sizeof(s->bus), dev,
                  &virtio_scsi_scsi_info, vdev->bus_name);
     /* override default SCSI bus hotplug-handler, with virtio-scsi's one */
-    qbus_set_hotplug_handler(BUS(&s->bus), OBJECT(dev), &error_abort);
+    qbus_set_hotplug_handler(BUS(&s->bus), OBJECT(dev));
 
     virtio_scsi_dataplane_setup(s, errp);
 }
@@ -958,7 +958,7 @@ static void virtio_scsi_device_unrealize(DeviceState *dev)
 {
     VirtIOSCSI *s = VIRTIO_SCSI(dev);
 
-    qbus_set_hotplug_handler(BUS(&s->bus), NULL, &error_abort);
+    qbus_set_hotplug_handler(BUS(&s->bus), NULL);
     virtio_scsi_common_unrealize(dev);
 }
 
diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
index ec5bf9ea34..df07ab6bfb 100644
--- a/hw/scsi/vmw_pvscsi.c
+++ b/hw/scsi/vmw_pvscsi.c
@@ -1147,7 +1147,7 @@ pvscsi_realizefn(PCIDevice *pci_dev, Error **errp)
     scsi_bus_new(&s->bus, sizeof(s->bus), DEVICE(pci_dev),
                  &pvscsi_scsi_info, NULL);
     /* override default SCSI bus hotplug-handler, with pvscsi's one */
-    qbus_set_hotplug_handler(BUS(&s->bus), OBJECT(s), &error_abort);
+    qbus_set_hotplug_handler(BUS(&s->bus), OBJECT(s));
     pvscsi_reset_state(s);
 }
 
diff --git a/hw/usb/dev-smartcard-reader.c b/hw/usb/dev-smartcard-reader.c
index ada18c1983..fcfe216594 100644
--- a/hw/usb/dev-smartcard-reader.c
+++ b/hw/usb/dev-smartcard-reader.c
@@ -1320,7 +1320,7 @@ static void ccid_realize(USBDevice *dev, Error **errp)
     usb_desc_init(dev);
     qbus_create_inplace(&s->bus, sizeof(s->bus), TYPE_CCID_BUS, DEVICE(dev),
                         NULL);
-    qbus_set_hotplug_handler(BUS(&s->bus), OBJECT(dev), &error_abort);
+    qbus_set_hotplug_handler(BUS(&s->bus), OBJECT(dev));
     s->intr = usb_ep_get(dev, USB_TOKEN_IN, CCID_INT_IN_EP);
     s->bulk = usb_ep_get(dev, USB_TOKEN_IN, CCID_BULK_IN_EP);
     s->card = NULL;
-- 
2.26.2

[PULL 17/28] hw: Fix error API violation around object_property_set_link()

[Markus Armbruster]

The Error ** argument must be NULL, &error_abort, &error_fatal, or a
pointer to a variable containing NULL.  Passing an argument of the
latter kind twice without clearing it in between is wrong: if the
first call sets an error, it no longer points to NULL for the second
call.

virtio_gpu_pci_base_realize(), virtio_vga_base_realize(),
sparc32_ledma_device_realize(), sparc32_dma_realize(),
sparc32_dma_realize() xilinx_axidma_realize(), mips_cps_realize(),
macio_realize_ide(), xilinx_enet_realize(), and
virtio_iommu_pci_realize() are wrong that way: they reuse the argument
they pass to object_property_set_link() for another call.

Harmless, because object_property_set_link() can't actually fail for
them: it fails when the property doesn't exist, is not settable, or
its .check() method fails.  Fix by passing &error_abort instead.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Cc: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
Cc: Alistair Francis <alistair@alistair23.me>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-arm@nongnu.org
Cc: Aleksandar Markovic <aleksandar.qemu.devel@gmail.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>
Cc: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-Id: <20200630090351.1247703-16-armbru@redhat.com>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/display/virtio-gpu-pci.c  |  2 +-
 hw/display/virtio-vga.c      |  2 +-
 hw/dma/sparc32_dma.c         |  6 +++---
 hw/dma/xilinx_axidma.c       | 12 ++----------
 hw/mips/cps.c                |  6 ++++--
 hw/misc/macio/macio.c        |  3 ++-
 hw/net/xilinx_axienet.c      | 12 ++----------
 hw/virtio/virtio-iommu-pci.c |  2 +-
 8 files changed, 16 insertions(+), 29 deletions(-)

diff --git a/hw/display/virtio-gpu-pci.c b/hw/display/virtio-gpu-pci.c
index b532fe8b5f..41b88b878d 100644
--- a/hw/display/virtio-gpu-pci.c
+++ b/hw/display/virtio-gpu-pci.c
@@ -44,7 +44,7 @@ static void virtio_gpu_pci_base_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
     for (i = 0; i < g->conf.max_outputs; i++) {
         object_property_set_link(OBJECT(g->scanout[i].con),
                                  OBJECT(vpci_dev),
-                                 "device", errp);
+                                 "device", &error_abort);
     }
 }
 
diff --git a/hw/display/virtio-vga.c b/hw/display/virtio-vga.c
index 68a062ece6..67f409e106 100644
--- a/hw/display/virtio-vga.c
+++ b/hw/display/virtio-vga.c
@@ -154,7 +154,7 @@ static void virtio_vga_base_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
     for (i = 0; i < g->conf.max_outputs; i++) {
         object_property_set_link(OBJECT(g->scanout[i].con),
                                  OBJECT(vpci_dev),
-                                 "device", errp);
+                                 "device", &error_abort);
     }
 }
 
diff --git a/hw/dma/sparc32_dma.c b/hw/dma/sparc32_dma.c
index f02aca6f40..2d7dbbb92d 100644
--- a/hw/dma/sparc32_dma.c
+++ b/hw/dma/sparc32_dma.c
@@ -346,7 +346,7 @@ static void sparc32_ledma_device_realize(DeviceState *dev, Error **errp)
     d = qdev_new(TYPE_LANCE);
     object_property_add_child(OBJECT(dev), "lance", OBJECT(d));
     qdev_set_nic_properties(d, nd);
-    object_property_set_link(OBJECT(d), OBJECT(dev), "dma", errp);
+    object_property_set_link(OBJECT(d), OBJECT(dev), "dma", &error_abort);
     sysbus_realize_and_unref(SYS_BUS_DEVICE(d), &error_fatal);
 }
 
@@ -379,7 +379,7 @@ static void sparc32_dma_realize(DeviceState *dev, Error **errp)
     }
 
     espdma = qdev_new(TYPE_SPARC32_ESPDMA_DEVICE);
-    object_property_set_link(OBJECT(espdma), iommu, "iommu", errp);
+    object_property_set_link(OBJECT(espdma), iommu, "iommu", &error_abort);
     object_property_add_child(OBJECT(s), "espdma", OBJECT(espdma));
     sysbus_realize_and_unref(SYS_BUS_DEVICE(espdma), &error_fatal);
 
@@ -394,7 +394,7 @@ static void sparc32_dma_realize(DeviceState *dev, Error **errp)
                                 sysbus_mmio_get_region(sbd, 0));
 
     ledma = qdev_new(TYPE_SPARC32_LEDMA_DEVICE);
-    object_property_set_link(OBJECT(ledma), iommu, "iommu", errp);
+    object_property_set_link(OBJECT(ledma), iommu, "iommu", &error_abort);
     object_property_add_child(OBJECT(s), "ledma", OBJECT(ledma));
     sysbus_realize_and_unref(SYS_BUS_DEVICE(ledma), &error_fatal);
 
diff --git a/hw/dma/xilinx_axidma.c b/hw/dma/xilinx_axidma.c
index 6a9df2c4db..a069637bf2 100644
--- a/hw/dma/xilinx_axidma.c
+++ b/hw/dma/xilinx_axidma.c
@@ -537,7 +537,6 @@ static void xilinx_axidma_realize(DeviceState *dev, Error **errp)
     XilinxAXIDMAStreamSlave *ds = XILINX_AXI_DMA_DATA_STREAM(&s->rx_data_dev);
     XilinxAXIDMAStreamSlave *cs = XILINX_AXI_DMA_CONTROL_STREAM(
                                                             &s->rx_control_dev);
-    Error *local_err = NULL;
     int i;
 
     object_property_add_link(OBJECT(ds), "dma", TYPE_XILINX_AXI_DMA,
@@ -548,11 +547,8 @@ static void xilinx_axidma_realize(DeviceState *dev, Error **errp)
                              (Object **)&cs->dma,
                              object_property_allow_set_link,
                              OBJ_PROP_LINK_STRONG);
-    object_property_set_link(OBJECT(ds), OBJECT(s), "dma", &local_err);
-    object_property_set_link(OBJECT(cs), OBJECT(s), "dma", &local_err);
-    if (local_err) {
-        goto xilinx_axidma_realize_fail;
-    }
+    object_property_set_link(OBJECT(ds), OBJECT(s), "dma", &error_abort);
+    object_property_set_link(OBJECT(cs), OBJECT(s), "dma", &error_abort);
 
     for (i = 0; i < 2; i++) {
         struct Stream *st = &s->streams[i];
@@ -567,10 +563,6 @@ static void xilinx_axidma_realize(DeviceState *dev, Error **errp)
 
     address_space_init(&s->as,
                        s->dma_mr ? s->dma_mr : get_system_memory(), "dma");
-    return;
-
-xilinx_axidma_realize_fail:
-    error_propagate(errp, local_err);
 }
 
 static void xilinx_axidma_init(Object *obj)
diff --git a/hw/mips/cps.c b/hw/mips/cps.c
index cdfab19826..5382bc86f7 100644
--- a/hw/mips/cps.c
+++ b/hw/mips/cps.c
@@ -150,8 +150,10 @@ static void mips_cps_realize(DeviceState *dev, Error **errp)
     object_property_set_int(OBJECT(&s->gcr), s->num_vp, "num-vp", &err);
     object_property_set_int(OBJECT(&s->gcr), 0x800, "gcr-rev", &err);
     object_property_set_int(OBJECT(&s->gcr), gcr_base, "gcr-base", &err);
-    object_property_set_link(OBJECT(&s->gcr), OBJECT(&s->gic.mr), "gic", &err);
-    object_property_set_link(OBJECT(&s->gcr), OBJECT(&s->cpc.mr), "cpc", &err);
+    object_property_set_link(OBJECT(&s->gcr), OBJECT(&s->gic.mr), "gic",
+                             &error_abort);
+    object_property_set_link(OBJECT(&s->gcr), OBJECT(&s->cpc.mr), "cpc",
+                             &error_abort);
     sysbus_realize(SYS_BUS_DEVICE(&s->gcr), &err);
     if (err != NULL) {
         error_propagate(errp, err);
diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
index 8ba7af073c..3251c79f46 100644
--- a/hw/misc/macio/macio.c
+++ b/hw/misc/macio/macio.c
@@ -136,7 +136,8 @@ static void macio_realize_ide(MacIOState *s, MACIOIDEState *ide,
     sysbus_connect_irq(sysbus_dev, 0, irq0);
     sysbus_connect_irq(sysbus_dev, 1, irq1);
     qdev_prop_set_uint32(DEVICE(ide), "channel", dmaid);
-    object_property_set_link(OBJECT(ide), OBJECT(&s->dbdma), "dbdma", errp);
+    object_property_set_link(OBJECT(ide), OBJECT(&s->dbdma), "dbdma",
+                             &error_abort);
     macio_ide_register_dma(ide);
 
     qdev_realize(DEVICE(ide), BUS(&s->macio_bus), errp);
diff --git a/hw/net/xilinx_axienet.c b/hw/net/xilinx_axienet.c
index c2f40b8ea9..679a359f9a 100644
--- a/hw/net/xilinx_axienet.c
+++ b/hw/net/xilinx_axienet.c
@@ -980,7 +980,6 @@ static void xilinx_enet_realize(DeviceState *dev, Error **errp)
     XilinxAXIEnetStreamSlave *ds = XILINX_AXI_ENET_DATA_STREAM(&s->rx_data_dev);
     XilinxAXIEnetStreamSlave *cs = XILINX_AXI_ENET_CONTROL_STREAM(
                                                             &s->rx_control_dev);
-    Error *local_err = NULL;
 
     object_property_add_link(OBJECT(ds), "enet", "xlnx.axi-ethernet",
                              (Object **) &ds->enet,
@@ -990,11 +989,8 @@ static void xilinx_enet_realize(DeviceState *dev, Error **errp)
                              (Object **) &cs->enet,
                              object_property_allow_set_link,
                              OBJ_PROP_LINK_STRONG);
-    object_property_set_link(OBJECT(ds), OBJECT(s), "enet", &local_err);
-    object_property_set_link(OBJECT(cs), OBJECT(s), "enet", &local_err);
-    if (local_err) {
-        goto xilinx_enet_realize_fail;
-    }
+    object_property_set_link(OBJECT(ds), OBJECT(s), "enet", &error_abort);
+    object_property_set_link(OBJECT(cs), OBJECT(s), "enet", &error_abort);
 
     qemu_macaddr_default_if_unset(&s->conf.macaddr);
     s->nic = qemu_new_nic(&net_xilinx_enet_info, &s->conf,
@@ -1008,10 +1004,6 @@ static void xilinx_enet_realize(DeviceState *dev, Error **errp)
 
     s->rxmem = g_malloc(s->c_rxmem);
     s->txmem = g_malloc(s->c_txmem);
-    return;
-
-xilinx_enet_realize_fail:
-    error_propagate(errp, local_err);
 }
 
 static void xilinx_enet_init(Object *obj)
diff --git a/hw/virtio/virtio-iommu-pci.c b/hw/virtio/virtio-iommu-pci.c
index 32e3215d1d..4588361d6b 100644
--- a/hw/virtio/virtio-iommu-pci.c
+++ b/hw/virtio/virtio-iommu-pci.c
@@ -56,7 +56,7 @@ static void virtio_iommu_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
     }
     object_property_set_link(OBJECT(dev),
                              OBJECT(pci_get_bus(&vpci_dev->pci_dev)),
-                             "primary-bus", errp);
+                             "primary-bus", &error_abort);
     qdev_realize(vdev, BUS(&vpci_dev->bus), errp);
 }
 
-- 
2.26.2

[PULL 18/28] hw/arm: Drop useless object_property_set_link() error handling

[Markus Armbruster]

object_property_set_link() fails when the property doesn't exist, is
not settable, or its .check() method fails.  These are all programming
errors here, so passing it &error_abort is appropriate.

Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: "Cédric Le Goater" <clg@kaod.org>
Cc: Andrew Jeffery <andrew@aj.id.au>
Cc: Joel Stanley <joel@jms.id.au>
Cc: qemu-arm@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Message-Id: <20200630090351.1247703-17-armbru@redhat.com>
---
 hw/arm/armsse.c         | 53 ++++++++++-------------------------------
 hw/arm/armv7m.c         |  7 ++----
 hw/arm/aspeed_ast2600.c | 20 ++++------------
 hw/arm/aspeed_soc.c     | 14 ++++-------
 hw/arm/nrf51_soc.c      |  6 +----
 5 files changed, 24 insertions(+), 76 deletions(-)

diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c
index 9ddde339ec..a851652b39 100644
--- a/hw/arm/armsse.c
+++ b/hw/arm/armsse.c
@@ -563,16 +563,8 @@ static void armsse_realize(DeviceState *dev, Error **errp)
                                                 &s->container, -1);
         }
         object_property_set_link(cpuobj, OBJECT(&s->cpu_container[i]),
-                                 "memory", &err);
-        if (err) {
-            error_propagate(errp, err);
-            return;
-        }
-        object_property_set_link(cpuobj, OBJECT(s), "idau", &err);
-        if (err) {
-            error_propagate(errp, err);
-            return;
-        }
+                                 "memory", &error_abort);
+        object_property_set_link(cpuobj, OBJECT(s), "idau", &error_abort);
         sysbus_realize(SYS_BUS_DEVICE(cpuobj), &err);
         if (err) {
             error_propagate(errp, err);
@@ -699,11 +691,7 @@ static void armsse_realize(DeviceState *dev, Error **errp)
             return;
         }
         object_property_set_link(OBJECT(&s->mpc[i]), OBJECT(&s->sram[i]),
-                                 "downstream", &err);
-        if (err) {
-            error_propagate(errp, err);
-            return;
-        }
+                                 "downstream", &error_abort);
         sysbus_realize(SYS_BUS_DEVICE(&s->mpc[i]), &err);
         if (err) {
             error_propagate(errp, err);
@@ -754,11 +742,8 @@ static void armsse_realize(DeviceState *dev, Error **errp)
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->timer0), 0,
                        armsse_get_common_irq_in(s, 3));
     mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->timer0), 0);
-    object_property_set_link(OBJECT(&s->apb_ppc0), OBJECT(mr), "port[0]", &err);
-    if (err) {
-        error_propagate(errp, err);
-        return;
-    }
+    object_property_set_link(OBJECT(&s->apb_ppc0), OBJECT(mr), "port[0]",
+                             &error_abort);
 
     qdev_prop_set_uint32(DEVICE(&s->timer1), "pclk-frq", s->mainclk_frq);
     sysbus_realize(SYS_BUS_DEVICE(&s->timer1), &err);
@@ -769,12 +754,8 @@ static void armsse_realize(DeviceState *dev, Error **errp)
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->timer1), 0,
                        armsse_get_common_irq_in(s, 4));
     mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->timer1), 0);
-    object_property_set_link(OBJECT(&s->apb_ppc0), OBJECT(mr), "port[1]", &err);
-    if (err) {
-        error_propagate(errp, err);
-        return;
-    }
-
+    object_property_set_link(OBJECT(&s->apb_ppc0), OBJECT(mr), "port[1]",
+                             &error_abort);
 
     qdev_prop_set_uint32(DEVICE(&s->dualtimer), "pclk-frq", s->mainclk_frq);
     sysbus_realize(SYS_BUS_DEVICE(&s->dualtimer), &err);
@@ -785,11 +766,8 @@ static void armsse_realize(DeviceState *dev, Error **errp)
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->dualtimer), 0,
                        armsse_get_common_irq_in(s, 5));
     mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->dualtimer), 0);
-    object_property_set_link(OBJECT(&s->apb_ppc0), OBJECT(mr), "port[2]", &err);
-    if (err) {
-        error_propagate(errp, err);
-        return;
-    }
+    object_property_set_link(OBJECT(&s->apb_ppc0), OBJECT(mr), "port[2]",
+                             &error_abort);
 
     if (info->has_mhus) {
         /*
@@ -814,12 +792,8 @@ static void armsse_realize(DeviceState *dev, Error **errp)
             port = g_strdup_printf("port[%d]", i + 3);
             mr = sysbus_mmio_get_region(mhu_sbd, 0);
             object_property_set_link(OBJECT(&s->apb_ppc0), OBJECT(mr),
-                                     port, &err);
+                                     port, &error_abort);
             g_free(port);
-            if (err) {
-                error_propagate(errp, err);
-                return;
-            }
 
             /*
              * Each MHU has an irq line for each CPU:
@@ -966,11 +940,8 @@ static void armsse_realize(DeviceState *dev, Error **errp)
     sysbus_connect_irq(SYS_BUS_DEVICE(&s->s32ktimer), 0,
                        armsse_get_common_irq_in(s, 2));
     mr = sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->s32ktimer), 0);
-    object_property_set_link(OBJECT(&s->apb_ppc1), OBJECT(mr), "port[0]", &err);
-    if (err) {
-        error_propagate(errp, err);
-        return;
-    }
+    object_property_set_link(OBJECT(&s->apb_ppc1), OBJECT(mr), "port[0]",
+                             &error_abort);
 
     sysbus_realize(SYS_BUS_DEVICE(&s->apb_ppc1), &err);
     if (err) {
diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index ce83586e03..3308211e9c 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -170,11 +170,8 @@ static void armv7m_realize(DeviceState *dev, Error **errp)
     object_property_set_link(OBJECT(s->cpu), OBJECT(&s->container), "memory",
                              &error_abort);
     if (object_property_find(OBJECT(s->cpu), "idau", NULL)) {
-        object_property_set_link(OBJECT(s->cpu), s->idau, "idau", &err);
-        if (err != NULL) {
-            error_propagate(errp, err);
-            return;
-        }
+        object_property_set_link(OBJECT(s->cpu), s->idau, "idau",
+                                 &error_abort);
     }
     if (object_property_find(OBJECT(s->cpu), "init-svtor", NULL)) {
         object_property_set_uint(OBJECT(s->cpu), s->init_svtor,
diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index 08b3592e36..4efac02e2b 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -341,11 +341,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
     }
 
     /* I2C */
-    object_property_set_link(OBJECT(&s->i2c), OBJECT(s->dram_mr), "dram", &err);
-    if (err) {
-        error_propagate(errp, err);
-        return;
-    }
+    object_property_set_link(OBJECT(&s->i2c), OBJECT(s->dram_mr), "dram",
+                             &error_abort);
     sysbus_realize(SYS_BUS_DEVICE(&s->i2c), &err);
     if (err) {
         error_propagate(errp, err);
@@ -363,11 +360,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
     }
 
     /* FMC, The number of CS is set at the board level */
-    object_property_set_link(OBJECT(&s->fmc), OBJECT(s->dram_mr), "dram", &err);
-    if (err) {
-        error_propagate(errp, err);
-        return;
-    }
+    object_property_set_link(OBJECT(&s->fmc), OBJECT(s->dram_mr), "dram",
+                             &error_abort);
     object_property_set_int(OBJECT(&s->fmc), sc->memmap[ASPEED_SDRAM],
                             "sdram-base", &err);
     if (err) {
@@ -388,11 +382,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
     /* SPI */
     for (i = 0; i < sc->spis_num; i++) {
         object_property_set_link(OBJECT(&s->spi[i]), OBJECT(s->dram_mr),
-                                 "dram", &err);
-        if (err) {
-            error_propagate(errp, err);
-            return;
-        }
+                                 "dram", &error_abort);
         object_property_set_int(OBJECT(&s->spi[i]), 1, "num-cs", &err);
         sysbus_realize(SYS_BUS_DEVICE(&s->spi[i]), &err);
         if (err) {
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index ec21de50ce..03b91bade6 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -300,11 +300,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
     }
 
     /* I2C */
-    object_property_set_link(OBJECT(&s->i2c), OBJECT(s->dram_mr), "dram", &err);
-    if (err) {
-        error_propagate(errp, err);
-        return;
-    }
+    object_property_set_link(OBJECT(&s->i2c), OBJECT(s->dram_mr), "dram",
+                             &error_abort);
     sysbus_realize(SYS_BUS_DEVICE(&s->i2c), &err);
     if (err) {
         error_propagate(errp, err);
@@ -315,11 +312,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
                        aspeed_soc_get_irq(s, ASPEED_I2C));
 
     /* FMC, The number of CS is set at the board level */
-    object_property_set_link(OBJECT(&s->fmc), OBJECT(s->dram_mr), "dram", &err);
-    if (err) {
-        error_propagate(errp, err);
-        return;
-    }
+    object_property_set_link(OBJECT(&s->fmc), OBJECT(s->dram_mr), "dram",
+                             &error_abort);
     object_property_set_int(OBJECT(&s->fmc), sc->memmap[ASPEED_SDRAM],
                             "sdram-base", &err);
     if (err) {
diff --git a/hw/arm/nrf51_soc.c b/hw/arm/nrf51_soc.c
index 5a8961ddbb..20dd8b5897 100644
--- a/hw/arm/nrf51_soc.c
+++ b/hw/arm/nrf51_soc.c
@@ -66,11 +66,7 @@ static void nrf51_soc_realize(DeviceState *dev_soc, Error **errp)
     }
 
     object_property_set_link(OBJECT(&s->cpu), OBJECT(&s->container), "memory",
-            &err);
-    if (err) {
-        error_propagate(errp, err);
-        return;
-    }
+                             &error_abort);
     sysbus_realize(SYS_BUS_DEVICE(&s->cpu), &err);
     if (err) {
         error_propagate(errp, err);
-- 
2.26.2

[PULL 19/28] riscv/sifive_u: Fix sifive_u_soc_realize() error API violations

[Markus Armbruster]

The Error ** argument must be NULL, &error_abort, &error_fatal, or a
pointer to a variable containing NULL.  Passing an argument of the
latter kind twice without clearing it in between is wrong: if the
first call sets an error, it no longer points to NULL for the second
call.

sifive_u_soc_realize() is wrong that way: it passes &err to
sysbus_realize() four times before checking it.  Harmless, because the
first three can't actually fail (I think).

Fix by checking for failure right away.

Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Alistair Francis <Alistair.Francis@wdc.com>
Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Cc: Bin Meng <bmeng.cn@gmail.com>
Cc: qemu-riscv@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-Id: <20200630090351.1247703-18-armbru@redhat.com>
---
 hw/riscv/sifive_u.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 7d051e7c92..a1d2edfe13 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -677,11 +677,15 @@ static void sifive_u_soc_realize(DeviceState *dev, Error **errp)
         memmap[SIFIVE_U_CLINT].size, ms->smp.cpus,
         SIFIVE_SIP_BASE, SIFIVE_TIMECMP_BASE, SIFIVE_TIME_BASE, false);
 
-    sysbus_realize(SYS_BUS_DEVICE(&s->prci), &err);
+    if (!sysbus_realize(SYS_BUS_DEVICE(&s->prci), errp)) {
+        return;
+    }
     sysbus_mmio_map(SYS_BUS_DEVICE(&s->prci), 0, memmap[SIFIVE_U_PRCI].base);
 
     qdev_prop_set_uint32(DEVICE(&s->gpio), "ngpio", 16);
-    sysbus_realize(SYS_BUS_DEVICE(&s->gpio), &err);
+    if (!sysbus_realize(SYS_BUS_DEVICE(&s->gpio), errp)) {
+        return;
+    }
     sysbus_mmio_map(SYS_BUS_DEVICE(&s->gpio), 0, memmap[SIFIVE_U_GPIO].base);
 
     /* Pass all GPIOs to the SOC layer so they are available to the board */
@@ -695,7 +699,9 @@ static void sifive_u_soc_realize(DeviceState *dev, Error **errp)
     }
 
     qdev_prop_set_uint32(DEVICE(&s->otp), "serial", s->serial);
-    sysbus_realize(SYS_BUS_DEVICE(&s->otp), &err);
+    if (!sysbus_realize(SYS_BUS_DEVICE(&s->otp), errp)) {
+        return;
+    }
     sysbus_mmio_map(SYS_BUS_DEVICE(&s->otp), 0, memmap[SIFIVE_U_OTP].base);
 
     if (nd->used) {
-- 
2.26.2

[PULL 20/28] riscv_hart: Fix riscv_harts_realize() error API violations

[Markus Armbruster]

The Error ** argument must be NULL, &error_abort, &error_fatal, or a
pointer to a variable containing NULL.  Passing an argument of the
latter kind twice without clearing it in between is wrong: if the
first call sets an error, it no longer points to NULL for the second
call.

riscv_harts_realize() is wrong that way: it passes @errp to
riscv_hart_realize() in a loop.  I can't tell offhand whether this can
fail.

Fix by checking for failure in each iteration.

Cc: Palmer Dabbelt <palmer@dabbelt.com>
Cc: Alistair Francis <Alistair.Francis@wdc.com>
Cc: Sagar Karandikar <sagark@eecs.berkeley.edu>
Cc: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Cc: Bin Meng <bmeng.cn@gmail.com>
Cc: qemu-riscv@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-Id: <20200630090351.1247703-19-armbru@redhat.com>
---
 hw/riscv/riscv_hart.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/hw/riscv/riscv_hart.c b/hw/riscv/riscv_hart.c
index e26c382259..f59fe52f0f 100644
--- a/hw/riscv/riscv_hart.c
+++ b/hw/riscv/riscv_hart.c
@@ -40,19 +40,13 @@ static void riscv_harts_cpu_reset(void *opaque)
     cpu_reset(CPU(cpu));
 }
 
-static void riscv_hart_realize(RISCVHartArrayState *s, int idx,
+static bool riscv_hart_realize(RISCVHartArrayState *s, int idx,
                                char *cpu_type, Error **errp)
 {
-    Error *err = NULL;
-
     object_initialize_child(OBJECT(s), "harts[*]", &s->harts[idx], cpu_type);
     s->harts[idx].env.mhartid = s->hartid_base + idx;
     qemu_register_reset(riscv_harts_cpu_reset, &s->harts[idx]);
-    qdev_realize(DEVICE(&s->harts[idx]), NULL, &err);
-    if (err) {
-        error_propagate(errp, err);
-        return;
-    }
+    return qdev_realize(DEVICE(&s->harts[idx]), NULL, errp);
 }
 
 static void riscv_harts_realize(DeviceState *dev, Error **errp)
@@ -63,7 +57,9 @@ static void riscv_harts_realize(DeviceState *dev, Error **errp)
     s->harts = g_new0(RISCVCPU, s->num_harts);
 
     for (n = 0; n < s->num_harts; n++) {
-        riscv_hart_realize(s, n, s->cpu_type, errp);
+        if (!riscv_hart_realize(s, n, s->cpu_type, errp)) {
+            return;
+        }
     }
 }
 
-- 
2.26.2

[PULL 21/28] mips/cps: Fix mips_cps_realize() error API violations

[Markus Armbruster]

The Error ** argument must be NULL, &error_abort, &error_fatal, or a
pointer to a variable containing NULL.  Passing an argument of the
latter kind twice without clearing it in between is wrong: if the
first call sets an error, it no longer points to NULL for the second
call.

mips_cps_realize() is wrong that way: it passes &err to multiple
object_property_set_FOO() without checking for failure, and then to
sysbus_realize().  Harmless, because the object_property_set_FOO()
can't actually fail here.

Fix by passing &error_abort instead.

Cc: Aleksandar Markovic <aleksandar.qemu.devel@gmail.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200630090351.1247703-20-armbru@redhat.com>
---
 hw/mips/cps.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/hw/mips/cps.c b/hw/mips/cps.c
index 5382bc86f7..0d7f3cf673 100644
--- a/hw/mips/cps.c
+++ b/hw/mips/cps.c
@@ -100,10 +100,12 @@ static void mips_cps_realize(DeviceState *dev, Error **errp)
     /* Inter-Thread Communication Unit */
     if (itu_present) {
         object_initialize_child(OBJECT(dev), "itu", &s->itu, TYPE_MIPS_ITU);
-        object_property_set_int(OBJECT(&s->itu), 16, "num-fifo", &err);
-        object_property_set_int(OBJECT(&s->itu), 16, "num-semaphores", &err);
+        object_property_set_int(OBJECT(&s->itu), 16, "num-fifo",
+                                &error_abort);
+        object_property_set_int(OBJECT(&s->itu), 16, "num-semaphores",
+                                &error_abort);
         object_property_set_bool(OBJECT(&s->itu), saar_present, "saar-present",
-                                 &err);
+                                 &error_abort);
         if (saar_present) {
             s->itu.saar = &env->CP0_SAAR;
         }
@@ -119,8 +121,10 @@ static void mips_cps_realize(DeviceState *dev, Error **errp)
 
     /* Cluster Power Controller */
     object_initialize_child(OBJECT(dev), "cpc", &s->cpc, TYPE_MIPS_CPC);
-    object_property_set_int(OBJECT(&s->cpc), s->num_vp, "num-vp", &err);
-    object_property_set_int(OBJECT(&s->cpc), 1, "vp-start-running", &err);
+    object_property_set_int(OBJECT(&s->cpc), s->num_vp, "num-vp",
+                            &error_abort);
+    object_property_set_int(OBJECT(&s->cpc), 1, "vp-start-running",
+                            &error_abort);
     sysbus_realize(SYS_BUS_DEVICE(&s->cpc), &err);
     if (err != NULL) {
         error_propagate(errp, err);
@@ -132,8 +136,10 @@ static void mips_cps_realize(DeviceState *dev, Error **errp)
 
     /* Global Interrupt Controller */
     object_initialize_child(OBJECT(dev), "gic", &s->gic, TYPE_MIPS_GIC);
-    object_property_set_int(OBJECT(&s->gic), s->num_vp, "num-vp", &err);
-    object_property_set_int(OBJECT(&s->gic), 128, "num-irq", &err);
+    object_property_set_int(OBJECT(&s->gic), s->num_vp, "num-vp",
+                            &error_abort);
+    object_property_set_int(OBJECT(&s->gic), 128, "num-irq",
+                            &error_abort);
     sysbus_realize(SYS_BUS_DEVICE(&s->gic), &err);
     if (err != NULL) {
         error_propagate(errp, err);
@@ -147,9 +153,12 @@ static void mips_cps_realize(DeviceState *dev, Error **errp)
     gcr_base = env->CP0_CMGCRBase << 4;
 
     object_initialize_child(OBJECT(dev), "gcr", &s->gcr, TYPE_MIPS_GCR);
-    object_property_set_int(OBJECT(&s->gcr), s->num_vp, "num-vp", &err);
-    object_property_set_int(OBJECT(&s->gcr), 0x800, "gcr-rev", &err);
-    object_property_set_int(OBJECT(&s->gcr), gcr_base, "gcr-base", &err);
+    object_property_set_int(OBJECT(&s->gcr), s->num_vp, "num-vp",
+                            &error_abort);
+    object_property_set_int(OBJECT(&s->gcr), 0x800, "gcr-rev",
+                            &error_abort);
+    object_property_set_int(OBJECT(&s->gcr), gcr_base, "gcr-base",
+                            &error_abort);
     object_property_set_link(OBJECT(&s->gcr), OBJECT(&s->gic.mr), "gic",
                              &error_abort);
     object_property_set_link(OBJECT(&s->gcr), OBJECT(&s->cpc.mr), "cpc",
-- 
2.26.2

[PULL 22/28] x86: Fix x86_cpu_new() error handling

[Markus Armbruster]

The Error ** argument must be NULL, &error_abort, &error_fatal, or a
pointer to a variable containing NULL.  Passing an argument of the
latter kind twice without clearing it in between is wrong: if the
first call sets an error, it no longer points to NULL for the second
call.

x86_cpu_new() is wrong that way: it passes &local_err to
object_property_set_uint() without checking it, and then to
qdev_realize().  If both fail, we'll trip error_setv()'s assertion.
To assess the bug's impact, we'd need to figure out how to make both
calls fail.  Too much work for ignorant me, sorry.

Fix by checking for failure right away.

Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200630090351.1247703-21-armbru@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/i386/x86.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/i386/x86.c b/hw/i386/x86.c
index 34229b45c7..93f7371a56 100644
--- a/hw/i386/x86.c
+++ b/hw/i386/x86.c
@@ -118,14 +118,16 @@ uint32_t x86_cpu_apic_id_from_index(X86MachineState *x86ms,
 
 void x86_cpu_new(X86MachineState *x86ms, int64_t apic_id, Error **errp)
 {
-    Object *cpu = NULL;
     Error *local_err = NULL;
-
-    cpu = object_new(MACHINE(x86ms)->cpu_type);
+    Object *cpu = object_new(MACHINE(x86ms)->cpu_type);
 
     object_property_set_uint(cpu, apic_id, "apic-id", &local_err);
+    if (local_err) {
+        goto out;
+    }
     qdev_realize(DEVICE(cpu), NULL, &local_err);
 
+out:
     object_unref(cpu);
     error_propagate(errp, local_err);
 }
-- 
2.26.2

[PULL 23/28] amd_iommu: Fix amdvi_realize() error API violation

[Markus Armbruster]

The Error ** argument must be NULL, &error_abort, &error_fatal, or a
pointer to a variable containing NULL.  Passing an argument of the
latter kind twice without clearing it in between is wrong: if the
first call sets an error, it no longer points to NULL for the second
call.

amdvi_realize() is wrong that way: it passes @errp to qdev_realize(),
object_property_get_int(), and msi_init() without checking it.  I
can't tell offhand whether qdev_realize() can fail here.  Fix by
checking it for failure.  object_property_get_int() can't.  Fix by
passing &error_abort instead.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200630090351.1247703-22-armbru@redhat.com>
---
 hw/i386/amd_iommu.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index b26d30da57..087f601666 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -1549,7 +1549,9 @@ static void amdvi_realize(DeviceState *dev, Error **errp)
 
     /* This device should take care of IOMMU PCI properties */
     x86_iommu->type = TYPE_AMD;
-    qdev_realize(DEVICE(&s->pci), &bus->qbus, errp);
+    if (!qdev_realize(DEVICE(&s->pci), &bus->qbus, errp)) {
+        return;
+    }
     ret = pci_add_capability(&s->pci.dev, AMDVI_CAPAB_ID_SEC, 0,
                                          AMDVI_CAPAB_SIZE, errp);
     if (ret < 0) {
@@ -1578,7 +1580,7 @@ static void amdvi_realize(DeviceState *dev, Error **errp)
     sysbus_init_mmio(SYS_BUS_DEVICE(s), &s->mmio);
     sysbus_mmio_map(SYS_BUS_DEVICE(s), 0, AMDVI_BASE_ADDR);
     pci_setup_iommu(bus, amdvi_host_dma_iommu, s);
-    s->devid = object_property_get_int(OBJECT(&s->pci), "addr", errp);
+    s->devid = object_property_get_int(OBJECT(&s->pci), "addr", &error_abort);
     msi_init(&s->pci.dev, 0, 1, true, false, errp);
     amdvi_init(s);
 }
-- 
2.26.2

[PULL 24/28] arm/stm32f205 arm/stm32f405: Fix realize error API violation

[Markus Armbruster]

The Error ** argument must be NULL, &error_abort, &error_fatal, or a
pointer to a variable containing NULL.  Passing an argument of the
latter kind twice without clearing it in between is wrong: if the
first call sets an error, it no longer points to NULL for the second
call.

stm32f205_soc_realize() and stm32f405_soc_realize() are wrong that
way: they pass &err to object_property_set_int() without checking it,
and then to qdev_realize().  Harmless, because the former can't
actually fail here.

Fix by passing &error_abort instead.

Cc: Alistair Francis <alistair@alistair23.me>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-arm@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-Id: <20200630090351.1247703-23-armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/arm/stm32f205_soc.c | 2 +-
 hw/arm/stm32f405_soc.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/arm/stm32f205_soc.c b/hw/arm/stm32f205_soc.c
index 19487544f0..56aef686c9 100644
--- a/hw/arm/stm32f205_soc.c
+++ b/hw/arm/stm32f205_soc.c
@@ -154,7 +154,7 @@ static void stm32f205_soc_realize(DeviceState *dev_soc, Error **errp)
 
     /* ADC 1 to 3 */
     object_property_set_int(OBJECT(s->adc_irqs), STM_NUM_ADCS,
-                            "num-lines", &err);
+                            "num-lines", &error_abort);
     qdev_realize(DEVICE(s->adc_irqs), NULL, &err);
     if (err != NULL) {
         error_propagate(errp, err);
diff --git a/hw/arm/stm32f405_soc.c b/hw/arm/stm32f405_soc.c
index c12d9f999d..cf9228d8e7 100644
--- a/hw/arm/stm32f405_soc.c
+++ b/hw/arm/stm32f405_soc.c
@@ -172,7 +172,7 @@ static void stm32f405_soc_realize(DeviceState *dev_soc, Error **errp)
         return;
     }
     object_property_set_int(OBJECT(&s->adc_irqs), STM_NUM_ADCS,
-                            "num-lines", &err);
+                            "num-lines", &error_abort);
     qdev_realize(DEVICE(&s->adc_irqs), NULL, &err);
     if (err != NULL) {
         error_propagate(errp, err);
-- 
2.26.2

[PULL 25/28] aspeed: Fix realize error API violation

[Markus Armbruster]

The Error ** argument must be NULL, &error_abort, &error_fatal, or a
pointer to a variable containing NULL.  Passing an argument of the
latter kind twice without clearing it in between is wrong: if the
first call sets an error, it no longer points to NULL for the second
call.

aspeed_soc_ast2600_realize() and aspeed_soc_realize() are wrong that
way: they pass &err to object_property_set_int() and
object_property_set_bool() without checking it, and then to
sysbus_realize().  Harmless, because the former can't actually fail
here.

Fix by passing &error_abort instead.

Cc: "Cédric Le Goater" <clg@kaod.org>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Andrew Jeffery <andrew@aj.id.au>
Cc: Joel Stanley <joel@jms.id.au>
Cc: qemu-arm@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200630090351.1247703-24-armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/arm/aspeed_ast2600.c | 5 +++--
 hw/arm/aspeed_soc.c     | 5 +++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/hw/arm/aspeed_ast2600.c b/hw/arm/aspeed_ast2600.c
index 4efac02e2b..59a7a1370b 100644
--- a/hw/arm/aspeed_ast2600.c
+++ b/hw/arm/aspeed_ast2600.c
@@ -383,7 +383,8 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
     for (i = 0; i < sc->spis_num; i++) {
         object_property_set_link(OBJECT(&s->spi[i]), OBJECT(s->dram_mr),
                                  "dram", &error_abort);
-        object_property_set_int(OBJECT(&s->spi[i]), 1, "num-cs", &err);
+        object_property_set_int(OBJECT(&s->spi[i]), 1, "num-cs",
+                                &error_abort);
         sysbus_realize(SYS_BUS_DEVICE(&s->spi[i]), &err);
         if (err) {
             error_propagate(errp, err);
@@ -434,7 +435,7 @@ static void aspeed_soc_ast2600_realize(DeviceState *dev, Error **errp)
     /* Net */
     for (i = 0; i < sc->macs_num; i++) {
         object_property_set_bool(OBJECT(&s->ftgmac100[i]), true, "aspeed",
-                                 &err);
+                                 &error_abort);
         sysbus_realize(SYS_BUS_DEVICE(&s->ftgmac100[i]), &err);
         if (err) {
             error_propagate(errp, err);
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index 03b91bade6..311458aa76 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -333,7 +333,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
 
     /* SPI */
     for (i = 0; i < sc->spis_num; i++) {
-        object_property_set_int(OBJECT(&s->spi[i]), 1, "num-cs", &err);
+        object_property_set_int(OBJECT(&s->spi[i]), 1, "num-cs",
+                                &error_abort);
         sysbus_realize(SYS_BUS_DEVICE(&s->spi[i]), &err);
         if (err) {
             error_propagate(errp, err);
@@ -384,7 +385,7 @@ static void aspeed_soc_realize(DeviceState *dev, Error **errp)
     /* Net */
     for (i = 0; i < sc->macs_num; i++) {
         object_property_set_bool(OBJECT(&s->ftgmac100[i]), true, "aspeed",
-                                 &err);
+                                 &error_abort);
         sysbus_realize(SYS_BUS_DEVICE(&s->ftgmac100[i]), &err);
         if (err) {
             error_propagate(errp, err);
-- 
2.26.2

[PULL 26/28] hw/arm/armsse: Fix armsse_realize() error API violation

[Markus Armbruster]

The Error ** argument must be NULL, &error_abort, &error_fatal, or a
pointer to a variable containing NULL.  Passing an argument of the
latter kind twice without clearing it in between is wrong: if the
first call sets an error, it no longer points to NULL for the second
call.

armsse_realize() is wrong that way: it passes &err to
object_property_set_int() multiple times without checking it, and then
to sysbus_realize().  Harmless, because the former can't actually fail
here.

Fix by passing &error_abort instead.

Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: qemu-arm@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200630090351.1247703-25-armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/arm/armsse.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/arm/armsse.c b/hw/arm/armsse.c
index a851652b39..2fbd970b4f 100644
--- a/hw/arm/armsse.c
+++ b/hw/arm/armsse.c
@@ -990,13 +990,13 @@ static void armsse_realize(DeviceState *dev, Error **errp)
     sysbus_mmio_map(SYS_BUS_DEVICE(&s->sysinfo), 0, 0x40020000);
     /* System control registers */
     object_property_set_int(OBJECT(&s->sysctl), info->sys_version,
-                            "SYS_VERSION", &err);
+                            "SYS_VERSION", &error_abort);
     object_property_set_int(OBJECT(&s->sysctl), info->cpuwait_rst,
-                            "CPUWAIT_RST", &err);
+                            "CPUWAIT_RST", &error_abort);
     object_property_set_int(OBJECT(&s->sysctl), s->init_svtor,
-                            "INITSVTOR0_RST", &err);
+                            "INITSVTOR0_RST", &error_abort);
     object_property_set_int(OBJECT(&s->sysctl), s->init_svtor,
-                            "INITSVTOR1_RST", &err);
+                            "INITSVTOR1_RST", &error_abort);
     sysbus_realize(SYS_BUS_DEVICE(&s->sysctl), &err);
     if (err) {
         error_propagate(errp, err);
-- 
2.26.2

[PULL 27/28] arm/{bcm2835, fsl-imx25, fsl-imx6}: Fix realize error API violations

[Markus Armbruster]

The Error ** argument must be NULL, &error_abort, &error_fatal, or a
pointer to a variable containing NULL.  Passing an argument of the
latter kind twice without clearing it in between is wrong: if the
first call sets an error, it no longer points to NULL for the second
call.

bcm2835_peripherals_realize(), fsl_imx25_realize() and
fsl_imx6_realize() are wrong that way: they pass &err to
object_property_set_uint() and object_property_set_bool() without
checking it, and then to sysbus_realize().  Harmless, because the
former can't actually fail here.

Fix by passing &error_abort instead.

Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Andrew Baumann <Andrew.Baumann@microsoft.com>
Cc: "Philippe Mathieu-Daudé" <philmd@redhat.com>
Cc: Jean-Christophe Dubois <jcd@tribudubois.net>
Cc: qemu-arm@nongnu.org
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200630090351.1247703-26-armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/arm/bcm2835_peripherals.c | 12 ++++--------
 hw/arm/fsl-imx25.c           | 12 +++++-------
 hw/arm/fsl-imx6.c            | 12 +++++-------
 3 files changed, 14 insertions(+), 22 deletions(-)

diff --git a/hw/arm/bcm2835_peripherals.c b/hw/arm/bcm2835_peripherals.c
index 1e975d7eec..7ffdf62067 100644
--- a/hw/arm/bcm2835_peripherals.c
+++ b/hw/arm/bcm2835_peripherals.c
@@ -283,16 +283,12 @@ static void bcm2835_peripherals_realize(DeviceState *dev, Error **errp)
      * For the exact details please refer to the Arasan documentation:
      *   SD3.0_Host_AHB_eMMC4.4_Usersguide_ver5.9_jan11_10.pdf
      */
-    object_property_set_uint(OBJECT(&s->sdhci), 3, "sd-spec-version", &err);
+    object_property_set_uint(OBJECT(&s->sdhci), 3, "sd-spec-version",
+                             &error_abort);
     object_property_set_uint(OBJECT(&s->sdhci), BCM2835_SDHC_CAPAREG, "capareg",
-                             &err);
+                             &error_abort);
     object_property_set_bool(OBJECT(&s->sdhci), true, "pending-insert-quirk",
-                             &err);
-    if (err) {
-        error_propagate(errp, err);
-        return;
-    }
-
+                             &error_abort);
     sysbus_realize(SYS_BUS_DEVICE(&s->sdhci), &err);
     if (err) {
         error_propagate(errp, err);
diff --git a/hw/arm/fsl-imx25.c b/hw/arm/fsl-imx25.c
index f32f9bce0f..7ab5c98fbe 100644
--- a/hw/arm/fsl-imx25.c
+++ b/hw/arm/fsl-imx25.c
@@ -260,15 +260,13 @@ static void fsl_imx25_realize(DeviceState *dev, Error **errp)
         };
 
         object_property_set_uint(OBJECT(&s->esdhc[i]), 2, "sd-spec-version",
-                                 &err);
+                                 &error_abort);
         object_property_set_uint(OBJECT(&s->esdhc[i]), IMX25_ESDHC_CAPABILITIES,
-                                 "capareg", &err);
+                                 "capareg",
+                                 &error_abort);
         object_property_set_uint(OBJECT(&s->esdhc[i]), SDHCI_VENDOR_IMX,
-                                 "vendor", &err);
-        if (err) {
-            error_propagate(errp, err);
-            return;
-        }
+                                 "vendor",
+                                 &error_abort);
         sysbus_realize(SYS_BUS_DEVICE(&s->esdhc[i]), &err);
         if (err) {
             error_propagate(errp, err);
diff --git a/hw/arm/fsl-imx6.c b/hw/arm/fsl-imx6.c
index d4bc4fae93..4ae3c3efc2 100644
--- a/hw/arm/fsl-imx6.c
+++ b/hw/arm/fsl-imx6.c
@@ -336,15 +336,13 @@ static void fsl_imx6_realize(DeviceState *dev, Error **errp)
 
         /* UHS-I SDIO3.0 SDR104 1.8V ADMA */
         object_property_set_uint(OBJECT(&s->esdhc[i]), 3, "sd-spec-version",
-                                 &err);
+                                 &error_abort);
         object_property_set_uint(OBJECT(&s->esdhc[i]), IMX6_ESDHC_CAPABILITIES,
-                                 "capareg", &err);
+                                 "capareg",
+                                 &error_abort);
         object_property_set_uint(OBJECT(&s->esdhc[i]), SDHCI_VENDOR_IMX,
-                                 "vendor", &err);
-        if (err) {
-            error_propagate(errp, err);
-            return;
-        }
+                                 "vendor",
+                                 &error_abort);
         sysbus_realize(SYS_BUS_DEVICE(&s->esdhc[i]), &err);
         if (err) {
             error_propagate(errp, err);
-- 
2.26.2

[PULL 28/28] migration/rdma: Plug memory leaks in qemu_rdma_registration_stop()

[Markus Armbruster]

qemu_rdma_registration_stop() uses the ERROR() macro to create, report
to stderr, and store an Error object.  The stored Error object is
never used, and its memory is leaked.

Even where ERROR() doesn't leak, it is ill-advised.  The whole point
of passing an Error to the caller is letting the caller handle the
error.  Error handling may report to stderr, to somewhere else, or not
at all.  Also reporting in the callee mixes up concerns that should be
kept separate.  Since I don't know what reporting to stderr is
supposed to accomplish, I'm not touching it.

Commit 2a1bc8bde7 "migration/rdma: rdma_accept_incoming_migration fix
error handling" plugged the same leak in
rdma_accept_incoming_migration().

Plug the memory leak the same way: keep the report part, delete the
store part.

The report part uses fprintf().  If it's truly an error, it should use
error_report() instead.  But I don't know, so I leave it alone, just
like commit 2a1bc8bde7 did.

Fixes: 2da776db4846eadcb808598a5d3484d149773c05
Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Cc: Juan Quintela <quintela@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20200630090351.1247703-27-armbru@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 migration/rdma.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/migration/rdma.c b/migration/rdma.c
index ec45d33ba3..3b18823268 100644
--- a/migration/rdma.c
+++ b/migration/rdma.c
@@ -3787,7 +3787,6 @@ static int qemu_rdma_registration_start(QEMUFile *f, void *opaque,
 static int qemu_rdma_registration_stop(QEMUFile *f, void *opaque,
                                        uint64_t flags, void *data)
 {
-    Error *local_err = NULL, **errp = &local_err;
     QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(opaque);
     RDMAContext *rdma;
     RDMAControlHeader head = { .len = 0, .repeat = 1 };
@@ -3832,7 +3831,7 @@ static int qemu_rdma_registration_stop(QEMUFile *f, void *opaque,
                     &reg_result_idx, rdma->pin_all ?
                     qemu_rdma_reg_whole_ram_blocks : NULL);
         if (ret < 0) {
-            ERROR(errp, "receiving remote info!");
+            fprintf(stderr, "receiving remote info!");
             return ret;
         }
 
@@ -3851,10 +3850,10 @@ static int qemu_rdma_registration_stop(QEMUFile *f, void *opaque,
          */
 
         if (local->nb_blocks != nb_dest_blocks) {
-            ERROR(errp, "ram blocks mismatch (Number of blocks %d vs %d) "
-                        "Your QEMU command line parameters are probably "
-                        "not identical on both the source and destination.",
-                        local->nb_blocks, nb_dest_blocks);
+            fprintf(stderr, "ram blocks mismatch (Number of blocks %d vs %d) "
+                    "Your QEMU command line parameters are probably "
+                    "not identical on both the source and destination.",
+                    local->nb_blocks, nb_dest_blocks);
             rdma->error_state = -EINVAL;
             return -EINVAL;
         }
@@ -3867,10 +3866,10 @@ static int qemu_rdma_registration_stop(QEMUFile *f, void *opaque,
 
             /* We require that the blocks are in the same order */
             if (rdma->dest_blocks[i].length != local->block[i].length) {
-                ERROR(errp, "Block %s/%d has a different length %" PRIu64
-                            "vs %" PRIu64, local->block[i].block_name, i,
-                            local->block[i].length,
-                            rdma->dest_blocks[i].length);
+                fprintf(stderr, "Block %s/%d has a different length %" PRIu64
+                        "vs %" PRIu64, local->block[i].block_name, i,
+                        local->block[i].length,
+                        rdma->dest_blocks[i].length);
                 rdma->error_state = -EINVAL;
                 return -EINVAL;
             }
-- 
2.26.2

Re: [PULL 00/28] Error reporting patches patches for 2020-07-02

[Peter Maydell]

On Thu, 2 Jul 2020 at 12:18, Markus Armbruster <armbru@redhat.com> wrote:
>
> The following changes since commit fc1bff958998910ec8d25db86cd2f53ff125f7ab:
>
>   hw/misc/pca9552: Add missing TypeInfo::class_size field (2020-06-29 21:16:10 +0100)
>
> are available in the Git repository at:
>
>   git://repo.or.cz/qemu/armbru.git tags/pull-error-2020-07-02
>
> for you to fetch changes up to 9cde9caa04beac25cef32a8a9d0bd26d6b91a41a:
>
>   migration/rdma: Plug memory leaks in qemu_rdma_registration_stop() (2020-07-02 11:54:48 +0200)
>
> ----------------------------------------------------------------
> Error reporting patches patches for 2020-07-02
>


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.1
for any user-visible changes.

-- PMM