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

[Anthony Liguori <anthony@codemonkey.ws>]

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<kraxel@redhat.com>  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.

Regards,

Anthony Liguori

>
> But 1 serves a second purpose, which is to allow the guest to do io
> exits while the server thread is processing the area update request
> issued for the screendump. So it makes sense regardless.
>
> =>  Option 2.