[PULL 0/1] NBD patches for 2021-05-11

[PULL 0/1] NBD patches for 2021-05-11

[Eric Blake]

The following changes since commit f9a576a818044133f8564e0d243ebd97df0b3280:

  Merge remote-tracking branch 'remotes/dgilbert-gitlab/tags/pull-virtiofs-20210506' into staging (2021-05-11 13:03:44 +0100)

are available in the Git repository at:

  https://repo.or.cz/qemu/ericb.git tags/pull-nbd-2021-05-11

for you to fetch changes up to 37179e9ea45d6428b29ae789209c119ac18c1d39:

  sockets: update SOCKET_ADDRESS_TYPE_FD listen(2) backlog (2021-05-11 12:43:26 -0500)

----------------------------------------------------------------
nbd patches for 2021-05-11

- fix fd passing to qemu-storage-daemon --nbd-server

----------------------------------------------------------------
Stefan Hajnoczi (1):
      sockets: update SOCKET_ADDRESS_TYPE_FD listen(2) backlog

 util/qemu-sockets.c | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

-- 
2.31.1

[PULL 1/1] sockets: update SOCKET_ADDRESS_TYPE_FD listen(2) backlog

[Eric Blake]

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

Re: [PULL 0/1] NBD patches for 2021-05-11

[Philippe Mathieu-Daudé]

Hi Eric,

On 5/11/21 9:28 PM, Eric Blake wrote:
> The following changes since commit f9a576a818044133f8564e0d243ebd97df0b3280:
> 
>   Merge remote-tracking branch 'remotes/dgilbert-gitlab/tags/pull-virtiofs-20210506' into staging (2021-05-11 13:03:44 +0100)
> 
> are available in the Git repository at:
> 
>   https://repo.or.cz/qemu/ericb.git tags/pull-nbd-2021-05-11
> 
> for you to fetch changes up to 37179e9ea45d6428b29ae789209c119ac18c1d39:
> 
>   sockets: update SOCKET_ADDRESS_TYPE_FD listen(2) backlog (2021-05-11 12:43:26 -0500)
> 
> ----------------------------------------------------------------
> nbd patches for 2021-05-11
> 
> - fix fd passing to qemu-storage-daemon --nbd-server
> 
> ----------------------------------------------------------------
> Stefan Hajnoczi (1):
>       sockets: update SOCKET_ADDRESS_TYPE_FD listen(2) backlog

Richard suggested to add the following tag to the commit if possible:
"Resolves: https://gitlab.com/qemu-project/qemu/-/issues/218"

https://lists.gnu.org/archive/html/qemu-devel/2021-05/msg03107.html

Re: [PULL 0/1] NBD patches for 2021-05-11

[Peter Maydell]

On Wed, 12 May 2021 at 04:43, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> Hi Eric,
>
> On 5/11/21 9:28 PM, Eric Blake wrote:
> > The following changes since commit f9a576a818044133f8564e0d243ebd97df0b3280:
> >
> >   Merge remote-tracking branch 'remotes/dgilbert-gitlab/tags/pull-virtiofs-20210506' into staging (2021-05-11 13:03:44 +0100)
> >
> > are available in the Git repository at:
> >
> >   https://repo.or.cz/qemu/ericb.git tags/pull-nbd-2021-05-11
> >
> > for you to fetch changes up to 37179e9ea45d6428b29ae789209c119ac18c1d39:
> >
> >   sockets: update SOCKET_ADDRESS_TYPE_FD listen(2) backlog (2021-05-11 12:43:26 -0500)
> >
> > ----------------------------------------------------------------
> > nbd patches for 2021-05-11
> >
> > - fix fd passing to qemu-storage-daemon --nbd-server
> >
> > ----------------------------------------------------------------
> > Stefan Hajnoczi (1):
> >       sockets: update SOCKET_ADDRESS_TYPE_FD listen(2) backlog
>
> Richard suggested to add the following tag to the commit if possible:
> "Resolves: https://gitlab.com/qemu-project/qemu/-/issues/218"
>
> https://lists.gnu.org/archive/html/qemu-devel/2021-05/msg03107.html

I left the pullreq unapplied for a while to give Eric the opportunity
to add that tag, but it's been 9 days now, so I'm going to go ahead
and apply it anyway.

-- PMM

Re: [PULL 0/1] NBD patches for 2021-05-11

[Peter Maydell]

On Tue, 11 May 2021 at 20:30, Eric Blake <eblake@redhat.com> wrote:
>
> The following changes since commit f9a576a818044133f8564e0d243ebd97df0b3280:
>
>   Merge remote-tracking branch 'remotes/dgilbert-gitlab/tags/pull-virtiofs-20210506' into staging (2021-05-11 13:03:44 +0100)
>
> are available in the Git repository at:
>
>   https://repo.or.cz/qemu/ericb.git tags/pull-nbd-2021-05-11
>
> for you to fetch changes up to 37179e9ea45d6428b29ae789209c119ac18c1d39:
>
>   sockets: update SOCKET_ADDRESS_TYPE_FD listen(2) backlog (2021-05-11 12:43:26 -0500)
>
> ----------------------------------------------------------------
> nbd patches for 2021-05-11
>
> - fix fd passing to qemu-storage-daemon --nbd-server
>
> ----------------------------------------------------------------
> Stefan Hajnoczi (1):
>       sockets: update SOCKET_ADDRESS_TYPE_FD listen(2) backlog
>
>  util/qemu-sockets.c | 29 ++++++++++++++++++++++-------
>  1 file changed, 22 insertions(+), 7 deletions(-)


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/6.1
for any user-visible changes.

-- PMM

Re: [PULL 0/1] NBD patches for 2021-05-11

[Eric Blake]

On Fri, May 21, 2021 at 09:55:38AM +0100, Peter Maydell wrote:
> > Richard suggested to add the following tag to the commit if possible:
> > "Resolves: https://gitlab.com/qemu-project/qemu/-/issues/218"
> >
> > https://lists.gnu.org/archive/html/qemu-devel/2021-05/msg03107.html
> 
> I left the pullreq unapplied for a while to give Eric the opportunity
> to add that tag, but it's been 9 days now, so I'm going to go ahead
> and apply it anyway.

Apologies that I did not respond in time; I've been fighting email
battles where I had some downtime with an inability to reply or even
read in a timely manner, but as evidenced by this mail, I should
finally be back online.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org