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

[Alon Levy]

On Fri, Jul 15, 2011 at 10:12:14AM +0200, Gerd Hoffmann wrote:
> >      case QXL_IO_DESTROY_PRIMARY:
> >          if (val != 0) {
> >-            qxl_guest_bug(d, "QXL_IO_DESTROY_PRIMARY: val != 0");
> >-            break;
> >+            qxl_guest_bug(d, "QXL_IO_DESTROY_PRIMARY (async=%d): val != 0",
> >+                          async);
> >+            goto cancel_async;
> >+        }
> >+        dprint(d, 1, "QXL_IO_DESTROY_PRIMARY (async=%d) (%s)\n", async,
> >+               qxl_mode_to_string(d->mode));
> >+        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.

> 
> >      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.

> 
> >  typedef struct PCIQXLDevice {
> >      PCIDevice          pci;
> >      SimpleSpiceDisplay ssd;
> >@@ -30,6 +32,11 @@ typedef struct PCIQXLDevice {
> >      int32_t            num_memslots;
> >      int32_t            num_surfaces;
> >
> >+#if SPICE_INTERFACE_QXL_MINOR>= 1
> >+    uint32_t           current_async;
> >+    QemuMutex          async_lock;
> >+#endif
> 
> No need to ifdef this.
> 

missed it.

> cheers,
>   Gerd