[Spice-devel] [PATCH spice-gtk v2] main: Handle file-xfer detailed errors

[Victor Toso]

Hi,

On Thu, Jun 01, 2017 at 08:57:12AM +0200, Pavel Grunt wrote:
> On Wed, 2017-05-31 at 23:31 +0200, Victor Toso wrote:
> > Hi,
> > 
> > On Mon, May 29, 2017 at 04:26:20PM +0200, Jakub Janků wrote:
> > > Log appropriate error messages when we receive
> > > VD_AGENT_FILE_XFER_STATUS_SESSION_LOCKED,
> > > VD_AGENT_FILE_XFER_STATUS_AGENT_NOT_CONNECTED,
> > > VD_AGENT_FILE_XFER_STATUS_DISABLED.
> > > ---
> > >  src/channel-main.c | 13 +++++++++++++
> > >  1 file changed, 13 insertions(+)
> > > 
> > > diff --git a/src/channel-main.c b/src/channel-main.c
> > > index af1350c..4d6c416 100644
> > > --- a/src/channel-main.c
> > > +++ b/src/channel-main.c
> > > @@ -1882,6 +1882,19 @@ static void
> > > main_agent_handle_xfer_status(SpiceMainChannel *channel,
> > >          g_free(file_size_str);
> > >          break;
> > >      }
> > > +    case VD_AGENT_FILE_XFER_STATUS_SESSION_LOCKED:
> > > +        error = g_error_new_literal(SPICE_CLIENT_ERROR,
> > > SPICE_CLIENT_ERROR_FAILED,
> > > +                                    _("User's session is locked
> > > and cannot transfer files, "
> > > +                                      "unlock it and try
> > > again."));
> > > +        break;
> > > +    case VD_AGENT_FILE_XFER_STATUS_AGENT_NOT_CONNECTED:
> > > +        error = g_error_new_literal(SPICE_CLIENT_ERROR,
> > > SPICE_CLIENT_ERROR_FAILED,
> > > +                                    _( "Agent not connected."));
> > 
> > - remove extra space: _("..."));
> > - I think it should be "Session agent is not connected" or "vdagent
> > is
> >   not connected"
> > 
> > > +        break;
> > > +    case VD_AGENT_FILE_XFER_STATUS_DISABLED:
> > > +        error = g_error_new_literal(SPICE_CLIENT_ERROR,
> > > SPICE_CLIENT_ERROR_FAILED,
> > > +                                    _("File transfer is disabled
> > > by spice agent."));
> > 
> > This is a cool one. An easy way to get this is trying file transfer
> > on
> > Login Screen.
>
> really? Should be VD_AGENT_FILE_XFER_STATUS_SESSION_LOCKED in that
> case. I would go only for "File transfer is disabled" - the transfer
> can be disabled in spice-server.

The guest session could be locked (user is logged in but locked it) or
it could be in login screen (no user is currently logged in).

In login screen we have a session agent for gdm/kdm/etc but that's not
real user and we disable the file-xfer in this situation.

So I think file-xfer is disabled makes sense although we don't have a
rationale on why it is disabled we know for sure that this is in the
agent.

The wip you have to send a message to the client if file-xfer is
disabled in the host could be using this extra field from
VD_AGENT_CAP_FILE_XFER_DETAILED_ERRORS to explicit say that file-xfer is
disabled in the host.

Cheers,

> 
> > 
> > My only doubt is the preposition 'by', shouldn't it be 'in'?
> > 
> > Just minor things,
> > Acked-by: Victor Toso <victortoso at redhat.com>
> > 
> > > +        break;
> > >      case VD_AGENT_FILE_XFER_STATUS_SUCCESS:
> > >          break;
> > >      default:
> > > -- 
> > > 2.13.0
> > > 
> > > _______________________________________________
> > > Spice-devel mailing list
> > > Spice-devel at lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/spice-devel
> > 
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170601/c7f4d000/attachment.sig>