[Spice-devel] [spice-gtk v4 09/24] main: do not check for xfer-task errors

[Victor Toso]

Hi,

On Fri, Jun 24, 2016 at 02:30:09PM -0500, Jonathon Jongsma wrote:
> On Thu, 2016-06-23 at 19:37 +0200, Victor Toso wrote:
> > file_xfer_flush_finish and file_xfer_data_flushed_cb are channel-main
> > function and should not check for SpiceFileTransferTask internal
> > errors.
>
> OK, but I'd like some additional justification of why we can skip this
> without causing problems. For example (as I understand it):

> The data will only be flushed if spice_file_transfer_task_read_async()
> was successful, and between the call to file_xfer_flush_async() and
> file_xfer_data_flushed_cb(), the task status will not change

Yes. Any pending change should affect SpiceFileTransferTask which then
will affect FileTransferOperation.

> (or is it possible that the task status can be changed by recieving
> a VD_AGENT_FILE_XFER_STATUS_ERROR message from the guest?)

In that case, we would call the task_complete() but I'm not sure if we
should cancel the flushing... Maybe it is easier to let agent ignore
the data if the cancel/error happens during the flush.

I'll try to improve the commit log!

>
> > ---
> >  src/channel-main.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/channel-main.c b/src/channel-main.c
> > index be5a454..fef72cd 100644
> > --- a/src/channel-main.c
> > +++ b/src/channel-main.c
> > @@ -1893,7 +1893,7 @@ static void file_xfer_data_flushed_cb(GObject *source_object,
> >      GError *error = NULL;
> >  
> >      file_xfer_flush_finish(channel, res, &error);
> > -    if (error || self->error) {
> > +    if (error) {
> >          spice_file_transfer_task_completed(self, error);
> >          return;
> >      }
> 
> Reviewed-by: Jonathon Jongsma <jjongsma at redhat.com>

Thanks,
  toso

> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel