[Spice-commits] 6 commits - server/main-channel-client.c server/red-channel-client.c

Christophe Fergau teuf at kemper.freedesktop.org
Tue Apr 11 09:21:39 UTC 2017


 server/main-channel-client.c |   79 -------------------------------------------
 server/red-channel-client.c  |   72 +++++++++++++++++++--------------------
 2 files changed, 36 insertions(+), 115 deletions(-)

New commits:
commit c765e9f2839396f819ef43137c24fbe54041f89c
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Fri Apr 7 15:57:45 2017 +0200

    main-channel-client: Remove unused statistics code
    
    Main-channel-client.c has code to send pings when the statistics code is
    enabled. This is done through a timer calling a ping_timer_cb callback.
    However, this timer is created but never started, so this code is
    actually dead-code.
    
    Since this code does not seem to be doing much apart from sending the
    pings (which can be achieved by simply setting monitor-latency to TRUE
    in main_channel_client_create()), this commit removes it.
    
    Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>
    Acked-by: Jonathon Jongsma <jjongsma at redhat.com>

diff --git a/server/main-channel-client.c b/server/main-channel-client.c
index c25a3471..3a379444 100644
--- a/server/main-channel-client.c
+++ b/server/main-channel-client.c
@@ -39,7 +39,6 @@ enum NetTestStage {
 };
 
 #define CLIENT_CONNECTIVITY_TIMEOUT (MSEC_PER_SEC * 30)
-#define PING_INTERVAL (MSEC_PER_SEC * 10)
 
 G_DEFINE_TYPE(MainChannelClient, main_channel_client, RED_TYPE_CHANNEL_CLIENT)
 
@@ -57,10 +56,6 @@ struct MainChannelClientPrivate {
     int net_test_stage;
     uint64_t latency;
     uint64_t bitrate_per_sec;
-#ifdef RED_STATISTICS
-    SpiceTimer *ping_timer;
-    int ping_interval;
-#endif
     int mig_wait_connect;
     int mig_connect_ok;
     int mig_wait_prev_complete;
@@ -167,38 +162,6 @@ static void main_channel_client_set_property(GObject *object,
     }
 }
 
-#ifdef RED_STATISTICS
-static void ping_timer_cb(void *opaque);
-#endif
-
-static void main_channel_client_constructed(GObject *object)
-{
-    G_OBJECT_CLASS(main_channel_client_parent_class)->constructed(object);
-#ifdef RED_STATISTICS
-    MainChannelClient *self = MAIN_CHANNEL_CLIENT(object);
-    RedsState *reds =
-        red_channel_get_server(red_channel_client_get_channel(RED_CHANNEL_CLIENT(object)));
-
-    self->priv->ping_timer = reds_core_timer_add(reds, ping_timer_cb, self);
-    if (!self->priv->ping_timer) {
-        spice_error("ping timer create failed");
-    }
-    self->priv->ping_interval = PING_INTERVAL;
-#endif
-}
-
-static void main_channel_client_finalize(GObject *object)
-{
-#ifdef RED_STATISTICS
-    MainChannelClient *self = MAIN_CHANNEL_CLIENT(object);
-    RedsState *reds =
-        red_channel_get_server(red_channel_client_get_channel(RED_CHANNEL_CLIENT(object)));
-
-    reds_core_timer_remove(reds, self->priv->ping_timer);
-#endif
-    G_OBJECT_CLASS(main_channel_client_parent_class)->finalize(object);
-}
-
 static uint8_t *
 main_channel_client_alloc_msg_rcv_buf(RedChannelClient *rcc,
                                       uint16_t type, uint32_t size)
@@ -235,8 +198,6 @@ static void main_channel_client_class_init(MainChannelClientClass *klass)
 
     object_class->get_property = main_channel_client_get_property;
     object_class->set_property = main_channel_client_set_property;
