Daniel P. Berrangé <[email protected]> writes: > On Fri, Aug 08, 2025 at 10:08:17AM +0200, Markus Armbruster wrote: >> net_slirp_register_poll_sock() and net_slirp_unregister_poll_sock() >> report WSAEventSelect() failure with error_setg(&error_warn, ...). >> >> I'm not familiar with liblirp, so I can't say whether the network > > ^^^^^^^^^ 'libslirp'
Will fix, thanks! >> backend will work after such a failure. If it doesn't, then this >> should be an error. If it does, then why bother the user with a >> warning that isn't actionable, and likely confusing? >> >> Regardless of that, error_setg_win32(&error_warn, ...) is undesirable >> just like error_setg(&error_fatal, ...) and error_setg(&error_abort, >> ...) are. Replace by warn_report(). >> >> Cc: Marc-André Lureau <[email protected]> >> Signed-off-by: Markus Armbruster <[email protected]> >> --- >> net/slirp.c | 6 ++++-- >> 1 file changed, 4 insertions(+), 2 deletions(-) > > Reviewed-by: Daniel P. Berrangé <[email protected]> > >> diff --git a/net/slirp.c b/net/slirp.c >> index 9657e86a84..d75b09f16b 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 > > IMHO this one ought to be considered fatal. If we can't select the > right events on the socket, then we're not going to have a good time > trying to poll on events. The libslirp callback API doesn't allow > us to return a success/failure code from this function, and IMHO it > is not appropriate to use error_fatal here because a fault with slirp > should not take down the whole of QEMU. So warn_report is the least > worst option I guess. At least it is a hint to the user that all is > not well - even if they can't action it, it might alert them if they > see network problems in their guest. So, we'd make this an error if we could. But we can't: the function is a callback that cannot fail, and outright exit(1) is too harsh. That leaves silence or warning. Warning is less bad. Correct? > >> } >> @@ -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()", >> + g_win32_error_message(WSAGetLastError())); >> } > > This one is reasonable to treat as non-fatal, since once we've > unregistered the socket for polling Whether clearing the event associated with the socket can fail is unclear. Whether it should be treated as an error is also unclear. If yes, then same argument as for net_slirp_register_poll_sock() above. If no, silence or warning. Warning is less bad. Correct? >> #endif >> } [...]
