[Spice-devel] [PATCH v5 4/9] server: add async io support

[Alon Levy]

On Tue, Jul 19, 2011 at 01:44:56PM +0200, Gerd Hoffmann wrote:
> >--- a/server/spice.h
> >+++ b/server/spice.h
> >@@ -122,6 +122,16 @@ struct QXLWorker {
> >      void (*reset_cursor)(QXLWorker *worker);
> >      void (*destroy_surface_wait)(QXLWorker *worker, uint32_t surface_id);
> >      void (*loadvm_commands)(QXLWorker *worker, struct QXLCommandExt *ext, uint32_t count);
> >+    /* async versions of commands. when complete spice calls async_complete */
> >+    void (*update_area_async)(QXLWorker *qxl_worker, uint32_t surface_id,
> >+                       struct QXLRect *area, uint32_t clear_dirty_region,
> >+                       uint64_t cookie);
> >+    void (*add_memslot_async)(QXLWorker *worker, QXLDevMemSlot *slot, uint64_t cookie);
> >+    void (*destroy_surfaces_async)(QXLWorker *worker, uint64_t cookie);
> >+    void (*destroy_primary_surface_async)(QXLWorker *worker, uint32_t surface_id, uint64_t cookie);
> >+    void (*create_primary_surface_async)(QXLWorker *worker, uint32_t surface_id,
> >+                                   QXLDevSurfaceCreate *surface, uint64_t cookie);
> >+    void (*destroy_surface_wait_async)(QXLWorker *worker, uint32_t surface_id, uint64_t cookie);
> >  };
> 
> Is there any reason to go this indirect route through QXLWorker in
> the first place (other than historical ones)?  We can do this

I don't know. I'm trying to figure out why would someone prefer callbacks
to exported functions. It's more OOPy? no idea.

> instead:
> 
>    SPICE_GNUC_VISIBLE void
>    spice_qxl_update_area(QXLInstance *qxl, uint32_t surface_id,
>                          QXLRect *qxl_area, QXLRect *qxl_dirty_rects,
>                          uint32_t num_dirty_rects,
>                          uint32_t clear_dirty_region)
>    {
>        qxl_worker_update_area(&qxl->st->dispatcher->base, surface_id,
>                               qxl_area, qxl_dirty_rects,
>                               num_dirty_rects, clear_dirty_region);
>    }
> 
> Then have qemu call spice_qxl_update_area() directly instead of
> worker->update_area().  Likewise for all others.  We obviously have
> to keep the old indirect code path for the existing calls for
> compatibility reasons.
> 

Right.

> I'd like to see all new calls go direct (and only direct).  Having
> direct calls for the old ones would be nice too.
> 
> This will (a) remove the pointless indirection and (b) make shared

Is this actually better then having a jump to the shared object table (I'm
assuming there is one, I haven't looked how it's actually implemented)?

> library versioning alot easier as we'll get new linker symbols which
> we can tag with the version they appeared in the shared library.

This sounds like a real win.

> 
> cheers,
>   Gerd
> 
> 
> cheers,
>   Gerd
>