[PULL 00/34] Error reporting patches for 2019-12-16

[PULL 00/34] Error reporting patches for 2019-12-16

[Markus Armbruster]

The following changes since commit cb88904a54903ef6ba21a68a61d9cd51e2166304:

  Merge remote-tracking branch 'remotes/amarkovic/tags/mips-queue-dec-16-2019' into staging (2019-12-16 14:07:56 +0000)

are available in the Git repository at:

  git://repo.or.cz/qemu/armbru.git tags/pull-error-2019-12-16

for you to fetch changes up to 0e7f83bab6559775cd71e418b12a49145e59faa7:

  nbd: assert that Error** is not NULL in nbd_iter_channel_error (2019-12-16 20:50:16 +0100)

----------------------------------------------------------------
Error reporting patches for 2019-12-16

----------------------------------------------------------------
Markus Armbruster (14):
      error: Fix -msg timestamp default
      crypto: Fix certificate file error handling crash bug
      crypto: Fix typo in QCryptoTLSSession's <example> comment
      io: Fix Error usage in a comment <example>
      tests: Clean up initialization of Error *err variables
      exec: Fix file_ram_alloc() error API violations
      hw/acpi: Fix legacy CPU plug error API violations
      hw/core: Fix fit_load_fdt() error API violations
      hw/ipmi: Fix realize() error API violations
      qga: Fix guest-get-fsinfo error API violations
      memory-device: Fix memory pre-plug error API violations
      error: Clean up unusual names of Error * variables
      hw/intc/s390: Simplify error handling in kvm_s390_flic_realize()
      tests-blockjob: Use error_free_or_abort()

Vladimir Sementsov-Ogievskiy (20):
      net/net: Clean up variable shadowing in net_client_init()
      error: make Error **errp const where it is appropriate
      hmp: drop Error pointer indirection in hmp_handle_error
      vnc: drop Error pointer indirection in vnc_client_io_error
      qdev-monitor: make Error **errp const where it is appropriate
      ppc: make Error **errp const where it is appropriate
      9pfs: make Error **errp const where it is appropriate
      hw/core/qdev: cleanup Error ** variables
      block/snapshot: rename Error ** parameter to more common errp
      hw/i386/amd_iommu: rename Error ** parameter to more common errp
      qga: rename Error ** parameter to more common errp
      monitor/qmp-cmds: rename Error ** parameter to more common errp
      hw/s390x: rename Error ** parameter to more common errp
      hw/sd: drop extra whitespace in sdhci_sysbus_realize() header
      hw/tpm: rename Error ** parameter to more common errp
      hw/usb: rename Error ** parameter to more common errp
      include/qom/object.h: rename Error ** parameter to more common errp
      backends/cryptodev: drop local_err from cryptodev_backend_complete()
      hw/vfio/ap: drop local_err from vfio_ap_realize
      nbd: assert that Error** is not NULL in nbd_iter_channel_error

 include/block/snapshot.h            |   2 +-
 include/crypto/tlssession.h         |   2 +-
 include/io/task.h                   |   2 +-
 include/monitor/hmp.h               |   2 +-
 include/qapi/error.h                |   6 +-
 include/qemu/error-report.h         |   3 +-
 include/qom/object.h                |   4 +-
 target/ppc/kvm_ppc.h                |   4 +-
 ui/vnc.h                            |   2 +-
 backends/cryptodev.c                |  11 +--
 block/nbd.c                         |   1 +
 block/snapshot.c                    |   4 +-
 crypto/tlscredsx509.c               |   2 +-
 dump/dump-hmp-cmds.c                |   4 +-
 exec.c                              |   6 +-
 hw/9pfs/9p-local.c                  |   2 +-
 hw/9pfs/9p-proxy.c                  |   2 +-
 hw/acpi/cpu_hotplug.c               |  10 +--
 hw/core/loader-fit.c                |  15 ++--
 hw/core/machine-hmp-cmds.c          |   6 +-
 hw/core/qdev.c                      |  28 +++----
 hw/i386/amd_iommu.c                 |  14 ++--
 hw/intc/s390_flic_kvm.c             |  16 ++--
 hw/ipmi/isa_ipmi_bt.c               |   7 +-
 hw/ipmi/isa_ipmi_kcs.c              |   7 +-
 hw/ipmi/pci_ipmi_bt.c               |   6 +-
 hw/ipmi/pci_ipmi_kcs.c              |   6 +-
 hw/mem/memory-device.c              |   6 +-
 hw/ppc/spapr.c                      |   2 +-
 hw/ppc/spapr_pci.c                  |  16 ++--
 hw/ppc/spapr_pci_nvlink2.c          |  10 +--
 hw/s390x/event-facility.c           |   2 +-
 hw/s390x/s390-stattrib.c            |   3 +-
 hw/sd/sdhci.c                       |   2 +-
 hw/tpm/tpm_emulator.c               |   8 +-
 hw/usb/dev-network.c                |   2 +-
 hw/vfio/ap.c                        |   9 +--
 monitor/hmp-cmds.c                  | 155 ++++++++++++++++++------------------
 monitor/qmp-cmds.c                  |   2 +-
 net/net.c                           |  17 ++--
 qdev-monitor.c                      |  16 ++--
 qga/commands-posix.c                |   8 +-
 qga/commands-win32.c                |   2 +-
 qga/commands.c                      |  12 +--
 qom/qom-hmp-cmds.c                  |   4 +-
 target/ppc/kvm.c                    |   2 +-
 tests/test-blockjob.c               |  15 ++--
 tests/test-qobject-output-visitor.c |   8 +-
 tests/test-string-output-visitor.c  |   4 +-
 ui/vnc.c                            |  20 ++---
 util/error.c                        |   6 +-
 util/qemu-error.c                   |   6 +-
 vl.c                                |   2 +-
 qemu-options.hx                     |  10 ++-
 54 files changed, 260 insertions(+), 263 deletions(-)

-- 
2.21.0

[PULL 01/34] error: Fix -msg timestamp default

[Markus Armbruster]

-msg parameter "timestamp" defaults to "off" if you don't specify msg,
and to "on" if you do.  Messed up right in commit 5e2ac51917 "add
timestamp to error_report()".  Mostly harmless, because "timestamp" is
the only parameter, so "if you do" is "-msg ''", which nobody does.

Change the default to "off" no matter what.

While there, rename enable_timestamp_msg to error_with_timestamp, and
polish documentation.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20191010081508.8978-1-armbru@redhat.com>
---
 include/qemu/error-report.h |  3 ++-
 util/qemu-error.c           |  6 ++++--
 vl.c                        |  2 +-
 qemu-options.hx             | 10 +++++++---
 4 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/include/qemu/error-report.h b/include/qemu/error-report.h
index 00d069b20f..87532d8596 100644
--- a/include/qemu/error-report.h
+++ b/include/qemu/error-report.h
@@ -73,6 +73,7 @@ void error_init(const char *argv0);
     })
 
 const char *error_get_progname(void);
-extern bool enable_timestamp_msg;
+
+extern bool error_with_timestamp;
 
 #endif
diff --git a/util/qemu-error.c b/util/qemu-error.c
index f373f3b3b0..dac7c7dc50 100644
--- a/util/qemu-error.c
+++ b/util/qemu-error.c
@@ -24,6 +24,9 @@ typedef enum {
     REPORT_TYPE_INFO,
 } report_type;
 
+/* Prepend timestamp to messages */
+bool error_with_timestamp;
+
 int error_printf(const char *fmt, ...)
 {
     va_list ap;
@@ -191,7 +194,6 @@ static void print_loc(void)
     }
 }
 
-bool enable_timestamp_msg;
 /*
  * Print a message to current monitor if we have one, else to stderr.
  * @report_type is the type of message: error, warning or informational.
@@ -204,7 +206,7 @@ static void vreport(report_type type, const char *fmt, va_list ap)
     GTimeVal tv;
     gchar *timestr;
 
-    if (enable_timestamp_msg && !cur_mon) {
+    if (error_with_timestamp && !cur_mon) {
         g_get_current_time(&tv);
         timestr = g_time_val_to_iso8601(&tv);
         error_printf("%s ", timestr);
diff --git a/vl.c b/vl.c
index c0904b365b..7ed92db055 100644
--- a/vl.c
+++ b/vl.c
@@ -1263,7 +1263,7 @@ static void realtime_init(void)
 
 static void configure_msg(QemuOpts *opts)
 {
-    enable_timestamp_msg = qemu_opt_get_bool(opts, "timestamp", true);
+    error_with_timestamp = qemu_opt_get_bool(opts, "timestamp", false);
 }
 
 
diff --git a/qemu-options.hx b/qemu-options.hx
index 3ff8c4e07a..c9bcd66e98 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -4246,13 +4246,17 @@ DEF("no-kvm", 0, QEMU_OPTION_no_kvm, "", QEMU_ARCH_I386)
 
 DEF("msg", HAS_ARG, QEMU_OPTION_msg,
     "-msg timestamp[=on|off]\n"
-    "                change the format of messages\n"
-    "                on|off controls leading timestamps (default:on)\n",
+    "                control error message format\n"
+    "                timestamp=on enables timestamps (default: off)\n",
     QEMU_ARCH_ALL)
 STEXI
 @item -msg timestamp[=on|off]
 @findex -msg
-prepend a timestamp to each log message.(default:on)
+Control error message format.
+@table @option
+@item timestamp=on|off
+Prefix messages with a timestamp.  Default is off.
+@end table
 ETEXI
 
 DEF("dump-vmstate", HAS_ARG, QEMU_OPTION_dump_vmstate,
-- 
2.21.0

[PULL 02/34] crypto: Fix certificate file error handling crash bug

[Markus Armbruster]

qcrypto_tls_creds_load_cert() passes uninitialized GError *gerr by
reference to g_file_get_contents().  When g_file_get_contents() fails,
it'll try to set a GError.  Unless @gerr is null by dumb luck, this
logs a ERROR_OVERWRITTEN_WARNING warning message and leaves @gerr
unchanged.  qcrypto_tls_creds_load_cert() then dereferences the
uninitialized @gerr.

Fix by initializing @gerr properly.

Fixes: 9a2fd4347c40321f5cbb4ab4220e759fcbf87d03
Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20191204093625.14836-2-armbru@redhat.com>
Acked-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 crypto/tlscredsx509.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c
index 01fc304e5d..53a4368f49 100644
--- a/crypto/tlscredsx509.c
+++ b/crypto/tlscredsx509.c
@@ -380,7 +380,7 @@ qcrypto_tls_creds_load_cert(QCryptoTLSCredsX509 *creds,
     gnutls_x509_crt_t cert = NULL;
     g_autofree char *buf = NULL;
     gsize buflen;
-    GError *gerr;
+    GError *gerr = NULL;
     int ret = -1;
     int err;
 
-- 
2.21.0

[PULL 03/34] crypto: Fix typo in QCryptoTLSSession's <example> comment

[Markus Armbruster]

Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20191204093625.14836-3-armbru@redhat.com>
Acked-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 include/crypto/tlssession.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/crypto/tlssession.h b/include/crypto/tlssession.h
index e01e1a9dc2..15b9cef086 100644
--- a/include/crypto/tlssession.h
+++ b/include/crypto/tlssession.h
@@ -56,7 +56,7 @@
  *
  * static int mysock_run_tls(int sockfd,
  *                           QCryptoTLSCreds *creds,
- *                           Error *errp)
+ *                           Error **errp)
  * {
  *    QCryptoTLSSession *sess;
  *
-- 
2.21.0

[PULL 04/34] io: Fix Error usage in a comment <example>

[Markus Armbruster]

Cc: "Daniel P. Berrangé" <berrange@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20191204093625.14836-4-armbru@redhat.com>
Acked-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 include/io/task.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/io/task.h b/include/io/task.h
index 5cb9faf9f2..1abbfb8b65 100644
--- a/include/io/task.h
+++ b/include/io/task.h
@@ -119,7 +119,7 @@ typedef void (*QIOTaskWorker)(QIOTask *task,
  *   gboolean myobject_operation_timer(gpointer opaque)
  *   {
  *      QIOTask *task = QIO_TASK(opaque);
- *      Error *err;*
+ *      Error *err = NULL;
  *
  *      ...check something important...
  *       if (err) {
-- 
2.21.0

[PULL 05/34] tests: Clean up initialization of Error *err variables

[Markus Armbruster]

Declaring a local Error *err without initializer looks suspicious.
Fuse the declaration with the initialization to avoid that.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20191204093625.14836-5-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/test-qobject-output-visitor.c | 8 ++++----
 tests/test-string-output-visitor.c  | 4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/tests/test-qobject-output-visitor.c b/tests/test-qobject-output-visitor.c
index 3e993e5ba8..d7761ebf84 100644
--- a/tests/test-qobject-output-visitor.c
+++ b/tests/test-qobject-output-visitor.c
@@ -145,10 +145,10 @@ static void test_visitor_out_enum_errors(TestOutputVisitorData *data,
                                          const void *unused)
 {
     EnumOne i, bad_values[] = { ENUM_ONE__MAX, -1 };
-    Error *err;
 
     for (i = 0; i < ARRAY_SIZE(bad_values) ; i++) {
-        err = NULL;
+        Error *err = NULL;
+
         visit_type_EnumOne(data->ov, "unused", &bad_values[i], &err);
         error_free_or_abort(&err);
         visitor_reset(data);
@@ -240,11 +240,11 @@ static void test_visitor_out_struct_errors(TestOutputVisitorData *data,
     EnumOne bad_values[] = { ENUM_ONE__MAX, -1 };
     UserDefOne u = {0};
     UserDefOne *pu = &u;
-    Error *err;
     int i;
 
     for (i = 0; i < ARRAY_SIZE(bad_values) ; i++) {
-        err = NULL;
+        Error *err = NULL;
+
         u.has_enum1 = true;
         u.enum1 = bad_values[i];
         visit_type_UserDefOne(data->ov, "unused", &pu, &err);
diff --git a/tests/test-string-output-visitor.c b/tests/test-string-output-visitor.c
index 02766c0f65..1be1540767 100644
--- a/tests/test-string-output-visitor.c
+++ b/tests/test-string-output-visitor.c
@@ -207,10 +207,10 @@ static void test_visitor_out_enum_errors(TestOutputVisitorData *data,
                                          const void *unused)
 {
     EnumOne i, bad_values[] = { ENUM_ONE__MAX, -1 };
-    Error *err;
 
     for (i = 0; i < ARRAY_SIZE(bad_values) ; i++) {
-        err = NULL;
+        Error *err = NULL;
+
         visit_type_EnumOne(data->ov, "unused", &bad_values[i], &err);
         error_free_or_abort(&err);
     }
-- 
2.21.0

[PULL 06/34] exec: Fix file_ram_alloc() error API violations

[Markus Armbruster]

When os_mem_prealloc() fails, file_ram_alloc() calls qemu_ram_munmap()
and returns null.  Except it doesn't when its @errp argument is null,
because it checks for failure with (errp && *errp).  Introduced in
commit 056b68af77 "fix qemu exit on memory hotplug when allocation
fails at prealloc time".

No caller actually passes null.

Fix anyway: splice in a local Error *err, and error_propagate().

Cc: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Message-Id: <20191204093625.14836-6-armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 exec.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/exec.c b/exec.c
index a34c348184..6bd4aa1d87 100644
--- a/exec.c
+++ b/exec.c
@@ -1843,6 +1843,7 @@ static void *file_ram_alloc(RAMBlock *block,
                             bool truncate,
                             Error **errp)
 {
+    Error *err = NULL;
     MachineState *ms = MACHINE(qdev_get_machine());
     void *area;
 
@@ -1900,8 +1901,9 @@ static void *file_ram_alloc(RAMBlock *block,
     }
 
     if (mem_prealloc) {
-        os_mem_prealloc(fd, area, memory, ms->smp.cpus, errp);
-        if (errp && *errp) {
+        os_mem_prealloc(fd, area, memory, ms->smp.cpus, &err);
+        if (err) {
+            error_propagate(errp, err);
             qemu_ram_munmap(fd, area, memory);
             return NULL;
         }
-- 
2.21.0

[PULL 07/34] hw/acpi: Fix legacy CPU plug error API violations

[Markus Armbruster]

legacy_acpi_cpu_plug_cb() dereferences @errp when
acpi_set_cpu_present_bit() fails.  That's wrong; see the big comment
in error.h.  Introduced in commit cc43364de7 "acpi/cpu-hotplug:
introduce helper function to keep bit setting in one place".

No caller actually passes null, and acpi_set_cpu_present_bit() can't
actually fail.

Fix anyway: drop acpi_set_cpu_present_bit()'s @errp parameter.

Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
Message-Id: <20191204093625.14836-7-armbru@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
---
 hw/acpi/cpu_hotplug.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/hw/acpi/cpu_hotplug.c b/hw/acpi/cpu_hotplug.c
index 3ac2045a95..9c3bcc84de 100644
--- a/hw/acpi/cpu_hotplug.c
+++ b/hw/acpi/cpu_hotplug.c
@@ -55,8 +55,7 @@ static const MemoryRegionOps AcpiCpuHotplug_ops = {
     },
 };
 
-static void acpi_set_cpu_present_bit(AcpiCpuHotplug *g, CPUState *cpu,
-                                     Error **errp)
+static void acpi_set_cpu_present_bit(AcpiCpuHotplug *g, CPUState *cpu)
 {
     CPUClass *k = CPU_GET_CLASS(cpu);
     int64_t cpu_id;
@@ -74,10 +73,7 @@ static void acpi_set_cpu_present_bit(AcpiCpuHotplug *g, CPUState *cpu,
 void legacy_acpi_cpu_plug_cb(HotplugHandler *hotplug_dev,
                              AcpiCpuHotplug *g, DeviceState *dev, Error **errp)
 {
-    acpi_set_cpu_present_bit(g, CPU(dev), errp);
-    if (*errp != NULL) {
-        return;
-    }
+    acpi_set_cpu_present_bit(g, CPU(dev));
     acpi_send_event(DEVICE(hotplug_dev), ACPI_CPU_HOTPLUG_STATUS);
 }
 
@@ -92,7 +88,7 @@ void legacy_acpi_cpu_hotplug_init(MemoryRegion *parent, Object *owner,
     gpe_cpu->device = owner;
 
     CPU_FOREACH(cpu) {
-        acpi_set_cpu_present_bit(gpe_cpu, cpu, &error_abort);
+        acpi_set_cpu_present_bit(gpe_cpu, cpu);
     }
 }
 
-- 
2.21.0

[PULL 08/34] hw/core: Fix fit_load_fdt() error API violations

[Markus Armbruster]

fit_load_fdt() passes @errp to fit_image_addr(), then recovers from
ENOENT failures.  Passing @errp is wrong, because it works only as
long as @errp is neither @error_fatal nor @error_abort.  Error
recovery dereferences @errp.  That's also wrong; see the big comment
in error.h.  Error recovery can leave *errp pointing to a freed
Error object.  Wrong, it must be null on success.  Messed up in
commit 3eb99edb48 "loader-fit: Wean off error_printf()".

No caller actually passes such values, or uses *errp on success.

Fix anyway: splice in a local Error *err, and error_propagate().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20191204093625.14836-8-armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 hw/core/loader-fit.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/hw/core/loader-fit.c b/hw/core/loader-fit.c
index 953b16bc82..c465921b8f 100644
--- a/hw/core/loader-fit.c
+++ b/hw/core/loader-fit.c
@@ -178,11 +178,12 @@ static int fit_load_fdt(const struct fit_loader *ldr, const void *itb,
                         int cfg, void *opaque, const void *match_data,
                         hwaddr kernel_end, Error **errp)
 {
+    Error *err = NULL;
     const char *name;
     const void *data;
     const void *load_data;
     hwaddr load_addr;
-    int img_off, err;
+    int img_off;
     size_t sz;
     int ret;
 
@@ -197,13 +198,13 @@ static int fit_load_fdt(const struct fit_loader *ldr, const void *itb,
         return -EINVAL;
     }
 
-    err = fit_image_addr(itb, img_off, "load", &load_addr, errp);
-    if (err == -ENOENT) {
+    ret = fit_image_addr(itb, img_off, "load", &load_addr, &err);
+    if (ret == -ENOENT) {
         load_addr = ROUND_UP(kernel_end, 64 * KiB) + (10 * MiB);
-        error_free(*errp);
-    } else if (err) {
-        error_prepend(errp, "unable to read FDT load address from FIT: ");
-        ret = err;
+        error_free(err);
+    } else if (ret) {
+        error_propagate_prepend(errp, err,
+                                "unable to read FDT load address from FIT: ");
         goto out;
     }
 
-- 
2.21.0

[PULL 09/34] hw/ipmi: Fix realize() error API violations

[Markus Armbruster]

isa_ipmi_bt_realize(), ipmi_isa_realize(), pci_ipmi_bt_realize(), and
pci_ipmi_kcs_realize() dereference @errp when IPMIInterfaceClass
method init() fails.  That's wrong; see the big comment in error.h.
Introduced in commit 0719029c47 "ipmi: Add an ISA KCS low-level
interface", then imitated in commit a9b74079cb "ipmi: Add a BT
low-level interface" and commit 12f983c6aa "ipmi: Add PCI IPMI
interfaces".

No caller actually passes null.

Fix anyway: splice in a local Error *err, and error_propagate().

Cc: Corey Minyard <cminyard@mvista.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20191204093625.14836-9-armbru@redhat.com>
---
 hw/ipmi/isa_ipmi_bt.c  | 7 +++++--
 hw/ipmi/isa_ipmi_kcs.c | 7 +++++--
 hw/ipmi/pci_ipmi_bt.c  | 6 ++++--
 hw/ipmi/pci_ipmi_kcs.c | 6 ++++--
 4 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/hw/ipmi/isa_ipmi_bt.c b/hw/ipmi/isa_ipmi_bt.c
index 9a87ffd3f0..9fba5ed383 100644
--- a/hw/ipmi/isa_ipmi_bt.c
+++ b/hw/ipmi/isa_ipmi_bt.c
@@ -70,6 +70,7 @@ static void isa_ipmi_bt_lower_irq(IPMIBT *ib)
 
 static void isa_ipmi_bt_realize(DeviceState *dev, Error **errp)
 {
+    Error *err = NULL;
     ISADevice *isadev = ISA_DEVICE(dev);
     ISAIPMIBTDevice *iib = ISA_IPMI_BT(dev);
     IPMIInterface *ii = IPMI_INTERFACE(dev);
@@ -85,9 +86,11 @@ static void isa_ipmi_bt_realize(DeviceState *dev, Error **errp)
     iib->bt.bmc->intf = ii;
     iib->bt.opaque = iib;
 
-    iic->init(ii, 0, errp);
-    if (*errp)
+    iic->init(ii, 0, &err);
+    if (err) {
+        error_propagate(errp, err);
         return;
+    }
 
     if (iib->isairq > 0) {
         isa_init_irq(isadev, &iib->irq, iib->isairq);
diff --git a/hw/ipmi/isa_ipmi_kcs.c b/hw/ipmi/isa_ipmi_kcs.c
index ca3ea36a3f..cc6bd817f2 100644
--- a/hw/ipmi/isa_ipmi_kcs.c
+++ b/hw/ipmi/isa_ipmi_kcs.c
@@ -69,6 +69,7 @@ static void isa_ipmi_kcs_lower_irq(IPMIKCS *ik)
 
 static void ipmi_isa_realize(DeviceState *dev, Error **errp)
 {
+    Error *err = NULL;
     ISADevice *isadev = ISA_DEVICE(dev);
     ISAIPMIKCSDevice *iik = ISA_IPMI_KCS(dev);
     IPMIInterface *ii = IPMI_INTERFACE(dev);
@@ -84,9 +85,11 @@ static void ipmi_isa_realize(DeviceState *dev, Error **errp)
     iik->kcs.bmc->intf = ii;
     iik->kcs.opaque = iik;
 
-    iic->init(ii, 0, errp);
-    if (*errp)
+    iic->init(ii, 0, &err);
+    if (err) {
+        error_propagate(errp, err);
         return;
+    }
 
     if (iik->isairq > 0) {
         isa_init_irq(isadev, &iik->irq, iik->isairq);
diff --git a/hw/ipmi/pci_ipmi_bt.c b/hw/ipmi/pci_ipmi_bt.c
index 6ed925a665..ba9cf016b5 100644
--- a/hw/ipmi/pci_ipmi_bt.c
+++ b/hw/ipmi/pci_ipmi_bt.c
@@ -54,6 +54,7 @@ static void pci_ipmi_lower_irq(IPMIBT *ik)
 
 static void pci_ipmi_bt_realize(PCIDevice *pd, Error **errp)
 {
+    Error *err = NULL;
     PCIIPMIBTDevice *pik = PCI_IPMI_BT(pd);
     IPMIInterface *ii = IPMI_INTERFACE(pd);
     IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
@@ -74,8 +75,9 @@ static void pci_ipmi_bt_realize(PCIDevice *pd, Error **errp)
     pik->bt.raise_irq = pci_ipmi_raise_irq;
     pik->bt.lower_irq = pci_ipmi_lower_irq;
 
-    iic->init(ii, 8, errp);
-    if (*errp) {
+    iic->init(ii, 8, &err);
+    if (err) {
+        error_propagate(errp, err);
         return;
     }
     pci_register_bar(pd, 0, PCI_BASE_ADDRESS_SPACE_IO, &pik->bt.io);
diff --git a/hw/ipmi/pci_ipmi_kcs.c b/hw/ipmi/pci_ipmi_kcs.c
index eeba63baa4..99f46152f4 100644
--- a/hw/ipmi/pci_ipmi_kcs.c
+++ b/hw/ipmi/pci_ipmi_kcs.c
@@ -54,6 +54,7 @@ static void pci_ipmi_lower_irq(IPMIKCS *ik)
 
 static void pci_ipmi_kcs_realize(PCIDevice *pd, Error **errp)
 {
+    Error *err = NULL;
     PCIIPMIKCSDevice *pik = PCI_IPMI_KCS(pd);
     IPMIInterface *ii = IPMI_INTERFACE(pd);
     IPMIInterfaceClass *iic = IPMI_INTERFACE_GET_CLASS(ii);
@@ -74,8 +75,9 @@ static void pci_ipmi_kcs_realize(PCIDevice *pd, Error **errp)
     pik->kcs.raise_irq = pci_ipmi_raise_irq;
     pik->kcs.lower_irq = pci_ipmi_lower_irq;
 
-    iic->init(ii, 8, errp);
-    if (*errp) {
+    iic->init(ii, 8, &err);
+    if (err) {
+        error_propagate(errp, err);
         return;
     }
     pci_register_bar(pd, 0, PCI_BASE_ADDRESS_SPACE_IO, &pik->kcs.io);
-- 
2.21.0

[PULL 10/34] qga: Fix guest-get-fsinfo error API violations

[Markus Armbruster]

build_guest_fsinfo_for_virtual_device() dereferences @errp when
build_guest_fsinfo_for_device() fails.  That's wrong; see the big
comment in error.h.  Introduced in commit 46d4c5723e "qga: Add
guest-get-fsinfo command".

No caller actually passes null.

Fix anyway: splice in a local Error *err, and error_propagate().

Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20191204093625.14836-10-armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 qga/commands-posix.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 0be301a4ea..10d818c3b2 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -1060,6 +1060,7 @@ static void build_guest_fsinfo_for_virtual_device(char const *syspath,
                                                   GuestFilesystemInfo *fs,
                                                   Error **errp)
 {
+    Error *err = NULL;
     DIR *dir;
     char *dirpath;
     struct dirent *entry;
@@ -1089,10 +1090,11 @@ static void build_guest_fsinfo_for_virtual_device(char const *syspath,
 
             g_debug(" slave device '%s'", entry->d_name);
             path = g_strdup_printf("%s/slaves/%s", syspath, entry->d_name);
-            build_guest_fsinfo_for_device(path, fs, errp);
+            build_guest_fsinfo_for_device(path, fs, &err);
             g_free(path);
 
-            if (*errp) {
+            if (err) {
+                error_propagate(errp, err);
                 break;
             }
         }
-- 
2.21.0

[PULL 11/34] memory-device: Fix memory pre-plug error API violations

[Markus Armbruster]

memory_device_get_free_addr() dereferences @errp when
memory_device_check_addable() fails.  That's wrong; see the big
comment in error.h.  Introduced in commit 1b6d6af21b "pc-dimm: factor
out capacity and slot checks into MemoryDevice".

No caller actually passes null.

Fix anyway: splice in a local Error *err, and error_propagate().

Cc: David Hildenbrand <david@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Message-Id: <20191204093625.14836-11-armbru@redhat.com>
---
 hw/mem/memory-device.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/mem/memory-device.c b/hw/mem/memory-device.c
index aef148c1d7..4bc9cf0917 100644
--- a/hw/mem/memory-device.c
+++ b/hw/mem/memory-device.c
@@ -99,6 +99,7 @@ static uint64_t memory_device_get_free_addr(MachineState *ms,
                                             uint64_t align, uint64_t size,
                                             Error **errp)
 {
+    Error *err = NULL;
     GSList *list = NULL, *item;
     Range as, new = range_empty;
 
@@ -123,8 +124,9 @@ static uint64_t memory_device_get_free_addr(MachineState *ms,
         return 0;
     }
 
-    memory_device_check_addable(ms, size, errp);
-    if (*errp) {
+    memory_device_check_addable(ms, size, &err);
+    if (err) {
+        error_propagate(errp, err);
         return 0;
     }
 
-- 
2.21.0

[PULL 12/34] error: Clean up unusual names of Error * variables

[Markus Armbruster]

Local Error * variables are conventionally named @err or @local_err,
and Error ** parameters @errp.  Naming local variables like parameters
is confusing.  Clean that up.

Naming parameters like local variables is also confusing.  Left for
another day.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20191204093625.14836-17-armbru@redhat.com>
---
 hw/intc/s390_flic_kvm.c    | 10 +++++-----
 hw/ppc/spapr_pci.c         | 16 ++++++++--------
 hw/ppc/spapr_pci_nvlink2.c | 10 +++++-----
 tests/test-blockjob.c      | 16 ++++++++--------
 4 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c
index c9ee80eaae..30d50c2369 100644
--- a/hw/intc/s390_flic_kvm.c
+++ b/hw/intc/s390_flic_kvm.c
@@ -582,10 +582,10 @@ static void kvm_s390_flic_realize(DeviceState *dev, Error **errp)
     struct kvm_create_device cd = {0};
     struct kvm_device_attr test_attr = {0};
     int ret;
-    Error *errp_local = NULL;
+    Error *err = NULL;
 
-    KVM_S390_FLIC_GET_CLASS(dev)->parent_realize(dev, &errp_local);
-    if (errp_local) {
+    KVM_S390_FLIC_GET_CLASS(dev)->parent_realize(dev, &err);
+    if (err) {
         goto fail;
     }
     flic_state->fd = -1;
@@ -593,7 +593,7 @@ static void kvm_s390_flic_realize(DeviceState *dev, Error **errp)
     cd.type = KVM_DEV_TYPE_FLIC;
     ret = kvm_vm_ioctl(kvm_state, KVM_CREATE_DEVICE, &cd);
     if (ret < 0) {
-        error_setg_errno(&errp_local, errno, "Creating the KVM device failed");
+        error_setg_errno(&err, errno, "Creating the KVM device failed");
         trace_flic_create_device(errno);
         goto fail;
     }
@@ -605,7 +605,7 @@ static void kvm_s390_flic_realize(DeviceState *dev, Error **errp)
                                             KVM_HAS_DEVICE_ATTR, test_attr);
     return;
 fail:
-    error_propagate(errp, errp_local);
+    error_propagate(errp, err);
 }
 
 static void kvm_s390_flic_reset(DeviceState *dev)
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index f6fbcf99ed..723373de73 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -2042,13 +2042,13 @@ void spapr_phb_dma_reset(SpaprPhbState *sphb)
 static void spapr_phb_reset(DeviceState *qdev)
 {
     SpaprPhbState *sphb = SPAPR_PCI_HOST_BRIDGE(qdev);
-    Error *errp = NULL;
+    Error *err = NULL;
 
     spapr_phb_dma_reset(sphb);
     spapr_phb_nvgpu_free(sphb);
-    spapr_phb_nvgpu_setup(sphb, &errp);
-    if (errp) {
-        error_report_err(errp);
+    spapr_phb_nvgpu_setup(sphb, &err);
+    if (err) {
+        error_report_err(err);
     }
 
     /* Reset the IOMMU state */
@@ -2326,7 +2326,7 @@ int spapr_dt_phb(SpaprMachineState *spapr, SpaprPhbState *phb,
                                 cpu_to_be32(phb->numa_node)};
     SpaprTceTable *tcet;
     SpaprDrc *drc;
-    Error *errp = NULL;
+    Error *err = NULL;
 
     /* Start populating the FDT */
     _FDT(bus_off = fdt_add_subnode(fdt, 0, phb->dtbusname));
@@ -2408,9 +2408,9 @@ int spapr_dt_phb(SpaprMachineState *spapr, SpaprPhbState *phb,
         return ret;
     }
 
-    spapr_phb_nvgpu_populate_dt(phb, fdt, bus_off, &errp);
-    if (errp) {
-        error_report_err(errp);
+    spapr_phb_nvgpu_populate_dt(phb, fdt, bus_off, &err);
+    if (err) {
+        error_report_err(err);
     }
     spapr_phb_nvgpu_ram_populate_dt(phb, fdt);
 
diff --git a/hw/ppc/spapr_pci_nvlink2.c b/hw/ppc/spapr_pci_nvlink2.c
index 4aa89ede23..8332d5694e 100644
--- a/hw/ppc/spapr_pci_nvlink2.c
+++ b/hw/ppc/spapr_pci_nvlink2.c
@@ -57,7 +57,7 @@ struct SpaprPhbPciNvGpuConfig {
     uint64_t nv2_atsd_current;
     int num; /* number of non empty (i.e. tgt!=0) entries in slots[] */
     SpaprPhbPciNvGpuSlot slots[NVGPU_MAX_NUM];
-    Error *errp;
+    Error *err;
 };
 
 static SpaprPhbPciNvGpuSlot *
@@ -153,7 +153,7 @@ static void spapr_phb_pci_collect_nvgpu(PCIBus *bus, PCIDevice *pdev,
             spapr_pci_collect_nvnpu(nvgpus, pdev, tgt, MEMORY_REGION(mr_npu),
                                     &local_err);
         }
-        error_propagate(&nvgpus->errp, local_err);
+        error_propagate(&nvgpus->err, local_err);
     }
     if ((pci_default_read_config(pdev, PCI_HEADER_TYPE, 1) !=
          PCI_HEADER_TYPE_BRIDGE)) {
@@ -187,9 +187,9 @@ void spapr_phb_nvgpu_setup(SpaprPhbState *sphb, Error **errp)
     pci_for_each_device(bus, pci_bus_num(bus),
                         spapr_phb_pci_collect_nvgpu, sphb->nvgpus);
 
-    if (sphb->nvgpus->errp) {
-        error_propagate(errp, sphb->nvgpus->errp);
-        sphb->nvgpus->errp = NULL;
+    if (sphb->nvgpus->err) {
+        error_propagate(errp, sphb->nvgpus->err);
+        sphb->nvgpus->err = NULL;
         goto cleanup_exit;
     }
 
diff --git a/tests/test-blockjob.c b/tests/test-blockjob.c
index 7844c9ffcb..e670a20617 100644
--- a/tests/test-blockjob.c
+++ b/tests/test-blockjob.c
@@ -34,13 +34,13 @@ static BlockJob *mk_job(BlockBackend *blk, const char *id,
                         int flags)
 {
     BlockJob *job;
-    Error *errp = NULL;
+    Error *err = NULL;
 
     job = block_job_create(id, drv, NULL, blk_bs(blk),
                            0, BLK_PERM_ALL, 0, flags, block_job_cb,
-                           NULL, &errp);
+                           NULL, &err);
     if (should_succeed) {
-        g_assert_null(errp);
+        g_assert_null(err);
         g_assert_nonnull(job);
         if (id) {
             g_assert_cmpstr(job->job.id, ==, id);
@@ -48,9 +48,9 @@ static BlockJob *mk_job(BlockBackend *blk, const char *id,
             g_assert_cmpstr(job->job.id, ==, blk_name(blk));
         }
     } else {
-        g_assert_nonnull(errp);
+        g_assert_nonnull(err);
         g_assert_null(job);
-        error_free(errp);
+        error_free(err);
     }
 
     return job;
@@ -80,9 +80,9 @@ static BlockBackend *create_blk(const char *name)
     bdrv_unref(bs);
 
     if (name) {
-        Error *errp = NULL;
-        monitor_add_blk(blk, name, &errp);
-        g_assert_null(errp);
+        Error *err = NULL;
+        monitor_add_blk(blk, name, &err);
+        g_assert_null(err);
     }
 
     return blk;
-- 
2.21.0

[PULL 13/34] hw/intc/s390: Simplify error handling in kvm_s390_flic_realize()

[Markus Armbruster]

Cc: Halil Pasic <pasic@linux.ibm.com>
Cc: Cornelia Huck <cohuck@redhat.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Acked-by: Halil Pasic <pasic@linux.ibm.com>
Message-Id: <20191204093625.14836-18-armbru@redhat.com>
---
 hw/intc/s390_flic_kvm.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/hw/intc/s390_flic_kvm.c b/hw/intc/s390_flic_kvm.c
index 30d50c2369..dddd33ea61 100644
--- a/hw/intc/s390_flic_kvm.c
+++ b/hw/intc/s390_flic_kvm.c
@@ -586,16 +586,17 @@ static void kvm_s390_flic_realize(DeviceState *dev, Error **errp)
 
     KVM_S390_FLIC_GET_CLASS(dev)->parent_realize(dev, &err);
     if (err) {
-        goto fail;
+        error_propagate(errp, err);
+        return;
     }
     flic_state->fd = -1;
 
     cd.type = KVM_DEV_TYPE_FLIC;
     ret = kvm_vm_ioctl(kvm_state, KVM_CREATE_DEVICE, &cd);
     if (ret < 0) {
-        error_setg_errno(&err, errno, "Creating the KVM device failed");
+        error_setg_errno(errp, errno, "Creating the KVM device failed");
         trace_flic_create_device(errno);
-        goto fail;
+        return;
     }
     flic_state->fd = cd.fd;
 
@@ -603,9 +604,6 @@ static void kvm_s390_flic_realize(DeviceState *dev, Error **errp)
     test_attr.group = KVM_DEV_FLIC_CLEAR_IO_IRQ;
     flic_state->clear_io_supported = !ioctl(flic_state->fd,
                                             KVM_HAS_DEVICE_ATTR, test_attr);
-    return;
-fail:
-    error_propagate(errp, err);
 }
 
 static void kvm_s390_flic_reset(DeviceState *dev)
-- 
2.21.0

[PULL 14/34] tests-blockjob: Use error_free_or_abort()

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20191204093625.14836-19-armbru@redhat.com>
---
 tests/test-blockjob.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tests/test-blockjob.c b/tests/test-blockjob.c
index e670a20617..4eeb184caf 100644
--- a/tests/test-blockjob.c
+++ b/tests/test-blockjob.c
@@ -48,9 +48,8 @@ static BlockJob *mk_job(BlockBackend *blk, const char *id,
             g_assert_cmpstr(job->job.id, ==, blk_name(blk));
         }
     } else {
-        g_assert_nonnull(err);
+        error_free_or_abort(&err);
         g_assert_null(job);
-        error_free(err);
     }
 
     return job;
-- 
2.21.0

[PULL 15/34] net/net: Clean up variable shadowing in net_client_init()

[Markus Armbruster]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Variable int err in inner scope shadows Error *err in outer scope.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20191205174635.18758-3-vsementsov@virtuozzo.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 net/net.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/net/net.c b/net/net.c
index 84aa6d8d00..9e93c3f8a1 100644
--- a/net/net.c
+++ b/net/net.c
@@ -1126,16 +1126,13 @@ static int net_client_init(QemuOpts *opts, bool is_netdev, Error **errp)
 
             prefix_addr = substrings[0];
 
-            if (substrings[1]) {
-                /* User-specified prefix length.  */
-                int err;
-
-                err = qemu_strtoul(substrings[1], NULL, 10, &prefix_len);
-                if (err) {
-                    error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
-                               "ipv6-prefixlen", "a number");
-                    goto out;
-                }
+            /* Handle user-specified prefix length. */
+            if (substrings[1] &&
+                qemu_strtoul(substrings[1], NULL, 10, &prefix_len))
+            {
+                error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
+                           "ipv6-prefixlen", "a number");
+                goto out;
             }
 
             qemu_opt_set(opts, "ipv6-prefix", prefix_addr, &error_abort);
-- 
2.21.0

[PULL 16/34] error: make Error **errp const where it is appropriate

[Markus Armbruster]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Mostly, Error ** is for returning error from the function, so the
callee sets it. However these three functions get already filled errp
parameter. They don't change the pointer itself, only change the
internal state of referenced Error object. So we can make it
Error *const * errp, to stress the behavior. It will also help
coccinelle script (in future) to distinguish such cases from common
errp usage.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20191205174635.18758-4-vsementsov@virtuozzo.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[Commit message typo fixed]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/qapi/error.h | 6 +++---
 util/error.c         | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/qapi/error.h b/include/qapi/error.h
index 3f95141a01..ad5b6e896d 100644
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -233,13 +233,13 @@ void error_propagate_prepend(Error **dst_errp, Error *local_err,
  * Prepend some text to @errp's human-readable error message.
  * The text is made by formatting @fmt, @ap like vprintf().
  */
-void error_vprepend(Error **errp, const char *fmt, va_list ap);
+void error_vprepend(Error *const *errp, const char *fmt, va_list ap);
 
 /*
  * Prepend some text to @errp's human-readable error message.
  * The text is made by formatting @fmt, ... like printf().
  */
-void error_prepend(Error **errp, const char *fmt, ...)
+void error_prepend(Error *const *errp, const char *fmt, ...)
     GCC_FMT_ATTR(2, 3);
 
 /*
@@ -256,7 +256,7 @@ void error_prepend(Error **errp, const char *fmt, ...)
  * May be called multiple times.  The resulting hint should end with a
  * newline.
  */
-void error_append_hint(Error **errp, const char *fmt, ...)
+void error_append_hint(Error *const *errp, const char *fmt, ...)
     GCC_FMT_ATTR(2, 3);
 
 /*
diff --git a/util/error.c b/util/error.c
index d4532ce318..b6c89d1412 100644
--- a/util/error.c
+++ b/util/error.c
@@ -121,7 +121,7 @@ void error_setg_file_open_internal(Error **errp,
                               "Could not open '%s'", filename);
 }
 
-void error_vprepend(Error **errp, const char *fmt, va_list ap)
+void error_vprepend(Error *const *errp, const char *fmt, va_list ap)
 {
     GString *newmsg;
 
@@ -136,7 +136,7 @@ void error_vprepend(Error **errp, const char *fmt, va_list ap)
     (*errp)->msg = g_string_free(newmsg, 0);
 }
 
-void error_prepend(Error **errp, const char *fmt, ...)
+void error_prepend(Error *const *errp, const char *fmt, ...)
 {
     va_list ap;
 
@@ -145,7 +145,7 @@ void error_prepend(Error **errp, const char *fmt, ...)
     va_end(ap);
 }
 
-void error_append_hint(Error **errp, const char *fmt, ...)
+void error_append_hint(Error *const *errp, const char *fmt, ...)
 {
     va_list ap;
     int saved_errno = errno;
-- 
2.21.0

[PULL 17/34] hmp: drop Error pointer indirection in hmp_handle_error

[Markus Armbruster]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

We don't need Error **, as all callers pass local Error object, which
isn't used after the call. Use Error * instead.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20191205174635.18758-5-vsementsov@virtuozzo.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/monitor/hmp.h      |   2 +-
 dump/dump-hmp-cmds.c       |   4 +-
 hw/core/machine-hmp-cmds.c |   6 +-
 monitor/hmp-cmds.c         | 155 ++++++++++++++++++-------------------
 qdev-monitor.c             |   4 +-
 qom/qom-hmp-cmds.c         |   4 +-
 6 files changed, 87 insertions(+), 88 deletions(-)

diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index a0e9511440..3d329853b2 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -16,7 +16,7 @@
 
 #include "qemu/readline.h"
 
-void hmp_handle_error(Monitor *mon, Error **errp);
+void hmp_handle_error(Monitor *mon, Error *err);
 
 void hmp_info_name(Monitor *mon, const QDict *qdict);
 void hmp_info_version(Monitor *mon, const QDict *qdict);
diff --git a/dump/dump-hmp-cmds.c b/dump/dump-hmp-cmds.c
index 3dbf44372c..e5053b04cd 100644
--- a/dump/dump-hmp-cmds.c
+++ b/dump/dump-hmp-cmds.c
@@ -32,7 +32,7 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
 
     if (zlib + lzo + snappy + win_dmp > 1) {
         error_setg(&err, "only one of '-z|-l|-s|-w' can be set");
-        hmp_handle_error(mon, &err);
+        hmp_handle_error(mon, err);
         return;
     }
 
@@ -66,7 +66,7 @@ void hmp_dump_guest_memory(Monitor *mon, const QDict *qdict)
 
     qmp_dump_guest_memory(paging, prot, true, detach, has_begin, begin,
                           has_length, length, true, dump_format, &err);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
     g_free(prot);
 }
 
diff --git a/hw/core/machine-hmp-cmds.c b/hw/core/machine-hmp-cmds.c
index cd970cc4c5..b76f7223af 100644
--- a/hw/core/machine-hmp-cmds.c
+++ b/hw/core/machine-hmp-cmds.c
@@ -55,7 +55,7 @@ void hmp_cpu_add(Monitor *mon, const QDict *qdict)
 
     cpuid = qdict_get_int(qdict, "id");
     qmp_cpu_add(cpuid, &err);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_hotpluggable_cpus(Monitor *mon, const QDict *qdict)
@@ -66,7 +66,7 @@ void hmp_hotpluggable_cpus(Monitor *mon, const QDict *qdict)
     CpuInstanceProperties *c;
 
     if (err != NULL) {
-        hmp_handle_error(mon, &err);
+        hmp_handle_error(mon, err);
         return;
     }
 
@@ -135,7 +135,7 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict)
     monitor_printf(mon, "\n");
 
     qapi_free_MemdevList(memdev_list);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_info_numa(Monitor *mon, const QDict *qdict)
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index b2551c16d1..c5dea307b6 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -60,11 +60,10 @@
 #include <spice/enums.h>
 #endif
 
-void hmp_handle_error(Monitor *mon, Error **errp)
+void hmp_handle_error(Monitor *mon, Error *err)
 {
-    assert(errp);
-    if (*errp) {
-        error_reportf_err(*errp, "Error: ");
+    if (err) {
+        error_reportf_err(err, "Error: ");
     }
 }
 
@@ -734,7 +733,7 @@ void hmp_info_vnc(Monitor *mon, const QDict *qdict)
 
     info2l = qmp_query_vnc_servers(&err);
     if (err) {
-        hmp_handle_error(mon, &err);
+        hmp_handle_error(mon, err);
         return;
     }
     if (!info2l) {
@@ -850,7 +849,7 @@ void hmp_info_balloon(Monitor *mon, const QDict *qdict)
 
     info = qmp_query_balloon(&err);
     if (err) {
-        hmp_handle_error(mon, &err);
+        hmp_handle_error(mon, err);
         return;
     }
 
@@ -1172,7 +1171,7 @@ void hmp_sync_profile(Monitor *mon, const QDict *qdict)
         Error *err = NULL;
 
         error_setg(&err, QERR_INVALID_PARAMETER, op);
-        hmp_handle_error(mon, &err);
+        hmp_handle_error(mon, err);
     }
 }
 
@@ -1191,7 +1190,7 @@ void hmp_exit_preconfig(Monitor *mon, const QDict *qdict)
     Error *err = NULL;
 
     qmp_x_exit_preconfig(&err);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_cpu(Monitor *mon, const QDict *qdict)
@@ -1220,7 +1219,7 @@ void hmp_memsave(Monitor *mon, const QDict *qdict)
     }
 
     qmp_memsave(addr, size, filename, true, cpu_index, &err);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_pmemsave(Monitor *mon, const QDict *qdict)
@@ -1231,7 +1230,7 @@ void hmp_pmemsave(Monitor *mon, const QDict *qdict)
     Error *err = NULL;
 
     qmp_pmemsave(addr, size, filename, &err);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_ringbuf_write(Monitor *mon, const QDict *qdict)
@@ -1242,7 +1241,7 @@ void hmp_ringbuf_write(Monitor *mon, const QDict *qdict)
 
     qmp_ringbuf_write(chardev, data, false, 0, &err);
 
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_ringbuf_read(Monitor *mon, const QDict *qdict)
@@ -1255,7 +1254,7 @@ void hmp_ringbuf_read(Monitor *mon, const QDict *qdict)
 
     data = qmp_ringbuf_read(chardev, size, false, 0, &err);
     if (err) {
-        hmp_handle_error(mon, &err);
+        hmp_handle_error(mon, err);
         return;
     }
 
@@ -1280,7 +1279,7 @@ void hmp_cont(Monitor *mon, const QDict *qdict)
     Error *err = NULL;
 
     qmp_cont(&err);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_system_wakeup(Monitor *mon, const QDict *qdict)
@@ -1288,7 +1287,7 @@ void hmp_system_wakeup(Monitor *mon, const QDict *qdict)
     Error *err = NULL;
 
     qmp_system_wakeup(&err);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_nmi(Monitor *mon, const QDict *qdict)
@@ -1296,7 +1295,7 @@ void hmp_nmi(Monitor *mon, const QDict *qdict)
     Error *err = NULL;
 
     qmp_inject_nmi(&err);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_set_link(Monitor *mon, const QDict *qdict)
@@ -1306,7 +1305,7 @@ void hmp_set_link(Monitor *mon, const QDict *qdict)
     Error *err = NULL;
 
     qmp_set_link(name, up, &err);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_block_passwd(Monitor *mon, const QDict *qdict)
@@ -1316,7 +1315,7 @@ void hmp_block_passwd(Monitor *mon, const QDict *qdict)
     Error *err = NULL;
 
     qmp_block_passwd(true, device, false, NULL, password, &err);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_balloon(Monitor *mon, const QDict *qdict)
@@ -1325,7 +1324,7 @@ void hmp_balloon(Monitor *mon, const QDict *qdict)
     Error *err = NULL;
 
     qmp_balloon(value, &err);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_block_resize(Monitor *mon, const QDict *qdict)
@@ -1335,7 +1334,7 @@ void hmp_block_resize(Monitor *mon, const QDict *qdict)
     Error *err = NULL;
 
     qmp_block_resize(true, device, false, NULL, size, &err);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_drive_mirror(Monitor *mon, const QDict *qdict)
@@ -1358,11 +1357,11 @@ void hmp_drive_mirror(Monitor *mon, const QDict *qdict)
 
     if (!filename) {
         error_setg(&err, QERR_MISSING_PARAMETER, "target");
-        hmp_handle_error(mon, &err);
+        hmp_handle_error(mon, err);
         return;
     }
     qmp_drive_mirror(&mirror, &err);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_drive_backup(Monitor *mon, const QDict *qdict)
@@ -1388,12 +1387,12 @@ void hmp_drive_backup(Monitor *mon, const QDict *qdict)
 
     if (!filename) {
         error_setg(&err, QERR_MISSING_PARAMETER, "target");
-        hmp_handle_error(mon, &err);
+        hmp_handle_error(mon, err);
         return;
     }
 
     qmp_drive_backup(&backup, &err);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict)
@@ -1409,7 +1408,7 @@ void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict)
         /* In the future, if 'snapshot-file' is not specified, the snapshot
            will be taken internally. Today it's actually required. */
         error_setg(&err, QERR_MISSING_PARAMETER, "snapshot-file");
-        hmp_handle_error(mon, &err);
+        hmp_handle_error(mon, err);
         return;
     }
 
@@ -1418,7 +1417,7 @@ void hmp_snapshot_blkdev(Monitor *mon, const QDict *qdict)
                                filename, false, NULL,
                                !!format, format,
                                true, mode, &err);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_snapshot_blkdev_internal(Monitor *mon, const QDict *qdict)
@@ -1428,7 +1427,7 @@ void hmp_snapshot_blkdev_internal(Monitor *mon, const QDict *qdict)
     Error *err = NULL;
 
     qmp_blockdev_snapshot_internal_sync(device, name, &err);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_snapshot_delete_blkdev_internal(Monitor *mon, const QDict *qdict)
@@ -1440,7 +1439,7 @@ void hmp_snapshot_delete_blkdev_internal(Monitor *mon, const QDict *qdict)
 
     qmp_blockdev_snapshot_delete_internal_sync(device, !!id, id,
                                                true, name, &err);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_loadvm(Monitor *mon, const QDict *qdict)
@@ -1454,7 +1453,7 @@ void hmp_loadvm(Monitor *mon, const QDict *qdict)
     if (load_snapshot(name, &err) == 0 && saved_vm_running) {
         vm_start();
     }
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_savevm(Monitor *mon, const QDict *qdict)
@@ -1462,7 +1461,7 @@ void hmp_savevm(Monitor *mon, const QDict *qdict)
     Error *err = NULL;
 
     save_snapshot(qdict_get_try_str(qdict, "name"), &err);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_delvm(Monitor *mon, const QDict *qdict)
@@ -1476,7 +1475,7 @@ void hmp_delvm(Monitor *mon, const QDict *qdict)
                       "deleting snapshot on device '%s': ",
                       bdrv_get_device_name(bs));
     }
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_info_snapshots(Monitor *mon, const QDict *qdict)
@@ -1652,7 +1651,7 @@ void hmp_migrate_continue(Monitor *mon, const QDict *qdict)
         qmp_migrate_continue(val, &err);
     }
 
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_migrate_incoming(Monitor *mon, const QDict *qdict)
@@ -1662,7 +1661,7 @@ void hmp_migrate_incoming(Monitor *mon, const QDict *qdict)
 
     qmp_migrate_incoming(uri, &err);
 
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_migrate_recover(Monitor *mon, const QDict *qdict)
@@ -1672,7 +1671,7 @@ void hmp_migrate_recover(Monitor *mon, const QDict *qdict)
 
     qmp_migrate_recover(uri, &err);
 
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_migrate_pause(Monitor *mon, const QDict *qdict)
@@ -1681,7 +1680,7 @@ void hmp_migrate_pause(Monitor *mon, const QDict *qdict)
 
     qmp_migrate_pause(&err);
 
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 /* Kept for backwards compatibility */
@@ -1697,7 +1696,7 @@ void hmp_migrate_set_cache_size(Monitor *mon, const QDict *qdict)
     Error *err = NULL;
 
     qmp_migrate_set_cache_size(value, &err);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 /* Kept for backwards compatibility */
