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

[Alon Levy]

On Fri, Feb 17, 2012 at 02:11:40PM +0100, Gerd Hoffmann wrote:
> On 02/17/12 09:44, Alon Levy wrote:
> > Will be used in the next patch.
> > 
> > Signed-off-by: Alon Levy <address@hidden>
> > ---
> >  hw/qxl-render.c    |    2 +-
> >  hw/qxl.c           |   60 
> > +++++++++++++++++++++++++++++++++++++--------------
> >  hw/qxl.h           |    2 +-
> >  ui/spice-display.c |   26 ++++++++++++++++++++--
> >  ui/spice-display.h |   12 ++++++++++
> >  5 files changed, 80 insertions(+), 22 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..02708e3 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);
> > +        }
> 
> Hmm, why that?  If the callers want a cookie, then they should
> explicitly pass in what they want instead of expecting the function to
> magically do the correct thing.  If the callers don't need a cookie,
> then use something like QXL_COOKIE_TYPE_UNUSED or simply pass on the
> NULL pointer.

I can move it to the users if it makes it a little less magic.

Note that I will be sending a revized version, since:
1) I forgot about my previous attempt (october 2010,
http://lists.gnu.org/archive/html/qemu-devel/2011-10/msg02758.html)
2) there are a few bugs with this approach, that I fixed correctly back
then: one is the async monitor command, which I will not send this time
since I don't want to get into the same argument.
3) the second is the dirty rectangles, for which I have to raise
SPICE_INTERFACE_QXL_MINOR.

> 
> cheers,
>   Gerd
>