[Qemu-devel] [PATCH v3 0/5] Fix multifd with big number of channels

[Qemu-devel] [PATCH v3 0/5] Fix multifd with big number of channels

[Juan Quintela]

Hi

In this version:
- updated to latest upstream
- moved trace to suggested position (danp)

Please review.

Later, Juan.


Juan Quintela (5):
  socket: Add backlog parameter to socket_listen
  socket: Add num connections to qio_channel_socket_sync()
  socket: Add num connections to qio_channel_socket_async()
  socket: Add num connections to qio_net_listener_open_sync()
  multifd: Use number of channels as listen backlog

 blockdev-nbd.c                 |  2 +-
 chardev/char-socket.c          |  2 +-
 include/io/channel-socket.h    |  4 ++++
 include/io/net-listener.h      |  2 ++
 include/qemu/sockets.h         |  2 +-
 io/channel-socket.c            | 35 +++++++++++++++++++++++++---------
 io/net-listener.c              |  3 ++-
 io/trace-events                |  4 ++--
 migration/socket.c             |  7 ++++++-
 qemu-nbd.c                     |  2 +-
 qga/channel-posix.c            |  2 +-
 scsi/qemu-pr-helper.c          |  3 ++-
 tests/test-char.c              |  4 ++--
 tests/test-io-channel-socket.c |  4 ++--
 tests/test-util-sockets.c      | 12 ++++++------
 tests/tpm-emu.c                |  2 +-
 ui/vnc.c                       |  4 ++--
 util/qemu-sockets.c            | 33 +++++++++++++++++++++-----------
 util/trace-events              |  3 +++
 19 files changed, 87 insertions(+), 43 deletions(-)

-- 
2.21.0

[Qemu-devel] [PATCH v3 1/5] socket: Add backlog parameter to socket_listen

[Juan Quintela]

Current parameter was always one.  We continue with that value for now
in all callers.

Signed-off-by: Juan Quintela <quintela@redhat.com>

---
Moved trace to socket_listen
---
 include/qemu/sockets.h    |  2 +-
 io/channel-socket.c       |  2 +-
 qga/channel-posix.c       |  2 +-
 tests/test-util-sockets.c | 12 ++++++------
 util/qemu-sockets.c       | 33 ++++++++++++++++++++++-----------
 util/trace-events         |  3 +++
 6 files changed, 34 insertions(+), 20 deletions(-)

diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index 8140fea685..57cd049d6e 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -41,7 +41,7 @@ int unix_connect(const char *path, Error **errp);
 
 SocketAddress *socket_parse(const char *str, Error **errp);
 int socket_connect(SocketAddress *addr, Error **errp);
-int socket_listen(SocketAddress *addr, Error **errp);
+int socket_listen(SocketAddress *addr, int num, Error **errp);
 void socket_listen_cleanup(int fd, Error **errp);
 int socket_dgram(SocketAddress *remote, SocketAddress *local, Error **errp);
 
