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

[Frediano Ziglio]

> 
> 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_SEMI_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?

Frediano