[Spice-devel] [PATCH spice-server 4/6] red-qxl: Avoid to use AsyncCommand for GL_DRAW_ASYNC message

[Frediano Ziglio]

> 
> OK. The invalid cookie value is a bit odd, but looks safe.
> 

Not that odd, CreateFile, open, mmap and iconv for instance return similar values

Frediano

> Acked-by: Jonathon Jongsma <jjongsma at redhat.com>
> 
> 
> On Thu, 2017-09-07 at 12:40 +0100, Frediano Ziglio wrote:
> > AsyncCommand is used to handle asynchronous messages from the
> > dispatcher.
> > GL_DRAW_ASYNC is mainly using it to store the cookie.
> > 
> > The value of GL_DRAW_COOKIE_INVALID was choosen to allow implementing
> > cookies (which basically are handles) either using indexes (where 0
> > is
> > valid) or pointers (where 0 is invalid). Currently Qemu uses
> > pointers.
> > 
> > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > ---
> >  server/red-qxl.c | 22 +++++++++++++---------
> >  1 file changed, 13 insertions(+), 9 deletions(-)
> > 
> > diff --git a/server/red-qxl.c b/server/red-qxl.c
> > index bec063c17..b1804cf17 100644
> > --- a/server/red-qxl.c
> > +++ b/server/red-qxl.c
> > @@ -62,9 +62,11 @@ struct QXLState {
> >  
> >      pthread_mutex_t scanout_mutex;
> >      SpiceMsgDisplayGlScanoutUnix scanout;
> > -    struct AsyncCommand *gl_draw_async;
> > +    uint64_t gl_draw_cookie;
> >  };
> >  
> > +#define GL_DRAW_COOKIE_INVALID (~((uint64_t) 0))
> > +
> >  int red_qxl_check_qxl_version(QXLInstance *qxl, int major, int
> > minor)
> >  {
> >      int qxl_major = qxl_get_interface(qxl)->base.major_version;
> > @@ -833,7 +835,7 @@ void spice_qxl_gl_scanout(QXLInstance *qxl,
> >      spice_return_if_fail(qxl != NULL);
> >  
> >      QXLState *qxl_state = qxl->st;
> > -    spice_return_if_fail(qxl_state->gl_draw_async == NULL);
> > +    spice_return_if_fail(qxl_state->gl_draw_cookie ==
> > GL_DRAW_COOKIE_INVALID);
> >  
> >      pthread_mutex_lock(&qxl_state->scanout_mutex);
> >  
> > @@ -877,13 +879,15 @@ void spice_qxl_gl_draw_async(QXLInstance *qxl,
> >      spice_return_if_fail(qxl != NULL);
> >      qxl_state = qxl->st;
> >      if (qxl_state->scanout.drm_dma_buf_fd == -1) {
> > +        QXLInterface *interface = qxl_get_interface(qxl);
> > +
> >          spice_warning("called spice_qxl_gl_draw_async without a
> > buffer");
> > -        red_qxl_async_complete(qxl, async_command_alloc(qxl_state,
> > message, cookie));
> > +        interface->async_complete(qxl, cookie);
> >          return;
> >      }
> > -    spice_return_if_fail(qxl_state->gl_draw_async == NULL);
> > +    spice_return_if_fail(qxl_state->gl_draw_cookie ==
> > GL_DRAW_COOKIE_INVALID);
> >  
> > -    qxl_state->gl_draw_async = async_command_alloc(qxl_state,
> > message, cookie);
> > +    qxl_state->gl_draw_cookie = cookie;
> >      dispatcher_send_message(qxl_state->dispatcher, message, &draw);
> >  }
> >  
> > @@ -899,7 +903,6 @@ void red_qxl_async_complete(QXLInstance *qxl,
> > AsyncCommand *async_command)
> >      case RED_WORKER_MESSAGE_DESTROY_SURFACE_WAIT_ASYNC:
> >      case RED_WORKER_MESSAGE_FLUSH_SURFACES_ASYNC:
> >      case RED_WORKER_MESSAGE_MONITORS_CONFIG_ASYNC:
> > -    case RED_WORKER_MESSAGE_GL_DRAW_ASYNC:
> >          break;
> >      case RED_WORKER_MESSAGE_CREATE_PRIMARY_SURFACE_ASYNC:
> >          red_qxl_create_primary_surface_complete(qxl->st);
> > @@ -916,10 +919,11 @@ void red_qxl_async_complete(QXLInstance *qxl,
> > AsyncCommand *async_command)
> >  
> >  void red_qxl_gl_draw_async_complete(QXLInstance *qxl)
> >  {
> > +    QXLInterface *interface = qxl_get_interface(qxl);
> >      /* this reset before usage prevent a possible race condition */
> > -    struct AsyncCommand *async = qxl->st->gl_draw_async;
> > -    qxl->st->gl_draw_async = NULL;
> > -    red_qxl_async_complete(qxl, async);
> > +    uint64_t cookie = qxl->st->gl_draw_cookie;
> > +    qxl->st->gl_draw_cookie = GL_DRAW_COOKIE_INVALID;
> > +    interface->async_complete(qxl, cookie);
> >  }
> >  
> >  void red_qxl_init(RedsState *reds, QXLInstance *qxl)
>