Re: [Qemu-devel] [PATCH v2 1/2] console: add hw_screen_dump_async

[Luiz Capitulino]

On Tue, 06 Mar 2012 07:51:29 -0600
Anthony Liguori <address@hidden> 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.

Completely agree, I was going to suggest that too.