Re: [Qemu-devel] [PATCH 4/4] add qmp screendump-async

[Alon Levy <alevy@redhat.com>]

On Tue, Feb 28, 2012 at 05:10:39PM -0300, Luiz Capitulino wrote:
> On Sat, 25 Feb 2012 10:46:07 +0200
> Alon Levy <alevy@redhat.com> wrote:
> 
> > On Fri, Feb 24, 2012 at 04:40:15PM -0600, Anthony Liguori wrote:
> > > On 02/24/2012 03:22 PM, Alon Levy wrote:
> > > >This is an across the board change since I wanted to keep the existing
> > > >(good imo) single graphic_console_init callback setter, instead of
> > > >introducing a new cb that isn't set by it but instead by a second
> > > >initialization function.
> > > >
> > > >Signed-off-by: Alon Levy<alevy@redhat.com>
> > > 
> > > What's the rationale for this?
> > 
> > There is a hang possible with the current screendump command, qxl, a
> > spice client using libvirt and spice-gtk such as virt-viewer /
> > remote-viewer, where you have:
> > 1. libvirt waiting for screendump to complete
> > 2. screendump waiting for spice server thread to render
> > 3. spice server thread waiting for spice client to read messages
> 
> Which messages?

spice display channel messages.

> 
> > 4. spice client == libvirt client, waiting for screendump completion
> 
> The way I had understood this problem is that qxl takes a long time to
> perform a screen dump, which would cause the global mutex to be held for
> a long time. If this is really serious, then a async command for it
> makes sense IMO.

That is true, but it is not the immediate problem the bz is dealing with
- if it was only this there would be no hang.

> 
> Now, the problem you describe above is different and maybe it shouldn't
> be solved at QMP level. Can you describe it in more detail?
> 

I can just point to the diagram I put into the mail from october:

"""
This fixes a hang that is caused when the spice client is also a qemu
monitor client and the client is single threaded:


  io thread                   worker thread                client

   <---------------------------------------------------- screendump
  do_screen_dump-> read------->
                               flush, read
                               client socket------------->
"""

I have not managed to reproduce this myself, so I'm cc'ing Marc-Andre
who has and reported the fix of using an asyncronous operation
internally (unrelated to the asynchronous qevent in this patchset)
between the monitor thread and the spice thread fixed the problem.

> > > I really don't want a plethora of pseudo-async commands.
> 
> Agreed, but this is not supposed to be pseudo-async. If a command returns
> immediately and sends an event when it later completes, it's really async.
Thanks for saying this. I didn't understand the pseudo-async angle.