[Spice-devel] [spice-gtk] main: abort previous migration on switch-host message

[Victor Toso]

Hi,

On Thu, Oct 10, 2019 at 05:48:39AM -0400, Frediano Ziglio wrote:
> > 
> > From: Victor Toso <me at victortoso.com>
> > 
> > In the host, it is possible that the migration goes faster than
> > client side would expect. In case we receive a migrate-begin message
> > followed by switch-host message, we should be sure to abort previous
> 
> Aren't these message exclusives? Is there a bug in the server?

I thought for a while on it and I don't think the server needs a
patch. I have one that fixes the issue there, it sends a
migrate-cancel before switch-host and client would cancel
migration + swap connection as well. But I don't think that a
migrate-cancel is a must as intention really is to change host.

So, migrate-begin + migrate-end is the normal in semi-seamless,
if it happens too quick (for the client) it might happen a
migrate-begin + migrate-switch-host without migrate-end and if it
happens that client sent the msgc-migrated-connected reply, it
will get a migrate-cancel too.

There are quite a few possibilities but I don't think they would
happen in real use case scenarios as migration takes some time.

> > migration to avoid keeping some unused objects in memory plus
> > critical messages in subsequent migrations as below:
> > 
> >  > GSpice-CRITICAL **: spice_session_set_migration_session:
> >  > assertion 'session->priv->migration == NULL' failed
> > 
> > Found by testing with spice/tests/migraton.py without QCOW image.
> > 
> 
> Do you mean tests/migrate.py ? I don't have spice/tests/migraton.py
> (not even spice/tests/migration.py)

Yes, migrate.py.

> 
> > Signed-off-by: Victor Toso <victortoso at redhat.com>
> > ---
> >  src/channel-main.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/src/channel-main.c b/src/channel-main.c
> > index 4305dcd..4afaa3f 100644
> > --- a/src/channel-main.c
> > +++ b/src/channel-main.c
> > @@ -2506,6 +2506,11 @@ static void
> > main_handle_migrate_switch_host(SpiceChannel *channel, SpiceMsgIn *i
> >      }
> >  
> >      session = spice_channel_get_session(channel);
> > +    /* It is possible that we were migrating before receiving the
> > switch-host
> > +     * message without receiving a migrate-cancel message. Simply cancel
> > +     * previous migration before switching host */
> > +    spice_session_abort_migration(session);
> > +
> >      spice_session_set_migration_state(session,
> >      SPICE_SESSION_MIGRATION_SWITCHING);
> >      g_object_set(session,
> >                   "host", host,
> 
> Patch seems fine.
> Is the migration seamless or not?

I did both ways, critical would appear in both and patch fixes
both. Should I sent a v2 with commit log fix?

Cheers,
-------------- 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/20191010/ee2e7187/attachment.sig>