util/qemu-sockets.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Our default of a backlog of 1 connection is rather puny, particularly
for scenarios where we expect multiple listeners to connect (such as
qemu-nbd -e X). For Unix sockets, there's no real harm in supporting
a larger backlog, and a definite benefit to the clients: at least on
Linux, a client trying to connect to a Unix socket with a backlog gets
an EAGAIN failure with no way to poll() for when the backlog is no
longer present short of sleeping an arbitrary amount of time before
retrying.
See https://bugzilla.redhat.com/1925045 for a demonstration of where
our low backlog prevents libnbd from connecting as many parallel
clients as it wants.
Reported-by: Richard W.M. Jones <rjones@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
util/qemu-sockets.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 8af0278f15c6..a7573e9f0fda 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -1059,7 +1059,7 @@ int unix_listen(const char *str, Error **errp)
saddr = g_new0(UnixSocketAddress, 1);
saddr->path = g_strdup(str);
- sock = unix_listen_saddr(saddr, 1, errp);
+ sock = unix_listen_saddr(saddr, SOMAXCONN, errp);
qapi_free_UnixSocketAddress(saddr);
return sock;
}
--
2.30.0
On Thu, Feb 04, 2021 at 04:20:18PM -0600, Eric Blake wrote: > Our default of a backlog of 1 connection is rather puny, particularly > for scenarios where we expect multiple listeners to connect (such as > qemu-nbd -e X). For Unix sockets, there's no real harm in supporting > a larger backlog, and a definite benefit to the clients: at least on > Linux, a client trying to connect to a Unix socket with a backlog gets > an EAGAIN failure with no way to poll() for when the backlog is no > longer present short of sleeping an arbitrary amount of time before > retrying. > > See https://bugzilla.redhat.com/1925045 for a demonstration of where > our low backlog prevents libnbd from connecting as many parallel > clients as it wants. > > Reported-by: Richard W.M. Jones <rjones@redhat.com> > Signed-off-by: Eric Blake <eblake@redhat.com> > --- > util/qemu-sockets.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c > index 8af0278f15c6..a7573e9f0fda 100644 > --- a/util/qemu-sockets.c > +++ b/util/qemu-sockets.c > @@ -1059,7 +1059,7 @@ int unix_listen(const char *str, Error **errp) > > saddr = g_new0(UnixSocketAddress, 1); > saddr->path = g_strdup(str); > - sock = unix_listen_saddr(saddr, 1, errp); > + sock = unix_listen_saddr(saddr, SOMAXCONN, errp); > qapi_free_UnixSocketAddress(saddr); > return sock; > } This method is a legacy back compat function, only used by the QEMU guest agent, so this can't explain the NBD problems, which use the QIONetListener class. IOW, the problem is in the qemu-nbd.c / blockdev-nbd.c code I believe Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 2/5/21 3:55 AM, Daniel P. Berrangé wrote: >> +++ b/util/qemu-sockets.c >> @@ -1059,7 +1059,7 @@ int unix_listen(const char *str, Error **errp) >> >> saddr = g_new0(UnixSocketAddress, 1); >> saddr->path = g_strdup(str); >> - sock = unix_listen_saddr(saddr, 1, errp); >> + sock = unix_listen_saddr(saddr, SOMAXCONN, errp); >> qapi_free_UnixSocketAddress(saddr); >> return sock; >> } > > This method is a legacy back compat function, only used by the QEMU > guest agent, so this can't explain the NBD problems, which use the > QIONetListener class. > > IOW, the problem is in the qemu-nbd.c / blockdev-nbd.c code I believe D'oh. Serves me right for trying to guess the spot using just a grep on listen() rather than running under gdb with a breakpoint to find the actual backtrace. v2 posted with a slightly changed subject line, and this time tested to actually work. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
On Thu, Feb 04, 2021 at 04:20:18PM -0600, Eric Blake wrote: > Our default of a backlog of 1 connection is rather puny, particularly > for scenarios where we expect multiple listeners to connect (such as > qemu-nbd -e X). For Unix sockets, there's no real harm in supporting > a larger backlog, and a definite benefit to the clients: at least on > Linux, a client trying to connect to a Unix socket with a backlog gets > an EAGAIN failure with no way to poll() for when the backlog is no > longer present short of sleeping an arbitrary amount of time before > retrying. > > See https://bugzilla.redhat.com/1925045 for a demonstration of where > our low backlog prevents libnbd from connecting as many parallel > clients as it wants. > > Reported-by: Richard W.M. Jones <rjones@redhat.com> > Signed-off-by: Eric Blake <eblake@redhat.com> > --- > util/qemu-sockets.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c > index 8af0278f15c6..a7573e9f0fda 100644 > --- a/util/qemu-sockets.c > +++ b/util/qemu-sockets.c > @@ -1059,7 +1059,7 @@ int unix_listen(const char *str, Error **errp) > > saddr = g_new0(UnixSocketAddress, 1); > saddr->path = g_strdup(str); > - sock = unix_listen_saddr(saddr, 1, errp); > + sock = unix_listen_saddr(saddr, SOMAXCONN, errp); > qapi_free_UnixSocketAddress(saddr); > return sock; > } Does this fix the problem with qemu-nbd? Not for me - I still seem to see the old behaviour with this patch applied. Also looking more generally at util/qemu-sockets.c I think it would be better if all of those "num" parameters were renamed "backlog". Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
© 2016 - 2024 Red Hat, Inc.