Hi,
On Fri, Jun 03, 2016 at 12:22:09PM -0500, Jonathon Jongsma wrote:
> On Mon, 2016-05-30 at 11:55 +0200, Victor Toso wrote:
> > Application can start multiple file-transfers in one operation and
> > cancel a few of them while the operation is ongoing. In that case, we
> > should remove the file-size of the transfer operation otherwise we
> > will send incorrect progress data.
> >
> > Taking in consideration the split of SpiceFileTransferTask, this patch
> > includes spice_file_transfer_task_get_file_size() internal helper
> > function.
> > ---
> > src/channel-main.c | 13 ++++++++++---
> > 1 file changed, 10 insertions(+), 3 deletions(-)
> >
> > diff --git a/src/channel-main.c b/src/channel-main.c
> > index e204a1e..890d939 100644
> > --- a/src/channel-main.c
> > +++ b/src/channel-main.c
> > @@ -61,6 +61,7 @@ typedef void
> > (*SpiceFileTransferTaskFlushCb)(SpiceFileTransferTask *xfer_task,
> > static guint32 spice_file_transfer_task_get_id(SpiceFileTransferTask *self);
> > static SpiceMainChannel
> > *spice_file_transfer_task_get_channel(SpiceFileTransferTask *self);
> > static GCancellable
> > *spice_file_transfer_task_get_cancellable(SpiceFileTransferTask *self);
> > +static guint64 spice_file_transfer_task_get_file_size(SpiceFileTransferTask
> > *self);
> > static void spice_file_transfer_task_flush_done(SpiceFileTransferTask *self,
> > GError *error);
> > static GList *spice_file_transfer_task_create_tasks(SpiceMainChannel
> > *channel,
> > GFile **files,
> > @@ -1927,6 +1928,7 @@ static void file_xfer_end_callback(GObject
> > *source_object,
> > {
> > GTask *task;
> > FileTransferOperation *xfer_op;
> > + SpiceFileTransferTask *xfer_task;
> >
> > task = G_TASK(res);
> > if (!g_task_had_error(task))
> > @@ -1934,7 +1936,9 @@ static void file_xfer_end_callback(GObject
> > *source_object,
> > * file_transfer_operation_task_finished */
> > return;
> >
> > + xfer_task = SPICE_FILE_TRANSFER_TASK(source_object);
> > xfer_op = user_data;
> > + xfer_op->transfer_size -=
> > spice_file_transfer_task_get_file_size(xfer_task);
>
> I don't think this is sufficient. The file might get cancelled after it has
> transferred some of its data already. In this case, that already-transferred
> data will have been added to total_sent, but we will subtract the file size from
> transfer_size. Consider the situation where you're transferring two 1MB files.
> If you cancel one of these files after sending 75% of both files, the next
> progress update will indicate that you've sent 1.5MB of 1MB (or 150%) even
> though you still have 25% of the other file to send yet.
>
> Reviewed-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
You are right!
I guess the correct would be by having "total_flushed" data in each
SpiceFileTransferTask and use that to subtract from total_sent.
I'll fix it, thanks!
>
>
> >
> > if (xfer_op->error != NULL)
> > return;
> > @@ -1948,9 +1952,6 @@ static void file_xfer_end_callback(GObject
> > *source_object,
> > * without GCancellabe */
> > if (g_error_matches(xfer_op->error, G_IO_ERROR, G_IO_ERROR_CANCELLED) &&
> > xfer_op->cancellable == NULL) {
> > - SpiceFileTransferTask *xfer_task;
> > -
> > - xfer_task = SPICE_FILE_TRANSFER_TASK(source_object);
> > spice_debug("file-transfer %u was cancelled",
> > spice_file_transfer_task_get_id(xfer_task));
> > g_clear_error(&xfer_op->error);
> > @@ -3382,6 +3383,12 @@ static GCancellable
> > *spice_file_transfer_task_get_cancellable(SpiceFileTransferT
> > return self->cancellable;
> > }
> >
> > +static guint64 spice_file_transfer_task_get_file_size(SpiceFileTransferTask
> > *self)
> > +{
> > + g_return_val_if_fail(self != NULL, 0);
> > + return self->file_size;
> > +}
> > +
> > static void spice_file_transfer_task_flush_done(SpiceFileTransferTask *self,
> > GError *error)
> > {
> > g_return_if_fail(self != NULL);
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel