error: Kill @error_warn

We added @error_warn some two years ago in commit 3ffef1a55ca (error:
add global &error_warn destination).  It has multiple issues:

* error.h's big comment was not updated for it.

* Function contracts were not updated for it.

* ERRP_GUARD() is unaware of @error_warn, and fails to mask it from
  error_prepend() and such.  These crash on @error_warn, as pointed
  out by Akihiko Odaki.

All fixable.  However, after more than two years, we had just of 15
uses, of which the last few patches removed seven as unclean or
otherwise undesirable, adding back five elsewhere.  I didn't look
closely enough at the remaining seven to decide whether they are
desirable or not.

I don't think this feature earns its keep.  Drop it.

Thanks-to: Akihiko  Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Message-ID: <20250923091000.3180122-14-armbru@redhat.com>
Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>

diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 0a1a625b0ea6cf26cb0d799171a57ed3d3ab2442..de3590221308d5f98826a5ef93f2f0d5fe187842 100644 (file)
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -242,6 +242,7 @@ static uint32_t calc_image_hostmem(pixman_format_code_t pformat,
 static void virtio_gpu_resource_create_2d(VirtIOGPU *g,
                                           struct virtio_gpu_ctrl_command *cmd)
 {
+    Error *err = NULL;
     pixman_format_code_t pformat;
     struct virtio_gpu_simple_resource *res;
     struct virtio_gpu_resource_create_2d c2d;
@@ -293,7 +294,8 @@ static void virtio_gpu_resource_create_2d(VirtIOGPU *g,
                 c2d.width,
                 c2d.height,
                 c2d.height ? res->hostmem / c2d.height : 0,
-                &error_warn)) {
+                &err)) {
+            warn_report_err(err);
             goto end;
         }
     }
@@ -1282,6 +1284,7 @@ static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size,
                            const VMStateField *field)
 {
     VirtIOGPU *g = opaque;
+    Error *err = NULL;
     struct virtio_gpu_simple_resource *res;
     uint32_t resource_id, pformat;
     int i;
@@ -1317,7 +1320,8 @@ static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size,
                                              res->width,
                                              res->height,
                                              res->height ? res->hostmem / res->height : 0,
-                                             &error_warn)) {
+                                             &err)) {
+            warn_report_err(err);
             g_free(res);
             return -EINVAL;
         }

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 6b5b5dace334af12e9b77a8a2765c88443cee235..7848e26278adadfd293fbbb9b0b2ff7c95f61d55 100644 (file)
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1289,6 +1289,8 @@ exit:
 
 static bool virtio_net_load_ebpf(VirtIONet *n, Error **errp)
 {
+    Error *err = NULL;
+
     if (!virtio_net_attach_ebpf_to_backend(n->nic, -1)) {
         return true;
     }
@@ -1306,7 +1308,11 @@ static bool virtio_net_load_ebpf(VirtIONet *n, Error **errp)
         return virtio_net_load_ebpf_fds(n, errp);
     }
 
-    ebpf_rss_load(&n->ebpf_rss, &error_warn);
+    ebpf_rss_load(&n->ebpf_rss, &err);
+    /* Beware, ebpf_rss_load() can return false with @err unset */
+    if (err) {
+        warn_report_err(err);
+    }
     return true;
 }
 

diff --git a/include/qapi/error.h b/include/qapi/error.h
index 41e38163804904874e5d114b699a3e2a93d3fc6d..b16c6303f83eec134c5be858bff001d99891fdf0 100644 (file)
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -533,12 +533,6 @@ static inline void error_propagator_cleanup(ErrorPropagator *prop)
 
 G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(ErrorPropagator, error_propagator_cleanup);
 
-/*
- * Special error destination to warn on error.
- * See error_setg() and error_propagate() for details.
- */
-extern Error *error_warn;
-
 /*
  * Special error destination to abort on error.
  * See error_setg() and error_propagate() for details.

diff --git a/include/system/os-win32.h b/include/system/os-win32.h
index 3aa6cee4c232cc1ebebee0a1aba485cc81cce5bb..22d72babdf40724c4d184e48474251cd6b019c79 100644 (file)
--- a/include/system/os-win32.h
+++ b/include/system/os-win32.h
@@ -168,11 +168,14 @@ static inline void qemu_funlockfile(FILE *f)
 #endif
 }
 
-/* Helper for WSAEventSelect, to report errors */
+/* Helpers for WSAEventSelect() */
 bool qemu_socket_select(int sockfd, WSAEVENT hEventObject,
                         long lNetworkEvents, Error **errp);
+void qemu_socket_select_nofail(int sockfd, WSAEVENT hEventObject,
+                               long lNetworkEvents);
 
 bool qemu_socket_unselect(int sockfd, Error **errp);
+void qemu_socket_unselect_nofail(int sockfd);
 
 /* We wrap all the sockets functions so that we can set errno based on
  * WSAGetLastError(), and use file-descriptors instead of SOCKET.

diff --git a/io/channel-socket.c b/io/channel-socket.c
index fdc767086753c45b369eb846ced3ec74ea4dfa60..712b793eaf2cd1caf53c6aa2ab6176cfd57d6bc3 100644 (file)
--- a/io/channel-socket.c
+++ b/io/channel-socket.c
@@ -454,7 +454,7 @@ static void qio_channel_socket_finalize(Object *obj)
             }
         }
 #ifdef WIN32
-        qemu_socket_unselect(ioc->fd, &error_warn);
+        qemu_socket_unselect_nofail(ioc->fd);
 #endif
         close(ioc->fd);
         ioc->fd = -1;
@@ -929,7 +929,7 @@ qio_channel_socket_close(QIOChannel *ioc,
 
     if (sioc->fd != -1) {
 #ifdef WIN32
-        qemu_socket_unselect(sioc->fd, &error_warn);
+        qemu_socket_unselect_nofail(sioc->fd);
 #endif
         if (qio_channel_has_feature(ioc, QIO_CHANNEL_FEATURE_LISTEN)) {
             socket_listen_cleanup(sioc->fd, errp);

diff --git a/io/channel-watch.c b/io/channel-watch.c
index ec76bd1ec6a17d489f093bffe89b2ebca31d6f6d..018648b36b4cf66e387a6486680bb1cf27806a33 100644 (file)
--- a/io/channel-watch.c
+++ b/io/channel-watch.c
@@ -281,9 +281,9 @@ GSource *qio_channel_create_socket_watch(QIOChannel *ioc,
     GSource *source;
     QIOChannelSocketSource *ssource;
 
-    qemu_socket_select(sockfd, ioc->event,
-                       FD_READ | FD_ACCEPT | FD_CLOSE |
-                       FD_CONNECT | FD_WRITE | FD_OOB, &error_warn);
+    qemu_socket_select_nofail(sockfd, ioc->event,
+                              FD_READ | FD_ACCEPT | FD_CLOSE |
+                              FD_CONNECT | FD_WRITE | FD_OOB);
 
     source = g_source_new(&qio_channel_socket_source_funcs,
                           sizeof(QIOChannelSocketSource));

diff --git a/tests/unit/test-error-report.c b/tests/unit/test-error-report.c
index 54319c86c927f172c3efbdba1efe559fbab9beb3..0cbde3c4cf5f4ef24a835704a37a3ec0b3cfedd5 100644 (file)
--- a/tests/unit/test-error-report.c
+++ b/tests/unit/test-error-report.c
@@ -104,22 +104,6 @@ test_error_report_timestamp(void)
 ");
 }
 
-static void
-test_error_warn(void)
-{
-    if (g_test_subprocess()) {
-        error_setg(&error_warn, "Testing &error_warn");
-        return;
-    }
-
-    g_test_trap_subprocess(NULL, 0, 0);
-    g_test_trap_assert_passed();
-    g_test_trap_assert_stderr("\
-test-error-report: warning: Testing &error_warn*\
-");
-}
-
-
 int
 main(int argc, char *argv[])
 {
@@ -133,7 +117,6 @@ main(int argc, char *argv[])
     g_test_add_func("/error-report/glog", test_error_report_glog);
     g_test_add_func("/error-report/once", test_error_report_once);
     g_test_add_func("/error-report/timestamp", test_error_report_timestamp);
-    g_test_add_func("/error-report/warn", test_error_warn);
 
     return g_test_run();
 }

diff --git a/ui/gtk.c b/ui/gtk.c
index e91d093a49e5d05c10f1dbea5b9b94ff1389b456..9a08cadc88ffb713f1c53e3219ae7f8ea6181332 100644 (file)
--- a/ui/gtk.c
+++ b/ui/gtk.c
@@ -1181,6 +1181,7 @@ static gboolean gd_touch_event(GtkWidget *widget, GdkEventTouch *touch,
                                void *opaque)
 {
     VirtualConsole *vc = opaque;
+    Error *err = NULL;
     uint64_t num_slot = GPOINTER_TO_UINT(touch->sequence);
     int type = -1;
 
@@ -1203,7 +1204,10 @@ static gboolean gd_touch_event(GtkWidget *widget, GdkEventTouch *touch,
     console_handle_touch_event(vc->gfx.dcl.con, touch_slots,
                                num_slot, surface_width(vc->gfx.ds),
                                surface_height(vc->gfx.ds), touch->x,
-                               touch->y, type, &error_warn);
+                               touch->y, type, &err);
+    if (err) {
+        warn_report_err(err);
+    }
     return TRUE;
 }
 

diff --git a/util/aio-win32.c b/util/aio-win32.c
index b125924433fa2c3bbe7a5938ae9e7311d48b5837..c6fbce64c2cb8180852d7041e2e8d75f28d0ccac 100644 (file)
--- a/util/aio-win32.c
+++ b/util/aio-win32.c
@@ -121,7 +121,7 @@ void aio_set_fd_handler(AioContext *ctx,
 
         QLIST_INSERT_HEAD_RCU(&ctx->aio_handlers, node, node);
         event = event_notifier_get_handle(&ctx->notifier);
-        qemu_socket_select(fd, event, bitmask, &error_warn);
+        qemu_socket_select_nofail(fd, event, bitmask);
     }
     if (old_node) {
         aio_remove_fd_handler(ctx, old_node);

diff --git a/util/error.c b/util/error.c
index daea2142f30121abc46e5342526f5eec40ea246e..0ae08225c095a90da846410b61cf1ed87be8acb8 100644 (file)
--- a/util/error.c
+++ b/util/error.c
@@ -19,7 +19,6 @@
 
 Error *error_abort;
 Error *error_fatal;
-Error *error_warn;
 
 static void error_handle(Error **errp, Error *err)
 {
@@ -41,9 +40,7 @@ static void error_handle(Error **errp, Error *err)
         error_report_err(err);
         exit(1);
     }
-    if (errp == &error_warn) {
-        warn_report_err(err);
-    } else if (errp && !*errp) {
+    if (errp && !*errp) {
         *errp = err;
     } else {
         error_free(err);

diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index 6a2367c89defc98a6e1b1ce94c670cc259fbf6e1..84bc65a76575b399a8c5de8d1faf8e59014f1e36 100644 (file)
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -182,7 +182,7 @@ bool qemu_set_blocking(int fd, bool block, Error **errp)
     unsigned long opt = block ? 0 : 1;
 
     if (block) {
-        qemu_socket_unselect(fd, &error_warn);
+        qemu_socket_unselect_nofail(fd);
     }
 
     if (ioctlsocket(fd, FIONBIO, &opt) != NO_ERROR) {
@@ -311,6 +311,25 @@ bool qemu_socket_unselect(int sockfd, Error **errp)
     return qemu_socket_select(sockfd, NULL, 0, errp);
 }
 
+void qemu_socket_select_nofail(int sockfd, WSAEVENT hEventObject,
+                               long lNetworkEvents)
+{
+    Error *err = NULL;
+
+    if (!qemu_socket_select(sockfd, hEventObject, lNetworkEvents, &err)) {
+        warn_report_err(err);
+    }
+}
+
+void qemu_socket_unselect_nofail(int sockfd)
+{
+    Error *err = NULL;
+
+    if (!qemu_socket_unselect(sockfd, &err)) {
+        warn_report_err(err);
+    }
+}
+
 int qemu_socketpair(int domain, int type, int protocol, int sv[2])
 {
     struct sockaddr_un addr = {