Re: [Qemu-devel] [PATCHv5 09/12] qxl: async io support using new spice api

[Gerd Hoffmann]

  Hi,

> > > +        qxl_destroy_primary(d, async);
> > > +#if SPICE_INTERFACE_QXL_MINOR>= 1
> > > +        if (d->mode == QXL_MODE_UNDEFINED&&   async == QXL_ASYNC) {
> > > +            dprint(d, 1, "QXL_IO_DESTROY_PRIMARY_ASYNC in %s, ignored\n",
> > > +                    qxl_mode_to_string(d->mode));
> > > +            qxl_send_events(d, QXL_INTERRUPT_IO_CMD);
> > > +            goto cancel_async;
> > >           }
> >
> > Hmm?  Why this is needed?
> In this case we don't do any operation (i.e. qxl_destroy_primary is a nop,
> it checkes d->mode == QXL_MODE_UNDEFINED too) so there will never be an 
> async_complete,
> so there will never be a qxl_send_events, so we need to send it now.

Ok.  Maybe make qxl_destroy_primary return an error in case there is no 
primary to destroy, then catch this here?  Makes more clear what is 
going on here.

> > >       default:
> > >           fprintf(stderr, "%s: ioport=0x%x, abort()\n", __FUNCTION__, io_port);
> > >           abort();
> > >       }
> > > +    return;
> > > +cancel_async:
> > > +#if SPICE_INTERFACE_QXL_MINOR>= 1
> > > +    if (async) {
> > > +        qemu_mutex_lock(&d->async_lock);
> > > +        d->current_async = QXL_UNDEFINED_IO;
> > > +        qemu_mutex_unlock(&d->async_lock);
> >
> > Add "qxl_send_events(d, QXL_INTERRUPT_IO_CMD)" here?
>
> no, we want that wen the command is actually complete, on async_complete,
> where it is already done.

That is the cancel code path, i.e. the guest did a async request but we 
don't submit one to the spice-server because some sanity check failed, 
so we never get a complete callback.  Thats why I think we should raise 
the QXL_INTERRUPT_IO_CMD interrupt here instead.

cheers,
  Gerd