-    object_class->finalize = main_channel_client_finalize;
-    object_class->constructed = main_channel_client_constructed;
 
     client_class->alloc_recv_buf = main_channel_client_alloc_msg_rcv_buf;
     client_class->release_recv_buf = main_channel_client_release_msg_rcv_buf;
@@ -646,45 +607,6 @@ gboolean main_channel_client_migrate_src_complete(MainChannelClient *mcc,
     return ret;
 }
 
-#ifdef RED_STATISTICS
-static void do_ping_client(MainChannelClient *mcc,
-    const char *opt, int has_interval, int interval)
-{
-    RedChannel *channel = red_channel_client_get_channel(RED_CHANNEL_CLIENT(mcc));
-    spice_printerr("");
-    if (!opt) {
-        main_channel_client_push_ping(mcc, 0);
-    } else if (!strcmp(opt, "on")) {
-        if (has_interval && interval > 0) {
-            mcc->priv->ping_interval = interval * MSEC_PER_SEC;
-        }
-        reds_core_timer_start(red_channel_get_server(channel),
-                              mcc->priv->ping_timer, mcc->priv->ping_interval);
-    } else if (!strcmp(opt, "off")) {
-        reds_core_timer_cancel(red_channel_get_server(channel),
-                               mcc->priv->ping_timer);
-    } else {
-        return;
-    }
-}
-
-static void ping_timer_cb(void *opaque)
-{
-    MainChannelClient *mcc = opaque;
-    RedChannel *channel = red_channel_client_get_channel(RED_CHANNEL_CLIENT(mcc));
-
-    if (!red_channel_client_is_connected(RED_CHANNEL_CLIENT(mcc))) {
-        spice_printerr("not connected to peer, ping off");
-        reds_core_timer_cancel(red_channel_get_server(channel),
-                               mcc->priv->ping_timer);
-        return;
-    }
-    do_ping_client(mcc, NULL, 0, 0);
-    reds_core_timer_start(red_channel_get_server(channel),
-                          mcc->priv->ping_timer, mcc->priv->ping_interval);
-}
-#endif /* RED_STATISTICS */
-
 MainChannelClient *main_channel_client_create(MainChannel *main_chan, RedClient *client,
                                               RedsStream *stream, uint32_t connection_id,
                                               RedChannelCapabilities *caps)
commit cf3657d2e4dabf6038249f2c6c0a161a6346b540
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Thu Apr 6 18:03:25 2017 +0200

    Use red_channel_client_is_blocked
    
    Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>
    Acked-by: Jonathon Jongsma <jjongsma at redhat.com>

diff --git a/server/red-channel-client.c b/server/red-channel-client.c
index db561b38..802cd46a 100644
--- a/server/red-channel-client.c
+++ b/server/red-channel-client.c
@@ -637,7 +637,7 @@ static void red_channel_client_msg_sent(RedChannelClient *rcc)
     }
 
     red_channel_client_clear_sent_item(rcc);
-    if (rcc->priv->send_data.blocked) {
+    if (red_channel_client_is_blocked(rcc)) {
         SpiceCoreInterfaceInternal *core = red_channel_get_core_interface(rcc->priv->channel);
         rcc->priv->send_data.blocked = FALSE;
         core->watch_update_mask(core, rcc->priv->stream->watch,
@@ -650,7 +650,7 @@ static void red_channel_client_msg_sent(RedChannelClient *rcc)
         red_channel_client_begin_send_message(rcc);
     } else {
         if (rcc->priv->latency_monitor.timer
-            && !rcc->priv->send_data.blocked
+            && !red_channel_client_is_blocked(rcc)
             && g_queue_is_empty(&rcc->priv->pipe)) {
             /* It is possible that the socket will become idle, so we may be able to test latency */
             red_channel_client_restart_ping_timer(rcc);
@@ -747,7 +747,7 @@ static void red_channel_client_connectivity_timer(void *opaque)
 
     if (monitor->state == CONNECTIVITY_STATE_BLOCKED) {
         if (!monitor->received_bytes && !monitor->sent_bytes) {
-            if (!rcc->priv->send_data.blocked && !red_channel_client_waiting_for_ack(rcc)) {
+            if (!red_channel_client_is_blocked(rcc) && !red_channel_client_waiting_for_ack(rcc)) {
                 spice_error("mismatch between rcc-state and connectivity-state");
             }
             spice_debug("rcc is blocked; connection is idle");
@@ -768,7 +768,7 @@ static void red_channel_client_connectivity_timer(void *opaque)
         SpiceCoreInterfaceInternal *core = red_channel_get_core_interface(rcc->priv->channel);
         monitor->received_bytes = false;
         monitor->sent_bytes = false;
-        if (rcc->priv->send_data.blocked || red_channel_client_waiting_for_ack(rcc)) {
+        if (red_channel_client_is_blocked(rcc) || red_channel_client_waiting_for_ack(rcc)) {
             monitor->state = CONNECTIVITY_STATE_BLOCKED;
         } else if (rcc->priv->latency_monitor.state == PING_STATE_WARMUP ||
                    rcc->priv->latency_monitor.state == PING_STATE_LATENCY) {
@@ -1279,7 +1279,7 @@ void red_channel_client_send(RedChannelClient *rcc)
 
 static inline RedPipeItem *red_channel_client_pipe_item_get(RedChannelClient *rcc)
 {
-    if (!rcc || rcc->priv->send_data.blocked
+    if (!rcc || red_channel_client_is_blocked(rcc)
              || red_channel_client_waiting_for_ack(rcc)) {
         return NULL;
     }
@@ -1296,11 +1296,11 @@ void red_channel_client_push(RedChannelClient *rcc)
         return;
     }
     g_object_ref(rcc);
-    if (rcc->priv->send_data.blocked) {
+    if (red_channel_client_is_blocked(rcc)) {
         red_channel_client_send(rcc);
     }
 
-    if (!red_channel_client_no_item_being_sent(rcc) && !rcc->priv->send_data.blocked) {
+    if (!red_channel_client_no_item_being_sent(rcc) && !red_channel_client_is_blocked(rcc)) {
         rcc->priv->send_data.blocked = TRUE;
         spice_printerr("ERROR: an item waiting to be sent and not blocked");
     }
commit 24020ab931c1fd1eb82f0a0c883ea04d29f00df6
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Thu Apr 6 18:01:34 2017 +0200

    Use bool in RedChannelClientLatencyMonitor
    
    Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>
    Acked-by: Jonathon Jongsma <jjongsma at redhat.com>

diff --git a/server/red-channel-client.c b/server/red-channel-client.c
index 536d9911..db561b38 100644
--- a/server/red-channel-client.c
+++ b/server/red-channel-client.c
@@ -79,8 +79,8 @@ typedef struct RedChannelClientLatencyMonitor {
     uint64_t last_pong_time;
     SpiceTimer *timer;
     uint32_t id;
-    int tcp_nodelay;
-    int warmup_was_sent;
+    bool tcp_nodelay;
+    bool warmup_was_sent;
 
     int64_t roundtrip;
 } RedChannelClientLatencyMonitor;
@@ -557,13 +557,13 @@ static void red_channel_client_send_ping(RedChannelClient *rcc)
     if (!rcc->priv->latency_monitor.warmup_was_sent) { // latency test start
         int delay_val;
 
-        rcc->priv->latency_monitor.warmup_was_sent = TRUE;
+        rcc->priv->latency_monitor.warmup_was_sent = true;
         /*
          * When testing latency, TCP_NODELAY must be switched on, otherwise,
          * sending the ping message is delayed by Nagle algorithm, and the
          * roundtrip measurement is less accurate (bigger).
          */
-        rcc->priv->latency_monitor.tcp_nodelay = 1;
+        rcc->priv->latency_monitor.tcp_nodelay = true;
         delay_val = reds_stream_get_no_delay(rcc->priv->stream);
         if (delay_val != -1) {
             rcc->priv->latency_monitor.tcp_nodelay = delay_val;
@@ -682,7 +682,7 @@ static void red_channel_client_push_ping(RedChannelClient *rcc)
 {
     spice_assert(rcc->priv->latency_monitor.state == PING_STATE_NONE);
     rcc->priv->latency_monitor.state = PING_STATE_WARMUP;
-    rcc->priv->latency_monitor.warmup_was_sent = FALSE;
+    rcc->priv->latency_monitor.warmup_was_sent = false;
     rcc->priv->latency_monitor.id = rand();
     red_channel_client_pipe_add_type(rcc, RED_PIPE_ITEM_TYPE_PING);
     red_channel_client_pipe_add_type(rcc, RED_PIPE_ITEM_TYPE_PING);
commit 6ccd8721a431a01fd4de44fab82da90d29fd1e44
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Thu Apr 6 17:52:11 2017 +0200

    Use enum rather than int in RedChannelClient{Latency,Connectivity}Monitor
    
    Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>
    Acked-by: Jonathon Jongsma <jjongsma at redhat.com>

diff --git a/server/red-channel-client.c b/server/red-channel-client.c
index 17432778..536d9911 100644
--- a/server/red-channel-client.c
+++ b/server/red-channel-client.c
@@ -67,8 +67,15 @@ struct SpiceDataHeaderOpaque {
     get_msg_size_proc get_msg_size;
 };
 
+typedef enum {
+    PING_STATE_NONE,
+    PING_STATE_TIMER,
+    PING_STATE_WARMUP,
+    PING_STATE_LATENCY,
+} QosPingState;
+
 typedef struct RedChannelClientLatencyMonitor {
-    int state;
+    QosPingState state;
     uint64_t last_pong_time;
     SpiceTimer *timer;
     uint32_t id;
@@ -78,8 +85,15 @@ typedef struct RedChannelClientLatencyMonitor {
     int64_t roundtrip;
 } RedChannelClientLatencyMonitor;
 
+typedef enum {
+    CONNECTIVITY_STATE_CONNECTED,
+    CONNECTIVITY_STATE_BLOCKED,
+    CONNECTIVITY_STATE_WAIT_PONG,
+    CONNECTIVITY_STATE_DISCONNECTED,
+} ConnectivityState;
+
 typedef struct RedChannelClientConnectivityMonitor {
-    int state;
+    ConnectivityState state;
     bool sent_bytes;
     bool received_bytes;
     uint32_t timeout;
@@ -197,20 +211,6 @@ enum {
 #define PING_TEST_TIMEOUT_MS (MSEC_PER_SEC * 15)
 #define PING_TEST_IDLE_NET_TIMEOUT_MS (MSEC_PER_SEC / 10)
 
-enum QosPingState {
-    PING_STATE_NONE,
-    PING_STATE_TIMER,
-    PING_STATE_WARMUP,
-    PING_STATE_LATENCY,
-};
-
-enum ConnectivityState {
-    CONNECTIVITY_STATE_CONNECTED,
-    CONNECTIVITY_STATE_BLOCKED,
-    CONNECTIVITY_STATE_WAIT_PONG,
-    CONNECTIVITY_STATE_DISCONNECTED,
-};
-
 typedef struct RedEmptyMsgPipeItem {
     RedPipeItem base;
     int msg;
commit 54146811b245c43ec95202fc4e25206c3fa140d1
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Thu Apr 6 17:25:09 2017 +0200

    Use bool in ConnectivityMonitor
    
    Their uint32_t value is never used, all that matters is whether we
    received data or not.
    
    Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>
    Acked-by: Jonathon Jongsma <jjongsma at redhat.com>

diff --git a/server/red-channel-client.c b/server/red-channel-client.c
index 4eac231b..17432778 100644
--- a/server/red-channel-client.c
+++ b/server/red-channel-client.c
@@ -80,8 +80,8 @@ typedef struct RedChannelClientLatencyMonitor {
 
 typedef struct RedChannelClientConnectivityMonitor {
     int state;
-    uint32_t out_bytes;
-    uint32_t in_bytes;
+    bool sent_bytes;
+    bool received_bytes;
     uint32_t timeout;
     SpiceTimer *timer;
 } RedChannelClientConnectivityMonitor;
@@ -465,7 +465,7 @@ RedChannel* red_channel_client_get_channel(RedChannelClient *rcc)
 static void red_channel_client_data_sent(RedChannelClient *rcc, int n)
 {
     if (rcc->priv->connectivity_monitor.timer) {
-        rcc->priv->connectivity_monitor.out_bytes += n;
+        rcc->priv->connectivity_monitor.sent_bytes = true;
     }
     stat_inc_counter(rcc->priv->out_bytes, n);
 }
@@ -473,7 +473,7 @@ static void red_channel_client_data_sent(RedChannelClient *rcc, int n)
 static void red_channel_client_data_read(RedChannelClient *rcc, int n)
 {
     if (rcc->priv->connectivity_monitor.timer) {
-        rcc->priv->connectivity_monitor.in_bytes += n;
+        rcc->priv->connectivity_monitor.received_bytes = true;
     }
 }
 
@@ -746,7 +746,7 @@ static void red_channel_client_connectivity_timer(void *opaque)
     int is_alive = TRUE;
 
     if (monitor->state == CONNECTIVITY_STATE_BLOCKED) {
-        if (monitor->in_bytes == 0 && monitor->out_bytes == 0) {
+        if (!monitor->received_bytes && !monitor->sent_bytes) {
             if (!rcc->priv->send_data.blocked && !red_channel_client_waiting_for_ack(rcc)) {
                 spice_error("mismatch between rcc-state and connectivity-state");
             }
@@ -754,7 +754,7 @@ static void red_channel_client_connectivity_timer(void *opaque)
             is_alive = FALSE;
         }
     } else if (monitor->state == CONNECTIVITY_STATE_WAIT_PONG) {
-        if (monitor->in_bytes == 0) {
+        if (!monitor->received_bytes) {
             if (rcc->priv->latency_monitor.state != PING_STATE_WARMUP &&
                 rcc->priv->latency_monitor.state != PING_STATE_LATENCY) {
                 spice_error("mismatch between rcc-state and connectivity-state");
@@ -766,8 +766,8 @@ static void red_channel_client_connectivity_timer(void *opaque)
 
     if (is_alive) {
         SpiceCoreInterfaceInternal *core = red_channel_get_core_interface(rcc->priv->channel);
-        monitor->in_bytes = 0;
-        monitor->out_bytes = 0;
+        monitor->received_bytes = false;
+        monitor->sent_bytes = false;
         if (rcc->priv->send_data.blocked || red_channel_client_waiting_for_ack(rcc)) {
             monitor->state = CONNECTIVITY_STATE_BLOCKED;
         } else if (rcc->priv->latency_monitor.state == PING_STATE_WARMUP ||
commit 9770428c0d5501bed9332c0fc0a1d627e6d1adb7
Author: Christophe Fergeau <cfergeau at redhat.com>
Date:   Thu Apr 6 17:24:34 2017 +0200

    Remove one more unused "monitor_latency" arguments
    
    This was missed in commit db9dcd9.
    
    Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>
    Acked-by: Jonathon Jongsma <jjongsma at redhat.com>

diff --git a/server/main-channel-client.c b/server/main-channel-client.c
index 9eb60c99..c25a3471 100644
--- a/server/main-channel-client.c
+++ b/server/main-channel-client.c
@@ -696,7 +696,6 @@ MainChannelClient *main_channel_client_create(MainChannel *main_chan, RedClient
                          "channel", RED_CHANNEL(main_chan),
                          "client", client,
                          "stream", stream,
-                         "monitor-latency", FALSE,
                          "caps", caps,
                          "connection-id", connection_id,
                          NULL);


More information about the Spice-commits mailing list