[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 1/2] console: add hw_screen_dump_async
From: |
Alon Levy |
Subject: |
Re: [Qemu-devel] [PATCH v2 1/2] console: add hw_screen_dump_async |
Date: |
Tue, 6 Mar 2012 18:26:58 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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<address@hidden> 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<vm-name> /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
.
>
> Regards,
>
> Anthony Liguori
>
- Re: [Qemu-devel] [PATCH v2 1/2] console: add hw_screen_dump_async, (continued)
- Re: [Qemu-devel] [PATCH v2 1/2] console: add hw_screen_dump_async, Alon Levy, 2012/03/06
- Re: [Qemu-devel] [PATCH v2 1/2] console: add hw_screen_dump_async, Luiz Capitulino, 2012/03/06
- Re: [Qemu-devel] [PATCH v2 1/2] console: add hw_screen_dump_async, Alon Levy, 2012/03/06
- Re: [Qemu-devel] [PATCH v2 1/2] console: add hw_screen_dump_async, Anthony Liguori, 2012/03/06
- Re: [Qemu-devel] [PATCH v2 1/2] console: add hw_screen_dump_async, Luiz Capitulino, 2012/03/06
- Re: [Qemu-devel] [PATCH v2 1/2] console: add hw_screen_dump_async, Alon Levy, 2012/03/06
- Re: [Qemu-devel] [PATCH v2 1/2] console: add hw_screen_dump_async, Anthony Liguori, 2012/03/06
- Re: [Qemu-devel] [PATCH v2 1/2] console: add hw_screen_dump_async, Alon Levy, 2012/03/06
- Re: [Qemu-devel] [PATCH v2 1/2] console: add hw_screen_dump_async, Alon Levy, 2012/03/06
- Re: [Qemu-devel] [PATCH v2 1/2] console: add hw_screen_dump_async, Anthony Liguori, 2012/03/06
- Re: [Qemu-devel] [PATCH v2 1/2] console: add hw_screen_dump_async,
Alon Levy <=
- Re: [Qemu-devel] [PATCH v2 1/2] console: add hw_screen_dump_async, Anthony Liguori, 2012/03/06
- Re: [Qemu-devel] [PATCH v2 1/2] console: add hw_screen_dump_async, Gerd Hoffmann, 2012/03/07