[Spice-devel] [PATCH v3] server: add QXLInterface::surface_updated callback

[Alon Levy]

On Mon, Jul 11, 2011 at 01:48:38PM +0300, Alon Levy wrote:
> On Sun, Jul 10, 2011 at 09:55:21AM +0300, Yonit Halperin wrote:
> > On 07/07/2011 07:33 PM, Alon Levy wrote:
> > >used to move dirty rectangle notification from update_area to surface_updated.
> > >
> > >This is RfC quality. Specifically where to call surface_updated. Currently I
> > >only call it from stop. This should be good enough for migration, which is
> > >the only use case I think. This means after stop qemu is guranteed to know
> > >the dirty regions of all surfaces.
> > >---
> > >  server/red_dispatcher.c          |   20 +++--------
> > >  server/red_worker.c              |   71 +++++++++++++++++++++++++------------
> > >  server/spice.h                   |    8 ++--
> > >  server/tests/test_display_base.c |    2 +-
> > >  4 files changed, 58 insertions(+), 43 deletions(-)
> > >
> > >diff --git a/server/red_dispatcher.c b/server/red_dispatcher.c
> > >index 3dfd4e5..996e1ed 100644
> > >--- a/server/red_dispatcher.c
> > >+++ b/server/red_dispatcher.c
> > >@@ -210,9 +210,7 @@ static void update_client_mouse_allowed(void)
> > >  }
> > >
> > >  static void qxl_worker_update_area_helper(QXLWorker *qxl_worker, uint32_t surface_id,
> > >-                                   QXLRect *qxl_area, QXLRect *qxl_dirty_rects,
> > >-                                   uint32_t num_dirty_rects, uint32_t clear_dirty_region,
> > >-                                   int async, uint64_t cookie)
> > >+                                   QXLRect *qxl_area, int async, uint64_t cookie)
> > >  {
> > >      RedDispatcher *dispatcher = (RedDispatcher *)qxl_worker;
> > >      RedWorkerMessage message;
> > >@@ -228,9 +226,6 @@ static void qxl_worker_update_area_helper(QXLWorker *qxl_worker, uint32_t surfac
> > >      }
> > >      send_data(dispatcher->channel,&surface_id, sizeof(uint32_t));
> > >      send_data(dispatcher->channel,&qxl_area, sizeof(QXLRect *));
> > >-    send_data(dispatcher->channel,&qxl_dirty_rects, sizeof(QXLRect *));
> > >-    send_data(dispatcher->channel,&num_dirty_rects, sizeof(uint32_t));
> > >-    send_data(dispatcher->channel,&clear_dirty_region, sizeof(uint32_t));
> > >      if (async) {
> > >          return;
> > >      }
> > >@@ -239,22 +234,17 @@ static void qxl_worker_update_area_helper(QXLWorker *qxl_worker, uint32_t surfac
> > >  }
> > >
> > >  static void qxl_worker_update_area(QXLWorker *qxl_worker, uint32_t surface_id,
> > >-                                   QXLRect *qxl_area, QXLRect *qxl_dirty_rects,
> > >-                                   uint32_t num_dirty_rects, uint32_t clear_dirty_region)
> > >+                                   QXLRect *qxl_area)
> > >  {
> > >-    qxl_worker_update_area_helper(qxl_worker, surface_id, qxl_area, qxl_dirty_rects, num_dirty_rects,
> > >-        clear_dirty_region, 0, 0);
> > >+    qxl_worker_update_area_helper(qxl_worker, surface_id, qxl_area, 0, 0);
> > >  }
> > >
> > >  static void qxl_worker_update_area_async(QXLWorker *qxl_worker, uint32_t surface_id,
> > >-                                   QXLRect *qxl_area, QXLRect *qxl_dirty_rects,
> > >-                                   uint32_t num_dirty_rects, uint32_t clear_dirty_region, uint64_t cookie)
> > >+                                         QXLRect *qxl_area, uint64_t cookie)
> > >  {
> > >-    qxl_worker_update_area_helper(qxl_worker, surface_id, qxl_area, qxl_dirty_rects, num_dirty_rects,
> > >-        clear_dirty_region, 1, cookie);
> > >+    qxl_worker_update_area_helper(qxl_worker, surface_id, qxl_area, 1, cookie);
> > >  }
> > >
> > >-
> > >  static void qxl_worker_add_memslot(QXLWorker *qxl_worker, QXLDevMemSlot *mem_slot)
> > >  {
> > >      RedDispatcher *dispatcher = (RedDispatcher *)qxl_worker;
> > >diff --git a/server/red_worker.c b/server/red_worker.c
> > >index cb84754..f5b5570 100644
> > >--- a/server/red_worker.c
> > >+++ b/server/red_worker.c
> > >@@ -9415,25 +9415,56 @@ static void red_wait_pipe_item_sent(RedChannel *channel, PipeItem *item)
> > >      red_unref_channel(channel);
> > >  }
> > >
> > >+static int surface_updated_available(RedWorker *worker)
> > >+{
> > >+    int minor = worker->qxl->st->qif->base.minor_version;
> > >+    int major = worker->qxl->st->qif->base.major_version;
> > >+
> > >+    return (major>  3 || (major == 3&&  minor>= 1))&&
> > >+            worker->qxl->st->qif->surface_updated != NULL;
> > >+}
> > Hi,
> > can qxl < 3.1 be compiled with the interface changes to update_area?
> > 
> 
> no. To use a newer spice a newer qxl will also need to be used.

on second thought, I'll keep update_area as is and just have the new update_area_async
not accept dirty_rects.

So:
 if qxl < 3.1 || qxl >= 3.1 and surface_updated == NULL:
  spice will never call surface_updated, never reset the dirty_rects by itself.
 else (if qxl >= 3.1 && surface_updated != NULL):
  spice will expect update_area to never be called (won't crash, but just treat those calls
  as update_area_async, not returning any dirty rects information, and log this)

> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel