On Thu, 2016-06-23 at 19:37 +0200, Victor Toso wrote:
> Introduced functions (private):
> * void spice_file_transfer_task_init_task_async()
> * GFileInfo *spice_file_transfer_task_init_task_finish()
>
> The init process of SpiceFileTransferTask does initialize its
> GFileInputStream (for reading the file) and also its GFileInfo
> (necessary to protocol in order to start the file-transfer with agent)
>
> Due the logic changed involved, some functions were renamed to better
> match its place and purpose:
>
> * file_xfer_info_async_cb -> file_xfer_init_task_async_cb
> It is channel-main's callback for each _init_task_async()
>
> * file_xfer_read_async_cb -> spice_file_transfer_task_read_file_cb
> * file_xfer_info_async_cb -> spice_file_transfer_task_query_info_cb
> Both should be private to SpiceFileTransferTask now.
>
> As the _init_task_async() uses GTask, some error handling was moved to
> channel-main's after _init_task_finish() is called.
>
> This change is related to split SpiceFileTransferTask from
> channel-main.
> ---
> src/channel-main.c | 181 +++++++++++++++++++++++++++++++++++++---------------
> -
> 1 file changed, 126 insertions(+), 55 deletions(-)
>
> diff --git a/src/channel-main.c b/src/channel-main.c
> index 1411fb3..ceca7e3 100644
> --- a/src/channel-main.c
> +++ b/src/channel-main.c
> @@ -65,6 +65,12 @@ static GHashTable
> *spice_file_transfer_task_create_tasks(GFile **files,
> gpointer
> progress_callback_data,
> GAsyncReadyCallback
> callback,
> gpointer user_data);
> +static void spice_file_transfer_task_init_task_async(SpiceFileTransferTask
> *self,
> + GAsyncReadyCallback
> callback,
> + gpointer userdata);
> +static GFileInfo
> *spice_file_transfer_task_init_task_finish(SpiceFileTransferTask *xfer_task,
> + GAsyncResult
> *result,
> + GError **error);
>
> /**
> * SECTION:file-transfer-task
> @@ -3025,35 +3031,32 @@ signal:
> }
>
>
> -static void file_xfer_info_async_cb(GObject *obj, GAsyncResult *res, gpointer
> data)
> +static void file_xfer_init_task_async_cb(GObject *obj, GAsyncResult *res,
> gpointer data)
> {
> GFileInfo *info;
> - GFile *file = G_FILE(obj);
> - GError *error = NULL;
> - GKeyFile *keyfile = NULL;
> - gchar *basename = NULL;
> + SpiceFileTransferTask *xfer_task;
> + SpiceMainChannel *channel;
> + gchar *string, *basename;
> + GKeyFile *keyfile;
> VDAgentFileXferStartMessage msg;
> - gsize /*msg_size*/ data_len;
> - gchar *string;
> - SpiceFileTransferTask *self = SPICE_FILE_TRANSFER_TASK(data);
> + guint64 file_size;
> + gsize data_len;
> + GError *error = NULL;
>
> - self->pending = FALSE;
> - info = g_file_query_info_finish(file, res, &error);
> - if (error || self->error)
> + xfer_task = SPICE_FILE_TRANSFER_TASK(obj);
> +
> + info = spice_file_transfer_task_init_task_finish(xfer_task, res, &error);
> + if (info == NULL)
> goto failed;
>
> - self->file_info = info;
> - self->file_size =
> - g_file_info_get_attribute_uint64(info,
> G_FILE_ATTRIBUTE_STANDARD_SIZE);
> - g_object_notify(G_OBJECT(self), "progress");
> - keyfile = g_key_file_new();
> + channel = spice_file_transfer_task_get_channel(xfer_task);
> + basename = g_file_info_get_attribute_as_string(info,
> G_FILE_ATTRIBUTE_STANDARD_NAME);
> + file_size = g_file_info_get_attribute_uint64(info,
> G_FILE_ATTRIBUTE_STANDARD_SIZE);
>
> - /* File name */
> - basename = g_file_get_basename(file);
> + keyfile = g_key_file_new();
> g_key_file_set_string(keyfile, "vdagent-file-xfer", "name", basename);
> + g_key_file_set_uint64(keyfile, "vdagent-file-xfer", "size", file_size);
> g_free(basename);
> - /* File size */
> - g_key_file_set_uint64(keyfile, "vdagent-file-xfer", "size", self-
> >file_size);
>
> /* Save keyfile content to memory. TODO: more file attributions
> need to be sent to guest */
> @@ -3063,39 +3066,16 @@ static void file_xfer_info_async_cb(GObject *obj,
> GAsyncResult *res, gpointer da
> goto failed;
>
> /* Create file-xfer start message */
> - msg.id = self->id;
> - agent_msg_queue_many(self->channel, VD_AGENT_FILE_XFER_START,
> + msg.id = spice_file_transfer_task_get_id(xfer_task);
> + agent_msg_queue_many(channel, VD_AGENT_FILE_XFER_START,
> &msg, sizeof(msg),
> string, data_len + 1, NULL);
> g_free(string);
> - spice_channel_wakeup(SPICE_CHANNEL(self->channel), FALSE);
> + spice_channel_wakeup(SPICE_CHANNEL(channel), FALSE);
[NOTE: read comments below first]
If you follow my suggestions below, you'll need to unref 'info' here.
> return;
>
> failed:
> - spice_file_transfer_task_completed(self, error);
> -}
> -
> -static void file_xfer_read_async_cb(GObject *obj, GAsyncResult *res, gpointer
> data)
> -{
> - GFile *file = G_FILE(obj);
> - SpiceFileTransferTask *self = SPICE_FILE_TRANSFER_TASK(data);
> - GError *error = NULL;
> -
> - self->pending = FALSE;
> - self->file_stream = g_file_read_finish(file, res, &error);
> - if (error || self->error) {
> - spice_file_transfer_task_completed(self, error);
> - return;
> - }
> -
> - g_file_query_info_async(self->file,
> - G_FILE_ATTRIBUTE_STANDARD_SIZE,
> - G_FILE_QUERY_INFO_NONE,
> - G_PRIORITY_DEFAULT,
> - self->cancellable,
> - file_xfer_info_async_cb,
> - self);
> - self->pending = TRUE;
> + spice_file_transfer_task_completed(xfer_task, error);
> }
>
> static SpiceFileTransferTask *spice_file_transfer_task_new(SpiceMainChannel
> *channel,
> @@ -3184,12 +3164,10 @@ void spice_main_file_copy_async(SpiceMainChannel
> *channel,
> g_hash_table_iter_init(&iter, xfer_ht);
> while (g_hash_table_iter_next(&iter, &key, &value)) {
> guint32 task_id;
> - GFile *file;
> SpiceFileTransferTask *xfer_task;
>
> xfer_task = value;
> task_id = spice_file_transfer_task_get_id(xfer_task);
> - g_object_get(xfer_task, "file", &file, NULL);
>
> SPICE_DEBUG("Insert a xfer task:%u to task list", task_id);
>
> @@ -3197,12 +3175,9 @@ void spice_main_file_copy_async(SpiceMainChannel
> *channel,
> g_signal_connect(xfer_task, "finished", G_CALLBACK(task_finished),
> channel);
> g_signal_emit(channel, signals[SPICE_MAIN_NEW_FILE_TRANSFER], 0,
> xfer_task);
>
> - g_file_read_async(file,
> - G_PRIORITY_DEFAULT,
> - cancellable,
> - file_xfer_read_async_cb,
> - xfer_task);
> - xfer_task->pending = TRUE;
> + spice_file_transfer_task_init_task_async(xfer_task,
> + file_xfer_init_task_async_cb
> ,
> + NULL);
> }
> g_hash_table_unref(xfer_ht);
> }
> @@ -3296,6 +3271,102 @@ static GHashTable
> *spice_file_transfer_task_create_tasks(GFile **files,
> return xfer_ht;
> }
>
> +static void spice_file_transfer_task_query_info_cb(GObject *obj,
> + GAsyncResult *res,
> + gpointer user_data)
> +{
> + SpiceFileTransferTask *self;
> + GFileInfo *info;
> + GTask *task;
> + GError *error = NULL;
> +
> + task = G_TASK(user_data);
> + self = g_task_get_source_object(task);
> +
> + g_return_if_fail(self->pending == TRUE);
> + self->pending = FALSE;
> +
> + info = g_file_query_info_finish(G_FILE(obj), res, &error);
> + if (error || self->error) {
> + error = (error == NULL) ? self->error : error;
> + g_task_return_error(task, error);
> + return;
> + }
> +
> + self->file_info = info;
don't need to save this: see below.
> + self->file_size =
> + g_file_info_get_attribute_uint64(info,
> G_FILE_ATTRIBUTE_STANDARD_SIZE);
> +
> + /* SpiceFileTransferTask's init is done, handshake for file-trasfer will
> + * start soon. First "progress" can be emitted ~ 0% */
> + g_object_notify(G_OBJECT(self), "progress");
> +
> + g_task_return_boolean(task, TRUE);
Instead of returning a boolean here, I think it would be better to do:
g_task_return_pointer(task, info);
Then see suggested changes below to _init_task_finish()
> +}
> +
> +static void spice_file_transfer_task_read_file_cb(GObject *obj,
> + GAsyncResult *res,
> + gpointer user_data)
> +{
> + SpiceFileTransferTask *self;
> + GTask *task;
> + GError *error = NULL;
> +
> + task = G_TASK(user_data);
> + self = g_task_get_source_object(task);
> +
> + g_return_if_fail(self->pending == TRUE);
> +
> + self->file_stream = g_file_read_finish(G_FILE(obj), res, &error);
> + if (error || self->error) {
> + self->pending = FALSE;
> + error = (error == NULL) ? self->error : error;
> + g_task_return_error(task, error);
> + return;
> + }
> +
> + g_file_query_info_async(self->file,
> + "standard::*",
> + G_FILE_QUERY_INFO_NONE,
> + G_PRIORITY_DEFAULT,
> + self->cancellable,
> + spice_file_transfer_task_query_info_cb,
> + task);
> +}
> +
> +static void spice_file_transfer_task_init_task_async(SpiceFileTransferTask
> *self,
> + GAsyncReadyCallback
> callback,
> + gpointer userdata)
> +{
> + GTask *task;
> +
> + g_return_if_fail(self != NULL);
> + g_return_if_fail(self->pending == FALSE);
> +
> + task = g_task_new(self, self->cancellable, callback, userdata);
> +
> + self->pending = TRUE;
> + g_file_read_async(self->file,
> + G_PRIORITY_DEFAULT,
> + self->cancellable,
> + spice_file_transfer_task_read_file_cb,
> + task);
> +}
> +
> +static GFileInfo
> *spice_file_transfer_task_init_task_finish(SpiceFileTransferTask *self,
> + GAsyncResult
> *result,
> + GError **error)
> +{
> + GTask *task = G_TASK(result);
> +
> + g_return_val_if_fail(self != NULL, NULL);
> +
> + if (g_task_propagate_boolean(task, error))
> + return self->file_info;
> +
> + return NULL;
This function can basically just be:
return g_task_propagate_pointer(task, error);
> +}
> +
> static void
> spice_file_transfer_task_get_property(GObject *object,
> guint property_id,
Reviewed-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel