From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([140.186.70.92]:39109)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <kraxel@redhat.com>) id 1QejJX-0004nN-DP
	for qemu-devel@nongnu.org; Thu, 07 Jul 2011 03:48:01 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <kraxel@redhat.com>) id 1QejJV-0001T7-BX
	for qemu-devel@nongnu.org; Thu, 07 Jul 2011 03:47:59 -0400
Received: from mx1.redhat.com ([209.132.183.28]:53676)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <kraxel@redhat.com>) id 1QejJU-0001Sz-Na
	for qemu-devel@nongnu.org; Thu, 07 Jul 2011 03:47:57 -0400
Received: from int-mx09.intmail.prod.int.phx2.redhat.com
	(int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22])
	by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p677ltJ5024862
	(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK)
	for <qemu-devel@nongnu.org>; Thu, 7 Jul 2011 03:47:55 -0400
Message-ID: <4E1564A8.60403@redhat.com>
Date: Thu, 07 Jul 2011 09:47:52 +0200
From: Gerd Hoffmann <kraxel@redhat.com>
MIME-Version: 1.0
References: <1309954766-9728-1-git-send-email-alevy@redhat.com>
	<1309954766-9728-14-git-send-email-alevy@redhat.com>
In-Reply-To: <1309954766-9728-14-git-send-email-alevy@redhat.com>
Content-Type: text/plain; charset=ISO-8859-1; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH] qxl: async I/O
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Alon Levy <alevy@redhat.com>
Cc: yhalperi@redhat.com, qemu-devel@nongnu.org

   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