Re: [PATCH] sockets: update SOCKET_ADDRESS_TYPE_FD listen(2) backlog

["Richard W.M. Jones" <rjones@redhat.com>]

On Tue, May 11, 2021 at 09:23:10AM +0100, Stefan Hajnoczi wrote:
> On Tue, Mar 16, 2021 at 08:35:14AM -0500, Eric Blake wrote:
> > On 3/16/21 4:10 AM, Stefan Hajnoczi wrote:
> > > On Wed, Mar 10, 2021 at 05:30:04PM +0000, Stefan Hajnoczi wrote:
> > >> socket_get_fd() fails with the error "socket_get_fd: too many
> > >> connections" if the given listen backlog value is not 1.
> > >>
> > >> Not all callers set the backlog to 1. For example, commit
> > >> 582d4210eb2f2ab5baac328fe4b479cd86da1647 ("qemu-nbd: Use SOMAXCONN for
> > >> socket listen() backlog") uses SOMAXCONN. This will always fail with in
> > >> socket_get_fd().
> > >>
> > >> This patch calls listen(2) on the fd to update the backlog value. The
> > >> socket may already be in the listen state. I have tested that this works
> > >> on Linux 5.10 and macOS Catalina.
> > >>
> > >> As a bonus this allows us to detect when the fd cannot listen. Now we'll
> > >> be able to catch unbound or connected fds in socket_listen().
> > >>
> > >> Drop the num argument from socket_get_fd() since this function is also
> > >> called by socket_connect() where a listen backlog value does not make
> > >> sense.
> > >>
> > >> Fixes: e5b6353cf25c99c3f08bf51e29933352f7140e8f ("socket: Add backlog parameter to socket_listen")
> > >> Reported-by: Richard W.M. Jones <rjones@redhat.com>
> > >> Cc: Juan Quintela <quintela@redhat.com>
> > >> Cc: Eric Blake <eblake@redhat.com>
> > >> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> > >> ---
> > >>  util/qemu-sockets.c | 29 ++++++++++++++++++++++-------
> > >>  1 file changed, 22 insertions(+), 7 deletions(-)
> > > 
> > > Dan and Gerd: Can this go via one of your trees?
> > > 
> > 
> > As it showed up as a regression in qemu-nbd, I can also consider queuing
> > it in my NBD tree.  However, I claim it counts as a bug fix, so it is
> > fine for -rc1 even if it misses soft freeze.
> > 
> > I'm fine whichever maintainer takes this, although I've now flagged it
> > to go through an NBD pull request if it doesn't land elsewhere sooner.
> 
> Ping? I didn't see this land in qemu.git.

And a second reminder.  qemu-storage-daemon is broken in the released
qemu 6.0.0 so it'd be good to get this commit into the stable branch
as well.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html