On Tue, Feb 09, 2021 at 09:27:58AM -0600, Eric Blake wrote: > Our default of a backlog of 1 connection is rather puny; it gets in > the way when we are explicitly allowing multiple clients (such as > qemu-nbd -e N [--shared], or nbd-server-start with its default > "max-connections":0 for unlimited), but is even a problem when we > stick to qemu-nbd's default of only 1 active client but use -t > [--persistent] where a second client can start using the server once > the first finishes. While the effects are less noticeable on TCP > sockets (since the client can poll() to learn when the server is ready > again), it is definitely observable on Unix sockets, where on Unix, a > client will fail with EAGAIN and no recourse but to sleep an arbitrary > amount of time before retrying if the server backlog is already full. > > Since QMP nbd-server-start is always persistent, it now always > requests a backlog of SOMAXCONN; meanwhile, qemu-nbd will request > SOMAXCONN if persistent, otherwise its backlog should be based on the > expected number of clients. > > 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 <rjo...@redhat.com> > Signed-off-by: Eric Blake <ebl...@redhat.com> > CC: qemu-sta...@nongnu.org > --- > blockdev-nbd.c | 7 ++++++- > qemu-nbd.c | 10 +++++++++- > 2 files changed, 15 insertions(+), 2 deletions(-) > > diff --git a/blockdev-nbd.c b/blockdev-nbd.c > index d8443d235b73..b264620b98d8 100644 > --- a/blockdev-nbd.c > +++ b/blockdev-nbd.c > @@ -134,7 +134,12 @@ void nbd_server_start(SocketAddress *addr, const char > *tls_creds, > qio_net_listener_set_name(nbd_server->listener, > "nbd-listener"); > > - if (qio_net_listener_open_sync(nbd_server->listener, addr, 1, errp) < 0) > { > + /* > + * Because this server is persistent, a backlog of SOMAXCONN is > + * better than trying to size it to max_connections. > + */ > + if (qio_net_listener_open_sync(nbd_server->listener, addr, SOMAXCONN, > + errp) < 0) { > goto error; > } > > diff --git a/qemu-nbd.c b/qemu-nbd.c > index 608c63e82a25..1a340ea4858d 100644 > --- a/qemu-nbd.c > +++ b/qemu-nbd.c > @@ -964,8 +964,16 @@ int main(int argc, char **argv) > > server = qio_net_listener_new(); > if (socket_activation == 0) { > + int backlog; > + > + if (persistent) { > + backlog = SOMAXCONN; > + } else { > + backlog = MIN(shared, SOMAXCONN); > + } > saddr = nbd_build_socket_address(sockpath, bindto, port); > - if (qio_net_listener_open_sync(server, saddr, 1, &local_err) < 0) { > + if (qio_net_listener_open_sync(server, saddr, backlog, > + &local_err) < 0) { > object_unref(OBJECT(server)); > error_report_err(local_err); > exit(EXIT_FAILURE);
Works fine here, so: Tested-by: Richard W.M. Jones <rjo...@redhat.com> Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW
