From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:41853) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S4x4F-00038m-I0 for qemu-devel@nongnu.org; Tue, 06 Mar 2012 11:16:53 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S4x4D-0002ph-GU for qemu-devel@nongnu.org; Tue, 06 Mar 2012 11:16:51 -0500 Received: from mail-pw0-f45.google.com ([209.85.160.45]:34194) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S4x4D-0002pI-BA for qemu-devel@nongnu.org; Tue, 06 Mar 2012 11:16:49 -0500 Received: by pbcuo5 with SMTP id uo5so5954890pbc.4 for ; Tue, 06 Mar 2012 08:16:47 -0800 (PST) Message-ID: <4F56386A.8040700@codemonkey.ws> Date: Tue, 06 Mar 2012 10:16:42 -0600 From: Anthony Liguori MIME-Version: 1.0 References: <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> <4F561661.7080805@codemonkey.ws> <20120306155649.GF2240@garlic.redhat.com> In-Reply-To: <20120306155649.GF2240@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 09:56 AM, Alon Levy wrote: > On Tue, Mar 06, 2012 at 07:51:29AM -0600, Anthony Liguori wrote: >> 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. > > s/the problem/different problem/: > > NB: To get this hang I had to disable update_area's from the guest, just > because otherwise there is a very small window between suspending the > client and qemu waiting on the same stack trace below issued from the > guest update_area io out, instead of from the screendump monitor command. > > spice client, qemu, libvirt, virsh screenshot. > > libvirt controlled qemu with qxl device and spice connection. > qemu ... -vga qxl -spice disable-ticketing,port=5900... > spicec -h localhost -p 5900 > [boot until qxl driver is loaded, and guest is doing something (Running > toms 2d benchmark works), suspend spicec] > virsh screenshot /tmp/dump.ppm > > virsh will hang at: > #1 virCondWait > #2 qemuMonitorSend > #3 qemuMonitorJSONCommandWithFd > #4 qemuMonitorJSONCommand > #5 qemuMonitorJSONScreendump > > qemu is hung at: > main thread: > #0 read Is qxl doing a blocking read? If so, that's a bug in qxl. You cannot do a blocking read while holding qemu_mutex. Regards, Anthony Liguori