[Spice-devel] [PATCH v6 2/9] server: api: add spice_qxl_* calls based on QXLWorker contents

[Alon Levy]

On Wed, Jul 20, 2011 at 10:53:49AM +0200, Christophe Fergeau wrote:
> On Wed, Jul 20, 2011 at 11:19:52AM +0300, Alon Levy wrote:
> > -static void qxl_worker_update_area(QXLWorker *qxl_worker, uint32_t surface_id,
> > +static void red_dispatcher_update_area(RedDispatcher *dispatcher, uint32_t surface_id,
> >                                     QXLRect *qxl_area, QXLRect *qxl_dirty_rects,
> >                                     uint32_t num_dirty_rects, uint32_t clear_dirty_region)
> >  {
> > -    RedDispatcher *dispatcher = (RedDispatcher *)qxl_worker;
> >      RedWorkerMessage message = RED_WORKER_MESSAGE_UPDATE;
> >      SpiceRect *dirty_rects = spice_new0(SpiceRect, num_dirty_rects);
> >      SpiceRect *area = spice_new0(SpiceRect, 1);
> > @@ -241,9 +240,16 @@ static void qxl_worker_update_area(QXLWorker *qxl_worker, uint32_t surface_id,
> >      free(area);
> >  }
> >  
> > -static void qxl_worker_add_memslot(QXLWorker *qxl_worker, QXLDevMemSlot *mem_slot)
> > +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)
> > +{
> > +    red_dispatcher_update_area((RedDispatcher*)qxl_worker, surface_id, qxl_area,
> > +                               qxl_dirty_rects, num_dirty_rects, clear_dirty_region);
> > +}
> 
> 
> Aren't red_dispatcher_update_area/qxl_worker_update_area redundant? Can't
> we just keep the old qxl_worker_update_area and do the cast in there?

red_disptacher_update_area is used twice - once by qxl_worker_update_area which must remain
for backward compatibility (QXLWorker.update_area is set to it), and second by spice_qxl_update_area
which goes directly from the QXLInstance to the RedDispatcher without going through QXLWorker.

> 
> > diff --git a/server/spice.h b/server/spice.h
> > index f64ff41..86d9ffe 100644
> > --- a/server/spice.h
> > +++ b/server/spice.h
> > @@ -124,6 +124,25 @@ struct QXLWorker {
> >      void (*loadvm_commands)(QXLWorker *worker, struct QXLCommandExt *ext, uint32_t count);
> >  };
> 
> Will the function pointer above go away? My understanding is that the
> functions added below are meant to replace these?

They can't go away as long as we want to maintain backwards compatibility.

> 
> >  
> > +void spice_qxl_wakeup(QXLInstance *instance);
> > +void spice_qxl_oom(QXLInstance *instance);
> > +void spice_qxl_start(QXLInstance *instance);
> > +void spice_qxl_stop(QXLInstance *instance);
> > +void spice_qxl_update_area(QXLInstance *instance, uint32_t surface_id,
> > +                   struct QXLRect *area, struct QXLRect *dirty_rects,
> > +                   uint32_t num_dirty_rects, uint32_t clear_dirty_region);
> > +void spice_qxl_add_memslot(QXLInstance *instance, QXLDevMemSlot *slot);
> > +void spice_qxl_del_memslot(QXLInstance *instance, uint32_t slot_group_id, uint32_t slot_id);
> > +void spice_qxl_reset_memslots(QXLInstance *instance);
> > +void spice_qxl_destroy_surfaces(QXLInstance *instance);
> > +void spice_qxl_destroy_primary_surface(QXLInstance *instance, uint32_t surface_id);
> > +void spice_qxl_create_primary_surface(QXLInstance *instance, uint32_t surface_id,
> > +                               QXLDevSurfaceCreate *surface);
> > +void spice_qxl_reset_image_cache(QXLInstance *instance);
> > +void spice_qxl_reset_cursor(QXLInstance *instance);
> > +void spice_qxl_destroy_surface_wait(QXLInstance *instance, uint32_t surface_id);
> > +void spice_qxl_loadvm_commands(QXLInstance *instance, struct QXLCommandExt *ext, uint32_t count);
> 
> Christophe