[PULL 05/13] hw/remote/vfio-user: Clean up error reporting

Markus Armbruster posted 13 patches 1 month, 2 weeks ago
Maintainers: Jonathan Cameron <jonathan.cameron@huawei.com>, Fan Ni <fan.ni@samsung.com>, "Michael S. Tsirkin" <mst@redhat.com>, "Alex Bennée" <alex.bennee@linaro.org>, Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>, Dmitry Osipenko <dmitry.osipenko@collabora.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>
There is a newer version of this series
[PULL 05/13] hw/remote/vfio-user: Clean up error reporting
Posted by Markus Armbruster 1 month, 2 weeks ago
VFU_OBJECT_ERROR() reports the error with error_setg(&error_abort,
...) when auto-shutdown is enabled, else with error_report().

Issues:

1. The error is serious enough to warrant aborting the process when
auto-shutdown is enabled, yet harmless enough to permit carrying on
when it's disabled.  This makes no sense to me.

2. Like assert(), &error_abort is strictly for programming errors.  Is
this one?  Vladimir Sementsov-Ogievskiy tells me it's not.  Should we
exit(1) instead?

3. qapi/error.h advises "don't error_setg(&error_abort, ...), use
assert()."

This patch addresses just 3.  It adds a FIXME comment for the other
two.

Cc: Jagannathan Raman <jag.raman@oracle.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-ID: <20250923091000.3180122-6-armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
[FIXME comment added, commit message adjusted accordingly]
Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
 hw/remote/vfio-user-obj.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/hw/remote/vfio-user-obj.c b/hw/remote/vfio-user-obj.c
index ea6165ebdc..216b4876e2 100644
--- a/hw/remote/vfio-user-obj.c
+++ b/hw/remote/vfio-user-obj.c
@@ -75,12 +75,17 @@ OBJECT_DECLARE_TYPE(VfuObject, VfuObjectClass, VFU_OBJECT)
  */
 #define VFU_OBJECT_ERROR(o, fmt, ...)                                     \
     {                                                                     \
+        error_report((fmt), ## __VA_ARGS__);                              \
         if (vfu_object_auto_shutdown()) {                                 \
-            error_setg(&error_abort, (fmt), ## __VA_ARGS__);              \
-        } else {                                                          \
-            error_report((fmt), ## __VA_ARGS__);                          \
+            /*                                                            \
+             * FIXME This looks inappropriate.  The error is serious      \
+             * enough programming error to warrant aborting the process   \
+             * when auto-shutdown is enabled, yet harmless enough to      \
+             * permit carrying on when it's disabled.  Makes no sense.    \
+             */                                                           \
+            abort();                                                      \
         }                                                                 \
-    }                                                                     \
+    }
 
 struct VfuObjectClass {
     ObjectClass parent_class;
-- 
2.49.0