qemu-devel
[Top][All Lists]
Advanced

[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: Anthony Liguori
Subject: Re: [Qemu-devel] [PATCH v2 1/2] console: add hw_screen_dump_async
Date: Tue, 06 Mar 2012 10:16:42 -0600
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.23) Gecko/20110922 Lightning/1.0b2 Thunderbird/3.1.15

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.

Regards,

Anthony Liguori



reply via email to

[Prev in Thread] Current Thread [Next in Thread]