diff mbox

[RFC,3/7] qxl: introduce QXLCookie

Message ID 1329686886-6853-4-git-send-email-alevy@redhat.com
State New
Headers show

Commit Message

Alon Levy Feb. 19, 2012, 9:28 p.m. UTC 
Will be used in the next patch.

Signed-off-by: Alon Levy <alevy@redhat.com>
---
 hw/qxl-render.c    |    2 +-
 hw/qxl.c           |   65 +++++++++++++++++++++++++++++++++++++--------------
 hw/qxl.h           |    2 +-
 ui/spice-display.c |   26 ++++++++++++++++++--
 ui/spice-display.h |   12 +++++++++
 5 files changed, 84 insertions(+), 23 deletions(-)

Comments

Gerd Hoffmann Feb. 20, 2012, 10:56 a.m. UTC | #1 
> +        if (cookie == NULL) {
> +            cookie = qxl_cookie_new(QXL_COOKIE_TYPE_IO,
> +                                    QXL_IO_UPDATE_AREA_ASYNC,
> +                                    0);
> +        }

Automagic cookie creation is still there.
I think when cookie is NULL you should just pass it on ...

> -/* 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;

... and do "if (cookie == NULL) return;" here.

Which spice server version added async support?  IIRC this is 0.8.2?

I'm tempted to raise the minimal supported version to 0.8.latest and zap
a bunch of #ifdefs from the code.  What do you think?

cheers,
  Gerd
Alon Levy Feb. 20, 2012, 12:31 p.m. UTC | #2 
On Mon, Feb 20, 2012 at 11:56:58AM +0100, Gerd Hoffmann wrote:
> > +        if (cookie == NULL) {
> > +            cookie = qxl_cookie_new(QXL_COOKIE_TYPE_IO,
> > +                                    QXL_IO_UPDATE_AREA_ASYNC,
> > +                                    0);
> > +        }
> 
> Automagic cookie creation is still there.
> I think when cookie is NULL you should just pass it on ...

My bad, I thought I zapped it. I've fixed some issues, now it runs with
vnc and sdl fine (with my DisplayAllocation patch, didn't see yet if you
had any comments on it), so I'll make sure to remove this before sending
the next RFC.

> 
> > -/* 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;
> 
> ... and do "if (cookie == NULL) return;" here.

I can do that too.

> 
> Which spice server version added async support?  IIRC this is 0.8.2?
> 
> I'm tempted to raise the minimal supported version to 0.8.latest and zap
> a bunch of #ifdefs from the code.  What do you think?
> 

How does that work? raise the minimal supported version in configure?
and in RHEL we would do that as well? should be fine. And solves
headaches of testing this with multiple spice-server versions. I'll do
it.

> cheers,
>   Gerd
>
Gerd Hoffmann Feb. 20, 2012, 12:39 p.m. UTC | #3 
Hi,

>> Which spice server version added async support?  IIRC this is 0.8.2?
>>
>> I'm tempted to raise the minimal supported version to 0.8.latest and zap
>> a bunch of #ifdefs from the code.  What do you think?
>>
> 
> How does that work? raise the minimal supported version in configure?

Yes.  Then zap code handling older versions.  Maybe some cleanups
afterwards.

> and in RHEL we would do that as well? should be fine.

Probably, otherwise backporting becomes messy ;)

cheers,
  Gerd
diff mbox

Patch

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..9d3b848 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -143,15 +143,20 @@  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 = qxl_cookie_new(QXL_COOKIE_TYPE_IO,
+                                    QXL_IO_UPDATE_AREA_ASYNC,
+                                    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
@@ -171,11 +176,13 @@  static void qxl_spice_destroy_surface_wait(PCIQXLDevice *qxl, uint32_t id,
                                            qxl_async_io async)
 {
     if (async) {
-#if SPICE_INTERFACE_QXL_MINOR < 1
-        abort();
-#else
+#if SPICE_INTERFACE_QXL_MINOR >= 1
         spice_qxl_destroy_surface_async(&qxl->ssd.qxl, id,
-                                        (uint64_t)id);
+                (uint64_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO,
+                                         QXL_IO_DESTROY_SURFACE_ASYNC,
+                                         id));
+#else
+        abort();
 #endif
     } else {
         qxl->ssd.worker->destroy_surface_wait(qxl->ssd.worker, id);
@@ -186,7 +193,10 @@  static void qxl_spice_destroy_surface_wait(PCIQXLDevice *qxl, uint32_t id,
 #if SPICE_INTERFACE_QXL_MINOR >= 1
 static void qxl_spice_flush_surfaces_async(PCIQXLDevice *qxl)
 {
-    spice_qxl_flush_surfaces_async(&qxl->ssd.qxl, 0);
+ spice_qxl_flush_surfaces_async(&qxl->ssd.qxl,
+        (uint64_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO,
+                                 QXL_IO_FLUSH_SURFACES_ASYNC,
+                                 0));
 }
 #endif
 
@@ -217,10 +227,13 @@  static void qxl_spice_destroy_surfaces_complete(PCIQXLDevice *qxl)
 static void qxl_spice_destroy_surfaces(PCIQXLDevice *qxl, qxl_async_io async)
 {
     if (async) {
-#if SPICE_INTERFACE_QXL_MINOR < 1
-        abort();
+#if SPICE_INTERFACE_QXL_MINOR >= 1
+        spice_qxl_destroy_surfaces_async(&qxl->ssd.qxl,
+                (uint64_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO,
+                                         QXL_IO_DESTROY_ALL_SURFACES,
+                                         0));
 #else
-        spice_qxl_destroy_surfaces_async(&qxl->ssd.qxl, 0);
+        abort();
 #endif
     } else {
         qxl->ssd.worker->destroy_surfaces(qxl->ssd.worker);
@@ -737,18 +750,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 +772,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 +1108,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 +1244,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 7df00f1..71d5c35 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..680e6f4 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -60,12 +60,26 @@  void qemu_spice_rect_union(QXLRect *dest, const QXLRect *r)
     dest->right = MAX(dest->right, r->right);
 }
 
+QXLCookie *qxl_cookie_new(int type, uint64_t io, uint64_t data)
+{
+    QXLCookie *cookie;
+
+    cookie = g_malloc0(sizeof(*cookie));
+    cookie->type = type;
+    cookie->io = io;
+    cookie->data = data;
+    return cookie;
+}
+
 void qemu_spice_add_memslot(SimpleSpiceDisplay *ssd, QXLDevMemSlot *memslot,
                             qxl_async_io async)
 {
     if (async != QXL_SYNC) {
 #if SPICE_INTERFACE_QXL_MINOR >= 1
-        spice_qxl_add_memslot_async(&ssd->qxl, memslot, 0);
+        spice_qxl_add_memslot_async(&ssd->qxl, memslot,
+                (uint64_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO,
+                                         QXL_IO_MEMSLOT_ADD_ASYNC,
+                                         0));
 #else
         abort();
 #endif
@@ -85,7 +99,10 @@  void qemu_spice_create_primary_surface(SimpleSpiceDisplay *ssd, uint32_t id,
 {
     if (async != QXL_SYNC) {
 #if SPICE_INTERFACE_QXL_MINOR >= 1
-        spice_qxl_create_primary_surface_async(&ssd->qxl, id, surface, 0);
+        spice_qxl_create_primary_surface_async(&ssd->qxl, id, surface,
+                (uint64_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO,
+                                         QXL_IO_CREATE_PRIMARY_ASYNC,
+                                         0));
 #else
         abort();
 #endif
@@ -100,7 +117,10 @@  void qemu_spice_destroy_primary_surface(SimpleSpiceDisplay *ssd,
 {
     if (async != QXL_SYNC) {
 #if SPICE_INTERFACE_QXL_MINOR >= 1
-        spice_qxl_destroy_primary_surface_async(&ssd->qxl, id, 0);
+        spice_qxl_destroy_primary_surface_async(&ssd->qxl, id,
+                (uint64_t)qxl_cookie_new(QXL_COOKIE_TYPE_IO,
+                                         QXL_IO_DESTROY_PRIMARY_ASYNC,
+                                         0));
 #else
         abort();
 #endif
diff --git a/ui/spice-display.h b/ui/spice-display.h
index 5e52df9..c8747ab 100644
--- a/ui/spice-display.h
+++ b/ui/spice-display.h
@@ -48,6 +48,18 @@  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;
+
+QXLCookie *qxl_cookie_new(int type, uint64_t io, uint64_t data);
+
 typedef struct SimpleSpiceDisplay SimpleSpiceDisplay;
 typedef struct SimpleSpiceUpdate SimpleSpiceUpdate;