[Spice-commits] Branch '0.12' - 9 commits - NEWS configure.ac server/red_channel.c server/red_memslots.c server/reds-private.h server/reds.c server/spice-server.h server/spice-server.syms spice-common

Christophe Fergau teuf at kemper.freedesktop.org
Thu Apr 14 14:07:49 UTC 2016


 NEWS                     |    8 +++---
 configure.ac             |    2 -
 server/red_channel.c     |   14 +++++++----
 server/red_memslots.c    |    2 -
 server/reds-private.h    |    1 
 server/reds.c            |   55 +++++++++++++++++++++++++++++------------------
 server/spice-server.h    |    1 
 server/spice-server.syms |    5 ----
 spice-common             |    2 -
 9 files changed, 50 insertions(+), 40 deletions(-)

New commits:
commit cf43fa7bb342575c3358ed122184da6e6abcbd4b
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Thu Apr 14 16:01:58 2016 +0200

    Update NEWS file for 0.12.7

diff --git a/NEWS b/NEWS
index cf6f1d6..f00d445 100644
--- a/NEWS
+++ b/NEWS
@@ -1,11 +1,11 @@
 Changes in 0.12.7:
 ==================
-* Added public spice_server_set_keepalive_timeout() to make it possible
-  to tweak keepalive on all SPICE connection. This can prevent unwanted
-  idle disconnections if proxies are used between the client and the host.
+* spice-server will now send TCP keepalive probes on the TCP connections it
+  uses. This can prevent unwanted idle disconnections if proxies are used
+  between the client and the host.
 * Fix important memory usage when the webdav channel is used
 * Do not disconnect when the client requests an unsupported compression type
