Re: [spice-server] wip: to better sync server and client tokens

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Fri, Jun 03, 2016 at 11:52:17AM -0400, Frediano Ziglio wrote:
> > 
> > ---
> >  server/char-device.c | 6 ++++++
> >  server/reds.c        | 8 ++------
> >  2 files changed, 8 insertions(+), 6 deletions(-)
> > 
> > diff --git a/server/char-device.c b/server/char-device.c
> > index cb35aa2..84efdfb 100644
> > --- a/server/char-device.c
> > +++ b/server/char-device.c
> > @@ -851,6 +851,12 @@ void red_char_device_reset(RedCharDevice *dev)
> >          dev_client->num_send_tokens +=
> >          g_queue_get_length(dev_client->send_queue);
> >          g_queue_foreach(dev_client->send_queue, (GFunc)red_pipe_item_unref,
> >          NULL);
> >          g_queue_clear(dev_client->send_queue);
> > +
> > +        /* FIXME: WIP patch: If the device is reset and we clear all the
> > +         * WriteBuffers, we must garantee that the number of tokens is
> > exactly
> > +         * the same that we will send to the client upon agent
> > re-connection. */
> > +        dev_client->num_client_tokens += dev_client->num_client_tokens_free;
> > +        dev_client->num_client_tokens_free = 0;
> >      }
> >      red_char_device_reset_dev_instance(dev, NULL);
> >  }
>
> Yes, make sense. Perhaps a check that there are no connected client
> but should be done in a reset function, or perhaps a client should
> be passed to this function (well, multiple clients are really never
> going to work reliably probably).

You mean that the whole block that resets the clients are wrong then?
When this happens in the current bug, we are definitely connected.

The only problem that I can see related to multiple clients is the clear
of send_queue. I would say that we need to be sure that same data were
sent to each client before clearing the buffers.

I'll be testing this patch a little longer to see if I can see any
problems :)

Cheers,
  toso

>
> > diff --git a/server/reds.c b/server/reds.c
> > index 4fd1d35..1cd697d 100644
> > --- a/server/reds.c
> > +++ b/server/reds.c
> > @@ -553,12 +553,8 @@ static void reds_reset_vdp(RedsState *reds)
> >       *  In addition, there used to be a misshandling of AGENT_TOKENS message
> >       in spice-gtk: it
> >       *  overrides the amount of tokens, instead of adding the given amount.
> >       */
> > -    if (red_channel_test_remote_cap(&reds->main_channel->base,
> > -                                    SPICE_MAIN_CAP_AGENT_CONNECTED_TOKENS))
> > {
> > -        dev->priv->agent_attached = FALSE;
> > -    } else {
> > -        red_char_device_reset(RED_CHAR_DEVICE(dev));
> > -    }
> > +    dev->priv->agent_attached = FALSE;
> > +    red_char_device_reset(RED_CHAR_DEVICE(dev));
> >  
> >      sif = spice_char_device_get_interface(reds->vdagent);
> >      if (sif->state) {
> 
> Frediano
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]