[Spice-devel] [spice-gtk][PATCH v2] Added INFO messages about a file transfer
Christophe Fergeau
cfergeau at redhat.com
Wed Sep 17 04:10:51 PDT 2014
Hey,
On Wed, Sep 17, 2014 at 12:10:06PM +0200, Pavel Grunt wrote:
> When a file transfer starts / finishes an information message is printed (in INFO log level).
> Also INFO messages about the transfer progress are printed.
It seems this 2nd sentence is about what you changed in this v2. It's
nice to reviewers when you can make that explicit with a
changes since v2:
- added periodic information about file transfers in progress
after the '---' just below (ie not in the commit log so that the patch
applies cleanly with git am).
In this case, you could even had gone with an additional patch as the 2
changes can work indepently if I'm not mistaken.
>
> ---
> gtk/channel-main.c | 23 +++++++++++++++++++++++
> gtk/spice-widget.c | 44 ++++++++++++++++++++++++++++++++++++++++++--
> 2 files changed, 65 insertions(+), 2 deletions(-)
>
> diff --git a/gtk/channel-main.c b/gtk/channel-main.c
> index 021fa83..7a59613 100644
> --- a/gtk/channel-main.c
> +++ b/gtk/channel-main.c
> @@ -70,6 +70,7 @@ typedef struct SpiceFileXferTask {
> char buffer[FILE_XFER_CHUNK_SIZE];
> uint64_t read_bytes;
> uint64_t file_size;
> + GDateTime *start_time;
> GError *error;
> } SpiceFileXferTask;
>
> @@ -1538,6 +1539,7 @@ static void file_xfer_task_free(SpiceFileXferTask *task)
> g_clear_object(&task->channel);
> g_clear_object(&task->file);
> g_clear_object(&task->file_stream);
> + g_date_time_unref(task->start_time);
> g_free(task);
> }
>
> @@ -1548,6 +1550,9 @@ static void file_xfer_close_cb(GObject *object,
> {
> GSimpleAsyncResult *res;
> SpiceFileXferTask *task;
> + GDateTime *end;
> + char *basename;
> + double seconds;
> GError *error = NULL;
>
> task = user_data;
> @@ -1572,6 +1577,17 @@ static void file_xfer_close_cb(GObject *object,
> g_simple_async_result_take_error(res, task->error);
> g_simple_async_result_set_op_res_gboolean(res, FALSE);
> } else {
> + end = g_date_time_new_now_local();
> + seconds = (double) g_date_time_difference(end, task->start_time) / 1000000.0;
> + g_date_time_unref(end);
> + basename = g_file_get_basename(task->file);
> + if (basename != NULL) {
> + g_log(G_LOG_DOMAIN, G_LOG_LEVEL_INFO,
> + "transferred file %s of %.2f kB size in %.2f seconds (%.1f MB/s)",
> + basename, task->file_size / 1000.0, seconds,
> + (task->file_size/1000000.0)/seconds);
> + g_free(basename);
> + }
> g_simple_async_result_set_op_res_gboolean(res, TRUE);
> }
> g_simple_async_result_complete_in_idle(res);
> @@ -2739,6 +2755,7 @@ static void file_xfer_send_start_msg_async(SpiceMainChannel *channel,
> {
> SpiceMainChannelPrivate *c = channel->priv;
> SpiceFileXferTask *task;
> + char *basename;
> static uint32_t xfer_id; /* Used to identify task id */
>
> task = g_malloc0(sizeof(SpiceFileXferTask));
> @@ -2751,7 +2768,13 @@ static void file_xfer_send_start_msg_async(SpiceMainChannel *channel,
> task->progress_callback_data = progress_callback_data;
> task->callback = callback;
> task->user_data = user_data;
> + task->start_time = g_date_time_new_now_local();
>
> + basename = g_file_get_basename(task->file);
> + if (basename != NULL) {
> + g_log(G_LOG_DOMAIN, G_LOG_LEVEL_INFO, "transfer of file %s has started", basename);
> + g_free(basename);
> + }
> CHANNEL_DEBUG(task->channel, "Insert a xfer task:%d to task list", task->id);
> g_hash_table_insert(c->file_xfer_tasks, GUINT_TO_POINTER(task->id), task);
>
> diff --git a/gtk/spice-widget.c b/gtk/spice-widget.c
> index 1220030..39434ff 100644
> --- a/gtk/spice-widget.c
> +++ b/gtk/spice-widget.c
> @@ -473,6 +473,41 @@ static gboolean grab_broken(SpiceDisplay *self, GdkEventGrabBroken *event,
> return false;
> }
>
> +static void file_progress_callback(goffset current_num_bytes,
> + goffset total_num_bytes,
> + gpointer user_data)
> +{
> + GDateTime *now;
> + GTimeSpan diff;
> + const GTimeSpan interval = 20000000;/* microseconds */
This can be written 20 * G_USEC_PER_SEC; in which case the comment is not
needed. I think you have trailing whitespace after this comment.
> + GDateTime **last_time = user_data;
This can be just GDateTime *last_time = user_data; with the change I
suggest below
> +
> + g_return_if_fail(last_time != NULL);
> +
> + if (current_num_bytes == total_num_bytes) {
> + g_date_time_unref(*last_time);
> + g_free(last_time);
> + last_time = NULL;
> + return;
> + }
> +
> + now = g_date_time_new_now_local();
> + diff = g_date_time_difference(now, *last_time);
> +
> + if (diff < interval) {
> + g_date_time_unref(now);
> + return;
> + }
> +
> + g_date_time_unref(*last_time);
> + *last_time = now;
> +
> + g_log(G_LOG_DOMAIN, G_LOG_LEVEL_INFO, "transferred %.2f%%",
> + 100.0 * current_num_bytes / total_num_bytes );
> +
> +
> +}
> +
> static void drag_data_received_callback(SpiceDisplay *self,
> GdkDragContext *drag_context,
> gint x,
> @@ -488,6 +523,7 @@ static void drag_data_received_callback(SpiceDisplay *self,
> SpiceDisplayPrivate *d = self->priv;
> int i = 0;
> GFile **files;
> + GDateTime **progress_cb_time;
>
> /* We get a buf like:
> * file:///root/a.txt\r\nfile:///root/b.txt\r\n
> @@ -504,8 +540,12 @@ static void drag_data_received_callback(SpiceDisplay *self,
> }
> g_strfreev(file_urls);
>
> - spice_main_file_copy_async(d->main, files, 0, NULL, NULL,
> - NULL, NULL, NULL);
> + progress_cb_time = g_malloc0(sizeof(GDateTime *));
> + *progress_cb_time = g_date_time_new_now_local();
you could call it last_update or whatever generic name.
g_date_time_new_now_local() returns pointer to a newly allocated
GDateTime instance, so you can pass that directly as the user data to
use for file_progress_callback, this makes the code simpler than with
the additional g_malloc0.
> +
> + spice_main_file_copy_async(d->main, files, 0, NULL,
> + (file_progress_callback), progress_cb_time,
You don't need the parentheses around file_progress_callback here. Imo
this is harder to read as this makes it look like a cast.
Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20140917/012d1223/attachment.sig>
More information about the Spice-devel
mailing list