[Spice-commits] 3 commits - src/channel-main.c src/spice-file-transfer-task.c

Victor Toso de Carvalho victortoso at kemper.freedesktop.org
Wed Aug 10 07:59:36 UTC 2016


 src/channel-main.c             |   44 ++++++++++++++++++++++-------------------
 src/spice-file-transfer-task.c |   40 ++++++++++++++++++++-----------------
 2 files changed, 46 insertions(+), 38 deletions(-)

New commits:
commit 2481df264d57b5992fc123cee57ed824465101aa
Author: Victor Toso <victortoso at redhat.com>
Date:   Sat Aug 6 16:47:53 2016 +0200

    channel-main: make debug helpful in case of failures
    
    Debug should come before the g_return_if_fail() otherwise we lose
    important debug data.
    
    Acked-by: Jonathon Jongsma <jjongsma at redhat.com>

diff --git a/src/channel-main.c b/src/channel-main.c
index f3e666b..47e19e1 100644
--- a/src/channel-main.c
+++ b/src/channel-main.c
@@ -1841,12 +1841,12 @@ static void main_agent_handle_xfer_status(SpiceMainChannel *channel,
     FileTransferOperation *xfer_op;
     GError *error = NULL;
 
+    SPICE_DEBUG("xfer-task %u received response %u", msg->id, msg->result);
+
     xfer_task = spice_main_channel_find_xfer_task_by_task_id(channel, msg->id);
     g_return_if_fail(xfer_task != NULL);
     xfer_op = g_hash_table_lookup(channel->priv->file_xfer_tasks, GUINT_TO_POINTER(msg->id));
 
-    SPICE_DEBUG("xfer-task %u received response %u", msg->id, msg->result);
-
     switch (msg->result) {
     case VD_AGENT_FILE_XFER_STATUS_CAN_SEND_DATA:
         g_return_if_fail(spice_file_transfer_task_is_completed(xfer_task) == FALSE);
commit cbfcaa8c82b4e1633f86c52019b45f6f58d92b3d
Author: Victor Toso <victortoso at redhat.com>
Date:   Fri Aug 5 19:08:41 2016 +0200

    channel-main: improve code to use FileTransferOperation
    
    Since commit b26818d0e00666 we are grouping all transferred files per
    operation (drag and drop); That leaded to some design flaws in the
    code. The changes suggested in this patch are (in execution order):
    
    * On spice_file_transfer_task_read_async()
    - Passing FileTransferOperation data so in the callback and in further
      function calls, we don't need to look for this;
    
    Note that FileTransferOperation is not freed while any of its
    SpiceFileTransferTask exists and by design they are never finished
    while on pending state, so file_xfer_read_async_cb() should have a
    valid FileTransferOperation pointer.
    
    * On file_xfer_read_async_cb()
    - Removed unnecessary variables;
    
    * On file_xfer_flush_async()
    - Using SpiceFileTransferTask as parameter which can retrieve the
      SpiceMainChannel and the GCancellable;
    - Using SpiceFileTransferTask as source_object for GTask with
      FileTransferOperation as user_data which is helpful for its
      callback;
    
    * On file_xfer_flush_finish() and file_xfer_data_flushed_cb()
    - Using SpiceFileTransferTask as parameter and source_object check for
      GTask
    
    Acked-by: Jonathon Jongsma <jjongsma at redhat.com>

diff --git a/src/channel-main.c b/src/channel-main.c
index d9fd025..f3e666b 100644
--- a/src/channel-main.c
+++ b/src/channel-main.c
@@ -889,15 +889,22 @@ static void file_xfer_flushed(SpiceMainChannel *channel, gboolean success)
                                 GUINT_TO_POINTER(success));
 }
 
-static void file_xfer_flush_async(SpiceMainChannel *channel, GCancellable *cancellable,
-                                  GAsyncReadyCallback callback, gpointer user_data)
+static void file_xfer_flush_async(SpiceFileTransferTask *xfer_task,
+                                  GAsyncReadyCallback callback,
+                                  gpointer user_data)
 {
     GTask *task;
-    SpiceMainChannelPrivate *c = channel->priv;
+    SpiceMainChannel *channel;
+    SpiceMainChannelPrivate *c;
     gboolean was_empty;
 
-    task = g_task_new(channel, cancellable, callback, user_data);
+    channel = spice_file_transfer_task_get_channel(xfer_task);
+    task = g_task_new(xfer_task,
+                      spice_file_transfer_task_get_cancellable(xfer_task),
+                      callback,
+                      user_data);
 
+    c = channel->priv;
     was_empty = g_queue_is_empty(c->agent_msg_queue);
     if (was_empty) {
         g_task_return_boolean(task, TRUE);
@@ -909,12 +916,13 @@ static void file_xfer_flush_async(SpiceMainChannel *channel, GCancellable *cance
     g_hash_table_insert(c->flushing, g_queue_peek_tail(c->agent_msg_queue), task);
 }
 
-static gboolean file_xfer_flush_finish(SpiceMainChannel *channel, GAsyncResult *result,
+static gboolean file_xfer_flush_finish(SpiceFileTransferTask *xfer_task,
+                                       GAsyncResult *result,
                                        GError **error)
 {
     GTask *task = G_TASK(result);
 
-    g_return_val_if_fail(g_task_is_valid(result, channel), FALSE);
+    g_return_val_if_fail(g_task_is_valid(result, xfer_task), FALSE);
 
     return g_task_propagate_boolean(task, error);
 }
@@ -1756,11 +1764,10 @@ static void file_xfer_data_flushed_cb(GObject *source_object,
                                       GAsyncResult *res,
                                       gpointer user_data)
 {
-    SpiceFileTransferTask *xfer_task = user_data;
-    SpiceMainChannel *channel = (SpiceMainChannel *)source_object;
+    SpiceFileTransferTask *xfer_task = SPICE_FILE_TRANSFER_TASK(source_object);
     GError *error = NULL;
 
-    file_xfer_flush_finish(channel, res, &error);
+    file_xfer_flush_finish(xfer_task, res, &error);
     if (error) {
         spice_file_transfer_task_completed(xfer_task, error);
         return;
@@ -1770,7 +1777,7 @@ static void file_xfer_data_flushed_cb(GObject *source_object,
     if (!spice_file_transfer_task_is_completed(xfer_task)) {
         file_transfer_operation_send_progress(xfer_task);
         /* Read more data */
-        spice_file_transfer_task_read_async(xfer_task, file_xfer_read_async_cb, NULL);
+        spice_file_transfer_task_read_async(xfer_task, file_xfer_read_async_cb, user_data);
     }
 }
 
@@ -1801,11 +1808,10 @@ static void file_xfer_read_async_cb(GObject *source_object,
     SpiceMainChannel *channel;
     gssize count;
     char *buffer;
-    GCancellable *cancellable;
-    guint32 task_id;
     GError *error = NULL;
 
     xfer_task = SPICE_FILE_TRANSFER_TASK(source_object);
+    xfer_op = user_data;
 
     channel = spice_file_transfer_task_get_channel(xfer_task);
     count = spice_file_transfer_task_read_finish(xfer_task, res, &buffer, &error);
@@ -1822,13 +1828,9 @@ static void file_xfer_read_async_cb(GObject *source_object,
         return;
     }
 
-    task_id = spice_file_transfer_task_get_id(xfer_task);
-    xfer_op = g_hash_table_lookup(channel->priv->file_xfer_tasks, GUINT_TO_POINTER(task_id));
-    g_return_if_fail(xfer_op != NULL);
     xfer_op->stats.total_sent += count;
 
-    cancellable = spice_file_transfer_task_get_cancellable(xfer_task);
-    file_xfer_flush_async(channel, cancellable, file_xfer_data_flushed_cb, xfer_task);
+    file_xfer_flush_async(xfer_task, file_xfer_data_flushed_cb, xfer_op);
 }
 
 /* coroutine context */
@@ -1836,17 +1838,19 @@ static void main_agent_handle_xfer_status(SpiceMainChannel *channel,
                                           VDAgentFileXferStatusMessage *msg)
 {
     SpiceFileTransferTask *xfer_task;
+    FileTransferOperation *xfer_op;
     GError *error = NULL;
 
     xfer_task = spice_main_channel_find_xfer_task_by_task_id(channel, msg->id);
     g_return_if_fail(xfer_task != NULL);
+    xfer_op = g_hash_table_lookup(channel->priv->file_xfer_tasks, GUINT_TO_POINTER(msg->id));
 
     SPICE_DEBUG("xfer-task %u received response %u", msg->id, msg->result);
 
     switch (msg->result) {
     case VD_AGENT_FILE_XFER_STATUS_CAN_SEND_DATA:
         g_return_if_fail(spice_file_transfer_task_is_completed(xfer_task) == FALSE);
-        spice_file_transfer_task_read_async(xfer_task, file_xfer_read_async_cb, NULL);
+        spice_file_transfer_task_read_async(xfer_task, file_xfer_read_async_cb, xfer_op);
         return;
     case VD_AGENT_FILE_XFER_STATUS_CANCELLED:
         error = g_error_new(SPICE_CLIENT_ERROR, SPICE_CLIENT_ERROR_FAILED,
commit 7b9ba36bb5105de1a857cb99d81265d2484fd2ba
Author: Victor Toso <victortoso at redhat.com>
Date:   Wed Aug 3 17:52:32 2016 +0200

    file-transfer: Move initializations to _task_new()
    
    This was a request introduced at f6b3b697093a16de to be done after
    moving the SpiceFileTransferTask code to its own file.
    
    Acked-by: Jonathon Jongsma <jjongsma at redhat.com>

diff --git a/src/spice-file-transfer-task.c b/src/spice-file-transfer-task.c
index 2207ba4..4689b71 100644
--- a/src/spice-file-transfer-task.c
+++ b/src/spice-file-transfer-task.c
@@ -88,16 +88,34 @@ static guint task_signals[LAST_TASK_SIGNAL];
  ******************************************************************************/
 
 static SpiceFileTransferTask *
-spice_file_transfer_task_new(SpiceMainChannel *channel, GFile *file, GCancellable *cancellable)
+spice_file_transfer_task_new(SpiceMainChannel *channel,
+                             GFile *file,
+                             GFileCopyFlags flags,
+                             GCancellable *cancellable)
 {
     static uint32_t xfer_id = 1;    /* Used to identify task id */
+    GCancellable *task_cancellable = cancellable;
+    SpiceFileTransferTask *self;
+
+    /* if a cancellable object was not provided for the overall operation,
+     * create a separate object for each file so that they can be cancelled
+     * separately  */
+    if (!task_cancellable)
+        task_cancellable = g_cancellable_new();
 
-    return g_object_new(SPICE_TYPE_FILE_TRANSFER_TASK,
+    self = g_object_new(SPICE_TYPE_FILE_TRANSFER_TASK,
                         "id", xfer_id++,
                         "file", file,
                         "channel", channel,
-                        "cancellable", cancellable,
+                        "cancellable", task_cancellable,
                         NULL);
+    self->flags = flags;
+
+    /* if we created a GCancellable above, unref it */
+    if (!cancellable)
+        g_object_unref(task_cancellable);
+
+    return self;
 }
 
 static void spice_file_transfer_task_query_info_cb(GObject *obj,
@@ -364,24 +382,10 @@ GHashTable *spice_file_transfer_task_create_tasks(GFile **files,
     for (i = 0; files[i] != NULL && !g_cancellable_is_cancelled(cancellable); i++) {
         SpiceFileTransferTask *xfer_task;
         guint32 task_id;
-        GCancellable *task_cancellable = cancellable;
-
-        /* if a cancellable object was not provided for the overall operation,
-         * create a separate object for each file so that they can be cancelled
-         * separately  */
-        if (!task_cancellable)
-            task_cancellable = g_cancellable_new();
-
-        /* FIXME: Move the xfer-task initialization to spice_file_transfer_task_new() */
-        xfer_task = spice_file_transfer_task_new(channel, files[i], task_cancellable);
-        xfer_task->flags = flags;
 
+        xfer_task = spice_file_transfer_task_new(channel, files[i], flags, cancellable);
         task_id = spice_file_transfer_task_get_id(xfer_task);
         g_hash_table_insert(xfer_ht, GUINT_TO_POINTER(task_id), xfer_task);
-
-        /* if we created a per-task cancellable above, unref it */
-        if (!cancellable)
-            g_object_unref(task_cancellable);
     }
     return xfer_ht;
 }


More information about the Spice-commits mailing list