From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:35099) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S4unl-0000Gr-E1 for qemu-devel@nongnu.org; Tue, 06 Mar 2012 08:51:50 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S4ung-0006Zw-IA for qemu-devel@nongnu.org; Tue, 06 Mar 2012 08:51:40 -0500 Received: from mail-pz0-f46.google.com ([209.85.210.46]:37380) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S4ung-0006ZY-Bh for qemu-devel@nongnu.org; Tue, 06 Mar 2012 08:51:36 -0500 Received: by dajr28 with SMTP id r28so8030443daj.33 for ; Tue, 06 Mar 2012 05:51:34 -0800 (PST) Message-ID: <4F561661.7080805@codemonkey.ws> Date: Tue, 06 Mar 2012 07:51:29 -0600 From: Anthony Liguori MIME-Version: 1.0 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> <20120306131624.GC2240@garlic.redhat.com> In-Reply-To: <20120306131624.GC2240@garlic.redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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 , Gerd Hoffmann , Avi Kivity , qemu-devel@nongnu.org On 03/06/2012 07:16 AM, Alon Levy wrote: > 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. So let's start with a reproducible test case that demonstrates the problem before we start introducing new commands then if there's doubt about the nature of the problem. Regards, Anthony Liguori > > 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.