[Qemu-devel] [PATCH 00/17] Fix usage of error_append_hint()

[Greg Kurz]

A recent post unveiled a potential problem when passing errp to
error_append_hint():

https://patchwork.ozlabs.org/patch/1162171/

The issue is that error_append_hint() may end up not being called
at all if errp is equal to &error_fatal or &error_abort. It turns
out that the only way to ensure error_append_hint() can do its job
regardless of how the error is handled is to use a local error
object and to propagate it to the caller.

As suggested by Markus Armbruster, this series does:
(1) update error_append_hint()'s documentation in <qapi/error.h> to
    spell this out
(2) fix other misuses of error_append_hint() in the tree

The following command was used to detect candidates for (2), which
were then manually inspected. It is possible that many of them never
exhibit the unwanted behaviour in practice because errp happens to
never be equal to &error_fatal or &error_abort. It isn't trivial to
assess though, and this is fragile and can be easily broken if the
code gets rearranged. Also, passing errp directly is a shortcut that
doesn't bring much, neither for clarity, nor for performance that
we don't really care for on an error path). Better safe than sorry,
so fix them all.

I tried to group the changes that are supposed to go through the same
tree together.

$ git grep -n error_append_hint\(errp | cut -d: -f 1-2
block/backup.c:608
block/dirty-bitmap.c:256
block/file-posix.c:393
block/file-posix.c:854
block/file-posix.c:2282
block/gluster.c:480
block/gluster.c:483
block/gluster.c:489
block/gluster.c:702
block/gluster.c:711
block/qcow.c:162
block/qcow.c:195
block/qcow2.c:1363
block/vhdx-log.c:811
block/vpc.c:1033

=> Patch for block

chardev/spice.c:284

=> Patch for chardev

hw/9pfs/9p-local.c:1474
hw/9pfs/9p-proxy.c:1119

Already posted https://patchwork.ozlabs.org/patch/1162171/

hw/arm/msf2-soc.c:131
hw/arm/virt.c:1808
hw/arm/virt.c:1836
hw/intc/arm_gicv3_kvm.c:815
hw/misc/msf2-sysreg.c:135

=> Patch for arm

hw/pci-bridge/pcie_root_port.c:76
hw/pci-bridge/pcie_root_port.c:90

=> Patch for pci

hw/ppc/mac_newworld.c:622
hw/ppc/spapr.c:4341
hw/ppc/spapr_pci.c:1874
hw/ppc/spapr_pci.c:1876
hw/ppc/spapr_pci.c:1878

=> Patch for ppc

hw/rdma/vmw/pvrdma_main.c:670

=> Patch for pvrdma

hw/s390x/s390-ccw.c:63

=> Patch for s390

hw/scsi/scsi-disk.c:2624
hw/scsi/scsi-generic.c:679

=> Patch for scsi

hw/usb/ccid-card-emulated.c:516

=> Patch for usb

hw/vfio/common.c:1473
hw/vfio/common.c:1536
hw/vfio/pci.c:2532
hw/vfio/pci.c:2718

=> Patch for vfio

hw/virtio/virtio-pci.c:1548
hw/virtio/virtio-pci.c:1742

=> Patch for virtio

migration/migration.c:988
migration/migration.c:997

=> Patch for migration

nbd/client.c:230
nbd/server.c:1627

=> Patch for nbd

qdev-monitor.c:334
qdev-monitor.c:337
qdev-monitor.c:340
qdev-monitor.c:348
qdev-monitor.c:351
qdev-monitor.c:354
qdev-monitor.c:358

No misuse here as these aren't preceded by a call to error_setg() or
error_propagate() that could terminate QEMU.

target/ppc/kvm.c:246
target/ppc/kvm.c:2089
target/ppc/kvm.c:2092

=> Patch for kvm

util/qemu-option.c:160
util/qemu-option.c:669

=> Patch for option

util/qemu-sockets.c:886
util/qemu-sockets.c:956

=> Patch for sockets

As a bonus, also teach checkpatch to detect that. Since the convention
of using the errp naming seems to be strictly followed, the check is
just to detect "error_append_hint(errp" and emit a warning, for the sake
of simplicity.

--
Greg

---

Greg Kurz (17):
      error: Update error_append_hint()'s documentation
      block: Pass local error object pointer to error_append_hint()
      char/spice: Pass local error object pointer to error_append_hint()
      ppc: Pass local error object pointer to error_append_hint()
      arm: Pass local error object pointer to error_append_hint()
      vfio: Pass local error object pointer to error_append_hint()
      virtio-pci: Pass local error object pointer to error_append_hint()
      pcie_root_port: Pass local error object pointer to error_append_hint()
      hw/rdma: Fix missing conversion to rdma_error_report()
      s390x/css: Pass local error object pointer to error_append_hint()
      scsi: Pass local error object pointer to error_append_hint()
      migration: Pass local error object pointer to error_append_hint()
      nbd: Pass local error object pointer to error_append_hint()
      ccid-card-emul: Pass local error object pointer to error_append_hint()
      option: Pass local error object pointer to error_append_hint()
      socket: Pass local error object pointer to error_append_hint()
      checkpatch: Warn when errp is passed to error_append_hint()


 block/backup.c                 |    7 +++++--
 block/dirty-bitmap.c           |    7 +++++--
 block/file-posix.c             |   20 +++++++++++++-------
 block/gluster.c                |   23 +++++++++++++++--------
 block/qcow.c                   |   10 ++++++----
 block/qcow2.c                  |    7 +++++--
 block/vhdx-log.c               |    7 +++++--
 block/vpc.c                    |    7 +++++--
 chardev/spice.c                |    6 ++++--
 hw/arm/msf2-soc.c              |    5 +++--
 hw/arm/virt.c                  |   14 ++++++++++----
 hw/intc/arm_gicv3_kvm.c        |    5 +++--
 hw/misc/msf2-sysreg.c          |    6 ++++--
 hw/pci-bridge/pcie_root_port.c |   11 +++++++----
 hw/ppc/mac_newworld.c          |    7 +++++--
 hw/ppc/spapr.c                 |    7 +++++--
 hw/ppc/spapr_pci.c             |    9 +++++----
 hw/rdma/vmw/pvrdma_main.c      |    2 +-
 hw/s390x/s390-ccw.c            |    6 ++++--
 hw/scsi/scsi-disk.c            |    7 +++++--
 hw/scsi/scsi-generic.c         |    7 +++++--
 hw/usb/ccid-card-emulated.c    |    7 +++++--
 hw/vfio/common.c               |   14 ++++++++++----
 hw/vfio/pci.c                  |   12 ++++++++----
 hw/virtio/virtio-pci.c         |   14 ++++++++++----
 include/qapi/error.h           |   11 ++++++++++-
 migration/migration.c          |   14 ++++++++++----
 nbd/client.c                   |   24 +++++++++++++-----------
 nbd/server.c                   |    7 +++++--
 scripts/checkpatch.pl          |    4 ++++
 target/ppc/kvm.c               |   13 +++++++++----
 util/qemu-option.c             |   14 ++++++++++----
 util/qemu-sockets.c            |   14 ++++++++++----
 33 files changed, 224 insertions(+), 104 deletions(-)