diff mbox series

[v2,1/5] socket: Add backlog parameter to socket_listen

Message ID 20190820082459.2101-2-quintela@redhat.com
State New
Headers show
Series Fix multifd with big number of channels | expand

Commit Message

Juan Quintela Aug. 20, 2019, 8:24 a.m. UTC 
Current parameter was always one.  We continue with that value for now
in all callers.

Signed-off-by: Juan Quintela <quintela@redhat.com>
---
 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         |  2 ++
 6 files changed, 33 insertions(+), 20 deletions(-)

Comments

Daniel P. Berrangé Aug. 20, 2019, 8:39 a.m. UTC | #1 
On Tue, Aug 20, 2019 at 10:24:55AM +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>
> ---
>  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         |  2 ++
>  6 files changed, 33 insertions(+), 20 deletions(-)
> 
> diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> index e3a1666578..3f0a80404f 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,8 @@ static int inet_listen_saddr(InetSocketAddress *saddr,
>                      goto listen_failed;
>                  }
>              } else {
> -                if (!listen(slisten, 1)) {
> +                trace_inet_listen_saddr(num);

It is a bit odd to only have the trace event for inet sockets. I'd
prefer it in the caller for all sockets, with just "socket_listen"
name.

> +                if (!listen(slisten, num)) {
>                      goto listen_ok;
>                  }
>                  if (errno != EADDRINUSE) {

Regards,
Daniel
Juan Quintela Aug. 20, 2019, 9:14 a.m. UTC | #2 
Daniel P. Berrangé <berrange@redhat.com> wrote:
> On Tue, Aug 20, 2019 at 10:24:55AM +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>

>> @@ -309,7 +311,8 @@ static int inet_listen_saddr(InetSocketAddress *saddr,
>>                      goto listen_failed;
>>                  }
>>              } else {
>> -                if (!listen(slisten, 1)) {
>> +                trace_inet_listen_saddr(num);
>
> It is a bit odd to only have the trace event for inet sockets. I'd
> prefer it in the caller for all sockets, with just "socket_listen"
> name.

Ok. will change.

This is the one that I needed, I just changed an error_report() to a
trace O:-)
diff mbox series

Patch

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..3f0a80404f 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,8 @@  static int inet_listen_saddr(InetSocketAddress *saddr,
                     goto listen_failed;
                 }
             } else {
-                if (!listen(slisten, 1)) {
+                trace_inet_listen_saddr(num);
+                if (!listen(slisten, num)) {
                     goto listen_ok;
                 }
                 if (errno != EADDRINUSE) {
@@ -774,6 +777,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 +799,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 +840,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 +858,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 +920,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 +987,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 +1011,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 +1068,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 +1110,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 +1123,25 @@  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;
 
     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..106bbca6cb 100644
--- a/util/trace-events
+++ b/util/trace-events
@@ -64,6 +64,8 @@  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
+inet_listen_saddr(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)"