[Spice-commits] 6 commits - server/net-utils.c server/net-utils.h server/reds.cpp server/red-worker.cpp server/sys-socket.h tools/reds_stat.c

GitLab Mirror gitlab-mirror at kemper.freedesktop.org
Mon May 25 05:51:13 UTC 2020


 server/net-utils.c    |   50 ++++++++++++++++++++++++++++++++++++++++++++++----
 server/net-utils.h    |    1 +
 server/red-worker.cpp |    5 +++++
 server/reds.cpp       |    1 +
 server/sys-socket.h   |    4 ++++
 tools/reds_stat.c     |    7 +++----
 6 files changed, 60 insertions(+), 8 deletions(-)

New commits:
commit 88c25a18deb87877fcec98ba2bad2f544e1f1049
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Wed Apr 15 21:15:34 2020 +0100

    Fix compatibility with TCP sockets and Darwin
    
    Using socket pairs and trying to set a TCP level option on the
    socket setsockopt returns EINVAL error and not ENOTSUP as
    expected.
    Check this case and handle as ENOTSUP (ignoring it).
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>

diff --git a/server/net-utils.c b/server/net-utils.c
index f0aecc3d..e9778e73 100644
--- a/server/net-utils.c
+++ b/server/net-utils.c
@@ -45,6 +45,25 @@
 #define NOTSUP_ERROR(err) ((err) == ENOTSUP)
 #endif
 
