On Tue, Nov 04, 2025 at 12:08:42PM +0100, Kevin Wolf wrote: > Am 03.11.2025 um 21:10 hat Eric Blake geschrieben: > > Upcoming patches will adjust how net_listener watches for new client > > connections; adding trace points now makes it easier to debug that the > > changes work as intended. For example, adding > > --trace='qio_net_listener*' to the qemu-storage-daemon command line > > before --nbd-server will track when the server first starts listening > > for clients. > > > > Signed-off-by: Eric Blake <[email protected]> > > --- > > io/net-listener.c | 17 +++++++++++++++++ > > io/trace-events | 5 +++++ > > 2 files changed, 22 insertions(+) > > > > diff --git a/io/net-listener.c b/io/net-listener.c > > index 47405965a66..0adbc409cf2 100644 > > --- a/io/net-listener.c > > +++ b/io/net-listener.c > > @@ -23,6 +23,7 @@ > > #include "io/dns-resolver.h" > > #include "qapi/error.h" > > #include "qemu/module.h" > > +#include "trace.h" > > > > QIONetListener *qio_net_listener_new(void) > > { > > @@ -50,6 +51,7 @@ static gboolean qio_net_listener_channel_func(QIOChannel > > *ioc, > > return TRUE; > > } > > > > + trace_qio_net_listener_callback(listener, listener->io_func); > > if (listener->io_func) { > > listener->io_func(listener, sioc, listener->io_data); > > } > > Not necessarily a problem, but I wonder why you decided to have the > trace point unconditional here... > > > @@ -143,6 +147,9 @@ void > > qio_net_listener_set_client_func_full(QIONetListener *listener, > > { > > size_t i; > > > > + if (listener->io_func) { > > + trace_qio_net_listener_watch_disabled(listener, "set_client_func"); > > + } > > if (listener->io_notify) { > > listener->io_notify(listener->io_data); > > } > > ...while everywhere else you only call it with a non-NULL > listener->io_func.
In the former, the trace is printing out the address of which io_func (including NULL) is currently registered when the callback is reached. In the case where a single NetListener registers more than one socket address, but the user uninstalls their callback after the first client to connect, it is still possible that other pending connections have still raced, at which point the qio_net_listener_channel_func can still fire on those later connections even though there is no longer an io_func from the user. In all the latter cases, I was merely tracing when state transitioned between the user installing a handler or removing one. Unless you think it would be to noisy, I'm game for changing this in v2 to have all of the traces be unconditional. It is potentially noisier, but also would aid in spotting how many times a client requests no change to the current io_func or lack thereof. Also worth thinking about: in this patch, I added new traces under the names enabled/disabled, with the trace points being reacable from multiple locations. Then in 4/8, when refactoring to consolidate duplicate code, the trace points are reduced to a single usage in functions named watch/unwatch. It may be worth rethinking naming to have the tracepoint and function names share the same terminology. -- Eric Blake, Principal Software Engineer Red Hat, Inc. Virtualization: qemu.org | libguestfs.org
