[Spice-devel] [PATCH spice-gtk 06/13] migration: set connecting state before fd request

[Christophe Fergeau]

On Thu, Nov 13, 2014 at 07:26:10PM +0100, Marc-André Lureau wrote:
> On Thu, Nov 13, 2014 at 6:38 PM, Christophe Fergeau <cfergeau at redhat.com>
> wrote:
> 
> > Hey,
> >
> > On Thu, Nov 13, 2014 at 05:41:20PM +0100, Marc-André Lureau wrote:
> > > On Thu, Nov 13, 2014 at 10:07 AM, Christophe Fergeau <
> > cfergeau at redhat.com>
> > > wrote:
> > >
> > > > Ah, this is the bit I was missing thanks! The commit log needs to be
> > > > much more detailed and accurate, and mention explicitly the various
> > code
> > > > paths involved to trigger the error, where the looping occurs, ...
> > > >
> > >
> > > What about?
> > >
> > > migration: set connecting state before fd request
> > >
> > > During migration, the main channel initiating the process is waiting on
> > > connection completion of all channels in migrate_channel_event_cb()
> >
> > before yielding back to the main context.
> >
> 
> Not sure what you mean, the main channel context is "waiting", meaning it
> waits until it is yields back from migrate_channel_event_cb() when all
> channels are connected.

Hmm I only partially understood what your log meant, and tried to
improve it, but it's very partial. I think it's better to be explicit
about main context/coroutine context here, maybe something like

"During migration, the main channel initiating the process is waiting
in the main context for connection completion.
It's migrate_channel_event_cb() which is expected to yield back to the
main context once all channels have completed connection"

> 
> > or
> > > it will abort migration for unexpected channel events, such as
> > > SPICE_CHANNEL_CLOSED.
> >
> > s/or it will/It will//
> >
> 
> "is waiting on connection completion of all channels... or it will", I
> think this is correct as it's an alternative path if the completion isn't
> met.

Yup, not saying it's incorrect, it's just that the sentence was a bit
long, and the 'or' is not very important, so it was easy to split it
there. I'm fine either way.

> 
> 
> > >
> > > If the migration is cancelled before connection completes, but the
> > > channels state are still SPICE_CHANNEL_STATE_UNCONNECTED,
> >
> > but the channells are still in the SPICE_CHANNEL_STATE_UNCONNECTED state
> >
> 
> ok
> 
> 
> > > no events will
> > > be emitted in channel_disconnect(), and the source session main channel
> > > will remain frozen waiting for migration completion or failure.
> > >
> > > Currently, for client-fd channels, the channel state remains UNCONNECTED
> > > until the fd is provided. But if cancellation occurs, no channel events
> > are
> > > emitted and the source session is stuck.
> > >
> > > Before requesting the fd, set the channel state to connecting, so
> > > it
> > >
> > > will emit an error if disconnect happens, and it will finish cancelling
> > > the migration in source session main channel.
> >
> > Extra line breaks in the middle of the paragraph it seems.
> >
> 
> I use emacs default fill-paragraph, it may not match the mailer but it
> looks correct ~70 character per line.

I was not sure if this was a local issue or a wrongly formatted
commit log, this is all good if this looks correct in git log.

Christophe
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20141114/851c1d6b/attachment.sig>