[PATCH v3 00/13] Error reporting cleanup, a fix, and &error_warn removal

Markus Armbruster posted 13 patches 5 days, 5 hours ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20250923091000.3180122-1-armbru@redhat.com
Maintainers: Jonathan Cameron <jonathan.cameron@huawei.com>, Fan Ni <fan.ni@samsung.com>, "Alex Bennée" <alex.bennee@linaro.org>, Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>, Dmitry Osipenko <dmitry.osipenko@collabora.com>, "Michael S. Tsirkin" <mst@redhat.com>, Gustavo Romero <gustavo.romero@linaro.org>, Jason Wang <jasowang@redhat.com>, Elena Ufimtseva <elena.ufimtseva@oracle.com>, Jagannathan Raman <jag.raman@oracle.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Markus Armbruster <armbru@redhat.com>, Michael Roth <michael.roth@amd.com>, Stefan Weil <sw@weilnetz.de>, "Daniel P. Berrangé" <berrange@redhat.com>, Steve Sistare <steven.sistare@oracle.com>, Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>, "Dr. David Alan Gilbert" <dave@treblig.org>, Samuel Thibault <samuel.thibault@ens-lyon.org>, Richard Henderson <richard.henderson@linaro.org>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>, Fam Zheng <fam@euphon.net>
include/exec/gdbstub.h         |  3 ---
include/qapi/error.h           |  6 ------
include/system/os-win32.h      |  5 ++++-
hw/cxl/cxl-host.c              |  7 ++++---
hw/display/virtio-gpu.c        |  8 ++++++--
hw/misc/ivshmem-flat.c         |  8 ++++++--
hw/net/virtio-net.c            |  8 +++++++-
hw/remote/vfio-user-obj.c      |  9 +++------
io/channel-socket.c            |  4 ++--
io/channel-watch.c             |  6 +++---
migration/cpr.c                |  9 +++++----
monitor/hmp-cmds.c             |  7 ++++---
net/slirp.c                    |  9 +++++++--
tcg/region.c                   |  7 +++++--
tests/unit/test-error-report.c | 17 -----------------
ui/dbus-listener.c             | 22 +++++++++++++++-------
ui/gtk.c                       |  6 +++++-
ui/qemu-pixman.c               |  5 ++++-
ui/spice-core.c                |  6 ++++--
util/aio-win32.c               |  2 +-
util/error.c                   |  5 +----
util/oslib-win32.c             | 25 ++++++++++++++++++++-----
22 files changed, 106 insertions(+), 78 deletions(-)
[PATCH v3 00/13] Error reporting cleanup, a fix, and &error_warn removal
Posted by Markus Armbruster 5 days, 5 hours ago
>From qapi/error.h:

 * Please don't error_setg(&error_fatal, ...), use error_report() and
 * exit(), because that's more obvious.
 * Likewise, don't error_setg(&error_abort, ...), use assert().

Not mentioned, but just as undesirable: error_setg(&error_warn, ...).

This series eliminates such uses, and gets rid of &error_warn.
&error_warn has multiple issues and little use.  PATCH 12 has full
rationale.

A note on warnings: we don't use warnings much, and when we use them,
they're often pretty bad.  See my memo "Abuse of warnings for
unhandled errors and programming errors"
Message-ID: <87h5yijh3b.fsf@pond.sub.org>
https://lore.kernel.org/qemu-devel/87h5yijh3b.fsf@pond.sub.org/

v3:
* PATCH 02: Whitespace cleanup [Akihiko Odaki]
* PATCH 06+07: Memory leaks on error paths [Akihiko Odaki]
* PATCH 08+13: Rebase conflicts
* PATCH 12: New

v2:
* PATCH 03: Mention change of cxl_fmws_link() return value in commit
  message [Jonathan]
* PATCH 04: Change exit(1) to g_assert_not_reached(), because it's a
  programming error.
* PATCH 06+07: Replace questions in commit message by answers from
  review.
* PATCH 06: Fix a format string
* PATCH 08: Keep warnings instead of reverting to silence [Daniel]
* PATCH 12: Adjusted for replaced PATCH 08

Issues raised in review I decided not to address in this series:
* PATCH 03: messages could be improved further, in particular the
  "gdbstub: " prefix could be dropped
* ebpf_rss_load() can return false without setting an error
* Capture the discussion on how to deal with undhandled errors in
  cover letter and/or commit messages.

The first two could be done on top.

Markus Armbruster (13):
  monitor: Clean up HMP gdbserver error reporting
  tcg: Fix error reporting on mprotect() failure in  tcg_region_init()
  hw/cxl: Convert cxl_fmws_link() to Error
  migration/cpr: Clean up error reporting in cpr_resave_fd()
  hw/remote/vfio-user: Clean up error reporting
  net/slirp: Clean up error reporting
  ui/spice-core: Clean up error reporting
  util/oslib-win32: Do not treat null @errp as &error_warn
  ui/pixman: Consistent error handling in qemu_pixman_shareable_free()
  ui/dbus: Clean up dbus_update_gl_cb() error checking
  ui/dbus: Consistent handling of texture mutex failure
  ivshmem-flat: Mark an instance of missing error handling FIXME
  error: Kill @error_warn

 include/exec/gdbstub.h         |  3 ---
 include/qapi/error.h           |  6 ------
 include/system/os-win32.h      |  5 ++++-
 hw/cxl/cxl-host.c              |  7 ++++---
 hw/display/virtio-gpu.c        |  8 ++++++--
 hw/misc/ivshmem-flat.c         |  8 ++++++--
 hw/net/virtio-net.c            |  8 +++++++-
 hw/remote/vfio-user-obj.c      |  9 +++------
 io/channel-socket.c            |  4 ++--
 io/channel-watch.c             |  6 +++---
 migration/cpr.c                |  9 +++++----
 monitor/hmp-cmds.c             |  7 ++++---
 net/slirp.c                    |  9 +++++++--
 tcg/region.c                   |  7 +++++--
 tests/unit/test-error-report.c | 17 -----------------
 ui/dbus-listener.c             | 22 +++++++++++++++-------
 ui/gtk.c                       |  6 +++++-
 ui/qemu-pixman.c               |  5 ++++-
 ui/spice-core.c                |  6 ++++--
 util/aio-win32.c               |  2 +-
 util/error.c                   |  5 +----
 util/oslib-win32.c             | 25 ++++++++++++++++++++-----
 22 files changed, 106 insertions(+), 78 deletions(-)

-- 
2.49.0
Re: [PATCH v3 00/13] Error reporting cleanup, a fix, and &error_warn removal
Posted by Akihiko Odaki 4 days, 13 hours ago
On 2025/09/23 18:09, Markus Armbruster wrote:
>>From qapi/error.h:
> 
>   * Please don't error_setg(&error_fatal, ...), use error_report() and
>   * exit(), because that's more obvious.
>   * Likewise, don't error_setg(&error_abort, ...), use assert().
> 
> Not mentioned, but just as undesirable: error_setg(&error_warn, ...).
> 
> This series eliminates such uses, and gets rid of &error_warn.
> &error_warn has multiple issues and little use.  PATCH 12 has full
> rationale.
> 
> A note on warnings: we don't use warnings much, and when we use them,
> they're often pretty bad.  See my memo "Abuse of warnings for
> unhandled errors and programming errors"
> Message-ID: <87h5yijh3b.fsf@pond.sub.org>
> https://lore.kernel.org/qemu-devel/87h5yijh3b.fsf@pond.sub.org/
> 
> v3:
> * PATCH 02: Whitespace cleanup [Akihiko Odaki]
> * PATCH 06+07: Memory leaks on error paths [Akihiko Odaki]
> * PATCH 08+13: Rebase conflicts
> * PATCH 12: New
> 
> v2:
> * PATCH 03: Mention change of cxl_fmws_link() return value in commit
>    message [Jonathan]
> * PATCH 04: Change exit(1) to g_assert_not_reached(), because it's a
>    programming error.
> * PATCH 06+07: Replace questions in commit message by answers from
>    review.
> * PATCH 06: Fix a format string
> * PATCH 08: Keep warnings instead of reverting to silence [Daniel]
> * PATCH 12: Adjusted for replaced PATCH 08
> 
> Issues raised in review I decided not to address in this series:
> * PATCH 03: messages could be improved further, in particular the
>    "gdbstub: " prefix could be dropped
> * ebpf_rss_load() can return false without setting an error
> * Capture the discussion on how to deal with undhandled errors in
>    cover letter and/or commit messages.
> 
> The first two could be done on top.
> 
> Markus Armbruster (13):
>    monitor: Clean up HMP gdbserver error reporting
>    tcg: Fix error reporting on mprotect() failure in  tcg_region_init()
>    hw/cxl: Convert cxl_fmws_link() to Error
>    migration/cpr: Clean up error reporting in cpr_resave_fd()
>    hw/remote/vfio-user: Clean up error reporting
>    net/slirp: Clean up error reporting
>    ui/spice-core: Clean up error reporting
>    util/oslib-win32: Do not treat null @errp as &error_warn
>    ui/pixman: Consistent error handling in qemu_pixman_shareable_free()
>    ui/dbus: Clean up dbus_update_gl_cb() error checking
>    ui/dbus: Consistent handling of texture mutex failure
>    ivshmem-flat: Mark an instance of missing error handling FIXME
>    error: Kill @error_warn
> 
>   include/exec/gdbstub.h         |  3 ---
>   include/qapi/error.h           |  6 ------
>   include/system/os-win32.h      |  5 ++++-
>   hw/cxl/cxl-host.c              |  7 ++++---
>   hw/display/virtio-gpu.c        |  8 ++++++--
>   hw/misc/ivshmem-flat.c         |  8 ++++++--
>   hw/net/virtio-net.c            |  8 +++++++-
>   hw/remote/vfio-user-obj.c      |  9 +++------
>   io/channel-socket.c            |  4 ++--
>   io/channel-watch.c             |  6 +++---
>   migration/cpr.c                |  9 +++++----
>   monitor/hmp-cmds.c             |  7 ++++---
>   net/slirp.c                    |  9 +++++++--
>   tcg/region.c                   |  7 +++++--
>   tests/unit/test-error-report.c | 17 -----------------
>   ui/dbus-listener.c             | 22 +++++++++++++++-------
>   ui/gtk.c                       |  6 +++++-
>   ui/qemu-pixman.c               |  5 ++++-
>   ui/spice-core.c                |  6 ++++--
>   util/aio-win32.c               |  2 +-
>   util/error.c                   |  5 +----
>   util/oslib-win32.c             | 25 ++++++++++++++++++++-----
>   22 files changed, 106 insertions(+), 78 deletions(-)
> 

For the whole series:

Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>