@@ -1728,7 +1727,7 @@ void hmp_migrate_set_capability(Monitor *mon, const QDict *qdict)
 
 end:
     qapi_free_MigrationCapabilityStatusList(caps);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
@@ -1869,7 +1868,7 @@ void hmp_migrate_set_parameter(Monitor *mon, const QDict *qdict)
  cleanup:
     qapi_free_MigrateSetParameters(p);
     visit_free(v);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_client_migrate_info(Monitor *mon, const QDict *qdict)
@@ -1886,14 +1885,14 @@ void hmp_client_migrate_info(Monitor *mon, const QDict *qdict)
     qmp_client_migrate_info(protocol, hostname,
                             has_port, port, has_tls_port, tls_port,
                             !!cert_subject, cert_subject, &err);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_migrate_start_postcopy(Monitor *mon, const QDict *qdict)
 {
     Error *err = NULL;
     qmp_migrate_start_postcopy(&err);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_x_colo_lost_heartbeat(Monitor *mon, const QDict *qdict)
@@ -1901,7 +1900,7 @@ void hmp_x_colo_lost_heartbeat(Monitor *mon, const QDict *qdict)
     Error *err = NULL;
 
     qmp_x_colo_lost_heartbeat(&err);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_set_password(Monitor *mon, const QDict *qdict)
@@ -1912,7 +1911,7 @@ void hmp_set_password(Monitor *mon, const QDict *qdict)
     Error *err = NULL;
 
     qmp_set_password(protocol, password, !!connected, connected, &err);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_expire_password(Monitor *mon, const QDict *qdict)
@@ -1922,7 +1921,7 @@ void hmp_expire_password(Monitor *mon, const QDict *qdict)
     Error *err = NULL;
 
     qmp_expire_password(protocol, whenstr, &err);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_eject(Monitor *mon, const QDict *qdict)
@@ -1932,7 +1931,7 @@ void hmp_eject(Monitor *mon, const QDict *qdict)
     Error *err = NULL;
 
     qmp_eject(true, device, false, NULL, true, force, &err);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 #ifdef CONFIG_VNC
@@ -1978,7 +1977,7 @@ void hmp_change(Monitor *mon, const QDict *qdict)
                                 read_only,
                                 BLOCKDEV_CHANGE_READ_ONLY_MODE_RETAIN, &err);
             if (err) {
-                hmp_handle_error(mon, &err);
+                hmp_handle_error(mon, err);
                 return;
             }
         }
@@ -1988,7 +1987,7 @@ void hmp_change(Monitor *mon, const QDict *qdict)
                                    &err);
     }
 
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict)
@@ -2016,7 +2015,7 @@ void hmp_block_set_io_throttle(Monitor *mon, const QDict *qdict)
     }
 
     qmp_block_set_io_throttle(&throttle, &err);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_block_stream(Monitor *mon, const QDict *qdict)
@@ -2031,7 +2030,7 @@ void hmp_block_stream(Monitor *mon, const QDict *qdict)
                      BLOCKDEV_ON_ERROR_REPORT, false, false, false, false,
                      &error);
 
-    hmp_handle_error(mon, &error);
+    hmp_handle_error(mon, error);
 }
 
 void hmp_block_job_set_speed(Monitor *mon, const QDict *qdict)
@@ -2042,7 +2041,7 @@ void hmp_block_job_set_speed(Monitor *mon, const QDict *qdict)
 
     qmp_block_job_set_speed(device, value, &error);
 
-    hmp_handle_error(mon, &error);
+    hmp_handle_error(mon, error);
 }
 
 void hmp_block_job_cancel(Monitor *mon, const QDict *qdict)
@@ -2053,7 +2052,7 @@ void hmp_block_job_cancel(Monitor *mon, const QDict *qdict)
 
     qmp_block_job_cancel(device, true, force, &error);
 
-    hmp_handle_error(mon, &error);
+    hmp_handle_error(mon, error);
 }
 
 void hmp_block_job_pause(Monitor *mon, const QDict *qdict)
@@ -2063,7 +2062,7 @@ void hmp_block_job_pause(Monitor *mon, const QDict *qdict)
 
     qmp_block_job_pause(device, &error);
 
-    hmp_handle_error(mon, &error);
+    hmp_handle_error(mon, error);
 }
 
 void hmp_block_job_resume(Monitor *mon, const QDict *qdict)
@@ -2073,7 +2072,7 @@ void hmp_block_job_resume(Monitor *mon, const QDict *qdict)
 
     qmp_block_job_resume(device, &error);
 
-    hmp_handle_error(mon, &error);
+    hmp_handle_error(mon, error);
 }
 
 void hmp_block_job_complete(Monitor *mon, const QDict *qdict)
@@ -2083,7 +2082,7 @@ void hmp_block_job_complete(Monitor *mon, const QDict *qdict)
 
     qmp_block_job_complete(device, &error);
 
-    hmp_handle_error(mon, &error);
+    hmp_handle_error(mon, error);
 }
 
 typedef struct HMPMigrationStatus
@@ -2143,7 +2142,7 @@ void hmp_migrate(Monitor *mon, const QDict *qdict)
     qmp_migrate(uri, !!blk, blk, !!inc, inc,
                 false, false, true, resume, &err);
     if (err) {
-        hmp_handle_error(mon, &err);
+        hmp_handle_error(mon, err);
         return;
     }
 
@@ -2181,7 +2180,7 @@ void hmp_netdev_add(Monitor *mon, const QDict *qdict)
     }
 
 out:
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_netdev_del(Monitor *mon, const QDict *qdict)
@@ -2190,7 +2189,7 @@ void hmp_netdev_del(Monitor *mon, const QDict *qdict)
     Error *err = NULL;
 
     qmp_netdev_del(id, &err);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_object_add(Monitor *mon, const QDict *qdict)
@@ -2201,7 +2200,7 @@ void hmp_object_add(Monitor *mon, const QDict *qdict)
 
     opts = qemu_opts_from_qdict(qemu_find_opts("object"), qdict, &err);
     if (err) {
-        hmp_handle_error(mon, &err);
+        hmp_handle_error(mon, err);
         return;
     }
 
@@ -2209,7 +2208,7 @@ void hmp_object_add(Monitor *mon, const QDict *qdict)
     qemu_opts_del(opts);
 
     if (err) {
-        hmp_handle_error(mon, &err);
+        hmp_handle_error(mon, err);
     }
     if (obj) {
         object_unref(obj);
@@ -2222,7 +2221,7 @@ void hmp_getfd(Monitor *mon, const QDict *qdict)
     Error *err = NULL;
 
     qmp_getfd(fdname, &err);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_closefd(Monitor *mon, const QDict *qdict)
@@ -2231,7 +2230,7 @@ void hmp_closefd(Monitor *mon, const QDict *qdict)
     Error *err = NULL;
 
     qmp_closefd(fdname, &err);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_sendkey(Monitor *mon, const QDict *qdict)
@@ -2290,7 +2289,7 @@ void hmp_sendkey(Monitor *mon, const QDict *qdict)
     }
 
     qmp_send_key(head, has_hold_time, hold_time, &err);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 
 out:
     qapi_free_KeyValueList(head);
@@ -2309,7 +2308,7 @@ void hmp_screendump(Monitor *mon, const QDict *qdict)
     Error *err = NULL;
 
     qmp_screendump(filename, id != NULL, id, id != NULL, head, &err);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_nbd_server_start(Monitor *mon, const QDict *qdict)
@@ -2364,7 +2363,7 @@ void hmp_nbd_server_start(Monitor *mon, const QDict *qdict)
     qapi_free_BlockInfoList(block_list);
 
 exit:
-    hmp_handle_error(mon, &local_err);
+    hmp_handle_error(mon, local_err);
 }
 
 void hmp_nbd_server_add(Monitor *mon, const QDict *qdict)
@@ -2376,7 +2375,7 @@ void hmp_nbd_server_add(Monitor *mon, const QDict *qdict)
 
     qmp_nbd_server_add(device, !!name, name, true, writable,
                        false, NULL, &local_err);
-    hmp_handle_error(mon, &local_err);
+    hmp_handle_error(mon, local_err);
 }
 
 void hmp_nbd_server_remove(Monitor *mon, const QDict *qdict)
@@ -2387,7 +2386,7 @@ void hmp_nbd_server_remove(Monitor *mon, const QDict *qdict)
 
     /* Rely on NBD_SERVER_REMOVE_MODE_SAFE being the default */
     qmp_nbd_server_remove(name, force, NBD_SERVER_REMOVE_MODE_HARD, &err);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict)
@@ -2395,7 +2394,7 @@ void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict)
     Error *err = NULL;
 
     qmp_nbd_server_stop(&err);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_chardev_add(Monitor *mon, const QDict *qdict)
@@ -2411,7 +2410,7 @@ void hmp_chardev_add(Monitor *mon, const QDict *qdict)
         qemu_chr_new_from_opts(opts, NULL, &err);
         qemu_opts_del(opts);
     }
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_chardev_change(Monitor *mon, const QDict *qdict)
@@ -2445,7 +2444,7 @@ end:
     qapi_free_ChardevReturn(ret);
     qapi_free_ChardevBackend(backend);
     qemu_opts_del(opts);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_chardev_remove(Monitor *mon, const QDict *qdict)
@@ -2453,7 +2452,7 @@ void hmp_chardev_remove(Monitor *mon, const QDict *qdict)
     Error *local_err = NULL;
 
     qmp_chardev_remove(qdict_get_str(qdict, "id"), &local_err);
-    hmp_handle_error(mon, &local_err);
+    hmp_handle_error(mon, local_err);
 }
 
 void hmp_chardev_send_break(Monitor *mon, const QDict *qdict)
@@ -2461,7 +2460,7 @@ void hmp_chardev_send_break(Monitor *mon, const QDict *qdict)
     Error *local_err = NULL;
 
     qmp_chardev_send_break(qdict_get_str(qdict, "id"), &local_err);
-    hmp_handle_error(mon, &local_err);
+    hmp_handle_error(mon, local_err);
 }
 
 void hmp_qemu_io(Monitor *mon, const QDict *qdict)
@@ -2517,7 +2516,7 @@ void hmp_qemu_io(Monitor *mon, const QDict *qdict)
 
 fail:
     blk_unref(local_blk);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_object_del(Monitor *mon, const QDict *qdict)
@@ -2526,7 +2525,7 @@ void hmp_object_del(Monitor *mon, const QDict *qdict)
     Error *err = NULL;
 
     user_creatable_del(id, &err);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_info_memory_devices(Monitor *mon, const QDict *qdict)
@@ -2576,7 +2575,7 @@ void hmp_info_memory_devices(Monitor *mon, const QDict *qdict)
     }
 
     qapi_free_MemoryDeviceInfoList(info_list);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_info_iothreads(Monitor *mon, const QDict *qdict)
@@ -2605,7 +2604,7 @@ void hmp_rocker(Monitor *mon, const QDict *qdict)
 
     rocker = qmp_query_rocker(name, &err);
     if (err != NULL) {
-        hmp_handle_error(mon, &err);
+        hmp_handle_error(mon, err);
         return;
     }
 
@@ -2624,7 +2623,7 @@ void hmp_rocker_ports(Monitor *mon, const QDict *qdict)
 
     list = qmp_query_rocker_ports(name, &err);
     if (err != NULL) {
-        hmp_handle_error(mon, &err);
+        hmp_handle_error(mon, err);
         return;
     }
 
@@ -2653,7 +2652,7 @@ void hmp_rocker_of_dpa_flows(Monitor *mon, const QDict *qdict)
 
     list = qmp_query_rocker_of_dpa_flows(name, tbl_id != -1, tbl_id, &err);
     if (err != NULL) {
-        hmp_handle_error(mon, &err);
+        hmp_handle_error(mon, err);
         return;
     }
 
@@ -2804,7 +2803,7 @@ void hmp_rocker_of_dpa_groups(Monitor *mon, const QDict *qdict)
 
     list = qmp_query_rocker_of_dpa_groups(name, type != 9, type, &err);
     if (err != NULL) {
-        hmp_handle_error(mon, &err);
+        hmp_handle_error(mon, err);
         return;
     }
 
@@ -2911,7 +2910,7 @@ void hmp_info_vm_generation_id(Monitor *mon, const QDict *qdict)
     if (info) {
         monitor_printf(mon, "%s\n", info->guid);
     }
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
     qapi_free_GuidInfo(info);
 }
 
@@ -2930,5 +2929,5 @@ void hmp_info_memory_size_summary(Monitor *mon, const QDict *qdict)
 
         qapi_free_MemoryInfo(info);
     }
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
diff --git a/qdev-monitor.c b/qdev-monitor.c
index e6b112eb0a..29ed73e56a 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -890,7 +890,7 @@ void hmp_device_add(Monitor *mon, const QDict *qdict)
     Error *err = NULL;
 
     qmp_device_add((QDict *)qdict, NULL, &err);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_device_del(Monitor *mon, const QDict *qdict)
@@ -899,7 +899,7 @@ void hmp_device_del(Monitor *mon, const QDict *qdict)
     Error *err = NULL;
 
     qmp_device_del(id, &err);
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 BlockBackend *blk_by_qdev_id(const char *id, Error **errp)
diff --git a/qom/qom-hmp-cmds.c b/qom/qom-hmp-cmds.c
index a268e01eb4..cd08233a4c 100644
--- a/qom/qom-hmp-cmds.c
+++ b/qom/qom-hmp-cmds.c
@@ -37,7 +37,7 @@ void hmp_qom_list(Monitor *mon, const QDict *qdict)
         }
         qapi_free_ObjectPropertyInfoList(start);
     }
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 void hmp_qom_set(Monitor *mon, const QDict *qdict)
@@ -59,7 +59,7 @@ void hmp_qom_set(Monitor *mon, const QDict *qdict)
         }
         object_property_parse(obj, value, property, &err);
     }