diff --git a/io/channel-socket.c b/io/channel-socket.c
index bec3d931d1..a533c8bc11 100644
--- a/io/channel-socket.c
+++ b/io/channel-socket.c
@@ -202,7 +202,7 @@ int qio_channel_socket_listen_sync(QIOChannelSocket *ioc,
     int fd;
 
     trace_qio_channel_socket_listen_sync(ioc, addr);
-    fd = socket_listen(addr, errp);
+    fd = socket_listen(addr, 1, errp);
     if (fd < 0) {
         trace_qio_channel_socket_listen_fail(ioc);
         return -1;
diff --git a/qga/channel-posix.c b/qga/channel-posix.c
index 5a925a9818..8fc205ad21 100644
--- a/qga/channel-posix.c
+++ b/qga/channel-posix.c
@@ -215,7 +215,7 @@ static gboolean ga_channel_open(GAChannel *c, const gchar *path,
                 return false;
             }
 
-            fd = socket_listen(addr, &local_err);
+            fd = socket_listen(addr, 1, &local_err);
             qapi_free_SocketAddress(addr);
             if (local_err != NULL) {
                 g_critical("%s", error_get_pretty(local_err));
diff --git a/tests/test-util-sockets.c b/tests/test-util-sockets.c
index f1ebffee5a..c8e1893c11 100644
--- a/tests/test-util-sockets.c
+++ b/tests/test-util-sockets.c
@@ -93,7 +93,7 @@ static void test_socket_fd_pass_name_good(void)
     g_assert_cmpint(fd, !=, mon_fd);
     close(fd);
 
-    fd = socket_listen(&addr, &error_abort);
+    fd = socket_listen(&addr, 1, &error_abort);
     g_assert_cmpint(fd, !=, -1);
     g_assert_cmpint(fd, !=, mon_fd);
     close(fd);
@@ -124,7 +124,7 @@ static void test_socket_fd_pass_name_bad(void)
     g_assert_cmpint(fd, ==, -1);
     error_free_or_abort(&err);
 
-    fd = socket_listen(&addr, &err);
+    fd = socket_listen(&addr, 1, &err);
     g_assert_cmpint(fd, ==, -1);
     error_free_or_abort(&err);
 
@@ -151,7 +151,7 @@ static void test_socket_fd_pass_name_nomon(void)
     g_assert_cmpint(fd, ==, -1);
     error_free_or_abort(&err);
 
-    fd = socket_listen(&addr, &err);
+    fd = socket_listen(&addr, 1, &err);
     g_assert_cmpint(fd, ==, -1);
     error_free_or_abort(&err);
 
@@ -174,7 +174,7 @@ static void test_socket_fd_pass_num_good(void)
     fd = socket_connect(&addr, &error_abort);
     g_assert_cmpint(fd, ==, sfd);
 
-    fd = socket_listen(&addr, &error_abort);
+    fd = socket_listen(&addr, 1, &error_abort);
     g_assert_cmpint(fd, ==, sfd);
 
     g_free(addr.u.fd.str);
@@ -197,7 +197,7 @@ static void test_socket_fd_pass_num_bad(void)
     g_assert_cmpint(fd, ==, -1);
     error_free_or_abort(&err);
 
-    fd = socket_listen(&addr, &err);
+    fd = socket_listen(&addr, 1, &err);
     g_assert_cmpint(fd, ==, -1);
     error_free_or_abort(&err);
 
@@ -220,7 +220,7 @@ static void test_socket_fd_pass_num_nocli(void)
     g_assert_cmpint(fd, ==, -1);
     error_free_or_abort(&err);
 
-    fd = socket_listen(&addr, &err);
+    fd = socket_listen(&addr, 1, &err);
     g_assert_cmpint(fd, ==, -1);
     error_free_or_abort(&err);
 
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index e3a1666578..98ff3a1cce 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -31,6 +31,7 @@
 #include "qapi/qobject-input-visitor.h"
 #include "qapi/qobject-output-visitor.h"
 #include "qemu/cutils.h"
+#include "trace.h"
 
 #ifndef AI_ADDRCONFIG
 # define AI_ADDRCONFIG 0
@@ -207,6 +208,7 @@ static int try_bind(int socket, InetSocketAddress *saddr, struct addrinfo *e)
 
 static int inet_listen_saddr(InetSocketAddress *saddr,
                              int port_offset,
+                             int num,
                              Error **errp)
 {
     struct addrinfo ai,*res,*e;
@@ -309,7 +311,7 @@ static int inet_listen_saddr(InetSocketAddress *saddr,
                     goto listen_failed;
                 }
             } else {
-                if (!listen(slisten, 1)) {
+                if (!listen(slisten, num)) {
                     goto listen_ok;
                 }
                 if (errno != EADDRINUSE) {
@@ -774,6 +776,7 @@ static int vsock_connect_saddr(VsockSocketAddress *vaddr, Error **errp)
 }
 
 static int vsock_listen_saddr(VsockSocketAddress *vaddr,
+                              int num,
                               Error **errp)
 {
     struct sockaddr_vm svm;
@@ -795,7 +798,7 @@ static int vsock_listen_saddr(VsockSocketAddress *vaddr,
         return -1;
     }
 
-    if (listen(slisten, 1) != 0) {
+    if (listen(slisten, num) != 0) {
         error_setg_errno(errp, errno, "Failed to listen on socket");
         closesocket(slisten);
         return -1;
@@ -836,6 +839,7 @@ static int vsock_connect_saddr(VsockSocketAddress *vaddr, Error **errp)
 }
 
 static int vsock_listen_saddr(VsockSocketAddress *vaddr,
+                              int num,
                               Error **errp)
 {
     vsock_unsupported(errp);
@@ -853,6 +857,7 @@ static int vsock_parse(VsockSocketAddress *addr, const char *str,
 #ifndef _WIN32
 
 static int unix_listen_saddr(UnixSocketAddress *saddr,
+                             int num,
                              Error **errp)
 {
     struct sockaddr_un un;
@@ -914,7 +919,7 @@ static int unix_listen_saddr(UnixSocketAddress *saddr,
         error_setg_errno(errp, errno, "Failed to bind socket to %s", path);
         goto err;
     }
-    if (listen(sock, 1) < 0) {
+    if (listen(sock, num) < 0) {
         error_setg_errno(errp, errno, "Failed to listen on socket");
         goto err;
     }
@@ -981,6 +986,7 @@ static int unix_connect_saddr(UnixSocketAddress *saddr, Error **errp)
 #else
 
 static int unix_listen_saddr(UnixSocketAddress *saddr,
+                             int num,
                              Error **errp)
 {
     error_setg(errp, "unix sockets are not available on windows");
@@ -1004,7 +1010,7 @@ int unix_listen(const char *str, Error **errp)
 
     saddr = g_new0(UnixSocketAddress, 1);
     saddr->path = g_strdup(str);
-    sock = unix_listen_saddr(saddr, errp);
+    sock = unix_listen_saddr(saddr, 1, errp);
     qapi_free_UnixSocketAddress(saddr);
     return sock;
 }
@@ -1061,9 +1067,13 @@ fail:
     return NULL;
 }
 
-static int socket_get_fd(const char *fdstr, Error **errp)
+static int socket_get_fd(const char *fdstr, int num, Error **errp)
 {
     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) {
@@ -1099,7 +1109,7 @@ int socket_connect(SocketAddress *addr, Error **errp)
         break;
 
     case SOCKET_ADDRESS_TYPE_FD:
-        fd = socket_get_fd(addr->u.fd.str, errp);
+        fd = socket_get_fd(addr->u.fd.str, 1, errp);
         break;
 
     case SOCKET_ADDRESS_TYPE_VSOCK:
@@ -1112,25 +1122,26 @@ int socket_connect(SocketAddress *addr, Error **errp)
     return fd;
 }
 
-int socket_listen(SocketAddress *addr, Error **errp)
+int socket_listen(SocketAddress *addr, int num, Error **errp)
 {
     int fd;
 
+    trace_socket_listen(num);
     switch (addr->type) {
     case SOCKET_ADDRESS_TYPE_INET:
-        fd = inet_listen_saddr(&addr->u.inet, 0, errp);
+        fd = inet_listen_saddr(&addr->u.inet, 0, num, errp);
         break;
 
     case SOCKET_ADDRESS_TYPE_UNIX:
-        fd = unix_listen_saddr(&addr->u.q_unix, errp);
+        fd = unix_listen_saddr(&addr->u.q_unix, num, errp);
         break;
 
     case SOCKET_ADDRESS_TYPE_FD:
-        fd = socket_get_fd(addr->u.fd.str, errp);
+        fd = socket_get_fd(addr->u.fd.str, num, errp);
         break;
 
     case SOCKET_ADDRESS_TYPE_VSOCK:
-        fd = vsock_listen_saddr(&addr->u.vsock, errp);
+        fd = vsock_listen_saddr(&addr->u.vsock, num, errp);
         break;
 
     default:
diff --git a/util/trace-events b/util/trace-events
index 9dbd237dad..83b6639018 100644
--- a/util/trace-events
+++ b/util/trace-events
@@ -64,6 +64,9 @@ lockcnt_futex_wait(const void *lockcnt, int val) "lockcnt %p waiting on %d"
 lockcnt_futex_wait_resume(const void *lockcnt, int new) "lockcnt %p after wait: %d"
 lockcnt_futex_wake(const void *lockcnt) "lockcnt %p waking up one waiter"
 
+# qemu-sockets.c
+socket_listen(int num) "backlog: %d"
+
 # qemu-thread-common.h
 qemu_mutex_lock(void *mutex, const char *file, const int line) "waiting on mutex %p (%s:%d)"
 qemu_mutex_locked(void *mutex, const char *file, const int line) "taken mutex %p (%s:%d)"
-- 
2.21.0

[Qemu-devel] [PATCH v3 2/5] socket: Add num connections to qio_channel_socket_sync()

[Juan Quintela]

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 include/io/channel-socket.h    | 2 ++
 io/channel-socket.c            | 7 ++++---
 io/net-listener.c              | 2 +-
 io/trace-events                | 2 +-
 scsi/qemu-pr-helper.c          | 3 ++-
 tests/test-char.c              | 4 ++--
 tests/test-io-channel-socket.c | 2 +-
 tests/tpm-emu.c                | 2 +-
 8 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/include/io/channel-socket.h b/include/io/channel-socket.h
index d7134d2cd6..ed88e5b8c1 100644
--- a/include/io/channel-socket.h
+++ b/include/io/channel-socket.h
@@ -123,6 +123,7 @@ void qio_channel_socket_connect_async(QIOChannelSocket *ioc,
  * qio_channel_socket_listen_sync:
  * @ioc: the socket channel object
  * @addr: the address to listen to
+ * @num: the expected ammount of connections
  * @errp: pointer to a NULL-initialized error object
  *
  * Attempt to listen to the address @addr. This method
@@ -132,6 +133,7 @@ void qio_channel_socket_connect_async(QIOChannelSocket *ioc,
  */
 int qio_channel_socket_listen_sync(QIOChannelSocket *ioc,
                                    SocketAddress *addr,
+                                   int num,
                                    Error **errp);
 
 /**
diff --git a/io/channel-socket.c b/io/channel-socket.c
index a533c8bc11..6258c25983 100644
--- a/io/channel-socket.c
+++ b/io/channel-socket.c
@@ -197,12 +197,13 @@ void qio_channel_socket_connect_async(QIOChannelSocket *ioc,
 
 int qio_channel_socket_listen_sync(QIOChannelSocket *ioc,
                                    SocketAddress *addr,
+                                   int num,
                                    Error **errp)
 {
     int fd;
 
-    trace_qio_channel_socket_listen_sync(ioc, addr);
-    fd = socket_listen(addr, 1, errp);
+    trace_qio_channel_socket_listen_sync(ioc, addr, num);
+    fd = socket_listen(addr, num, errp);
     if (fd < 0) {
         trace_qio_channel_socket_listen_fail(ioc);
         return -1;
@@ -226,7 +227,7 @@ static void qio_channel_socket_listen_worker(QIOTask *task,
     SocketAddress *addr = opaque;
     Error *err = NULL;
 
-    qio_channel_socket_listen_sync(ioc, addr, &err);
+    qio_channel_socket_listen_sync(ioc, addr, 1, &err);
 
     qio_task_set_error(task, err);
 }
diff --git a/io/net-listener.c b/io/net-listener.c
index d8cfe52673..dc81150318 100644
--- a/io/net-listener.c
+++ b/io/net-listener.c
@@ -82,7 +82,7 @@ int qio_net_listener_open_sync(QIONetListener *listener,
     for (i = 0; i < nresaddrs; i++) {
         QIOChannelSocket *sioc = qio_channel_socket_new();
 
-        if (qio_channel_socket_listen_sync(sioc, resaddrs[i],
+        if (qio_channel_socket_listen_sync(sioc, resaddrs[i], 1,
                                            err ? NULL : &err) == 0) {
             success = true;
 
diff --git a/io/trace-events b/io/trace-events
index 378390521e..2e6aa1d749 100644
--- a/io/trace-events
+++ b/io/trace-events
@@ -17,7 +17,7 @@ qio_channel_socket_connect_sync(void *ioc, void *addr) "Socket connect sync ioc=
 qio_channel_socket_connect_async(void *ioc, void *addr) "Socket connect async ioc=%p addr=%p"
 qio_channel_socket_connect_fail(void *ioc) "Socket connect fail ioc=%p"
 qio_channel_socket_connect_complete(void *ioc, int fd) "Socket connect complete ioc=%p fd=%d"
-qio_channel_socket_listen_sync(void *ioc, void *addr) "Socket listen sync ioc=%p addr=%p"
+qio_channel_socket_listen_sync(void *ioc, void *addr, int num) "Socket listen sync ioc=%p addr=%p num=%d"
 qio_channel_socket_listen_async(void *ioc, void *addr) "Socket listen async ioc=%p addr=%p"
 qio_channel_socket_listen_fail(void *ioc) "Socket listen fail ioc=%p"
 qio_channel_socket_listen_complete(void *ioc, int fd) "Socket listen complete ioc=%p fd=%d"
diff --git a/scsi/qemu-pr-helper.c b/scsi/qemu-pr-helper.c
index a256ce490b..a8a74d1dba 100644
--- a/scsi/qemu-pr-helper.c
+++ b/scsi/qemu-pr-helper.c
@@ -1005,7 +1005,8 @@ int main(int argc, char **argv)
             .u.q_unix.path = socket_path,
         };
         server_ioc = qio_channel_socket_new();
-        if (qio_channel_socket_listen_sync(server_ioc, &saddr, &local_err) < 0) {
+        if (qio_channel_socket_listen_sync(server_ioc, &saddr,
+                                           1, &local_err) < 0) {
             object_unref(OBJECT(server_ioc));
             error_report_err(local_err);
             return 1;
diff --git a/tests/test-char.c b/tests/test-char.c
index f9440cdcfd..af131fc850 100644
--- a/tests/test-char.c
+++ b/tests/test-char.c
@@ -666,7 +666,7 @@ char_socket_addr_to_opt_str(SocketAddress *addr, bool fd_pass,
         char *optstr;
         g_assert(!reconnect);
         if (is_listen) {
-            qio_channel_socket_listen_sync(ioc, addr, &error_abort);
+            qio_channel_socket_listen_sync(ioc, addr, 1, &error_abort);
         } else {
             qio_channel_socket_connect_sync(ioc, addr, &error_abort);
         }
@@ -891,7 +891,7 @@ static void char_socket_client_test(gconstpointer opaque)
      */
     ioc = qio_channel_socket_new();
     g_assert_nonnull(ioc);
-    qio_channel_socket_listen_sync(ioc, config->addr, &error_abort);
+    qio_channel_socket_listen_sync(ioc, config->addr, 1, &error_abort);
     addr = qio_channel_socket_get_local_address(ioc, &error_abort);
     g_assert_nonnull(addr);
 
diff --git a/tests/test-io-channel-socket.c b/tests/test-io-channel-socket.c
index d2053c464c..6eebcee115 100644
--- a/tests/test-io-channel-socket.c
+++ b/tests/test-io-channel-socket.c
@@ -57,7 +57,7 @@ static void test_io_channel_setup_sync(SocketAddress *listen_addr,
     QIOChannelSocket *lioc;
 
     lioc = qio_channel_socket_new();
-    qio_channel_socket_listen_sync(lioc, listen_addr, &error_abort);
+    qio_channel_socket_listen_sync(lioc, listen_addr, 1, &error_abort);
 
     if (listen_addr->type == SOCKET_ADDRESS_TYPE_INET) {
         SocketAddress *laddr = qio_channel_socket_get_local_address(
diff --git a/tests/tpm-emu.c b/tests/tpm-emu.c
index 125e697181..c43ac4aef8 100644
--- a/tests/tpm-emu.c
+++ b/tests/tpm-emu.c
@@ -76,7 +76,7 @@ void *tpm_emu_ctrl_thread(void *data)
     QIOChannelSocket *lioc = qio_channel_socket_new();
     QIOChannel *ioc;
 
-    qio_channel_socket_listen_sync(lioc, s->addr, &error_abort);
+    qio_channel_socket_listen_sync(lioc, s->addr, 1, &error_abort);
 
     g_mutex_lock(&s->data_mutex);
     s->data_cond_signal = true;
-- 
2.21.0

[Qemu-devel] [PATCH v3 3/5] socket: Add num connections to qio_channel_socket_async()

[Juan Quintela]

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 include/io/channel-socket.h    |  2 ++
 io/channel-socket.c            | 30 +++++++++++++++++++++++-------
 io/trace-events                |  2 +-
 tests/test-io-channel-socket.c |  2 +-
 4 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/include/io/channel-socket.h b/include/io/channel-socket.h
index ed88e5b8c1..777ff5954e 100644
--- a/include/io/channel-socket.h
+++ b/include/io/channel-socket.h
@@ -140,6 +140,7 @@ int qio_channel_socket_listen_sync(QIOChannelSocket *ioc,
  * qio_channel_socket_listen_async:
  * @ioc: the socket channel object
  * @addr: the address to listen to
+ * @num: the expected ammount of connections
  * @callback: the function to invoke on completion
  * @opaque: user data to pass to @callback
  * @destroy: the function to free @opaque
@@ -155,6 +156,7 @@ int qio_channel_socket_listen_sync(QIOChannelSocket *ioc,
  */
 void qio_channel_socket_listen_async(QIOChannelSocket *ioc,
                                      SocketAddress *addr,
+                                     int num,
                                      QIOTaskFunc callback,
                                      gpointer opaque,
                                      GDestroyNotify destroy,
diff --git a/io/channel-socket.c b/io/channel-socket.c
index 6258c25983..b74f5b92a0 100644
--- a/io/channel-socket.c
+++ b/io/channel-socket.c
@@ -220,14 +220,27 @@ int qio_channel_socket_listen_sync(QIOChannelSocket *ioc,
 }
 
 
+struct QIOChannelListenWorkerData {
+    SocketAddress *addr;
+    int num; /* amount of expected connections */
+};
+
+static void qio_channel_listen_worker_free(gpointer opaque)
+{
+    struct QIOChannelListenWorkerData *data = opaque;
+
+    qapi_free_SocketAddress(data->addr);
+    g_free(data);
+}
+
 static void qio_channel_socket_listen_worker(QIOTask *task,
                                              gpointer opaque)
 {
     QIOChannelSocket *ioc = QIO_CHANNEL_SOCKET(qio_task_get_source(task));
-    SocketAddress *addr = opaque;
+    struct QIOChannelListenWorkerData *data = opaque;
     Error *err = NULL;
 
-    qio_channel_socket_listen_sync(ioc, addr, 1, &err);
+    qio_channel_socket_listen_sync(ioc, data->addr, data->num, &err);
 
     qio_task_set_error(task, err);
 }
@@ -235,6 +248,7 @@ static void qio_channel_socket_listen_worker(QIOTask *task,
 
 void qio_channel_socket_listen_async(QIOChannelSocket *ioc,
                                      SocketAddress *addr,
+                                     int num,
                                      QIOTaskFunc callback,
                                      gpointer opaque,
                                      GDestroyNotify destroy,
@@ -242,16 +256,18 @@ void qio_channel_socket_listen_async(QIOChannelSocket *ioc,
 {
     QIOTask *task = qio_task_new(
         OBJECT(ioc), callback, opaque, destroy);
-    SocketAddress *addrCopy;
+    struct QIOChannelListenWorkerData *data;
 
-    addrCopy = QAPI_CLONE(SocketAddress, addr);
+    data = g_new0(struct QIOChannelListenWorkerData, 1);
+    data->addr = QAPI_CLONE(SocketAddress, addr);
+    data->num = num;
 
     /* socket_listen() blocks in DNS lookups, so we must use a thread */
-    trace_qio_channel_socket_listen_async(ioc, addr);
+    trace_qio_channel_socket_listen_async(ioc, addr, num);
     qio_task_run_in_thread(task,
                            qio_channel_socket_listen_worker,
-                           addrCopy,
-                           (GDestroyNotify)qapi_free_SocketAddress,
+                           data,
+                           qio_channel_listen_worker_free,
                            context);
 }
 
diff --git a/io/trace-events b/io/trace-events
index 2e6aa1d749..d7bc70b966 100644
--- a/io/trace-events
+++ b/io/trace-events
@@ -18,7 +18,7 @@ qio_channel_socket_connect_async(void *ioc, void *addr) "Socket connect async io
 qio_channel_socket_connect_fail(void *ioc) "Socket connect fail ioc=%p"
 qio_channel_socket_connect_complete(void *ioc, int fd) "Socket connect complete ioc=%p fd=%d"
 qio_channel_socket_listen_sync(void *ioc, void *addr, int num) "Socket listen sync ioc=%p addr=%p num=%d"
-qio_channel_socket_listen_async(void *ioc, void *addr) "Socket listen async ioc=%p addr=%p"
+qio_channel_socket_listen_async(void *ioc, void *addr, int num) "Socket listen async ioc=%p addr=%p num=%d"
 qio_channel_socket_listen_fail(void *ioc) "Socket listen fail ioc=%p"
 qio_channel_socket_listen_complete(void *ioc, int fd) "Socket listen complete ioc=%p fd=%d"
 qio_channel_socket_dgram_sync(void *ioc, void *localAddr, void *remoteAddr) "Socket dgram sync ioc=%p localAddr=%p remoteAddr=%p"
diff --git a/tests/test-io-channel-socket.c b/tests/test-io-channel-socket.c
index 6eebcee115..50235c1547 100644
--- a/tests/test-io-channel-socket.c
+++ b/tests/test-io-channel-socket.c
@@ -113,7 +113,7 @@ static void test_io_channel_setup_async(SocketAddress *listen_addr,
 
     lioc = qio_channel_socket_new();
     qio_channel_socket_listen_async(
-        lioc, listen_addr,
+        lioc, listen_addr, 1,
         test_io_channel_complete, &data, NULL, NULL);
 
     g_main_loop_run(data.loop);
-- 
2.21.0

[Qemu-devel] [PATCH v3 4/5] socket: Add num connections to qio_net_listener_open_sync()

[Juan Quintela]

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 blockdev-nbd.c            | 2 +-
 chardev/char-socket.c     | 2 +-
 include/io/net-listener.h | 2 ++
 io/net-listener.c         | 3 ++-
 migration/socket.c        | 2 +-
 qemu-nbd.c                | 2 +-
 ui/vnc.c                  | 4 ++--
 7 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 7a71da447f..c621686131 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -101,7 +101,7 @@ 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, errp) < 0) {
+    if (qio_net_listener_open_sync(nbd_server->listener, addr, 1, errp) < 0) {
         goto error;
     }
 
diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 7ca5d97af3..8c7c9da567 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -1160,7 +1160,7 @@ static int qmp_chardev_open_socket_server(Chardev *chr,
     qio_net_listener_set_name(s->listener, name);
     g_free(name);
 
-    if (qio_net_listener_open_sync(s->listener, s->addr, errp) < 0) {
+    if (qio_net_listener_open_sync(s->listener, s->addr, 1, errp) < 0) {
         object_unref(OBJECT(s->listener));
         s->listener = NULL;
         return -1;
diff --git a/include/io/net-listener.h b/include/io/net-listener.h
index 8081ac58a2..fb101703e3 100644
--- a/include/io/net-listener.h
+++ b/include/io/net-listener.h
@@ -95,6 +95,7 @@ void qio_net_listener_set_name(QIONetListener *listener,
  * qio_net_listener_open_sync:
  * @listener: the network listener object
  * @addr: the address to listen on
+ * @num: the amount of expected connections
  * @errp: pointer to a NULL initialized error object
  *
  * Synchronously open a listening connection on all
@@ -104,6 +105,7 @@ void qio_net_listener_set_name(QIONetListener *listener,
  */
 int qio_net_listener_open_sync(QIONetListener *listener,
                                SocketAddress *addr,
+                               int num,
                                Error **errp);
 
 /**
diff --git a/io/net-listener.c b/io/net-listener.c
index dc81150318..5d8a226872 100644
--- a/io/net-listener.c
+++ b/io/net-listener.c
@@ -62,6 +62,7 @@ static gboolean qio_net_listener_channel_func(QIOChannel *ioc,
 
 int qio_net_listener_open_sync(QIONetListener *listener,
                                SocketAddress *addr,
+                               int num,
                                Error **errp)
 {
     QIODNSResolver *resolver = qio_dns_resolver_get_instance();
@@ -82,7 +83,7 @@ int qio_net_listener_open_sync(QIONetListener *listener,
     for (i = 0; i < nresaddrs; i++) {
         QIOChannelSocket *sioc = qio_channel_socket_new();
 
-        if (qio_channel_socket_listen_sync(sioc, resaddrs[i], 1,
+        if (qio_channel_socket_listen_sync(sioc, resaddrs[i], num,
                                            err ? NULL : &err) == 0) {
             success = true;
 
diff --git a/migration/socket.c b/migration/socket.c
index 98efdc0286..e63f5e1612 100644
--- a/migration/socket.c
+++ b/migration/socket.c
@@ -181,7 +181,7 @@ static void socket_start_incoming_migration(SocketAddress *saddr,
 
     qio_net_listener_set_name(listener, "migration-socket-listener");
 
-    if (qio_net_listener_open_sync(listener, saddr, errp) < 0) {
+    if (qio_net_listener_open_sync(listener, saddr, 1, errp) < 0) {
         object_unref(OBJECT(listener));
         return;
     }
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 049645491d..83b6c32d73 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -1054,7 +1054,7 @@ int main(int argc, char **argv)
     server = qio_net_listener_new();
     if (socket_activation == 0) {
         saddr = nbd_build_socket_address(sockpath, bindto, port);
-        if (qio_net_listener_open_sync(server, saddr, &local_err) < 0) {
+        if (qio_net_listener_open_sync(server, saddr, 1, &local_err) < 0) {
             object_unref(OBJECT(server));
             error_report_err(local_err);
             exit(EXIT_FAILURE);
diff --git a/ui/vnc.c b/ui/vnc.c
index 4812ed29d0..258461f814 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -3762,7 +3762,7 @@ static int vnc_display_listen(VncDisplay *vd,
         qio_net_listener_set_name(vd->listener, "vnc-listen");
         for (i = 0; i < nsaddr; i++) {
             if (qio_net_listener_open_sync(vd->listener,
-                                           saddr[i],
+                                           saddr[i], 1,
                                            errp) < 0)  {
                 return -1;
             }
@@ -3777,7 +3777,7 @@ static int vnc_display_listen(VncDisplay *vd,
         qio_net_listener_set_name(vd->wslistener, "vnc-ws-listen");
         for (i = 0; i < nwsaddr; i++) {
             if (qio_net_listener_open_sync(vd->wslistener,
-                                           wsaddr[i],
+                                           wsaddr[i], 1,
                                            errp) < 0)  {
                 return -1;
             }
-- 
2.21.0

[Qemu-devel] [PATCH v3 5/5] multifd: Use number of channels as listen backlog

[Juan Quintela]

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 migration/socket.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/migration/socket.c b/migration/socket.c
index e63f5e1612..97c9efde59 100644
--- a/migration/socket.c
+++ b/migration/socket.c
@@ -178,10 +178,15 @@ static void socket_start_incoming_migration(SocketAddress *saddr,
 {
     QIONetListener *listener = qio_net_listener_new();
     size_t i;
+    int num = 1;
 
     qio_net_listener_set_name(listener, "migration-socket-listener");
 
-    if (qio_net_listener_open_sync(listener, saddr, 1, errp) < 0) {
+    if (migrate_use_multifd()) {
+        num = migrate_multifd_channels();
+    }
+
+    if (qio_net_listener_open_sync(listener, saddr, num, errp) < 0) {
         object_unref(OBJECT(listener));
         return;
     }
-- 
2.21.0

Re: [Qemu-devel] [PATCH v3 1/5] socket: Add backlog parameter to socket_listen

[Daniel P. Berrangé]

On Tue, Aug 20, 2019 at 12:48:32PM +0200, Juan Quintela wrote:
> Current parameter was always one.  We continue with that value for now
> in all callers.
> 
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> 
> ---
> Moved trace to socket_listen
> ---
>  include/qemu/sockets.h    |  2 +-
>  io/channel-socket.c       |  2 +-
>  qga/channel-posix.c       |  2 +-
>  tests/test-util-sockets.c | 12 ++++++------
>  util/qemu-sockets.c       | 33 ++++++++++++++++++++++-----------
>  util/trace-events         |  3 +++
>  6 files changed, 34 insertions(+), 20 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v3 1/5] socket: Add backlog parameter to socket_listen

[Juan Quintela]

Daniel P. Berrangé <berrange@redhat.com> wrote:
> On Tue, Aug 20, 2019 at 12:48:32PM +0200, Juan Quintela wrote:
>> Current parameter was always one.  We continue with that value for now
>> in all callers.
>> 
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> 
>> ---
>> Moved trace to socket_listen
>> ---
>>  include/qemu/sockets.h    |  2 +-
>>  io/channel-socket.c       |  2 +-
>>  qga/channel-posix.c       |  2 +-
>>  tests/test-util-sockets.c | 12 ++++++------
>>  util/qemu-sockets.c       | 33 ++++++++++++++++++++++-----------
>>  util/trace-events         |  3 +++
>>  6 files changed, 34 insertions(+), 20 deletions(-)
>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

Hi

Everything is reviewed by you, and it is mostly socket code.  Should I
do the pull request, or are you doing it?

Thanks, Juan.

Re: [Qemu-devel] [PATCH v3 1/5] socket: Add backlog parameter to socket_listen

[Daniel P. Berrangé]

On Tue, Aug 20, 2019 at 01:17:59PM +0200, Juan Quintela wrote:
> Daniel P. Berrangé <berrange@redhat.com> wrote:
> > On Tue, Aug 20, 2019 at 12:48:32PM +0200, Juan Quintela wrote:
> >> Current parameter was always one.  We continue with that value for now
> >> in all callers.
> >> 
> >> Signed-off-by: Juan Quintela <quintela@redhat.com>
> >> 
> >> ---
> >> Moved trace to socket_listen
> >> ---
> >>  include/qemu/sockets.h    |  2 +-
> >>  io/channel-socket.c       |  2 +-
> >>  qga/channel-posix.c       |  2 +-
> >>  tests/test-util-sockets.c | 12 ++++++------
> >>  util/qemu-sockets.c       | 33 ++++++++++++++++++++++-----------
> >>  util/trace-events         |  3 +++
> >>  6 files changed, 34 insertions(+), 20 deletions(-)
> >
> > Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> 
> Hi
> 
> Everything is reviewed by you, and it is mostly socket code.  Should I
> do the pull request, or are you doing it?

I'm fine if you want to send a PR, since this is ultimately about
fixing migration problems.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [Qemu-devel] [PATCH v3 4/5] socket: Add num connections to qio_net_listener_open_sync()

[Eric Blake]

[-- Attachment #1.1: Type: text/plain, Size: 2152 bytes --]

On 8/20/19 5:48 AM, Juan Quintela wrote:
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> ---
>  blockdev-nbd.c            | 2 +-
>  chardev/char-socket.c     | 2 +-
>  include/io/net-listener.h | 2 ++
>  io/net-listener.c         | 3 ++-
>  migration/socket.c        | 2 +-
>  qemu-nbd.c                | 2 +-
>  ui/vnc.c                  | 4 ++--
>  7 files changed, 10 insertions(+), 7 deletions(-)

Just now noticing this one, even though the pull request is already sent...

> 
> diff --git a/blockdev-nbd.c b/blockdev-nbd.c
> index 7a71da447f..c621686131 100644
> --- a/blockdev-nbd.c
> +++ b/blockdev-nbd.c
> @@ -101,7 +101,7 @@ 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, errp) < 0) {
> +    if (qio_net_listener_open_sync(nbd_server->listener, addr, 1, errp) < 0) {
>          goto error;
>      }

Does this interfere with the ability to have more than one client
connect to an NBD server during pull-mode incremental backup?  Or can
you still have multiple simultaneous clients, provided that the server
has finished accepting the connection from the first before the second
one starts?

> +++ b/qemu-nbd.c
> @@ -1054,7 +1054,7 @@ int main(int argc, char **argv)
>      server = qio_net_listener_new();
>      if (socket_activation == 0) {
>          saddr = nbd_build_socket_address(sockpath, bindto, port);
> -        if (qio_net_listener_open_sync(server, saddr, &local_err) < 0) {
> +        if (qio_net_listener_open_sync(server, saddr, 1, &local_err) < 0) {

Here, 'qemu-nbd -e $n' allows up to $n simultaneous clients.  Should we
be feeding in that number, instead of a hard-coded 1, to make it easier
for those clients to connect simultaneously?

We can make such changes as a followup patch.

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Qemu-devel] [PATCH v3 4/5] socket: Add num connections to qio_net_listener_open_sync()

[Juan Quintela]

Eric Blake <eblake@redhat.com> wrote:
> On 8/20/19 5:48 AM, Juan Quintela wrote:
>> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> ---
>>  blockdev-nbd.c            | 2 +-
>>  chardev/char-socket.c     | 2 +-
>>  include/io/net-listener.h | 2 ++
>>  io/net-listener.c         | 3 ++-
>>  migration/socket.c        | 2 +-
>>  qemu-nbd.c                | 2 +-
>>  ui/vnc.c                  | 4 ++--
>>  7 files changed, 10 insertions(+), 7 deletions(-)
>
> Just now noticing this one, even though the pull request is already sent...
>
>> 
>> diff --git a/blockdev-nbd.c b/blockdev-nbd.c
>> index 7a71da447f..c621686131 100644
>> --- a/blockdev-nbd.c
>> +++ b/blockdev-nbd.c
>> @@ -101,7 +101,7 @@ 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, errp) < 0) {
>> +    if (qio_net_listener_open_sync(nbd_server->listener, addr, 1, errp) < 0) {
>>          goto error;
>>      }
>
> Does this interfere with the ability to have more than one client
> connect to an NBD server during pull-mode incremental backup?  Or can
> you still have multiple simultaneous clients, provided that the server
> has finished accepting the connection from the first before the second
> one starts?

It is exactly the same than the old code.  Old code always use one.  We
need to have more than one for multifd.

Once told that, if the connections don't start "very" simultaneosly
(i..e. With multifd we start <num channels> connections in paraller),
you will never notice that the backlog is one (sie of queue of pending
connections nowadays).


>
>> +++ b/qemu-nbd.c
>> @@ -1054,7 +1054,7 @@ int main(int argc, char **argv)
>>      server = qio_net_listener_new();
>>      if (socket_activation == 0) {
>>          saddr = nbd_build_socket_address(sockpath, bindto, port);
>> -        if (qio_net_listener_open_sync(server, saddr, &local_err) < 0) {
>> +        if (qio_net_listener_open_sync(server, saddr, 1, &local_err) < 0) {
>
> Here, 'qemu-nbd -e $n' allows up to $n simultaneous clients.  Should we
> be feeding in that number, instead of a hard-coded 1, to make it easier
> for those clients to connect simultaneously?
>
> We can make such changes as a followup patch.

From the man page:

       The backlog argument defines the maximum length to which the  queue  of
       pending  connections  for  sockfd  may  grow.

So, except if you plan to start multiples connections at the same time,
you don't care.  And if the old code worked, this one makes no
difference.

To explain multifd problem, I was creating 10 threads, and each launched
a connect.  On the receiving side, I only got 8 connections, 2 of them
were missing.

Later, Juan.

Re: [Qemu-devel] [PATCH v3 4/5] socket: Add num connections to qio_net_listener_open_sync()

[Daniel P. Berrangé]

On Wed, Sep 04, 2019 at 03:19:21PM +0200, Juan Quintela wrote:
> Eric Blake <eblake@redhat.com> wrote:
> > On 8/20/19 5:48 AM, Juan Quintela wrote:
> >> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> >> Signed-off-by: Juan Quintela <quintela@redhat.com>
> >> ---
> >>  blockdev-nbd.c            | 2 +-
> >>  chardev/char-socket.c     | 2 +-
> >>  include/io/net-listener.h | 2 ++
> >>  io/net-listener.c         | 3 ++-
> >>  migration/socket.c        | 2 +-
> >>  qemu-nbd.c                | 2 +-
> >>  ui/vnc.c                  | 4 ++--
> >>  7 files changed, 10 insertions(+), 7 deletions(-)
> >
> > Just now noticing this one, even though the pull request is already sent...
> >
> >> 
> >> diff --git a/blockdev-nbd.c b/blockdev-nbd.c
> >> index 7a71da447f..c621686131 100644
> >> --- a/blockdev-nbd.c
> >> +++ b/blockdev-nbd.c
> >> @@ -101,7 +101,7 @@ 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, errp) < 0) {
> >> +    if (qio_net_listener_open_sync(nbd_server->listener, addr, 1, errp) < 0) {
> >>          goto error;
> >>      }
> >
> > Does this interfere with the ability to have more than one client
> > connect to an NBD server during pull-mode incremental backup?  Or can
> > you still have multiple simultaneous clients, provided that the server
> > has finished accepting the connection from the first before the second
> > one starts?
> 
> It is exactly the same than the old code.  Old code always use one.  We
> need to have more than one for multifd.
> 
> Once told that, if the connections don't start "very" simultaneosly
> (i..e. With multifd we start <num channels> connections in paraller),
> you will never notice that the backlog is one (sie of queue of pending
> connections nowadays).

If incremental backup needs multiple concurrent connections, then
you certainly *do* want to increase this value to something other
than 1, or you will get random failures. As Juan says, this is a
pre-existing problem with NBD though.


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|