Re: [Qemu-devel] [PATCH] qxl: async I/O

[Gerd Hoffmann]

  Hi,

> +void qxl_spice_update_area_async(PCIQXLDevice *qxl, uint32_t surface_id,
> +                           struct QXLRect *area, struct QXLRect *dirty_rects,
> +                           uint32_t num_dirty_rects, uint32_t 
> clear_dirty_region,
> +                           int async)
> +{
> +    qemu_mutex_lock(&qxl->ssd.wlock);
> +    if (async) {
> +        qxl->ssd.worker->update_area_async(qxl->ssd.worker, surface_id, area, 
> dirty_rects,
> +                                 num_dirty_rects, clear_dirty_region);
> +    } else {
> +        qxl->ssd.worker->update_area(qxl->ssd.worker, surface_id, area, 
> dirty_rects,
> +                                 num_dirty_rects, clear_dirty_region);
> +    }
> +    qemu_mutex_unlock(&qxl->ssd.wlock);
> +}

We need a plan to handle backward compatibility here.  Older 
libspice-server versions don't have the update_area_async op.  Option 
one is to just not support async mode with older libraries.  Option two 
is to handle the request syncronously even though the guest has asked 
for async.  I'd tend to pick option one, that makes things easier with 
S3 support because we just can't do that in any way with an older 
libspice-server.

>       switch (io_port) {
> +    case QXL_IO_UPDATE_AREA_ASYNC:
> +    case QXL_IO_NOTIFY_OOM_ASYNC:
> +    case QXL_IO_MEMSLOT_ADD_ASYNC:
> +    case QXL_IO_CREATE_PRIMARY_ASYNC:
> +    case QXL_IO_DESTROY_PRIMARY_ASYNC:
> +    case QXL_IO_DESTROY_SURFACE_ASYNC:
> +    case QXL_IO_DESTROY_ALL_SURFACES_ASYNC:
> +        async = 1;
> +        if (d->current_async != QXL_UNDEFINED_IO) {
> +            qxl_guest_bug(d, "%d async started before last (%d) complete\n",
> +                io_port, d->current_async);

Better return here, ignoring the invalid request?

cheers,
  Gerd