+static inline bool
+darwin_einval_on_unix_socket(int fd, int err)
+{
+#if defined(__APPLE__)
+    if (err == EINVAL) {
+        union {
+            struct sockaddr sa;
+            char buf[1024];
+        } addr;
+        socklen_t len = sizeof(addr);
+
+        if (getsockname(fd, &addr.sa, &len) == 0 && addr.sa.sa_family == AF_UNIX) {
+            return true;
+        }
+    }
+#endif
+    return false;
+}
+
 /**
  * red_socket_set_keepalive:
  * @fd: a socket file descriptor
@@ -57,7 +76,7 @@ bool red_socket_set_keepalive(int fd, bool enable, int timeout)
     int keepalive = !!enable;
 
     if (setsockopt(fd, SOL_SOCKET, SO_KEEPALIVE, &keepalive, sizeof(keepalive)) == -1) {
-        if (!NOTSUP_ERROR(errno)) {
+        if (!NOTSUP_ERROR(errno) && !darwin_einval_on_unix_socket(fd, errno)) {
             g_warning("setsockopt for keepalive failed, %s", strerror(errno));
             return false;
         }
@@ -69,7 +88,7 @@ bool red_socket_set_keepalive(int fd, bool enable, int timeout)
 
 #ifdef TCP_KEEPIDLE
     if (setsockopt(fd, IPPROTO_TCP, TCP_KEEPIDLE, &timeout, sizeof(timeout)) == -1) {
-        if (!NOTSUP_ERROR(errno)) {
+        if (!NOTSUP_ERROR(errno) && !darwin_einval_on_unix_socket(fd, errno)) {
             g_warning("setsockopt for keepalive timeout failed, %s", strerror(errno));
             return false;
         }
@@ -92,7 +111,8 @@ bool red_socket_set_no_delay(int fd, bool no_delay)
 
     if (setsockopt(fd, IPPROTO_TCP, TCP_NODELAY,
                    &optval, sizeof(optval)) != 0) {
-        if (!NOTSUP_ERROR(errno) && errno != ENOPROTOOPT) {
+        if (!NOTSUP_ERROR(errno) && errno != ENOPROTOOPT &&
+            !darwin_einval_on_unix_socket(fd, errno)) {
             spice_warning("setsockopt failed, %s", strerror(errno));
             return false;
         }
commit 4f82cec1974e9ca4f0200ef7554c8e37adff62f3
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Wed Apr 15 20:50:56 2020 +0100

    Fix compatibility with ENOTSUP and Darwin
    
    Darwin uses also the constant EOPNOTSUPP for the same reasons.
    In some versions EOPNOTSUPP is defined as ENOTSUP.
    Check both values, not only ENOTSUP.
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>

diff --git a/server/net-utils.c b/server/net-utils.c
index 32ceb3b8..f0aecc3d 100644
--- a/server/net-utils.c
+++ b/server/net-utils.c
@@ -39,6 +39,12 @@
 #define TCP_KEEPIDLE TCP_KEEPALIVE
 #endif
 
+#if defined(EOPNOTSUPP) && EOPNOTSUPP != ENOTSUP
+#define NOTSUP_ERROR(err) ((err) == ENOTSUP || (err) == EOPNOTSUPP)
+#else
+#define NOTSUP_ERROR(err) ((err) == ENOTSUP)
+#endif
+
 /**
  * red_socket_set_keepalive:
  * @fd: a socket file descriptor
@@ -51,7 +57,7 @@ bool red_socket_set_keepalive(int fd, bool enable, int timeout)
     int keepalive = !!enable;
 
     if (setsockopt(fd, SOL_SOCKET, SO_KEEPALIVE, &keepalive, sizeof(keepalive)) == -1) {
-        if (errno != ENOTSUP) {
+        if (!NOTSUP_ERROR(errno)) {
             g_warning("setsockopt for keepalive failed, %s", strerror(errno));
             return false;
         }
@@ -63,7 +69,7 @@ bool red_socket_set_keepalive(int fd, bool enable, int timeout)
 
 #ifdef TCP_KEEPIDLE
     if (setsockopt(fd, IPPROTO_TCP, TCP_KEEPIDLE, &timeout, sizeof(timeout)) == -1) {
-        if (errno != ENOTSUP) {
+        if (!NOTSUP_ERROR(errno)) {
             g_warning("setsockopt for keepalive timeout failed, %s", strerror(errno));
             return false;
         }
@@ -86,7 +92,7 @@ bool red_socket_set_no_delay(int fd, bool no_delay)
 
     if (setsockopt(fd, IPPROTO_TCP, TCP_NODELAY,
                    &optval, sizeof(optval)) != 0) {
-        if (errno != ENOTSUP && errno != ENOPROTOOPT) {
+        if (!NOTSUP_ERROR(errno) && errno != ENOPROTOOPT) {
             spice_warning("setsockopt failed, %s", strerror(errno));
             return false;
         }
commit 90453ddf0640ebc54cf7d35c0f028553fc9f5703
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Wed Apr 15 20:47:05 2020 +0100

    Fix compatibility with TCP_KEEPIDLE and Darwin
    
    Darwin uses for the same setting the TCP_KEEPALIVE option.
    Use TCP_KEEPALIVE instead of TCP_KEEPIDLE for Darwin.
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>

diff --git a/server/net-utils.c b/server/net-utils.c
index 78b94886..32ceb3b8 100644
--- a/server/net-utils.c
+++ b/server/net-utils.c
@@ -35,6 +35,10 @@
 #include "net-utils.h"
 #include "sys-socket.h"
 
+#if !defined(TCP_KEEPIDLE) && defined(TCP_KEEPALIVE) && defined(__APPLE__)
+#define TCP_KEEPIDLE TCP_KEEPALIVE
+#endif
+
 /**
  * red_socket_set_keepalive:
  * @fd: a socket file descriptor
@@ -57,7 +61,7 @@ bool red_socket_set_keepalive(int fd, bool enable, int timeout)
         return true;
     }
 
-#ifdef HAVE_TCP_KEEPIDLE
+#ifdef TCP_KEEPIDLE
     if (setsockopt(fd, IPPROTO_TCP, TCP_KEEPIDLE, &timeout, sizeof(timeout)) == -1) {
         if (errno != ENOTSUP) {
             g_warning("setsockopt for keepalive timeout failed, %s", strerror(errno));
commit 8772163cf9b0219c372fdcfc37def8bbbfc3ecd7
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Wed Apr 15 14:35:10 2020 +0100

    Fix compatibility with mremap and Darwin
    
    Darwin does not have mremap. Use munmap+mmap instead.
    That code is not in a hot path, number of nodes do not change very
    often.
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>

diff --git a/tools/reds_stat.c b/tools/reds_stat.c
index deffec1b..7d35c45b 100644
--- a/tools/reds_stat.c
+++ b/tools/reds_stat.c
@@ -86,7 +86,6 @@ int main(int argc, char **argv)
     pid_t kvm_pid = 0;
     uint32_t num_of_nodes = 0;
     size_t shm_size;
-    size_t shm_old_size;
     int shm_name_len;
     int ret = EXIT_FAILURE;
     int fd;
@@ -142,11 +141,11 @@ int main(int argc, char **argv)
         printf("spice statistics\n\n");
         if (num_of_nodes != reds_stat->num_of_nodes) {
             num_of_nodes = reds_stat->num_of_nodes;
-            shm_old_size = shm_size;
+            munmap(reds_stat, shm_size);
             shm_size = header_size + num_of_nodes * sizeof(SpiceStatNode);
-            reds_stat = mremap(reds_stat, shm_old_size, shm_size, MREMAP_MAYMOVE);
+            reds_stat = (SpiceStat *)mmap(NULL, shm_size, PROT_READ, MAP_SHARED, fd, 0);
             if (reds_stat == (SpiceStat *)MAP_FAILED) {
-                perror("mremap");
+                perror("mmap");
                 goto error;
             }
             reds_nodes = (SpiceStatNode *)((char *) reds_stat + header_size);
commit 6684a3b5ef9e1e93a4123e6887353ffe6354890c
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Wed Apr 15 14:30:37 2020 +0100

    Fix compatibility with pthread_setname_np and Darwin
    
    On Darwin pthread_setname_np accepts only an argument and
    set current thread name.
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>

diff --git a/server/red-worker.cpp b/server/red-worker.cpp
index ff101000..cbb89a66 100644
--- a/server/red-worker.cpp
+++ b/server/red-worker.cpp
@@ -1084,6 +1084,9 @@ static void *red_worker_main(void *arg)
     RedWorker *worker = (RedWorker *) arg;
 
     spice_debug("begin");
+#if defined(__APPLE__)
+    pthread_setname_np("SPICE Worker");
+#endif
     SPICE_VERIFY(MAX_PIPE_SIZE > WIDE_CLIENT_ACK_WINDOW &&
            MAX_PIPE_SIZE > NARROW_CLIENT_ACK_WINDOW); //ensure wakeup by ack message
 
@@ -1123,7 +1126,9 @@ bool red_worker_run(RedWorker *worker)
 #ifndef _WIN32
     pthread_sigmask(SIG_SETMASK, &curr_sig_mask, NULL);
 #endif
+#if !defined(__APPLE__)
     pthread_setname_np(worker->thread, "SPICE Worker");
+#endif
 
     return r == 0;
 }
commit ee4cf6a07ccbbe41ba70baeaca1000c7fc495c3c
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Wed Apr 15 13:36:13 2020 +0100

    Fix compatibility with MSG_NOSIGNAL and Darwin
    
    Darwin does not have MSG_NOSIGNAL but allows to set a SO_NOSIGPIPE
    option to disable sending SIGPIPE writing to closed socket.
    Note that *BSD has the SO_NOSIGPIPE option but does not affect all
    write calls so instead continue to use MSG_NOSIGNAL instead on
    that systems.
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>

diff --git a/server/net-utils.c b/server/net-utils.c
index 144bfd8f..78b94886 100644
--- a/server/net-utils.c
+++ b/server/net-utils.c
@@ -150,3 +150,15 @@ int red_socket_get_no_delay(int fd)
 
     return delay_val;
 }
+
+/**
+ * red_socket_set_nosigpipe
+ * @fd: a socket file descriptor
+ */
+void red_socket_set_nosigpipe(int fd, bool enable)
+{
+#if defined(SO_NOSIGPIPE) && defined(__APPLE__)
+    int val = !!enable;
+    setsockopt(fd, SOL_SOCKET, SO_NOSIGPIPE, (const void *) &val, sizeof(val));
+#endif
+}
diff --git a/server/net-utils.h b/server/net-utils.h
index 5b8c9163..62ff2f23 100644
--- a/server/net-utils.h
+++ b/server/net-utils.h
@@ -27,6 +27,7 @@ bool red_socket_set_keepalive(int fd, bool enable, int timeout);
 bool red_socket_set_no_delay(int fd, bool no_delay);
 int red_socket_get_no_delay(int fd);
 bool red_socket_set_non_blocking(int fd, bool non_blocking);
+void red_socket_set_nosigpipe(int fd, bool enable);
 
 SPICE_END_DECLS
 
diff --git a/server/reds.cpp b/server/reds.cpp
index 23c17f3d..fb6e7795 100644
--- a/server/reds.cpp
+++ b/server/reds.cpp
@@ -2401,6 +2401,7 @@ static RedLinkInfo *reds_init_client_connection(RedsState *reds, int socket)
     }
 
     red_socket_set_keepalive(socket, TRUE, KEEPALIVE_TIMEOUT);
+    red_socket_set_nosigpipe(socket, true);
 
     link = g_new0(RedLinkInfo, 1);
     link->reds = reds;
diff --git a/server/sys-socket.h b/server/sys-socket.h
index b13ecdee..f8916a2f 100644
--- a/server/sys-socket.h
+++ b/server/sys-socket.h
@@ -146,4 +146,8 @@ SPICE_END_DECLS
 
 #endif
 
+#if defined(SO_NOSIGPIPE) && defined(__APPLE__)
+#define MSG_NOSIGNAL 0
+#endif
+
 #endif // RED_SYS_SOCKET_H_


More information about the Spice-commits mailing list