-* Fix potential race condition when using multiple QXL devices
+* Fix a few race conditions
 * Fix display glitch when using XSpice
 * Improve help string for 'replay -s'
 * Fix crashes in corner cases (buggy spice-html5 + win10, vnc + SPICE port
commit 3c1eadb42f4d9a8c36b4587fb10473867e1148bc
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Thu Apr 14 14:50:13 2016 +0200

    Update libtool versioning for 0.12.7 release

diff --git a/configure.ac b/configure.ac
index 5e5978d..0521d02 100644
--- a/configure.ac
+++ b/configure.ac
@@ -12,7 +12,7 @@ AC_PREREQ([2.57])
 #   then set age to 0.
 #
 m4_define([SPICE_CURRENT], [11])
-m4_define([SPICE_REVISION], [0])
+m4_define([SPICE_REVISION], [1])
 m4_define([SPICE_AGE], [10])
 
 AC_INIT(spice, [m4_esyscmd(build-aux/git-version-gen .tarball-version)],
commit 1cf0f19f1479626434e0adf6e81b3700710e48d3
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Thu Apr 14 12:20:06 2016 +0200

    Update spice-common
    
    Revert spice-protocol/codegen changes.
    This was causing build failures when trying to compile an older spice-server
    release against a newer spice-protocol release with new message/channel
    additions
    
    spice-common changes this brings in:
    
    Christophe Fergeau (2):
          proto: Rename image_compress to image_compression
          proto: Use proper type for preferred_compression field
    
    Marc-André Lureau (1):
          Revert "Remove files moved to spice-protocol"

diff --git a/spice-common b/spice-common
index 80cc3f6..6ddcd54 160000
--- a/spice-common
+++ b/spice-common
@@ -1 +1 @@
-Subproject commit 80cc3f680a1da4cfc7b2bc1666903086072ce1de
+Subproject commit 6ddcd5468a63adae6ff37ba371e8469aef97b267
commit cd282cf5d50535d9606b01330a01e30c5bb969d8
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Tue Apr 12 16:28:07 2016 +0100

    red-channel: make red_client_{ref,unref} thread safe
    
    These function are called on both sides of dispatcher so the
    increment/decrement of the counter is done in multiple threads.
    This caused the counter to not get incremented correctly and
    freed the structure too early, leaving a dangling pointer in
    the other thread.
    
    This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1253375.
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Christophe Fergeau <cfergeau at redhat.com>
    Acked-by: Jonathon Jongsma <jjongsma at redhat.com>

diff --git a/server/red_channel.c b/server/red_channel.c
index 9bf9d11..e61282e 100644
--- a/server/red_channel.c
+++ b/server/red_channel.c
@@ -2073,13 +2073,13 @@ RedClient *red_client_new(int migrated)
 RedClient *red_client_ref(RedClient *client)
 {
     spice_assert(client);
-    client->refs++;
+    g_atomic_int_inc(&client->refs);
     return client;
 }
 
 RedClient *red_client_unref(RedClient *client)
 {
-    if (!--client->refs) {
+    if (g_atomic_int_dec_and_test(&client->refs)) {
         spice_debug("release client=%p", client);
         pthread_mutex_destroy(&client->lock);
         free(client);
commit 5311f4f0042032341c623eec32c650dd32d25220
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Wed Feb 17 21:22:22 2016 +0000

    memslot: do not crash if guest provide a wrong address
    
    This could happen with buggy driver.
    
    This fixes https://bugzilla.redhat.com/show_bug.cgi?id=1264356
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Pavel Grunt <pgrunt at redhat.com>

diff --git a/server/red_memslots.c b/server/red_memslots.c
index 1b3ec62..817f158 100644
--- a/server/red_memslots.c
+++ b/server/red_memslots.c
@@ -62,7 +62,7 @@ int validate_virt(RedMemSlotInfo *info, unsigned long virt, int slot_id,
 
     if (virt < slot->virt_start_addr || (virt + add_size) > slot->virt_end_addr) {
         print_memslots(info);
-        spice_critical("virtual address out of range\n"
+        spice_warning("virtual address out of range\n"
               "    virt=0x%lx+0x%x slot_id=%d group_id=%d\n"
               "    slot=0x%lx-0x%lx delta=0x%lx",
               virt, add_size, slot_id, group_id,
commit cac6a78594d8b35acf952762386ab128e2220681
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Tue Jan 19 10:35:48 2016 +0000

    channel: do not free rcc->stream in red_channel_client_disconnect
    
    This fixes a crash if red_channel_client disconnect is called
    handling a message.
    This can happen for instance while handling SPICE_MSGC_ACK which calls
    red_channel_client_push which tries to detect write errors while writing
    to a socket (for instance socket disconnection).
    Messages are read in a loop and red_channel_client_disconnect would
    cause rcc->stream to be NULL which will result in a use-after-free
    problem (stream in red_peer_handle_incoming will use cached stream value).
    
    Signed-off-by: Marc-André Lureau <marcandre.lureau at gmail.com>
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Jonathon Jongsma <jjongsma at redhat.com>

diff --git a/server/red_channel.c b/server/red_channel.c
index 3f6240a..9bf9d11 100644
--- a/server/red_channel.c
+++ b/server/red_channel.c
@@ -1235,6 +1235,10 @@ static void red_channel_client_unref(RedChannelClient *rcc)
 {
     if (!--rcc->refs) {
         spice_debug("destroy rcc=%p", rcc);
+
+        reds_stream_free(rcc->stream);
+        rcc->stream = NULL;
+
         if (rcc->send_data.main.marshaller) {
             spice_marshaller_destroy(rcc->send_data.main.marshaller);
         }
@@ -1758,7 +1762,7 @@ void red_channel_pipes_add_empty_msg(RedChannel *channel, int msg_type)
 int red_channel_client_is_connected(RedChannelClient *rcc)
 {
     if (!rcc->dummy) {
-        return rcc->stream != NULL;
+        return ring_item_is_linked(&rcc->channel_link);
     } else {
         return rcc->dummy_connected;
     }
@@ -1813,6 +1817,8 @@ static void red_channel_remove_client(RedChannelClient *rcc)
                       rcc->channel->type, rcc->channel->id,
                       rcc->channel->thread_id, pthread_self());
     }
+    spice_return_if_fail(ring_item_is_linked(&rcc->channel_link));
+
     ring_remove(&rcc->channel_link);
     spice_assert(rcc->channel->clients_num > 0);
     rcc->channel->clients_num--;
@@ -1854,8 +1860,6 @@ void red_channel_client_disconnect(RedChannelClient *rcc)
         rcc->channel->core->watch_remove(rcc->stream->watch);
         rcc->stream->watch = NULL;
     }
-    reds_stream_free(rcc->stream);
-    rcc->stream = NULL;
     if (rcc->latency_monitor.timer) {
         rcc->channel->core->timer_remove(rcc->latency_monitor.timer);
         rcc->latency_monitor.timer = NULL;
commit 3336d892c3d8647868423300905e5cad67de1db1
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Wed Mar 2 12:25:51 2016 +0100

    Set TCP_KEEPINTVL when enabling TCP keepalive
    
    Otherwise we are only changing the timeout before the first keepalive
    probe is sent.

diff --git a/server/reds.c b/server/reds.c
index 61bf735..f185dea 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -2271,6 +2271,14 @@ static bool reds_init_keepalive(int socket)
         }
     }
 
+    if (setsockopt(socket, SOL_TCP, TCP_KEEPINTVL,
+                   &keepalive_timeout, sizeof(keepalive_timeout)) == -1) {
+        if (errno != ENOTSUP) {
+            spice_printerr("setsockopt for keepalive interval failed, %s", strerror(errno));
+            return false;
+        }
+    }
+
     return true;
 }
 
commit f9c96d3e02dfc89eacfc13d5365987490acc0e17
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Wed Mar 2 12:25:23 2016 +0100

    Remove spice_server_set_keepalive_timeout
    
    This public API is no longer needed as the keepalive interval does not
    need to be configurable. This API was never in a stable 0.12 release, so
    it's OK to remove it now.

diff --git a/server/reds-private.h b/server/reds-private.h
index 4859542..790f61c 100644
--- a/server/reds-private.h
+++ b/server/reds-private.h
@@ -175,7 +175,6 @@ typedef struct RedsState {
     int vm_running;
     Ring char_devs_states; /* list of SpiceCharDeviceStateItem */
     int seamless_migration_enabled; /* command line arg */
-    int keepalive_timeout;
 
     SSL_CTX *ctx;
 
diff --git a/server/reds.c b/server/reds.c
index f07d24e..61bf735 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -3935,10 +3935,3 @@ SPICE_GNUC_VISIBLE void spice_server_set_seamless_migration(SpiceServer *s, int
     reds->seamless_migration_enabled = enable && !reds->allow_multiple_clients;
     spice_debug("seamless migration enabled=%d", enable);
 }
-
-SPICE_GNUC_VISIBLE void spice_server_set_keepalive_timeout(SpiceServer *s, int timeout)
-{
-    spice_assert(s == reds);
-    reds->keepalive_timeout = timeout;
-    spice_debug("keepalive timeout=%d", timeout);
-}
diff --git a/server/spice-server.h b/server/spice-server.h
index fa74136..c2ff61d 100644
--- a/server/spice-server.h
+++ b/server/spice-server.h
@@ -111,7 +111,6 @@ int spice_server_set_playback_compression(SpiceServer *s, int enable);
 int spice_server_set_agent_mouse(SpiceServer *s, int enable);
 int spice_server_set_agent_copypaste(SpiceServer *s, int enable);
 int spice_server_set_agent_file_xfer(SpiceServer *s, int enable);
-void spice_server_set_keepalive_timeout(SpiceServer *s, int timeout);
 
 int spice_server_get_sock_info(SpiceServer *s, struct sockaddr *sa, socklen_t *salen);
 int spice_server_get_peer_info(SpiceServer *s, struct sockaddr *sa, socklen_t *salen);
diff --git a/server/spice-server.syms b/server/spice-server.syms
index 4137546..d65e14d 100644
--- a/server/spice-server.syms
+++ b/server/spice-server.syms
@@ -162,8 +162,3 @@ global:
     spice_replay_next_cmd;
     spice_replay_free_cmd;
 } SPICE_SERVER_0.12.5;
-
-SPICE_SERVER_0.12.7 {
-global:
-    spice_server_set_keepalive_timeout;
-} SPICE_SERVER_0.12.6;
commit 4e17b9ee440dbfea6ca87565a09c4bca57a93043
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Wed Mar 2 12:24:11 2016 +0100

    Always enable TCP keepalive
    
    Always enabled, hardcoded interval
    as per https://bugzilla.redhat.com/show_bug.cgi?id=1298590

diff --git a/server/reds.c b/server/reds.c
index fc391ea..f07d24e 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -2249,6 +2249,31 @@ static void reds_handle_ssl_accept(int fd, int event, void *data)
     }
 }
 
+#define KEEPALIVE_TIMEOUT (10*60)
+
+static bool reds_init_keepalive(int socket)
+{
+    int keepalive = 1;
+    int keepalive_timeout = KEEPALIVE_TIMEOUT;
+
+    if (setsockopt(socket, SOL_SOCKET, SO_KEEPALIVE, &keepalive, sizeof(keepalive)) == -1) {
+        if (errno != ENOTSUP) {
+            spice_printerr("setsockopt for keepalive failed, %s", strerror(errno));
+            return false;
+        }
+    }
+
+    if (setsockopt(socket, SOL_TCP, TCP_KEEPIDLE,
+                   &keepalive_timeout, sizeof(keepalive_timeout)) == -1) {
+        if (errno != ENOTSUP) {
+            spice_printerr("setsockopt for keepalive timeout failed, %s", strerror(errno));
+            return false;
+        }
+    }
+
+    return true;
+}
+
 static RedLinkInfo *reds_init_client_connection(int socket)
 {
     RedLinkInfo *link;
@@ -2271,20 +2296,7 @@ static RedLinkInfo *reds_init_client_connection(int socket)
         }
     }
 
-    if (reds->keepalive_timeout > 0) {
-        int keepalive = 1;
-        if (setsockopt(socket, SOL_SOCKET, SO_KEEPALIVE, &keepalive, sizeof(keepalive)) == -1) {
-            if (errno != ENOTSUP) {
-                spice_printerr("setsockopt for keepalive failed, %s", strerror(errno));
-            }
-        }
-        if (setsockopt(socket, SOL_TCP, TCP_KEEPIDLE,
-                       &reds->keepalive_timeout, sizeof(reds->keepalive_timeout)) == -1) {
-            if (errno != ENOTSUP) {
-                spice_printerr("setsockopt for keepalive timeout failed, %s", strerror(errno));
-            }
-        }
-    }
+    reds_init_keepalive(socket);
 
     link = spice_new0(RedLinkInfo, 1);
     link->stream = reds_stream_new(socket);


More information about the Spice-commits mailing list