[Spice-devel] [PATCH 4/4] replay: Handle cursor commands

[Frediano Ziglio]

> 
> On Fri, 2016-06-03 at 10:59 +0100, Frediano Ziglio wrote:
> > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > ---
> >  server/tests/replay.c | 50
> >  ++++++++++++++++++++++++++++++++------------------
> >  1 file changed, 32 insertions(+), 18 deletions(-)
> > 
> > diff --git a/server/tests/replay.c b/server/tests/replay.c
> > index d552327..528609b 100644
> > --- a/server/tests/replay.c
> > +++ b/server/tests/replay.c
> > @@ -52,7 +52,8 @@ static gboolean print_count = FALSE;
> >  static guint ncommands = 0;
> >  static pid_t client_pid;
> >  static GMainLoop *loop = NULL;
> > -static GAsyncQueue *aqueue = NULL;
> > +static GAsyncQueue *cmd_queue = NULL;
> > +static GAsyncQueue *cursor_queue = NULL;
> >  static long total_size;
> >  
> >  static pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
> > @@ -113,10 +114,12 @@ static gboolean fill_queue_idle(gpointer user_data)
> >      gboolean keep = FALSE;
> >      gboolean wakeup = FALSE;
> >  
> > -    while (g_async_queue_length(aqueue) < 50) {
> > +    while ((g_async_queue_length(cmd_queue) +
> > +            g_async_queue_length(cursor_queue)) < 50) {
> >          QXLCommandExt *cmd = spice_replay_next_cmd(replay, qxl_worker);
> >          if (!cmd) {
> > -            g_async_queue_push(aqueue, GINT_TO_POINTER(-1));
> > +            g_async_queue_push(cmd_queue, GINT_TO_POINTER(-1));
> > +            g_async_queue_push(cursor_queue, GINT_TO_POINTER(-1));
> >              goto end;
> >          }
> >  
> > @@ -127,7 +130,11 @@ static gboolean fill_queue_idle(gpointer user_data)
> >          }
> >  
> >          wakeup = TRUE;
> > -        g_async_queue_push(aqueue, cmd);
> > +        if (cmd->cmd.type == QXL_CMD_CURSOR) {
> > +            g_async_queue_push(cursor_queue, cmd);
> > +        } else {
> > +            g_async_queue_push(cmd_queue, cmd);
> > +        }
> >      }
> >  
> >  end:
> > @@ -166,17 +173,21 @@ end:
> >  
> >  
> >  // called from spice_server thread (i.e. red_worker thread)
> > -static int get_command(QXLInstance *qin, QXLCommandExt *ext)
> > +static int get_command_from(QXLInstance *qin, QXLCommandExt *ext,
> > GAsyncQueue
> > *queue)
> >  {
> >      QXLCommandExt *cmd;
> >  
> > -    if (g_async_queue_length(aqueue) == 0) {
> > +    if (g_async_queue_length(cmd_queue) == 0 &&
> > +        g_async_queue_length(cursor_queue) == 0) {
> 
> why not just check "g_async_queue_length(queue) == 0"? if we're trying to get
> a
> command from the cursor_queue and it's empty, should the fact that the other
> queue is not empty prevent us from trying to fill the queue? It's probably
> fine
> this way, just curious.
> 

I think was trying to be consistent to the way was done before.
Changed (and tested), not an issue.

> >          /* could use a gcondition ? */
> >          fill_queue();
> >          return FALSE;
> >      }
> >  
> > -    cmd = g_async_queue_try_pop(aqueue);
> > +    cmd = g_async_queue_try_pop(queue);
> > +    if (cmd == NULL) {
> > +        return FALSE;
> > +    }
> >      if (GPOINTER_TO_INT(cmd) == -1) {
> >          g_main_loop_quit(loop);
> >          return FALSE;
> > @@ -187,8 +198,14 @@ static int get_command(QXLInstance *qin, QXLCommandExt
> > *ext)
> >      return TRUE;
> >  }
> >  
> > -static int req_cmd_notification(QXLInstance *qin)
> > +static int get_command(QXLInstance *qin, QXLCommandExt *ext)
> > +{
> > +    return get_command_from(qin, ext, cmd_queue);
> > +}
> > +
> > +static int req_notification(QXLInstance *qin)
> >  {
> > +    /* we don't have currently message pending */
> >      return TRUE;
> >  }
> >  
> > @@ -214,12 +231,7 @@ static void release_resource(QXLInstance *qin, struct
> > QXLReleaseInfoExt release_
> >  
> >  static int get_cursor_command(QXLInstance *qin, struct QXLCommandExt *ext)
> >  {
> > -    return FALSE;
> > -}
> > -
> > -static int req_cursor_notification(QXLInstance *qin)
> > -{
> > -    return TRUE;
> > +    return get_command_from(qin, ext, cursor_queue);
> >  }
> >  
> >  static void notify_update(QXLInstance *qin, uint32_t update_id)
> > @@ -243,10 +255,10 @@ static QXLInterface display_sif = {
> >      .set_mm_time = set_mm_time,
> >      .get_init_info = get_init_info,
> >      .get_command = get_command,
> > -    .req_cmd_notification = req_cmd_notification,
> > +    .req_cmd_notification = req_notification,
> >      .release_resource = release_resource,
> >      .get_cursor_command = get_cursor_command,
> > -    .req_cursor_notification = req_cursor_notification,
> > +    .req_cursor_notification = req_notification,
> >      .notify_update = notify_update,
> >      .flush_resources = flush_resources,
> >  };
> > @@ -379,7 +391,8 @@ int main(int argc, char **argv)
> >          exit(1);
> >      }
> >  
> > -    aqueue = g_async_queue_new();
> > +    cmd_queue = g_async_queue_new();
> > +    cursor_queue = g_async_queue_new();
> >      core = basic_event_loop_init();
> >      core->channel_event = replay_channel_event;
> >  
> > @@ -414,7 +427,8 @@ int main(int argc, char **argv)
> >          g_print("Counted %d commands\n", ncommands);
> >  
> >      end_replay();
> > -    g_async_queue_unref(aqueue);
> > +    g_async_queue_unref(cmd_queue);
> > +    g_async_queue_unref(cursor_queue);
> >  
> >      /* FIXME: there should be a way to join server threads before:
> >       * g_main_loop_unref(loop);
> 
> aside from the minor question above, looks fine.
> 
> Acked-by: Jonathon Jongsma <jjongsma at redhat.com>
> 

Frediano