Re: [Qemu-devel] [PATCH v3 1/4] qxl/spice-display: move pipe to ssd

[Alon Levy]

On Wed, Mar 16, 2011 at 05:39:32PM +0100, Jes Sorensen wrote:
> On 03/16/11 16:52, Alon Levy wrote:
> > This moves the int pipe[2] and pthread_t main data from the
> > PCIQXLDevice struct to the SimpleSpiceDisplay. This will let us
> > reuse it in the next patch for both -spice with no -qxl usage and
> > for vga mode from qxl.
> > 
> > Also move the pipe creation function (which is effectively completely 
> > rewritten
> > by this patch anyways) from hw/qxl.c to ui/spice-display.c, since
> > spice-display will depend on it after the next patch and qemu can be build
> > with ui/spice-display.c in combination with no hw/qxl.c.
> > ---
> >  hw/qxl.c           |   22 +++++-----------------
> >  hw/qxl.h           |    4 ----
> >  ui/spice-display.c |   21 +++++++++++++++++++++
> >  ui/spice-display.h |    8 ++++++++
> >  4 files changed, 34 insertions(+), 21 deletions(-)
> > 
> > diff --git a/hw/qxl.c b/hw/qxl.c
> > index fe4212b..201698f 100644
> > --- a/hw/qxl.c
> > +++ b/hw/qxl.c
> > @@ -1062,7 +1062,7 @@ static void pipe_read(void *opaque)
> >      int len;
> >  
> >      do {
> > -        len = read(d->pipe[0], &dummy, sizeof(dummy));
> > +        len = read(d->ssd.pipe[0], &dummy, sizeof(dummy));
> >      } while (len == sizeof(dummy));
> >      qxl_set_irq(d);
> >  }
> > @@ -1078,10 +1078,11 @@ static void qxl_send_events(PCIQXLDevice *d, 
> > uint32_t events)
> >      if ((old_pending & le_events) == le_events) {
> >          return;
> >      }
> > -    if (pthread_self() == d->main) {
> > +    if (pthread_self() == d->ssd.main) {
> > +        /* running in io_thread thread */
> [snip]
> > diff --git a/ui/spice-display.h b/ui/spice-display.h
> > index aef0464..3e6cf7c 100644
> > --- a/ui/spice-display.h
> > +++ b/ui/spice-display.h
> > @@ -43,6 +43,11 @@ typedef struct SimpleSpiceDisplay {
> >      QXLRect dirty;
> >      int notify;
> >      int running;
> > +
> > +    /* thread signaling - used both in qxl (in vga mode
> > +     * and in native mode) and without qxl */
> > +    pthread_t          main;
> > +    int                pipe[2];     /* to iothread */
> >  } SimpleSpiceDisplay;
> 
> This really should be using QemuThread rather than pthread_t directly. I
> am not quite sure what impact it will have on the use of pthread_self()
> above, there might be a need for an additional wrapper there.
> 

Agreed, but it's not introduced by this patch, that code is just being moved
from qxl.h to spice-display.h, from PCIQXLDevice to SimpleSpiceDisplay (which
is embedded in PCIQXLDevice as well as used standalone when there is no qxl 
device).
I'll make a separate patch for this.

> Cheers,
> Jes