-    hmp_handle_error(mon, &err);
+    hmp_handle_error(mon, err);
 }
 
 typedef struct QOMCompositionState {
-- 
2.21.0

[PULL 18/34] vnc: drop Error pointer indirection in vnc_client_io_error

[Markus Armbruster]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

We don't need Error **, as all callers pass local Error object, which
isn't used after the call, or NULL. Use Error * instead.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20191205174635.18758-6-vsementsov@virtuozzo.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 ui/vnc.h |  2 +-
 ui/vnc.c | 20 +++++++-------------
 2 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/ui/vnc.h b/ui/vnc.h
index fea79c2fc9..4e2637ce6c 100644
--- a/ui/vnc.h
+++ b/ui/vnc.h
@@ -547,7 +547,7 @@ uint32_t read_u32(uint8_t *data, size_t offset);
 
 /* Protocol stage functions */
 void vnc_client_error(VncState *vs);
-size_t vnc_client_io_error(VncState *vs, ssize_t ret, Error **errp);
+size_t vnc_client_io_error(VncState *vs, ssize_t ret, Error *err);
 
 void start_client_init(VncState *vs);
 void start_auth_vnc(VncState *vs);
diff --git a/ui/vnc.c b/ui/vnc.c
index 87b8045afe..4100d6e404 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1312,7 +1312,7 @@ void vnc_disconnect_finish(VncState *vs)
     g_free(vs);
 }
 
-size_t vnc_client_io_error(VncState *vs, ssize_t ret, Error **errp)
+size_t vnc_client_io_error(VncState *vs, ssize_t ret, Error *err)
 {
     if (ret <= 0) {
         if (ret == 0) {
@@ -1320,15 +1320,11 @@ size_t vnc_client_io_error(VncState *vs, ssize_t ret, Error **errp)
             vnc_disconnect_start(vs);
         } else if (ret != QIO_CHANNEL_ERR_BLOCK) {
             trace_vnc_client_io_error(vs, vs->ioc,
-                                      errp ? error_get_pretty(*errp) :
-                                      "Unknown");
+                                      err ? error_get_pretty(err) : "Unknown");
             vnc_disconnect_start(vs);
         }
 
-        if (errp) {
-            error_free(*errp);
-            *errp = NULL;
-        }
+        error_free(err);
         return 0;
     }
     return ret;
@@ -1361,10 +1357,9 @@ size_t vnc_client_write_buf(VncState *vs, const uint8_t *data, size_t datalen)
 {
     Error *err = NULL;
     ssize_t ret;
-    ret = qio_channel_write(
-        vs->ioc, (const char *)data, datalen, &err);
+    ret = qio_channel_write(vs->ioc, (const char *)data, datalen, &err);
     VNC_DEBUG("Wrote wire %p %zd -> %ld\n", data, datalen, ret);
-    return vnc_client_io_error(vs, ret, &err);
+    return vnc_client_io_error(vs, ret, err);
 }
 
 
@@ -1488,10 +1483,9 @@ size_t vnc_client_read_buf(VncState *vs, uint8_t *data, size_t datalen)
 {
     ssize_t ret;
     Error *err = NULL;
-    ret = qio_channel_read(
-        vs->ioc, (char *)data, datalen, &err);
+    ret = qio_channel_read(vs->ioc, (char *)data, datalen, &err);
     VNC_DEBUG("Read wire %p %zd -> %ld\n", data, datalen, ret);
-    return vnc_client_io_error(vs, ret, &err);
+    return vnc_client_io_error(vs, ret, err);
 }
 
 
-- 
2.21.0

[PULL 19/34] qdev-monitor: make Error **errp const where it is appropriate

[Markus Armbruster]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Mostly, Error ** is for returning error from the function, so the
callee sets it. However qbus_list_bus and qbus_list_dev get already
filled errp parameter. They don't change the pointer itself, only
change the internal state of referenced Error object. So we can make
it Error *const * errp, to stress the behavior. It will also help
coccinelle script (in future) to distinguish such cases from common
errp usage.

While there, rename the functions to
qbus_error_append_bus_list_hint(), qbus_error_append_dev_list_hint().

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20191205174635.18758-7-vsementsov@virtuozzo.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[Commit message replaced]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qdev-monitor.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/qdev-monitor.c b/qdev-monitor.c
index 29ed73e56a..3465a1e2d0 100644
--- a/qdev-monitor.c
+++ b/qdev-monitor.c
@@ -328,7 +328,8 @@ static Object *qdev_get_peripheral_anon(void)
     return dev;
 }
 
-static void qbus_list_bus(DeviceState *dev, Error **errp)
+static void qbus_error_append_bus_list_hint(DeviceState *dev,
+                                            Error *const *errp)
 {
     BusState *child;
     const char *sep = " ";
@@ -342,7 +343,8 @@ static void qbus_list_bus(DeviceState *dev, Error **errp)
     error_append_hint(errp, "\n");
 }
 
-static void qbus_list_dev(BusState *bus, Error **errp)
+static void qbus_error_append_dev_list_hint(BusState *bus,
+                                            Error *const *errp)
 {
     BusChild *kid;
     const char *sep = " ";
@@ -500,7 +502,7 @@ static BusState *qbus_find(const char *path, Error **errp)
         if (!dev) {
             error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
                       "Device '%s' not found", elem);
-            qbus_list_dev(bus, errp);
+            qbus_error_append_dev_list_hint(bus, errp);
             return NULL;
         }
 
@@ -518,7 +520,7 @@ static BusState *qbus_find(const char *path, Error **errp)
             if (dev->num_child_bus) {
                 error_setg(errp, "Device '%s' has multiple child buses",
                            elem);
-                qbus_list_bus(dev, errp);
+                qbus_error_append_bus_list_hint(dev, errp);
             } else {
                 error_setg(errp, "Device '%s' has no child bus", elem);
             }
@@ -534,7 +536,7 @@ static BusState *qbus_find(const char *path, Error **errp)
         bus = qbus_find_bus(dev, elem);
         if (!bus) {
             error_setg(errp, "Bus '%s' not found", elem);
-            qbus_list_bus(dev, errp);
+            qbus_error_append_bus_list_hint(dev, errp);
             return NULL;
         }
     }
-- 
2.21.0

[PULL 20/34] ppc: make Error **errp const where it is appropriate

[Markus Armbruster]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Mostly, Error ** is for returning error from the function, so the
callee sets it. However kvmppc_hint_smt_possible gets already filled
errp parameter. It doesn't change the pointer itself, only change the
internal state of referenced Error object. So we can make it Error
*const * errp, to stress the behavior. It will also help coccinelle
script (in future) to distinguish such cases from common errp usage.

While there, rename the function to
kvmppc_error_append_smt_possible_hint().

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <20191205174635.18758-8-vsementsov@virtuozzo.com>
Acked-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[Commit message replaced]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 target/ppc/kvm_ppc.h | 4 ++--
 hw/ppc/spapr.c       | 2 +-
 target/ppc/kvm.c     | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
index 98bd7d5da6..f22daabf51 100644
--- a/target/ppc/kvm_ppc.h
+++ b/target/ppc/kvm_ppc.h
@@ -28,7 +28,7 @@ void kvmppc_set_papr(PowerPCCPU *cpu);
 int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr);
 void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy);
 int kvmppc_smt_threads(void);
-void kvmppc_hint_smt_possible(Error **errp);
+void kvmppc_error_append_smt_possible_hint(Error *const *errp);
 int kvmppc_set_smt_threads(int smt);
 int kvmppc_clear_tsr_bits(PowerPCCPU *cpu, uint32_t tsr_bits);
 int kvmppc_or_tsr_bits(PowerPCCPU *cpu, uint32_t tsr_bits);
@@ -164,7 +164,7 @@ static inline int kvmppc_smt_threads(void)
     return 1;
 }
 
-static inline void kvmppc_hint_smt_possible(Error **errp)
+static inline void kvmppc_error_append_smt_possible_hint(Error *const *errp)
 {
     return;
 }
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 3ae7db1563..c5d8f0afb6 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2564,7 +2564,7 @@ static void spapr_set_vsmt_mode(SpaprMachineState *spapr, Error **errp)
                                       " requires the use of VSMT mode %d.\n",
                                       smp_threads, kvm_smt, spapr->vsmt);
                 }
-                kvmppc_hint_smt_possible(&local_err);
+                kvmppc_error_append_smt_possible_hint(&local_err);
                 goto out;
             }
         }
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index c77f9848ec..27ea3ce535 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -2076,7 +2076,7 @@ int kvmppc_set_smt_threads(int smt)
     return ret;
 }
 
-void kvmppc_hint_smt_possible(Error **errp)
+void kvmppc_error_append_smt_possible_hint(Error *const *errp)
 {
     int i;
     GString *g;
-- 
2.21.0

[PULL 21/34] 9pfs: make Error **errp const where it is appropriate

[Markus Armbruster]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Mostly, Error ** is for returning error from the function, so the
callee sets it. However error_append_security_model_hint and
error_append_socket_sockfd_hint get already filled errp
parameter. They don't change the pointer itself, only change the
internal state of referenced Error object. So we can make it Error
*const * errp, to stress the behavior. It will also help coccinelle
script (in future) to distinguish such cases from common errp usage.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Acked-by: Greg Kurz <groug@kaod.org>
Message-Id: <20191205174635.18758-9-vsementsov@virtuozzo.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[Commit message replaced]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/9pfs/9p-local.c | 2 +-
 hw/9pfs/9p-proxy.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/9pfs/9p-local.c b/hw/9pfs/9p-local.c
index 4708c0bd89..ca641390fb 100644
--- a/hw/9pfs/9p-local.c
+++ b/hw/9pfs/9p-local.c
@@ -1473,7 +1473,7 @@ static void local_cleanup(FsContext *ctx)
     g_free(data);
 }
 
-static void error_append_security_model_hint(Error **errp)
+static void error_append_security_model_hint(Error *const *errp)
 {
     error_append_hint(errp, "Valid options are: security_model="
                       "[passthrough|mapped-xattr|mapped-file|none]\n");
diff --git a/hw/9pfs/9p-proxy.c b/hw/9pfs/9p-proxy.c
index 97ab9c58a5..8136e1342d 100644
--- a/hw/9pfs/9p-proxy.c
+++ b/hw/9pfs/9p-proxy.c
@@ -1114,7 +1114,7 @@ static int connect_namedsocket(const char *path, Error **errp)
     return sockfd;
 }
 
-static void error_append_socket_sockfd_hint(Error **errp)
+static void error_append_socket_sockfd_hint(Error *const *errp)
 {
     error_append_hint(errp, "Either specify socket=/some/path where /some/path"
                       " points to a listening AF_UNIX socket or sock_fd=fd"
-- 
2.21.0

[PULL 22/34] hw/core/qdev: cleanup Error ** variables

[Markus Armbruster]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Rename Error ** parameter in check_only_migratable to common errp.

In device_set_realized:

 - Move "if (local_err != NULL)" closer to error setters.

 - Drop 'Error **local_errp': it doesn't save any LoCs, but it's very
   unusual.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20191205174635.18758-10-vsementsov@virtuozzo.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/core/qdev.c | 28 +++++++++++++---------------
 1 file changed, 13 insertions(+), 15 deletions(-)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index cf1ba28fe3..82d3ee590a 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -820,12 +820,12 @@ static bool device_get_realized(Object *obj, Error **errp)
     return dev->realized;
 }
 
-static bool check_only_migratable(Object *obj, Error **err)
+static bool check_only_migratable(Object *obj, Error **errp)
 {
     DeviceClass *dc = DEVICE_GET_CLASS(obj);
 
     if (!vmstate_check_only_migratable(dc->vmsd)) {
-        error_setg(err, "Device %s is not migratable, but "
+        error_setg(errp, "Device %s is not migratable, but "
                    "--only-migratable was specified",
                    object_get_typename(obj));
         return false;
@@ -874,10 +874,9 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
 
         if (dc->realize) {
             dc->realize(dev, &local_err);
-        }
-
-        if (local_err != NULL) {
-            goto fail;
+            if (local_err != NULL) {
+                goto fail;
+            }
         }
 
         DEVICE_LISTENER_CALL(realize, Forward, dev);
@@ -918,27 +917,26 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
        }
 
     } else if (!value && dev->realized) {
-        Error **local_errp = NULL;
+        /* We want local_err to track only the first error */
         QLIST_FOREACH(bus, &dev->child_bus, sibling) {
-            local_errp = local_err ? NULL : &local_err;
             object_property_set_bool(OBJECT(bus), false, "realized",
-                                     local_errp);
+                                     local_err ? NULL : &local_err);
         }
         if (qdev_get_vmsd(dev)) {
             vmstate_unregister(dev, qdev_get_vmsd(dev), dev);
         }
         if (dc->unrealize) {
-            local_errp = local_err ? NULL : &local_err;
-            dc->unrealize(dev, local_errp);
+            dc->unrealize(dev, local_err ? NULL : &local_err);
         }
         dev->pending_deleted_event = true;
         DEVICE_LISTENER_CALL(unrealize, Reverse, dev);
-    }
 
-    if (local_err != NULL) {
-        goto fail;
+        if (local_err != NULL) {
+            goto fail;
+        }
     }
 
+    assert(local_err == NULL);
     dev->realized = value;
     return;
 
@@ -976,7 +974,7 @@ static bool device_get_hotpluggable(Object *obj, Error **errp)
                                 qbus_is_hotpluggable(dev->parent_bus));
 }
 
-static bool device_get_hotplugged(Object *obj, Error **err)
+static bool device_get_hotplugged(Object *obj, Error **errp)
 {
     DeviceState *dev = DEVICE(obj);
 
-- 
2.21.0

[PULL 23/34] block/snapshot: rename Error ** parameter to more common errp

[Markus Armbruster]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@Redhat.com>
Message-Id: <20191205174635.18758-11-vsementsov@virtuozzo.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/block/snapshot.h | 2 +-
 block/snapshot.c         | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/block/snapshot.h b/include/block/snapshot.h
index b5d5084a12..2bfcd57578 100644
--- a/include/block/snapshot.h
+++ b/include/block/snapshot.h
@@ -78,7 +78,7 @@ int bdrv_snapshot_load_tmp_by_id_or_name(BlockDriverState *bs,
 
 bool bdrv_all_can_snapshot(BlockDriverState **first_bad_bs);
 int bdrv_all_delete_snapshot(const char *name, BlockDriverState **first_bsd_bs,
-                             Error **err);
+                             Error **errp);
 int bdrv_all_goto_snapshot(const char *name, BlockDriverState **first_bad_bs,
                            Error **errp);
 int bdrv_all_find_snapshot(const char *name, BlockDriverState **first_bad_bs);
diff --git a/block/snapshot.c b/block/snapshot.c
index 8081616ae9..bd9fb01817 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -426,7 +426,7 @@ fail:
 }
 
 int bdrv_all_delete_snapshot(const char *name, BlockDriverState **first_bad_bs,
-                             Error **err)
+                             Error **errp)
 {
     int ret = 0;
     BlockDriverState *bs;
@@ -441,7 +441,7 @@ int bdrv_all_delete_snapshot(const char *name, BlockDriverState **first_bad_bs,
             bdrv_snapshot_find(bs, snapshot, name) >= 0)
         {
             ret = bdrv_snapshot_delete(bs, snapshot->id_str,
-                                       snapshot->name, err);
+                                       snapshot->name, errp);
         }
         aio_context_release(ctx);
         if (ret < 0) {
-- 
2.21.0

[PULL 24/34] hw/i386/amd_iommu: rename Error ** parameter to more common errp

[Markus Armbruster]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20191205174635.18758-12-vsementsov@virtuozzo.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/i386/amd_iommu.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index d55dbf07fc..b1175e52c7 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -1533,7 +1533,7 @@ static void amdvi_reset(DeviceState *dev)
     amdvi_init(s);
 }
 
-static void amdvi_realize(DeviceState *dev, Error **err)
+static void amdvi_realize(DeviceState *dev, Error **errp)
 {
     int ret = 0;
     AMDVIState *s = AMD_IOMMU_DEVICE(dev);
@@ -1549,21 +1549,21 @@ static void amdvi_realize(DeviceState *dev, Error **err)
     /* This device should take care of IOMMU PCI properties */
     x86_iommu->type = TYPE_AMD;
     qdev_set_parent_bus(DEVICE(&s->pci), &bus->qbus);
-    object_property_set_bool(OBJECT(&s->pci), true, "realized", err);
+    object_property_set_bool(OBJECT(&s->pci), true, "realized", errp);
     ret = pci_add_capability(&s->pci.dev, AMDVI_CAPAB_ID_SEC, 0,
-                                         AMDVI_CAPAB_SIZE, err);
+                                         AMDVI_CAPAB_SIZE, errp);
     if (ret < 0) {
         return;
     }
     s->capab_offset = ret;
 
     ret = pci_add_capability(&s->pci.dev, PCI_CAP_ID_MSI, 0,
-                             AMDVI_CAPAB_REG_SIZE, err);
+                             AMDVI_CAPAB_REG_SIZE, errp);
     if (ret < 0) {
         return;
     }
     ret = pci_add_capability(&s->pci.dev, PCI_CAP_ID_HT, 0,
-                             AMDVI_CAPAB_REG_SIZE, err);
+                             AMDVI_CAPAB_REG_SIZE, errp);
     if (ret < 0) {
         return;
     }
@@ -1578,8 +1578,8 @@ static void amdvi_realize(DeviceState *dev, Error **err)
     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", err);
-    msi_init(&s->pci.dev, 0, 1, true, false, err);
+    s->devid = object_property_get_int(OBJECT(&s->pci), "addr", errp);
+    msi_init(&s->pci.dev, 0, 1, true, false, errp);
     amdvi_init(s);
 }
 
-- 
2.21.0

[PULL 25/34] qga: rename Error ** parameter to more common errp

[Markus Armbruster]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20191205174635.18758-13-vsementsov@virtuozzo.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qga/commands-posix.c |  2 +-
 qga/commands-win32.c |  2 +-
 qga/commands.c       | 12 ++++++------
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 10d818c3b2..93474ff770 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -2794,7 +2794,7 @@ static double ga_get_login_time(struct utmpx *user_info)
     return seconds + useconds;
 }
 
-GuestUserList *qmp_guest_get_users(Error **err)
+GuestUserList *qmp_guest_get_users(Error **errp)
 {
     GHashTable *cache = NULL;
     GuestUserList *head = NULL, *cur_item = NULL;
diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 55ba5b263a..2461fd19bf 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -1946,7 +1946,7 @@ typedef struct _GA_WTSINFOA {
 
 } GA_WTSINFOA;
 
-GuestUserList *qmp_guest_get_users(Error **err)
+GuestUserList *qmp_guest_get_users(Error **errp)
 {
 #define QGA_NANOSECONDS 10000000
 
diff --git a/qga/commands.c b/qga/commands.c
index 0c7d1385c2..43c323cead 100644
--- a/qga/commands.c
+++ b/qga/commands.c
@@ -143,7 +143,7 @@ static GuestExecInfo *guest_exec_info_find(int64_t pid_numeric)
     return NULL;
 }
 
-GuestExecStatus *qmp_guest_exec_status(int64_t pid, Error **err)
+GuestExecStatus *qmp_guest_exec_status(int64_t pid, Error **errp)
 {
     GuestExecInfo *gei;
     GuestExecStatus *ges;
@@ -152,7 +152,7 @@ GuestExecStatus *qmp_guest_exec_status(int64_t pid, Error **err)
 
     gei = guest_exec_info_find(pid);
     if (gei == NULL) {
-        error_setg(err, QERR_INVALID_PARAMETER, "pid");
+        error_setg(errp, QERR_INVALID_PARAMETER, "pid");
         return NULL;
     }
 
@@ -385,7 +385,7 @@ GuestExec *qmp_guest_exec(const char *path,
                        bool has_env, strList *env,
                        bool has_input_data, const char *input_data,
                        bool has_capture_output, bool capture_output,
-                       Error **err)
+                       Error **errp)
 {
     GPid pid;
     GuestExec *ge = NULL;
@@ -405,7 +405,7 @@ GuestExec *qmp_guest_exec(const char *path,
     arglist.next = has_arg ? arg : NULL;
 
     if (has_input_data) {
-        input = qbase64_decode(input_data, -1, &ninput, err);
+        input = qbase64_decode(input_data, -1, &ninput, errp);
         if (!input) {
             return NULL;
         }
@@ -424,7 +424,7 @@ GuestExec *qmp_guest_exec(const char *path,
             guest_exec_task_setup, NULL, &pid, has_input_data ? &in_fd : NULL,
             has_output ? &out_fd : NULL, has_output ? &err_fd : NULL, &gerr);
     if (!ret) {
-        error_setg(err, QERR_QGA_COMMAND_FAILED, gerr->message);
+        error_setg(errp, QERR_QGA_COMMAND_FAILED, gerr->message);
         g_error_free(gerr);
         goto done;
     }
@@ -499,7 +499,7 @@ int ga_parse_whence(GuestFileWhence *whence, Error **errp)
     return -1;
 }
 
-GuestHostName *qmp_guest_get_host_name(Error **err)
+GuestHostName *qmp_guest_get_host_name(Error **errp)
 {
     GuestHostName *result = NULL;
     gchar const *hostname = g_get_host_name();
-- 
2.21.0

[PULL 26/34] monitor/qmp-cmds: rename Error ** parameter to more common errp

[Markus Armbruster]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20191205174635.18758-14-vsementsov@virtuozzo.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 monitor/qmp-cmds.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
index 0880341a2d..c6faa3eaf0 100644
--- a/monitor/qmp-cmds.c
+++ b/monitor/qmp-cmds.c
@@ -109,7 +109,7 @@ void qmp_system_reset(Error **errp)
     qemu_system_reset_request(SHUTDOWN_CAUSE_HOST_QMP_SYSTEM_RESET);
 }
 
-void qmp_system_powerdown(Error **erp)
+void qmp_system_powerdown(Error **errp)
 {
     qemu_system_powerdown_request();
 }
-- 
2.21.0

[PULL 27/34] hw/s390x: rename Error ** parameter to more common errp

[Markus Armbruster]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Message-Id: <20191205174635.18758-15-vsementsov@virtuozzo.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/s390x/event-facility.c | 2 +-
 hw/s390x/s390-stattrib.c  | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
index cdcf9154c4..6afe278cad 100644
--- a/hw/s390x/event-facility.c
+++ b/hw/s390x/event-facility.c
@@ -441,7 +441,7 @@ static void sclp_event_set_allow_all_mask_sizes(Object *obj, bool value,
     ef->allow_all_mask_sizes = value;
 }
 
-static bool sclp_event_get_allow_all_mask_sizes(Object *obj, Error **e)
+static bool sclp_event_get_allow_all_mask_sizes(Object *obj, Error **errp)
 {
     SCLPEventFacility *ef = (SCLPEventFacility *)obj;
 
diff --git a/hw/s390x/s390-stattrib.c b/hw/s390x/s390-stattrib.c
index bf5ac014c4..58121b9f68 100644
--- a/hw/s390x/s390-stattrib.c
+++ b/hw/s390x/s390-stattrib.c
@@ -352,7 +352,8 @@ static void s390_stattrib_class_init(ObjectClass *oc, void *data)
     dc->realize = s390_stattrib_realize;
 }
 
-static inline bool s390_stattrib_get_migration_enabled(Object *obj, Error **e)
+static inline bool s390_stattrib_get_migration_enabled(Object *obj,
+                                                       Error **errp)
 {
     S390StAttribState *s = S390_STATTRIB(obj);
 
-- 
2.21.0

[PULL 28/34] hw/sd: drop extra whitespace in sdhci_sysbus_realize() header

[Markus Armbruster]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20191205174635.18758-16-vsementsov@virtuozzo.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/sd/sdhci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 88404d0e9d..18c0c052ce 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1459,7 +1459,7 @@ static void sdhci_sysbus_finalize(Object *obj)
     sdhci_uninitfn(s);
 }
 
-static void sdhci_sysbus_realize(DeviceState *dev, Error ** errp)
+static void sdhci_sysbus_realize(DeviceState *dev, Error **errp)
 {
     SDHCIState *s = SYSBUS_SDHCI(dev);
     SysBusDevice *sbd = SYS_BUS_DEVICE(dev);
-- 
2.21.0

[PULL 29/34] hw/tpm: rename Error ** parameter to more common errp

[Markus Armbruster]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
Message-Id: <20191205174635.18758-17-vsementsov@virtuozzo.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/tpm/tpm_emulator.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/tpm/tpm_emulator.c b/hw/tpm/tpm_emulator.c
index 22f9113432..10d587ed40 100644
--- a/hw/tpm/tpm_emulator.c
+++ b/hw/tpm/tpm_emulator.c
@@ -155,7 +155,7 @@ static int tpm_emulator_unix_tx_bufs(TPMEmulator *tpm_emu,
                                      const uint8_t *in, uint32_t in_len,
                                      uint8_t *out, uint32_t out_len,
                                      bool *selftest_done,
-                                     Error **err)
+                                     Error **errp)
 {
     ssize_t ret;
     bool is_selftest = false;
@@ -165,20 +165,20 @@ static int tpm_emulator_unix_tx_bufs(TPMEmulator *tpm_emu,
         is_selftest = tpm_util_is_selftest(in, in_len);
     }
 
-    ret = qio_channel_write_all(tpm_emu->data_ioc, (char *)in, in_len, err);
+    ret = qio_channel_write_all(tpm_emu->data_ioc, (char *)in, in_len, errp);
     if (ret != 0) {
         return -1;
     }
 
     ret = qio_channel_read_all(tpm_emu->data_ioc, (char *)out,
-              sizeof(struct tpm_resp_hdr), err);
+              sizeof(struct tpm_resp_hdr), errp);
     if (ret != 0) {
         return -1;
     }
 
     ret = qio_channel_read_all(tpm_emu->data_ioc,
               (char *)out + sizeof(struct tpm_resp_hdr),
-              tpm_cmd_get_size(out) - sizeof(struct tpm_resp_hdr), err);
+              tpm_cmd_get_size(out) - sizeof(struct tpm_resp_hdr), errp);
     if (ret != 0) {
         return -1;
     }
-- 
2.21.0

[PULL 30/34] hw/usb: rename Error ** parameter to more common errp

[Markus Armbruster]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20191205174635.18758-18-vsementsov@virtuozzo.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/usb/dev-network.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/usb/dev-network.c b/hw/usb/dev-network.c
index 5de6213cc4..b81a8abe83 100644
--- a/hw/usb/dev-network.c
+++ b/hw/usb/dev-network.c
@@ -1342,7 +1342,7 @@ static NetClientInfo net_usbnet_info = {
     .cleanup = usbnet_cleanup,
 };
 
-static void usb_net_realize(USBDevice *dev, Error **errrp)
+static void usb_net_realize(USBDevice *dev, Error **errp)
 {
     USBNetState *s = USB_NET(dev);
 
-- 
2.21.0

[PULL 31/34] include/qom/object.h: rename Error ** parameter to more common errp

[Markus Armbruster]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20191205174635.18758-19-vsementsov@virtuozzo.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/qom/object.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/qom/object.h b/include/qom/object.h
index 128d00c77f..716f6f655d 100644
--- a/include/qom/object.h
+++ b/include/qom/object.h
@@ -1635,9 +1635,9 @@ void object_class_property_add_uint32_ptr(ObjectClass *klass, const char *name,
  * property of type 'uint64'.
  */
 void object_property_add_uint64_ptr(Object *obj, const char *name,
-                                    const uint64_t *v, Error **Errp);
+                                    const uint64_t *v, Error **errp);
 void object_class_property_add_uint64_ptr(ObjectClass *klass, const char *name,
-                                          const uint64_t *v, Error **Errp);
+                                          const uint64_t *v, Error **errp);
 
 /**
  * object_property_add_alias:
-- 
2.21.0

[PULL 32/34] backends/cryptodev: drop local_err from cryptodev_backend_complete()

[Markus Armbruster]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

No reason for local_err here, use errp directly instead.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Gonglei <arei.gonglei@huawei.com>
Message-Id: <20191205174635.18758-20-vsementsov@virtuozzo.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 backends/cryptodev.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/backends/cryptodev.c b/backends/cryptodev.c
index 3c071eab95..5a9735684e 100644
--- a/backends/cryptodev.c
+++ b/backends/cryptodev.c
@@ -176,19 +176,10 @@ cryptodev_backend_complete(UserCreatable *uc, Error **errp)
 {
     CryptoDevBackend *backend = CRYPTODEV_BACKEND(uc);
     CryptoDevBackendClass *bc = CRYPTODEV_BACKEND_GET_CLASS(uc);
-    Error *local_err = NULL;
 
     if (bc->init) {
-        bc->init(backend, &local_err);
-        if (local_err) {
-            goto out;
-        }
+        bc->init(backend, errp);
     }
-
-    return;
-
-out:
-    error_propagate(errp, local_err);
 }
 
 void cryptodev_backend_set_used(CryptoDevBackend *backend, bool used)
-- 
2.21.0

[PULL 33/34] hw/vfio/ap: drop local_err from vfio_ap_realize

[Markus Armbruster]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

No reason for local_err here, use errp directly instead.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Message-Id: <20191205174635.18758-21-vsementsov@virtuozzo.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/vfio/ap.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
index da6a20669d..8fbaa724c2 100644
--- a/hw/vfio/ap.c
+++ b/hw/vfio/ap.c
@@ -89,14 +89,13 @@ static void vfio_ap_realize(DeviceState *dev, Error **errp)
 {
     int ret;
     char *mdevid;
-    Error *local_err = NULL;
     VFIOGroup *vfio_group;
     APDevice *apdev = AP_DEVICE(dev);
     VFIOAPDevice *vapdev = VFIO_AP_DEVICE(apdev);
 
-    vfio_group = vfio_ap_get_group(vapdev, &local_err);
+    vfio_group = vfio_ap_get_group(vapdev, errp);
     if (!vfio_group) {
-        goto out_err;
+        return;
     }
 
     vapdev->vdev.ops = &vfio_ap_ops;
@@ -113,7 +112,7 @@ static void vfio_ap_realize(DeviceState *dev, Error **errp)
      */
     vapdev->vdev.balloon_allowed = true;
 
-    ret = vfio_get_device(vfio_group, mdevid, &vapdev->vdev, &local_err);
+    ret = vfio_get_device(vfio_group, mdevid, &vapdev->vdev, errp);
     if (ret) {
         goto out_get_dev_err;
     }
@@ -123,8 +122,6 @@ static void vfio_ap_realize(DeviceState *dev, Error **errp)
 out_get_dev_err:
     vfio_ap_put_device(vapdev);
     vfio_put_group(vfio_group);
-out_err:
-    error_propagate(errp, local_err);
 }
 
 static void vfio_ap_unrealize(DeviceState *dev, Error **errp)
-- 
2.21.0

[PULL 34/34] nbd: assert that Error** is not NULL in nbd_iter_channel_error

[Markus Armbruster]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

All callers of nbd_iter_channel_error() pass the address of a
local_err variable, and only call this function if an error has
already occurred, using this function to propagate that error.
This is already implied by its name (local_err instead of the classic
errp), but it is worth additionally stressing this by adding an
assertion to make it part of the function contract.

The local_err parameter is not here to return information about
nbd_iter_channel_error failure. Instead it's assumed to be filled when
passed to the function. This is already stressed by its name
(local_err, instead of classic errp). Stress it additionally by
assertion.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20191205174635.18758-22-vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 block/nbd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/block/nbd.c b/block/nbd.c
index 5f18f78a94..d085554f21 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -866,6 +866,7 @@ typedef struct NBDReplyChunkIter {
 static void nbd_iter_channel_error(NBDReplyChunkIter *iter,
                                    int ret, Error **local_err)
 {
+    assert(local_err && *local_err);
     assert(ret < 0);
 
     if (!iter->ret) {
-- 
2.21.0

Re: [PULL 34/34] nbd: assert that Error** is not NULL in nbd_iter_channel_error

[Eric Blake]

On 12/17/19 12:26 AM, Markus Armbruster wrote:
> From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> 
> All callers of nbd_iter_channel_error() pass the address of a
> local_err variable, and only call this function if an error has
> already occurred, using this function to propagate that error.
> This is already implied by its name (local_err instead of the classic
> errp), but it is worth additionally stressing this by adding an
> assertion to make it part of the function contract.
> 
> The local_err parameter is not here to return information about
> nbd_iter_channel_error failure. Instead it's assumed to be filled when
> passed to the function. This is already stressed by its name
> (local_err, instead of classic errp). Stress it additionally by
> assertion.

Redundant paragraph, but probably too late to worry about it now that we 
have a pull request.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PULL 00/34] Error reporting patches for 2019-12-16

[Peter Maydell]

On Tue, 17 Dec 2019 at 06:33, Markus Armbruster <armbru@redhat.com> wrote:
>
> The following changes since commit cb88904a54903ef6ba21a68a61d9cd51e2166304:
>
>   Merge remote-tracking branch 'remotes/amarkovic/tags/mips-queue-dec-16-2019' into staging (2019-12-16 14:07:56 +0000)
>
> are available in the Git repository at:
>
>   git://repo.or.cz/qemu/armbru.git tags/pull-error-2019-12-16
>
> for you to fetch changes up to 0e7f83bab6559775cd71e418b12a49145e59faa7:
>
>   nbd: assert that Error** is not NULL in nbd_iter_channel_error (2019-12-16 20:50:16 +0100)
>
> ----------------------------------------------------------------
> Error reporting patches for 2019-12-16
>
> ----------------------------------------------------------------

This gets conflicts:
diff --cc target/ppc/kvm.c
index 7406d18945,27ea3ce535..0000000000
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@@ -2076,7 -2076,7 +2076,11 @@@ int kvmppc_set_smt_threads(int smt
      return ret;
  }

++<<<<<<< HEAD
 +void kvmppc_error_append_smt_possible_hint(Error **errp_in)
++=======
+ void kvmppc_error_append_smt_possible_hint(Error *const *errp)
++>>>>>>> remotes/armbru/tags/pull-error-2019-12-16
  {
      int i;
      GString *g;
diff --cc target/ppc/kvm_ppc.h
index 47b08a4030,f22daabf51..0000000000
--- a/target/ppc/kvm_ppc.h
+++ b/target/ppc/kvm_ppc.h
@@@ -28,7 -28,7 +28,11 @@@ void kvmppc_set_papr(PowerPCCPU *cpu)
  int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr);
  void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy);
  int kvmppc_smt_threads(void);
++<<<<<<< HEAD
 +void kvmppc_error_append_smt_possible_hint(Error **errp_in);
++=======
+ void kvmppc_error_append_smt_possible_hint(Error *const *errp);
++>>>>>>> remotes/armbru/tags/pull-error-2019-12-16
  int kvmppc_set_smt_threads(int smt);
  int kvmppc_clear_tsr_bits(PowerPCCPU *cpu, uint32_t tsr_bits);
  int kvmppc_or_tsr_bits(PowerPCCPU *cpu, uint32_t tsr_bits);
@@@ -164,7 -164,7 +168,11 @@@ static inline int kvmppc_smt_threads(vo
      return 1;
  }

++<<<<<<< HEAD
 +static inline void kvmppc_error_append_smt_possible_hint(Error **errp_in)
++=======
+ static inline void kvmppc_error_append_smt_possible_hint(Error *const *errp)
++>>>>>>> remotes/armbru/tags/pull-error-2019-12-16
  {
      return;
  }

Furthermore, it turns out that the conflicts are due to
different patches from the same author to the same function
("ppc: well form kvmppc_hint_smt_possible error hint helper"
and "ppc: make Error **errp const where it is appropriate")
which both seem to be addressing broadly the same thing
but conflict with each other and arrived via different
pull requests.

So I'm just bouncing this one back for you to fix and
figure out which version you want...

thanks
-- PMM

Re: [PULL 34/34] nbd: assert that Error** is not NULL in nbd_iter_channel_error

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 12/17/19 12:26 AM, Markus Armbruster wrote:
>> From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>
>> All callers of nbd_iter_channel_error() pass the address of a
>> local_err variable, and only call this function if an error has
>> already occurred, using this function to propagate that error.
>> This is already implied by its name (local_err instead of the classic
>> errp), but it is worth additionally stressing this by adding an
>> assertion to make it part of the function contract.
>>
>> The local_err parameter is not here to return information about
>> nbd_iter_channel_error failure. Instead it's assumed to be filled when
>> passed to the function. This is already stressed by its name
>> (local_err, instead of classic errp). Stress it additionally by
>> assertion.
>
> Redundant paragraph, but probably too late to worry about it now that
> we have a pull request.

I'll have to respin anyway.  I'll drop the second paragraph.

Re: [PULL 00/34] Error reporting patches for 2019-12-16

[Markus Armbruster]

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

> On Tue, 17 Dec 2019 at 06:33, Markus Armbruster <armbru@redhat.com> wrote:
>>
>> The following changes since commit cb88904a54903ef6ba21a68a61d9cd51e2166304:
>>
>>   Merge remote-tracking branch 'remotes/amarkovic/tags/mips-queue-dec-16-2019' into staging (2019-12-16 14:07:56 +0000)
>>
>> are available in the Git repository at:
>>
>>   git://repo.or.cz/qemu/armbru.git tags/pull-error-2019-12-16
>>
>> for you to fetch changes up to 0e7f83bab6559775cd71e418b12a49145e59faa7:
>>
>>   nbd: assert that Error** is not NULL in nbd_iter_channel_error (2019-12-16 20:50:16 +0100)
>>
>> ----------------------------------------------------------------
>> Error reporting patches for 2019-12-16
>>
>> ----------------------------------------------------------------
>
> This gets conflicts:
> diff --cc target/ppc/kvm.c
> index 7406d18945,27ea3ce535..0000000000
> --- a/target/ppc/kvm.c
> +++ b/target/ppc/kvm.c
> @@@ -2076,7 -2076,7 +2076,11 @@@ int kvmppc_set_smt_threads(int smt
>       return ret;
>   }
>
> ++<<<<<<< HEAD
>  +void kvmppc_error_append_smt_possible_hint(Error **errp_in)
> ++=======
> + void kvmppc_error_append_smt_possible_hint(Error *const *errp)
> ++>>>>>>> remotes/armbru/tags/pull-error-2019-12-16
>   {
>       int i;
>       GString *g;
> diff --cc target/ppc/kvm_ppc.h
> index 47b08a4030,f22daabf51..0000000000
> --- a/target/ppc/kvm_ppc.h
> +++ b/target/ppc/kvm_ppc.h
> @@@ -28,7 -28,7 +28,11 @@@ void kvmppc_set_papr(PowerPCCPU *cpu)
>   int kvmppc_set_compat(PowerPCCPU *cpu, uint32_t compat_pvr);
>   void kvmppc_set_mpic_proxy(PowerPCCPU *cpu, int mpic_proxy);
>   int kvmppc_smt_threads(void);
> ++<<<<<<< HEAD
>  +void kvmppc_error_append_smt_possible_hint(Error **errp_in);
> ++=======
> + void kvmppc_error_append_smt_possible_hint(Error *const *errp);
> ++>>>>>>> remotes/armbru/tags/pull-error-2019-12-16
>   int kvmppc_set_smt_threads(int smt);
>   int kvmppc_clear_tsr_bits(PowerPCCPU *cpu, uint32_t tsr_bits);
>   int kvmppc_or_tsr_bits(PowerPCCPU *cpu, uint32_t tsr_bits);
> @@@ -164,7 -164,7 +168,11 @@@ static inline int kvmppc_smt_threads(vo
>       return 1;
>   }
>
> ++<<<<<<< HEAD
>  +static inline void kvmppc_error_append_smt_possible_hint(Error **errp_in)
> ++=======
> + static inline void kvmppc_error_append_smt_possible_hint(Error *const *errp)
> ++>>>>>>> remotes/armbru/tags/pull-error-2019-12-16
>   {
>       return;
>   }
>
> Furthermore, it turns out that the conflicts are due to
> different patches from the same author to the same function
> ("ppc: well form kvmppc_hint_smt_possible error hint helper"
> and "ppc: make Error **errp const where it is appropriate")
> which both seem to be addressing broadly the same thing
> but conflict with each other and arrived via different
> pull requests.
>
> So I'm just bouncing this one back for you to fix and
> figure out which version you want...

You got an outdated version of the patch via David's pull request.
Happens.  I'll fix things up and repost.

Re: [PULL 34/34] nbd: assert that Error** is not NULL in nbd_iter_channel_error

[Markus Armbruster]

Markus Armbruster <armbru@redhat.com> writes:

> Eric Blake <eblake@redhat.com> writes:
>
>> On 12/17/19 12:26 AM, Markus Armbruster wrote:
>>> From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>>>
>>> All callers of nbd_iter_channel_error() pass the address of a
>>> local_err variable, and only call this function if an error has
>>> already occurred, using this function to propagate that error.
>>> This is already implied by its name (local_err instead of the classic
>>> errp), but it is worth additionally stressing this by adding an
>>> assertion to make it part of the function contract.
>>>
>>> The local_err parameter is not here to return information about
>>> nbd_iter_channel_error failure. Instead it's assumed to be filled when
>>> passed to the function. This is already stressed by its name
>>> (local_err, instead of classic errp). Stress it additionally by
>>> assertion.
>>
>> Redundant paragraph, but probably too late to worry about it now that
>> we have a pull request.
>
> I'll have to respin anyway.  I'll drop the second paragraph.

Rats, I forgot.