[Spice-devel] [PATCH spice-gtk 05/13] migration: fail with client provided fd

[Marc-André Lureau]

----- Original Message -----
> On Sun, Nov 09, 2014 at 05:31:37PM +0100, Marc-André Lureau wrote:
> > This is not yet supported, so fail early.
> 
> Seems like this bit from your cover letter would fit in the log here:
> "Currently the fd request is done on the migration session, which is
> not connected with the client session, so the client has no way to
> provide fd for the migration. And the original and migration session
> ends up stuck. Failing early seems the best for now. "
> 
> > ---
> >  TODO                |  2 +-
> >  gtk/spice-channel.c |  4 ++++
> >  gtk/spice-session.c | 18 +++++++++++++-----
> >  3 files changed, 18 insertions(+), 6 deletions(-)
> > 
> > diff --git a/TODO b/TODO
> > index 34e2c39..68ea4d5 100644
> > --- a/TODO
> > +++ b/TODO
> > @@ -1,5 +1,5 @@
> > +* implement migration support with client fd
> >  * create a ChannelBaseAudio
> > -* missing TunnelChannel implementation
> 
> This removed line is unrelated to this commit.
> 

I will push it separately, the tunnel channel is long deprecated, no need to have that in todo here.

> >  * revive the win32 GDI backend
> >  
> >  See also list of bugs open:
> > diff --git a/gtk/spice-channel.c b/gtk/spice-channel.c
> > index ed18e09..93c2a78 100644
> > --- a/gtk/spice-channel.c
> > +++ b/gtk/spice-channel.c
> > @@ -2468,6 +2468,10 @@ static gboolean channel_connect(SpiceChannel
> > *channel)
> >  
> >      if (spice_session_get_client_provided_socket(c->session)) {
> >          if (c->fd == -1) {
> > +            CHANNEL_DEBUG(channel, "requesting fd");
> > +            /* FIXME: no way for client to provide fd atm. */
> > +            /* It could either chain on parent channel.. */
> > +            /* or register migration channel on parent session, or ? */
> >              g_signal_emit(channel, signals[SPICE_CHANNEL_OPEN_FD], 0,
> >              c->tls);
> >              return true;
> 
> return FALSE; here maybe?

It's not yet a connect error, so it must return TRUE. Agree, that dealing with async and sync error report sucks, but we don't want to break API.