From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:59568) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S4uFu-0000gl-K0 for qemu-devel@nongnu.org; Tue, 06 Mar 2012 08:16:47 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S4uFp-0007r2-N4 for qemu-devel@nongnu.org; Tue, 06 Mar 2012 08:16:42 -0500 Received: from mx1.redhat.com ([209.132.183.28]:41540) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S4uFp-0007qj-FG for qemu-devel@nongnu.org; Tue, 06 Mar 2012 08:16:37 -0500 Date: Tue, 6 Mar 2012 15:16:24 +0200 From: Alon Levy Message-ID: <20120306131624.GC2240@garlic.redhat.com> References: <1330956981-5001-1-git-send-email-alevy@redhat.com> <4F54CEA2.10808@codemonkey.ws> <4F54F5F8.70105@redhat.com> <4F54F76B.70403@codemonkey.ws> <20120305143142.1a1a53e2@doriath.home> <20120305180958.GC20937@garlic.tlv.redhat.com> <4F550350.7030703@redhat.com> <4F55BE82.3030205@redhat.com> <20120306092427.35103975@doriath.home> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20120306092427.35103975@doriath.home> Subject: Re: [Qemu-devel] [PATCH v2 1/2] console: add hw_screen_dump_async List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Luiz Capitulino Cc: qemu-devel@nongnu.org, Gerd Hoffmann , Anthony Liguori , Avi Kivity On Tue, Mar 06, 2012 at 09:24:27AM -0300, Luiz Capitulino wrote: > On Tue, 06 Mar 2012 08:36:34 +0100 > Gerd Hoffmann wrote: > > > Hi, > > > > >> How would the parallel execution facility be opaque to the implementer? > > >> screendump returns, screendump_async needs to pass a closure. You can > > >> automatically generate any amount of code, but you can only have a > > >> single function implementation with longjmp/coroutine, or having a > > >> saparate thread per command but that would mean taking locks for > > >> anything not trivial, which avoids the no-change-to-implementation. Is > > >> this what you have in mind? > > > > > > It would not be opaque to the implementer. But it would avoid > > > introducing new commands and events, instead we have a unified mechanism > > > to signal completion. > > > > Ok. We have a async mechanism today: .mhandler.cmd_async = ... > > > > I know it has its problems like no cancelation and is deprecated and > > all. But still: how about using it as interim until QAPI-based async > > monitor support is ready? We could unbreak qxl screendumps without > > having to introduce a new (but temporary!) screendump_async command + > > completion event. > > There are a few problems here, but the blocking one is that a command > can't go from sync to async. This is an incompatible change. > > If you mind adding the temporary command and if this issue is so rare > that none can reproduce it, then I'd suggest to wait for 1.2. > There are two options really: 1. revert the patches that changed qxl screendump to save the ppm before (possibly) updating the framebuffer. 2. introduce a new command that is really async The third option, what Gerd proposes, doesn't break the blocking chain going from the A, the dual purpose spice client and libvirt client, through libvirt, qemu, spice and back to A. If no one can reproduce the block then it would seem 1 makes sense. But 1 serves a second purpose, which is to allow the guest to do io exits while the server thread is processing the area update request issued for the screendump. So it makes sense regardless. => Option 2.