[PATCH v2 06/12] net/slirp: Clean up error reporting

Markus Armbruster posted 12 patches 1 month, 4 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>, 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
[PATCH v2 06/12] net/slirp: Clean up error reporting
Posted by Markus Armbruster 1 month, 4 weeks ago
net_slirp_register_poll_sock() and net_slirp_unregister_poll_sock()
report WSAEventSelect() failure with error_setg(&error_warn, ...).
error_setg_win32(&error_warn, ...) is undesirable just like
error_setg(&error_fatal, ...) and error_setg(&error_abort, ...)  are.
Replace by warn_report().

The failures should probably be errors, but these functions implement
callbacks that cannot fail, exit(1) would be too harsh, and silent
failure we don't want.  Thus, warnings.

Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 net/slirp.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/slirp.c b/net/slirp.c
index 9657e86a84..62f2684609 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -262,7 +262,8 @@ static void net_slirp_register_poll_sock(slirp_os_socket fd, void *opaque)
     if (WSAEventSelect(fd, event_notifier_get_handle(&ctxt->notifier),
                        FD_READ | FD_ACCEPT | FD_CLOSE |
                        FD_CONNECT | FD_WRITE | FD_OOB) != 0) {
-        error_setg_win32(&error_warn, WSAGetLastError(), "failed to WSAEventSelect()");
+        warn_report("failed to WSAEventSelect(): %s",
+                    g_win32_error_message(WSAGetLastError()));
     }
 #endif
 }
@@ -271,7 +272,8 @@ static void net_slirp_unregister_poll_sock(slirp_os_socket fd, void *opaque)
 {
 #ifdef WIN32
     if (WSAEventSelect(fd, NULL, 0) != 0) {
-        error_setg_win32(&error_warn, WSAGetLastError(), "failed to WSAEventSelect()");
+        warn_report("failed to WSAEventSelect(): %s",
+                    g_win32_error_message(WSAGetLastError()));
     }
 #endif
 }
-- 
2.49.0


Re: [PATCH v2 06/12] net/slirp: Clean up error reporting
Posted by Akihiko Odaki 1 month, 3 weeks ago
On 2025/09/17 20:52, Markus Armbruster wrote:
> net_slirp_register_poll_sock() and net_slirp_unregister_poll_sock()
> report WSAEventSelect() failure with error_setg(&error_warn, ...).
> error_setg_win32(&error_warn, ...) is undesirable just like
> error_setg(&error_fatal, ...) and error_setg(&error_abort, ...)  are.
> Replace by warn_report().
> 
> The failures should probably be errors, but these functions implement
> callbacks that cannot fail, exit(1) would be too harsh, and silent
> failure we don't want.  Thus, warnings.
> 
> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   net/slirp.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/net/slirp.c b/net/slirp.c
> index 9657e86a84..62f2684609 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -262,7 +262,8 @@ static void net_slirp_register_poll_sock(slirp_os_socket fd, void *opaque)
>       if (WSAEventSelect(fd, event_notifier_get_handle(&ctxt->notifier),
>                          FD_READ | FD_ACCEPT | FD_CLOSE |
>                          FD_CONNECT | FD_WRITE | FD_OOB) != 0) {
> -        error_setg_win32(&error_warn, WSAGetLastError(), "failed to WSAEventSelect()");
> +        warn_report("failed to WSAEventSelect(): %s",
> +                    g_win32_error_message(WSAGetLastError()));

https://docs.gtk.org/glib-win32/func.error_message.html says:
 > The caller of the function takes ownership of the data, and is
 > responsible for freeing it.

>       }
>   #endif
>   }
> @@ -271,7 +272,8 @@ static void net_slirp_unregister_poll_sock(slirp_os_socket fd, void *opaque)
>   {
>   #ifdef WIN32
>       if (WSAEventSelect(fd, NULL, 0) != 0) {
> -        error_setg_win32(&error_warn, WSAGetLastError(), "failed to WSAEventSelect()");
> +        warn_report("failed to WSAEventSelect(): %s",
> +                    g_win32_error_message(WSAGetLastError()));
>       }
>   #endif
>   }


Re: [PATCH v2 06/12] net/slirp: Clean up error reporting
Posted by Markus Armbruster 1 month, 3 weeks ago
Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp> writes:

> On 2025/09/17 20:52, Markus Armbruster wrote:
>> net_slirp_register_poll_sock() and net_slirp_unregister_poll_sock()
>> report WSAEventSelect() failure with error_setg(&error_warn, ...).
>> error_setg_win32(&error_warn, ...) is undesirable just like
>> error_setg(&error_fatal, ...) and error_setg(&error_abort, ...)  are.
>> Replace by warn_report().
>> The failures should probably be errors, but these functions implement
>> callbacks that cannot fail, exit(1) would be too harsh, and silent
>> failure we don't want.  Thus, warnings.
>> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> ---
>>   net/slirp.c | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>> diff --git a/net/slirp.c b/net/slirp.c
>> index 9657e86a84..62f2684609 100644
>> --- a/net/slirp.c
>> +++ b/net/slirp.c
>> @@ -262,7 +262,8 @@ static void net_slirp_register_poll_sock(slirp_os_socket fd, void *opaque)
>>       if (WSAEventSelect(fd, event_notifier_get_handle(&ctxt->notifier),
>>                          FD_READ | FD_ACCEPT | FD_CLOSE |
>>                          FD_CONNECT | FD_WRITE | FD_OOB) != 0) {
>> -        error_setg_win32(&error_warn, WSAGetLastError(), "failed to WSAEventSelect()");
>> +        warn_report("failed to WSAEventSelect(): %s",
>> +                    g_win32_error_message(WSAGetLastError()));
>
> https://docs.gtk.org/glib-win32/func.error_message.html says:
>> The caller of the function takes ownership of the data, and is
>> responsible for freeing it.

I'll fix it.  Thanks!

>
>>       }
>>   #endif
>>   }
>> @@ -271,7 +272,8 @@ static void net_slirp_unregister_poll_sock(slirp_os_socket fd, void *opaque)
>>   {
>>   #ifdef WIN32
>>       if (WSAEventSelect(fd, NULL, 0) != 0) {
>> -        error_setg_win32(&error_warn, WSAGetLastError(), "failed to WSAEventSelect()");
>> +        warn_report("failed to WSAEventSelect(): %s",
>> +                    g_win32_error_message(WSAGetLastError()));
>>       }
>>   #endif
>>   }