Hi,
On Fri, Jun 24, 2016 at 02:17:33PM -0500, Jonathon Jongsma wrote:
> On Thu, 2016-06-23 at 19:37 +0200, Victor Toso wrote:
> > * spice_file_transfer_task_read_async() will call the callback with
> > error in case it is called on pending state
>
> OK, this maintains behavior, since it just moves the error reporting down a
> level to the called function.
>
> >
> > * on VD_AGENT_FILE_XFER_STATUS_SUCCESS it should not be possible to be
> > on pending state as spice_file_transfer_task_read_async() would
> > return immediately in case all file is read
>
> On the other hand, this appears to change behavior. If the guest sends an early
> STATUS_SUCCESS to the client for some reason (buggy agent thinks it received all
> data even though the client is still sending data), we will no longer report an
> error. I'm not sure that's a good idea.
Agreed!
Seems that we can move this down to spice_file_transfer_task_completed()
as this function should be called to complete a task, it seems wrong to
complete a task without error but in a pending state.
<snip>
- if (self->pending)
+ if (self->pending) {
+ /* Complete but pending is okay only if error is set */
+ if (self->error == NULL) {
+ self->error = g_error_new(SPICE_CLIENT_ERROR,
+ SPICE_CLIENT_ERROR_FAILED,
+ "Cannot complete task in pending
state");
+ }
return;
+ }
</snip>
This will require some tests, I guess (to avoid regressions)
toso
>
>
> > ---
> > src/channel-main.c | 8 --------
> > 1 file changed, 8 deletions(-)
> >
> > diff --git a/src/channel-main.c b/src/channel-main.c
> > index 9787613..4b728fe 100644
> > --- a/src/channel-main.c
> > +++ b/src/channel-main.c
> > @@ -1993,11 +1993,6 @@ static void
> > spice_file_transfer_task_handle_status(SpiceFileTransferTask *task,
> >
> > switch (msg->result) {
> > case VD_AGENT_FILE_XFER_STATUS_CAN_SEND_DATA:
> > - if (task->pending) {
> > - error = g_error_new(SPICE_CLIENT_ERROR,
> > SPICE_CLIENT_ERROR_FAILED,
> > - "transfer received CAN_SEND_DATA in pending
> > state");
> > - break;
> > - }
> > spice_file_transfer_task_read_async(task, file_xfer_read_async_cb,
> > NULL);
> > return;
> > case VD_AGENT_FILE_XFER_STATUS_CANCELLED:
> > @@ -2009,9 +2004,6 @@ static void
> > spice_file_transfer_task_handle_status(SpiceFileTransferTask *task,
> > "some errors occurred in the spice agent");
> > break;
> > case VD_AGENT_FILE_XFER_STATUS_SUCCESS:
> > - if (task->pending)
> > - error = g_error_new(SPICE_CLIENT_ERROR,
> > SPICE_CLIENT_ERROR_FAILED,
> > - "transfer received success in pending
> > state");
> > break;
> > default:
> > g_warn_if_reached();
>
> Reviewed-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel