Re: [Qemu-devel] [PATCHv3] qxl-render/qxl: split out qxl_save_ppm

[Luiz Capitulino]

On Wed, 13 Jul 2011 15:56:55 +0300
Alon Levy <address@hidden> wrote:

> On Wed, Jul 13, 2011 at 09:33:26AM -0300, Luiz Capitulino wrote:
> > On Wed, 13 Jul 2011 14:29:16 +0300
> > Alon Levy <address@hidden> wrote:
> > 
> > > On Wed, Jul 13, 2011 at 12:41:48PM +0200, Gerd Hoffmann wrote:
> > > > On 07/13/11 11:29, Alon Levy wrote:
> > > > >On Wed, Jul 13, 2011 at 09:10:19AM +0200, Gerd Hoffmann wrote:
> > > > >>On 07/12/11 15:55, Alon Levy wrote:
> > > > >>>Later the save will happen asynchronously on surface_updated 
> > > > >>>callback.
> > > > >>
> > > > >>Hmm.  I can see why you are doing that.  It makes the file being
> > > > >>written *after* the monitor command finishes though, which I think
> > > > >>we should avoid.
> > > > >
> > > > >I think the simplest thing would be to add a specific cond for this - 
> > > > >ppm_save_filename_cond. ok?
> > > > 
> > > > Not sure.  Luiz, do we have async monitor commands meanwhile?
> > > > 
> > > > Background: screendump for qxl vga can take a while as the
> > > > spice-server might have to render everything first ...
> > > 
> > > I'd rather try the MONITOR_CMD_ASYNC thing then the cond variable, it's 
> > > becoming pretty
> > > ugly.
> > 
> > IIRC, that interface doesn't work as expected and is going to be replaced
> > by the QAPI's one.
> 
> In what way is in wrong? it seems to work fine here - monitor is blocked until
> I call the callback, after which it returns. Didn't test the qmp part though.

One problem is that the error is global and could be overwritten by other
handlers. Another problem is that there's no way for a client to kill an async
handler that got stuck, this could cause a client to wait for the handler
forever.

> 
> > 
> > > Also I guess what Daniel described is possible, but it changes the usage 
> > > of screendump
> > > even more. Is turning do_screen_dump to async viable? I think I'll work 
> > > on it.
> > > 
> > > > 
> > > > cheers,
> > > >   Gerd
> > > > 
> > > 
> > 
>