All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Blake <eblake@redhat.com>
To: qemu-devel@nongnu.org
Cc: "Daniel P. Berrangé" <berrange@redhat.com>,
	"Juan Quintela" <quintela@redhat.com>,
	qemu-stable@nongnu.org,
	"Richard W . M . Jones" <rjones@redhat.com>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Stefan Hajnoczi" <stefanha@redhat.com>,
	"Stefano Garzarella" <sgarzare@redhat.com>
Subject: [PULL 1/1] sockets: update SOCKET_ADDRESS_TYPE_FD listen(2) backlog
Date: Tue, 11 May 2021 14:28:02 -0500	[thread overview]
Message-ID: <20210511192802.552706-2-eblake@redhat.com> (raw)
In-Reply-To: <20210511192802.552706-1-eblake@redhat.com>

From: Stefan Hajnoczi <stefanha@redhat.com>

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>
Message-Id: <20210310173004.420190-1-stefanha@redhat.com>
Tested-by: Richard W.M. Jones <rjones@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 util/qemu-sockets.c | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 8af0278f15c6..2463c49773ea 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -1116,14 +1116,10 @@ fail:
     return NULL;
 }

-static int socket_get_fd(const char *fdstr, int num, Error **errp)
+static int socket_get_fd(const char *fdstr, Error **errp)
 {
     Monitor *cur_mon = monitor_cur();
     int fd;
-    if (num != 1) {
-        error_setg_errno(errp, EINVAL, "socket_get_fd: too many connections");
-        return -1;
-    }
     if (cur_mon) {
         fd = monitor_get_fd(cur_mon, fdstr, errp);
         if (fd < 0) {
@@ -1159,7 +1155,7 @@ int socket_connect(SocketAddress *addr, Error **errp)
         break;

     case SOCKET_ADDRESS_TYPE_FD:
-        fd = socket_get_fd(addr->u.fd.str, 1, errp);
+        fd = socket_get_fd(addr->u.fd.str, errp);
         break;

     case SOCKET_ADDRESS_TYPE_VSOCK:
@@ -1187,7 +1183,26 @@ int socket_listen(SocketAddress *addr, int num, Error **errp)
         break;

     case SOCKET_ADDRESS_TYPE_FD:
-        fd = socket_get_fd(addr->u.fd.str, num, errp);
+        fd = socket_get_fd(addr->u.fd.str, errp);
+        if (fd < 0) {
+            return -1;
+        }
+
+        /*
+         * If the socket is not yet in the listen state, then transition it to
+         * the listen state now.
+         *
+         * If it's already listening then this updates the backlog value as
+         * requested.
+         *
+         * If this socket cannot listen because it's already in another state
+         * (e.g. unbound or connected) then we'll catch the error here.
+         */
+        if (listen(fd, num) != 0) {
+            error_setg_errno(errp, errno, "Failed to listen on fd socket");
+            closesocket(fd);
+            return -1;
+        }
         break;

     case SOCKET_ADDRESS_TYPE_VSOCK:
-- 
2.31.1



  reply	other threads:[~2021-05-11 19:29 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-11 19:28 [PULL 0/1] NBD patches for 2021-05-11 Eric Blake
2021-05-11 19:28 ` Eric Blake [this message]
2021-05-12  3:42 ` Philippe Mathieu-Daudé
2021-05-21  8:55   ` Peter Maydell
2021-05-24 19:30     ` Eric Blake
2021-05-21 11:02 ` Peter Maydell

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=20210511192802.552706-2-eblake@redhat.com \
    --to=eblake@redhat.com \
    --cc=berrange@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-stable@nongnu.org \
    --cc=quintela@redhat.com \
    --cc=rjones@redhat.com \
    --cc=sgarzare@redhat.com \
    --cc=stefanha@redhat.com \
    /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.