All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Richard W.M. Jones" <rjones@redhat.com>
To: Eric Blake <eblake@redhat.com>
Cc: berrange@redhat.com, qemu-devel@nongnu.org,
	qemu-block@nongnu.org, Gerd Hoffmann <kraxel@redhat.com>
Subject: Re: [PATCH] sockets: Use SOMAXCONN for Unix socket listen()
Date: Fri, 5 Feb 2021 10:01:22 +0000	[thread overview]
Message-ID: <20210205100122.GY30079@redhat.com> (raw)
In-Reply-To: <20210204222018.1432848-1-eblake@redhat.com>

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



      parent reply	other threads:[~2021-02-05 10:04 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-04 22:20 [PATCH] sockets: Use SOMAXCONN for Unix socket listen() Eric Blake
2021-02-05  9:55 ` Daniel P. Berrangé
2021-02-05 19:04   ` Eric Blake
2021-02-05 10:01 ` Richard W.M. Jones [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210205100122.GY30079@redhat.com \
    --to=rjones@redhat.com \
    --cc=berrange@redhat.com \
    --cc=eblake@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.