[Spice-commits] 3 commits - src/channel-main.c

Jonathon Jongsma jjongsma at kemper.freedesktop.org
Thu Oct 22 08:40:05 PDT 2015

 src/channel-main.c |   50 +++++++++++++++++++++++++++++++++-----------------
 1 file changed, 33 insertions(+), 17 deletions(-)

New commits:
commit 51e5400d48efe24526c368e95689d085aa5cc959
Author: Jonathon Jongsma <jjongsma at redhat.com>
Date:   Wed Oct 21 15:26:54 2015 -0500

    Remove noisy debug statement
    This isn't terribly useful for general debugging and makes the log
    pretty noisy when transferring large files.

diff --git a/src/channel-main.c b/src/channel-main.c
index 2493ead..0eb40b9 100644
--- a/src/channel-main.c
+++ b/src/channel-main.c
@@ -972,7 +972,6 @@ static gboolean file_xfer_flush_finish(SpiceMainChannel *channel, GAsyncResult *
         return FALSE;
-    CHANNEL_DEBUG(channel, "flushed finished!");
     return g_simple_async_result_get_op_res_gboolean(simple);
commit e0ecb6985d18903d54e822089de1888329614001
Author: Jonathon Jongsma <jjongsma at redhat.com>
Date:   Wed Oct 21 14:52:57 2015 -0500

    Don't print error message on successful file transfer
    In certain circumstances we were printing an error message even though
    the file transfer had completed successfully. It didn't cause any
    problems, but it pointed out an issue in the handling of outgoing agent
    The described behavior was generally only encountered when there were multiple
    files being transferred at once, and most often when one or two files were
    significantly smaller than another file being transferred. For example, two
    20kB files and another 3MB file. The failure mechanism is basically as follows:
    For each file transfer, we read a chunk for the file and then we queue a series
    of messages to the guest and wait for them to be flushed before continuing to
    read a new chunk of the file. (Since the maximum agent message size is 2kB, each
    64kB chunk of file being read can generate up to 32 messages at a time). The
    file transfer task remains in "pending" state until the flush operation is
    complete. Since all agent messages go into a single channel-wide queue, waiting
    for the messages to be flushed meant waiting for *all* outgoing messages to be
    flushed, including those that were queued after we called flush_async().
    Since agent messages can only be sent if the client has enough agent tokens, it
    can take a little while for all queued messages to be sent.  This means that
    even if a file transfer task has sent out all of its data to the guest, it can
    be kept in "pending" state for some time if other file transfer tasks or other
    agent messages are added to the queue. In this situation, The guest might
    notice that it has received all of the data for a file transfer task and send a
    XFER_STATUS_SUCCESS message while we're still in "pending" state.
    This patch changes to code so that flush_async() only waits for the
    currently-queued messages to be sent. This means that it is not affected by any
    new messages that get added to the outgoing queue after we've called
    flush_async(). This means that the task will exit "pending" state immediately
    after sending out our last data packet.
    Fixes: rhbz#1265562

diff --git a/src/channel-main.c b/src/channel-main.c
index a833078..2493ead 100644
--- a/src/channel-main.c
+++ b/src/channel-main.c
@@ -152,7 +152,7 @@ struct _SpiceMainChannelPrivate  {
     gint                        timer_id;
     GQueue                      *agent_msg_queue;
     GHashTable                  *file_xfer_tasks;
-    GSList                      *flushing;
+    GHashTable                  *flushing;
     guint                       switch_host_delayed_id;
     guint                       migrate_delayed_id;
@@ -289,6 +289,8 @@ static void spice_main_channel_init(SpiceMainChannel *channel)
     c->agent_msg_queue = g_queue_new();
     c->file_xfer_tasks = g_hash_table_new_full(g_direct_hash, g_direct_equal,
                                                NULL, g_object_unref);
+    c->flushing = g_hash_table_new_full(g_direct_hash, g_direct_equal, NULL,
+                                        g_object_unref);
     c->cancellable_volume_info = g_cancellable_new();
@@ -406,6 +408,7 @@ static void spice_main_channel_dispose(GObject *obj)
     g_clear_pointer(&c->file_xfer_tasks, g_hash_table_unref);
+    g_clear_pointer (&c->flushing, g_hash_table_unref);
@@ -917,20 +920,22 @@ static void agent_free_msg_queue(SpiceMainChannel *channel)
     c->agent_msg_queue = NULL;
-/* Here, flushing algorithm is stolen from spice-channel.c */
-static void file_xfer_flushed(SpiceMainChannel *channel, gboolean success)
+static gboolean flush_foreach_remove(gpointer key G_GNUC_UNUSED,
+                                     gpointer value, gpointer user_data)
-    SpiceMainChannelPrivate *c = channel->priv;
-    GSList *l;
+    gboolean success = GPOINTER_TO_UINT(user_data);
+    GSimpleAsyncResult *result = G_SIMPLE_ASYNC_RESULT(value);
-    for (l = c->flushing; l != NULL; l = l->next) {
-        GSimpleAsyncResult *result = G_SIMPLE_ASYNC_RESULT(l->data);
-        g_simple_async_result_set_op_res_gboolean(result, success);
-        g_simple_async_result_complete_in_idle(result);
-    }
+    g_simple_async_result_set_op_res_gboolean(result, success);
+    g_simple_async_result_complete_in_idle(result);
+    return TRUE;
-    g_slist_free_full(c->flushing, g_object_unref);
-    c->flushing = NULL;
+static void file_xfer_flushed(SpiceMainChannel *channel, gboolean success)
+    SpiceMainChannelPrivate *c = channel->priv;
+    g_hash_table_foreach_remove(c->flushing, flush_foreach_remove,
+                                GUINT_TO_POINTER(success));
 static void file_xfer_flush_async(SpiceMainChannel *channel, GCancellable *cancellable,
@@ -951,7 +956,8 @@ static void file_xfer_flush_async(SpiceMainChannel *channel, GCancellable *cance
-    c->flushing = g_slist_append(c->flushing, simple);
+    /* wait until the last message currently in the queue has been sent */
+    g_hash_table_insert(c->flushing, g_queue_peek_tail(c->agent_msg_queue), simple);
 static gboolean file_xfer_flush_finish(SpiceMainChannel *channel, GAsyncResult *result,
@@ -978,11 +984,22 @@ static void agent_send_msg_queue(SpiceMainChannel *channel)
     while (c->agent_tokens > 0 &&
            !g_queue_is_empty(c->agent_msg_queue)) {
+        GSimpleAsyncResult *simple;
         out = g_queue_pop_head(c->agent_msg_queue);
+        simple = g_hash_table_lookup(c->flushing, out);
+        if (simple) {
+            /* if there's a flush task waiting for this message, finish it */
+            g_simple_async_result_set_op_res_gboolean(simple, TRUE);
+            g_simple_async_result_complete_in_idle(simple);
+            g_hash_table_remove(c->flushing, out);
+        }
-    if (g_queue_is_empty(c->agent_msg_queue) && c->flushing != NULL) {
+    if (g_queue_is_empty(c->agent_msg_queue) &&
+        g_hash_table_size(c->flushing) != 0) {
+        g_warning("unexpected flush task in list, clearing");
         file_xfer_flushed(channel, TRUE);
commit 0c7643cb7f5c5411daff5f6e5d8a823b48ac874a
Author: Jonathon Jongsma <jjongsma at redhat.com>
Date:   Wed Oct 21 14:30:43 2015 -0500

    MainChannel: move task free from finalize to dispose
    In order to avoid reference cycles, you're supposed to release
    references in dispose, especially to those objects that can hold
    references to yourself. This probably wasn't causing any leaks, since
    the file transfer tasks generally are not alive when the main channel is
    destroyed, but it's more proper.

diff --git a/src/channel-main.c b/src/channel-main.c
index 3a8c1dd..a833078 100644
--- a/src/channel-main.c
+++ b/src/channel-main.c
@@ -405,6 +405,8 @@ static void spice_main_channel_dispose(GObject *obj)
         c->migrate_delayed_id = 0;
+    g_clear_pointer(&c->file_xfer_tasks, g_hash_table_unref);
@@ -418,8 +420,6 @@ static void spice_main_channel_finalize(GObject *obj)
-    if (c->file_xfer_tasks)
-        g_hash_table_unref(c->file_xfer_tasks);
     if (G_OBJECT_CLASS(spice_main_channel_parent_class)->finalize)

More information about the Spice-commits mailing list