From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:38402) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S4xXl-0008Ge-G2 for qemu-devel@nongnu.org; Tue, 06 Mar 2012 11:47:28 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S4xXa-0001Ry-3x for qemu-devel@nongnu.org; Tue, 06 Mar 2012 11:47:21 -0500 Received: from mail-pz0-f46.google.com ([209.85.210.46]:59359) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S4xXZ-0001RL-QQ for qemu-devel@nongnu.org; Tue, 06 Mar 2012 11:47:10 -0500 Received: by dajr28 with SMTP id r28so8209328daj.33 for ; Tue, 06 Mar 2012 08:47:07 -0800 (PST) Message-ID: <4F563F87.7030004@codemonkey.ws> Date: Tue, 06 Mar 2012 10:47:03 -0600 From: Anthony Liguori MIME-Version: 1.0 References: <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> <4F56386A.8040700@codemonkey.ws> <20120306162658.GK2240@garlic.redhat.com> In-Reply-To: <20120306162658.GK2240@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 10:26 AM, Alon Levy wrote: > On Tue, Mar 06, 2012 at 10:16:42AM -0600, Anthony Liguori wrote: >> 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. > > What are you saying, that that read should be fixed? then I guess it's > good that patches fixing it have landed. That stack was prior to "qxl: > make qxl_render_update async", 81fb6f1504fb9ef71f2382f44af34756668296e8 I'm sorry, I'm thoroughly confused by this thread. Can you start a new thread with a clear explanation of the problem you're trying to solve or at least point me to an existing note? Regards, Anthony Liguori > . > >> >> Regards, >> >> Anthony Liguori >>