Re: [Qemu-devel] [PATCH 1/2] qxl: introduce QXLCookie

[Alon Levy]

On Fri, Feb 17, 2012 at 01:11:27AM +0200, Alon Levy wrote:
> Will be used in the next patch.

Self NACK, some errors in this patchset, sending v2 with cleanup
(qxl_cookie_new to reduce the duplicate code).

> 
> Signed-off-by: Alon Levy <address@hidden>
> ---
>  hw/qxl-render.c    |    2 +-
>  hw/qxl.c           |   63 +++++++++++++++++++++++++++++++++++++++------------
>  hw/qxl.h           |    2 +-
>  ui/spice-display.c |   24 +++++++++++++++++--
>  ui/spice-display.h |   10 ++++++++
>  5 files changed, 81 insertions(+), 20 deletions(-)
> 
> diff --git a/hw/qxl-render.c b/hw/qxl-render.c
> index 133d093..b238b96 100644
> --- a/hw/qxl-render.c
> +++ b/hw/qxl-render.c
> @@ -133,7 +133,7 @@ void qxl_render_update(PCIQXLDevice *qxl)
>  
>      memset(dirty, 0, sizeof(dirty));
>      qxl_spice_update_area(qxl, 0, &update,
> -                          dirty, ARRAY_SIZE(dirty), 1, QXL_SYNC);
> +                          dirty, ARRAY_SIZE(dirty), 1, QXL_SYNC, NULL);
>      if (redraw) {
>          memset(dirty, 0, sizeof(dirty));
>          dirty[0] = update;
> diff --git a/hw/qxl.c b/hw/qxl.c
> index ac69125..59d2b13 100644
> --- a/hw/qxl.c
> +++ b/hw/qxl.c
> @@ -143,15 +143,21 @@ void qxl_spice_update_area(PCIQXLDevice *qxl, uint32_t 
> surface_id,
>                             struct QXLRect *area, struct QXLRect *dirty_rects,
>                             uint32_t num_dirty_rects,
>                             uint32_t clear_dirty_region,
> -                           qxl_async_io async)
> +                           qxl_async_io async, QXLCookie *cookie)
>  {
>      if (async == QXL_SYNC) {
>          qxl->ssd.worker->update_area(qxl->ssd.worker, surface_id, area,
>                          dirty_rects, num_dirty_rects, clear_dirty_region);
>      } else {
>  #if SPICE_INTERFACE_QXL_MINOR >= 1
> +        if (cookie == NULL) {
> +            cookie = g_malloc0(sizeof(*cookie));
> +            cookie->type = QXL_COOKIE_TYPE_IO;
> +            cookie->io = QXL_IO_UPDATE_AREA_ASYNC;
> +            cookie->data = 0;
> +        }
>          spice_qxl_update_area_async(&qxl->ssd.qxl, surface_id, area,
> -                                    clear_dirty_region, 0);
> +                                    clear_dirty_region, (uint64_t)cookie);
>  #else
>          abort();
>  #endif
> @@ -170,12 +176,17 @@ static void 
> qxl_spice_destroy_surface_wait_complete(PCIQXLDevice *qxl,
>  static void qxl_spice_destroy_surface_wait(PCIQXLDevice *qxl, uint32_t id,
>                                             qxl_async_io async)
>  {
> +    QXLCookie *cookie;
> +
>      if (async) {
> -#if SPICE_INTERFACE_QXL_MINOR < 1
> -        abort();
> +#if SPICE_INTERFACE_QXL_MINOR >= 1
> +        cookie = g_malloc0(sizeof(*cookie));
> +        cookie->type = QXL_COOKIE_TYPE_IO;
> +        cookie->io = QXL_IO_DESTROY_SURFACE_ASYNC;
> +        cookie->data = id;
> +        spice_qxl_destroy_surface_async(&qxl->ssd.qxl, id, (uint64_t)cookie);
>  #else
> -        spice_qxl_destroy_surface_async(&qxl->ssd.qxl, id,
> -                                        (uint64_t)id);
> +        abort();
>  #endif
>      } else {
>          qxl->ssd.worker->destroy_surface_wait(qxl->ssd.worker, id);
> @@ -216,11 +227,17 @@ static void 
> qxl_spice_destroy_surfaces_complete(PCIQXLDevice *qxl)
>  
>  static void qxl_spice_destroy_surfaces(PCIQXLDevice *qxl, qxl_async_io async)
>  {
> +    QXLCookie *cookie;
> +
>      if (async) {
>  #if SPICE_INTERFACE_QXL_MINOR < 1
>          abort();
>  #else
> -        spice_qxl_destroy_surfaces_async(&qxl->ssd.qxl, 0);
> +        cookie = g_malloc0(sizeof(*cookie));
> +        cookie->type = QXL_COOKIE_TYPE_IO;
> +        cookie->io = QXL_IO_DESTROY_ALL_SURFACES;
> +        cookie->data = 0;
> +        spice_qxl_destroy_surfaces_async(&qxl->ssd.qxl, (uint64_t)cookie);
>  #endif
>      } else {
>          qxl->ssd.worker->destroy_surfaces(qxl->ssd.worker);
> @@ -737,18 +754,20 @@ static void 
> qxl_create_guest_primary_complete(PCIQXLDevice *d);
>  
>  #if SPICE_INTERFACE_QXL_MINOR >= 1
>  
> -/* called from spice server thread context only */
> -static void interface_async_complete(QXLInstance *sin, uint64_t cookie)
> +static void interface_async_complete_io(PCIQXLDevice *qxl, QXLCookie *cookie)
>  {
> -    PCIQXLDevice *qxl = container_of(sin, PCIQXLDevice, ssd.qxl);
>      uint32_t current_async;
>  
>      qemu_mutex_lock(&qxl->async_lock);
>      current_async = qxl->current_async;
>      qxl->current_async = QXL_UNDEFINED_IO;
>      qemu_mutex_unlock(&qxl->async_lock);
> +    dprint(qxl, 2, "async_complete: %d (%p) done\n", current_async, cookie);
> +    if (current_async != cookie->io) {
> +        fprintf(stderr, "qxl: %s: error: current_async = %d != %ld = 
> cookie->io\n",
> +                __func__, current_async, cookie->io);
> +    }
>  
> -    dprint(qxl, 2, "async_complete: %d (%ld) done\n", current_async, cookie);
>      switch (current_async) {
>      case QXL_IO_CREATE_PRIMARY_ASYNC:
>          qxl_create_guest_primary_complete(qxl);
> @@ -757,12 +776,28 @@ static void interface_async_complete(QXLInstance *sin, 
> uint64_t cookie)
>          qxl_spice_destroy_surfaces_complete(qxl);
>          break;
>      case QXL_IO_DESTROY_SURFACE_ASYNC:
> -        qxl_spice_destroy_surface_wait_complete(qxl, (uint32_t)cookie);
> +        qxl_spice_destroy_surface_wait_complete(qxl, (uint32_t)cookie->data);
>          break;
>      }
>      qxl_send_events(qxl, QXL_INTERRUPT_IO_CMD);
>  }
>  
> +/* called from spice server thread context only */
> +static void interface_async_complete(QXLInstance *sin, uint64_t cookie_token)
> +{
> +    PCIQXLDevice *qxl = container_of(sin, PCIQXLDevice, ssd.qxl);
> +    QXLCookie *cookie = (QXLCookie*)cookie_token;
> +
> +    switch (cookie->type) {
> +    case QXL_COOKIE_TYPE_IO:
> +        interface_async_complete_io(qxl, cookie);
> +        break;
> +    default:
> +        fprintf(stderr, "qxl: %s: unexpected cookie type %d\n", __func__, 
> cookie->type);
> +    }
> +    g_free(cookie);
> +}
> +
>  #endif
>  
>  static const QXLInterface qxl_interface = {
> @@ -1077,9 +1112,7 @@ static int qxl_destroy_primary(PCIQXLDevice *d, 
> qxl_async_io async)
>      if (d->mode == QXL_MODE_UNDEFINED) {
>          return 0;
>      }
> -
>      dprint(d, 1, "%s\n", __FUNCTION__);
> -
>      d->mode = QXL_MODE_UNDEFINED;
>      qemu_spice_destroy_primary_surface(&d->ssd, 0, async);
>      qxl_spice_reset_cursor(d);
> @@ -1215,7 +1248,7 @@ async_common:
>      {
>          QXLRect update = d->ram->update_area;
>          qxl_spice_update_area(d, d->ram->update_surface,
> -                              &update, NULL, 0, 0, async);
> +                              &update, NULL, 0, 0, async, NULL);
>          break;
>      }
>      case QXL_IO_NOTIFY_CMD:
> diff --git a/hw/qxl.h b/hw/qxl.h
> index 766aa6d..666dd78 100644
> --- a/hw/qxl.h
> +++ b/hw/qxl.h
> @@ -118,7 +118,7 @@ void qxl_spice_update_area(PCIQXLDevice *qxl, uint32_t 
> surface_id,
>                             struct QXLRect *area, struct QXLRect *dirty_rects,
>                             uint32_t num_dirty_rects,
>                             uint32_t clear_dirty_region,
> -                           qxl_async_io async);
> +                           qxl_async_io async, QXLCookie *cookie);
>  void qxl_spice_loadvm_commands(PCIQXLDevice *qxl, struct QXLCommandExt *ext,
>                                 uint32_t count);
>  void qxl_spice_oom(PCIQXLDevice *qxl);
> diff --git a/ui/spice-display.c b/ui/spice-display.c
> index 6c302a3..7f3f629 100644
> --- a/ui/spice-display.c
> +++ b/ui/spice-display.c
> @@ -63,9 +63,15 @@ void qemu_spice_rect_union(QXLRect *dest, const QXLRect *r)
>  void qemu_spice_add_memslot(SimpleSpiceDisplay *ssd, QXLDevMemSlot *memslot,
>                              qxl_async_io async)
>  {
> +    QXLCookie *cookie = g_malloc0(sizeof(*cookie));
> +
>      if (async != QXL_SYNC) {
>  #if SPICE_INTERFACE_QXL_MINOR >= 1
> -        spice_qxl_add_memslot_async(&ssd->qxl, memslot, 0);
> +        cookie = g_malloc0(sizeof(*cookie));
> +        cookie->type = QXL_COOKIE_TYPE_IO;
> +        cookie->io = QXL_IO_MEMSLOT_ADD_ASYNC;
> +        cookie->data = 0;
> +        spice_qxl_add_memslot_async(&ssd->qxl, memslot, (uint64_t)cookie);
>  #else
>          abort();
>  #endif
> @@ -83,9 +89,15 @@ void qemu_spice_create_primary_surface(SimpleSpiceDisplay 
> *ssd, uint32_t id,
>                                         QXLDevSurfaceCreate *surface,
>                                         qxl_async_io async)
>  {
> +    QXLCookie *cookie;
> +
>      if (async != QXL_SYNC) {
>  #if SPICE_INTERFACE_QXL_MINOR >= 1
> -        spice_qxl_create_primary_surface_async(&ssd->qxl, id, surface, 0);
> +        cookie = g_malloc0(sizeof(*cookie));
> +        cookie->type = QXL_COOKIE_TYPE_IO;
> +        cookie->io = QXL_IO_CREATE_PRIMARY_ASYNC;
> +        cookie->data = 0;
> +        spice_qxl_create_primary_surface_async(&ssd->qxl, id, surface, 
> (uint64_t)cookie);
>  #else
>          abort();
>  #endif
> @@ -98,9 +110,15 @@ void qemu_spice_create_primary_surface(SimpleSpiceDisplay 
> *ssd, uint32_t id,
>  void qemu_spice_destroy_primary_surface(SimpleSpiceDisplay *ssd,
>                                          uint32_t id, qxl_async_io async)
>  {
> +    QXLCookie *cookie;
> +
>      if (async != QXL_SYNC) {
>  #if SPICE_INTERFACE_QXL_MINOR >= 1
> -        spice_qxl_destroy_primary_surface_async(&ssd->qxl, id, 0);
> +        cookie = g_malloc0(sizeof(*cookie));
> +        cookie->type = QXL_COOKIE_TYPE_IO;
> +        cookie->io = QXL_IO_DESTROY_PRIMARY_ASYNC;
> +        cookie->data = 0;
> +        spice_qxl_destroy_primary_surface_async(&ssd->qxl, id, 
> (uint64_t)cookie);
>  #else
>          abort();
>  #endif
> diff --git a/ui/spice-display.h b/ui/spice-display.h
> index 5e52df9..63cb009 100644
> --- a/ui/spice-display.h
> +++ b/ui/spice-display.h
> @@ -48,6 +48,16 @@ typedef enum qxl_async_io {
>      QXL_ASYNC,
>  } qxl_async_io;
>  
> +enum {
> +    QXL_COOKIE_TYPE_IO,
> +};
> +
> +typedef struct QXLCookie {
> +    int      type;
> +    uint64_t io;
> +    uint64_t data;
> +} QXLCookie;
> +
>  typedef struct SimpleSpiceDisplay SimpleSpiceDisplay;
>  typedef struct SimpleSpiceUpdate SimpleSpiceUpdate;
>  
> -- 
> 1.7.9
> 
>