[Spice-devel] [PATCH 18/22] server: multiple clients works ok if we limit the pipe to the slowest client

[Frediano Ziglio]

> 
> On Wed, Dec 2, 2015 at 5:20 PM, Frediano Ziglio <fziglio at redhat.com> wrote:
> > From: Alon Levy <alon at pobox.com>
> >
> > ---
> >  server/red_worker.c | 8 ++++----
> >  server/reds.c       | 2 +-
> >  2 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/server/red_worker.c b/server/red_worker.c
> > index e31b872..25d2e0c 100644
> > --- a/server/red_worker.c
> > +++ b/server/red_worker.c
> > @@ -174,7 +174,7 @@ static int red_process_cursor(RedWorker *worker,
> > uint32_t max_pipe_size, int *ri
> >
> >      *ring_is_empty = FALSE;
> >      while (!cursor_is_connected(worker) ||
> > -           red_channel_min_pipe_size(RED_CHANNEL(worker->cursor_channel))
> > <= max_pipe_size) {
> > +           red_channel_max_pipe_size(RED_CHANNEL(worker->cursor_channel))
> > <= max_pipe_size) {
> >          if (!worker->qxl->st->qif->get_cursor_command(worker->qxl,
> >          &ext_cmd)) {
> >              *ring_is_empty = TRUE;
> >              if (worker->cursor_poll_tries < CMD_RING_POLL_RETRIES) {
> > @@ -244,10 +244,10 @@ static int red_process_display(RedWorker *worker,
> > uint32_t max_pipe_size, int *r
> >                  return n;
> >              }
> >
> > -
> > -            // TODO: change to average pipe size?
> > +            /* this is safe but slow. in the future client groups will
> > rule the world, and
> > +             * dial up will live with T1 pipes in harmony */
> >              if
> >              (red_channel_min_pipe_size(RED_CHANNEL(worker->display_channel))
> >              > max_pipe_size) {
> > -                spice_info("too much item in the display clients pipe
> > already");
> > +                spice_info("too many items in the display clients pipe
> > already");
> >                  return n;
> >              }
> >          }

Why in this second hunk red_channel_min_pipe_size is not changed to
red_channel_max_pipe_size ?

I think should be changed even here. Basically we avoid that a fast client
keep the pipeline empty while a slow one keeps accumulating items.

Actually I would prefer if there were an algorithm that start discarding
updates for slower clients putting items only on faster channels and
sending updates only when possible.

Looks like also from code that we stop processing commands if a client
is connected and cannot receive data fast enough.

> > diff --git a/server/reds.c b/server/reds.c
> > index 509c346..a494fa0 100644
> > --- a/server/reds.c
> > +++ b/server/reds.c
> > @@ -3347,7 +3347,7 @@ static int do_spice_init(SpiceCoreInterface
> > *core_interface)
> >
> >      reds->allow_multiple_clients = getenv(SPICE_DEBUG_ALLOW_MC_ENV) !=
> >      NULL;
> >      if (reds->allow_multiple_clients) {
> > -        spice_warning("spice: allowing multiple client connections
> > (crashy)");
> > +        spice_warning("spice: allowing multiple client connections");
> >      }
> >      atexit(reds_exit);
> >      return 0;
> > --
> > 2.4.3
> >
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/spice-devel
> 
> Not taht much related to the patch itself, but would be nice to have a
> explicit mention somewhere that even with multi client not crashing
> with this patch, there are still a few discussions needed in order to
> have a functional multi client support (like, how do we treat multiple
> mouse/keyboards? shall we just allow the first one and the others will
> be just "viewers"? how much work is necessary to have a full
> collaborative multi client support?).
> 
> Acked-by: Fabiano Fidêncio <fidencio at redhat.com>
> 

I think we can do like X with multiple mouse/keyboard attached. For
keyboard we handle all key events (press and release) as were coming
from a single keyboard. For relative pointers (like mouse) we add
all movements coming (so if I go left with one mouse and right with
another I can get a fixed pointer :) ). For absolute pointers (like
tablet) usually the last win.

Frediano