[Spice-devel] [spice-gtk v1] file-xfer: differentiate error from host/guest and client

[Jonathon Jongsma]

On Thu, 2016-12-22 at 11:23 -0600, Jonathon Jongsma wrote:
> 
> 
> On Thu, 2016-12-22 at 14:17 +0100, Victor Toso wrote:
> > Hi,
> > 
> > On Thu, Dec 22, 2016 at 11:35:13AM +0100, Pavel Grunt wrote:
> > > Hi,
> > > 
> > > it works. But do we need a new public symbol ? It is an internal
> > > stuff
> > > for channel-main.
> > > 
> > > Pavel
> > 
> > Yeah, I was also thinking if this is really necessary.
> > 
> > Application can track SpiceFileTransferTask "finished" signal too,
> > so
> > I
> > thought this distinction in GError would make sense even though the
> > main
> > API function still uses SPICE_CLIENT_ERROR_FAILED error code in
> > that
> > case.
> > 
> > The changes in the code are quite minor this way too, but this is
> > not
> > really important.
> > 
> > Pinging Jonathon for hints/ideas/opinions :)
> 
> Can you expand a little bit about what actual behavior you're trying
> to
> fix? I'm not quite sure I understand the problem yet.

Sorry, totally overlooked the bugzilla link on your git commit log.
Nevermind that question.


> 
> 
> 
> > 
> >   toso
> > 
> > > 
> > > On Wed, 2016-12-21 at 18:55 +0100, Victor Toso wrote:
> > > > From: Victor Toso <me at victortoso.com>
> > > > 
> > > > During file transfer, we can have error and cancellation in
> > > > client,
> > > > host or guest side.
> > > > 
> > > > If error happens in the client side, we must send a message to
> > > > the
> > > > guest, VD_AGENT_FILE_XFER_STATUS, with either _STATUS_CANCELLED
> > > > or
> > > > _STATUS_ERROR.
> > > > 
> > > > But the current code is also sending a message to the guest
> > > > when
> > > > error/cancellation does not come from client, leading to
> > > > unexpected
> > > > messages to be received in host/guest.
> > > > 
> > > > SPICE_CLIENT_ERROR_FILE_TRANSFER_FAILED is introduced in this
> > > > patch
> > > > to
> > > > mark a SpiceFileTransferTask to be failed due host or guest
> > > > problems.
> > > > 
> > > > The generic error code SPICE_CLIENT_ERROR_FAILED is still used
> > > > in
> > > > spice_main_file_copy_async() API.
> > > > 
> > > > Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=99170
> > > > Signed-off-by: Victor Toso <victortoso at redhat.com>
> > > > Reported-by: Pavel Grunt <pgrunt at redhat.com>
> > > > ---
> > > >  src/channel-main.c | 11 +++++++----
> > > >  src/spice-client.h |  1 +
> > > >  2 files changed, 8 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/src/channel-main.c b/src/channel-main.c
> > > > index ed5d611..e92d363 100644
> > > > --- a/src/channel-main.c
> > > > +++ b/src/channel-main.c
> > > > @@ -1862,18 +1862,18 @@ static void
> > > > main_agent_handle_xfer_status(SpiceMainChannel *channel,
> > > >          spice_file_transfer_task_read_async(xfer_task,
> > > > file_xfer_read_async_cb, xfer_op);
> > > >          return;
> > > >      case VD_AGENT_FILE_XFER_STATUS_CANCELLED:
> > > > -        error = g_error_new_literal(SPICE_CLIENT_ERROR,
> > > > SPICE_CLIENT_ERROR_FAILED,
> > > > +        error = g_error_new_literal(SPICE_CLIENT_ERROR,
> > > > SPICE_CLIENT_ERROR_FILE_TRANSFER_FAILED,
> > > >                                      _("The spice agent
> > > > cancelled
> > > > the file transfer"));
> > > >          break;
> > > >      case VD_AGENT_FILE_XFER_STATUS_ERROR:
> > > > -        error = g_error_new_literal(SPICE_CLIENT_ERROR,
> > > > SPICE_CLIENT_ERROR_FAILED,
> > > > +        error = g_error_new_literal(SPICE_CLIENT_ERROR,
> > > > SPICE_CLIENT_ERROR_FILE_TRANSFER_FAILED,
> > > >                                      _("The spice agent
> > > > reported
> > > > an
> > > > error during the file transfer"));
> > > >          break;
> > > >      case VD_AGENT_FILE_XFER_STATUS_SUCCESS:
> > > >          break;
> > > >      default:
> > > >          g_warn_if_reached();
> > > > -        error = g_error_new(SPICE_CLIENT_ERROR,
> > > > SPICE_CLIENT_ERROR_FAILED,
> > > > +        error = g_error_new(SPICE_CLIENT_ERROR,
> > > > SPICE_CLIENT_ERROR_FILE_TRANSFER_FAILED,
> > > >                              "unhandled status type: %u", msg-
> > > > > result);
> > > > 
> > > >          break;
> > > >      }
> > > > @@ -2960,7 +2960,10 @@ static void
> > > > file_transfer_operation_task_finished(SpiceFileTransferTask
> > > > *xfer_ta
> > > >      task_id = spice_file_transfer_task_get_id(xfer_task);
> > > >      g_return_if_fail(task_id != 0);
> > > >  
> > > > -    if (error) {
> > > > +    if (g_error_matches(error, SPICE_CLIENT_ERROR,
> > > > +                        SPICE_CLIENT_ERROR_FILE_TRANSFER_FAILE
> > > > D)
> > > > ) {
> > > > +        spice_debug("xfer task: %u failed due cancel/error on
> > > > server or agent", task_id);
> > > > +    } else if (error) {
> > > >          VDAgentFileXferStatusMessage msg;
> > > >          msg.id = task_id;
> > > >          if (g_error_matches(error, G_IO_ERROR,
> > > > G_IO_ERROR_CANCELLED)) {
> > > > diff --git a/src/spice-client.h b/src/spice-client.h
> > > > index 32b79ea..8297537 100644
> > > > --- a/src/spice-client.h
> > > > +++ b/src/spice-client.h
> > > > @@ -82,6 +82,7 @@ typedef enum
> > > >      SPICE_CLIENT_ERROR_AUTH_NEEDS_USERNAME,
> > > >      SPICE_CLIENT_ERROR_AUTH_NEEDS_PASSWORD_AND_USERNAME,
> > > >      SPICE_CLIENT_ERROR_USB_SERVICE,
> > > > +    SPICE_CLIENT_ERROR_FILE_TRANSFER_FAILED,
> > > >  } SpiceClientError;
> > > >  
> > > >  #ifndef SPICE_DISABLE_DEPRECATED
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel