[Spice-devel] [PATCH 5/7] Don't increment num_clients_mig_wait twice

[Pavel Grunt]

On Wed, 2016-10-12 at 10:13 -0400, Frediano Ziglio wrote:
> > 
> > On Wed, 2016-10-12 at 06:57 -0400, Frediano Ziglio wrote:
> > > > 
> > > > 
> > > > When MainChannelClient was split to a separate file, the
> > > > responsibility
> > > > for incrementing this field was supposed to belong to the
> > > > MainChannel
> > > > function (main_channel_connect_semi_seamless()), but by
> > > > mistake it
> > > > was
> > > > incremented both there and in the client function
> > > > (main_channel_client_connect_semi_seamless()).
> > > > 
> > > > The bug was introduced in
> > > > a11b785f191d2381932f8c1bb6281539f2afe483
> > > > ---
> > > >  server/main-channel-client.c | 2 --
> > > >  1 file changed, 2 deletions(-)
> > > > 
> > > > diff --git a/server/main-channel-client.c b/server/main-
> > > > channel-
> > > > client.c
> > > > index b30083f..0913028 100644
> > > > --- a/server/main-channel-client.c
> > > > +++ b/server/main-channel-client.c
> > > > @@ -704,7 +704,6 @@ void
> > > > main_channel_client_migrate(RedChannelClient *rcc)
> > > >  gboolean
> > > > main_channel_client_connect_semi_seamless(MainChannelClient
> > > > *mcc)
> > > >  {
> > > >      RedChannelClient *rcc = RED_CHANNEL_CLIENT(mcc);
> > > > -    MainChannel* main_channel =
> > > > MAIN_CHANNEL(red_channel_client_get_channel(rcc));
> > > >      if (red_channel_client_test_remote_cap(rcc,
> > > >                                             SPICE_MAIN_CAP_SEM
> > > > I_SEA
> > > > MLESS_MIGRATE))
> > > >                                             {
> > > >          RedClient *client =
> > > > red_channel_client_get_client(rcc);
> > > > @@ -718,7 +717,6 @@ gboolean
> > > > main_channel_client_connect_semi_seamless(MainChannelClient
> > > > *mcc)
> > > >              mcc->priv->mig_wait_connect = TRUE;
> > > >          }
> > > >          mcc->priv->mig_connect_ok = FALSE;
> > > > -        main_channel->num_clients_mig_wait++;
> > > >          return TRUE;
> > > >      }
> > > >      return FALSE;
> > > 
> > > Acked-by: Frediano Ziglio <fziglio at redhat.com>
> > > 
> > > I think we have a lack of testing, at least fast ones.
> > 
> > Yes, definitely.
> > 
> > > How did you discover this?
> > 
> > Just code inspection. While splitting out the MainChannelPrivate
> > stuff
> > into a separate patch, I noticed that this hunk was included in
> > the
> > "Convert RedChannel Hierarchy to GObject" patch. And it looked odd
> > to
> > me, so I went to look at the history of this particular code.
> > 
> > > Did you manage to reproduce the issue?
> > 
> > No.
> > 
> 
> I remember Uri can test migration locally but I never dig around
> enough.
> Uri, how to test it locally?

Take a look at:
https://cgit.freedesktop.org/spice/spice/tree/tests/migrate.py

> 
> Frediano
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel