[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH 00/17] Fix usage of error_append_hint()
From: |
Greg Kurz |
Subject: |
[Qemu-devel] [PATCH 00/17] Fix usage of error_append_hint() |
Date: |
Tue, 17 Sep 2019 12:20:30 +0200 |
User-agent: |
StGit/unknown-version |
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(-)
- [Qemu-devel] [PATCH 00/17] Fix usage of error_append_hint(),
Greg Kurz <=
Re: [Qemu-devel] [PATCH 02/17] block: Pass local error object pointer to error_append_hint(), Eric Blake, 2019/09/17