Re: [Qemu-devel] [RFC 5/7] qxl-render: call ppm_save on callback

[Luiz Capitulino]

On Wed, 22 Feb 2012 17:35:15 +0100
Alon Levy <address@hidden> wrote:

> On Wed, Feb 22, 2012 at 01:55:45PM -0200, Luiz Capitulino wrote:
> > On Wed, 22 Feb 2012 15:29:33 +0100
> > Alon Levy <address@hidden> wrote:
> > 
> > > On Wed, Feb 22, 2012 at 03:22:11PM +0100, Gerd Hoffmann wrote:
> > > >   Hi,
> > > > 
> > > > > Honestly, for this particular case, I'm not 100% sure that having an 
> > > > > id is
> > > > > _required_, as I don't expect a client to submit multiple screendump 
> > > > > calls
> > > > > in parallel and we don't "officially" support multiple QMP clients 
> > > > > either.
> > > > > Also, having the screendump filename in the event will serve as a 
> > > > > form of
> > > > > identifier too.
> > > > 
> > > > That is exactly my thinking, echo the filename written in the event.
> > > > 
> > > > > Btw, are you planning to add the event to the already existing 
> > > > > screendump
> > > > > command? Adding a new command that doesn't use the monitor async API 
> > > > > and
> > > > > is truly asynchronous wouldn't better?
> > > > 
> > > > Good question.  I'd tend to just let the existing command send trigger
> > > > an event.  But libvirt needs some way to figure whenever it should wait
> > > > for an event ...
> > > 
> > > Right, that's the second reason I think a new command is needed.
> > > Additionally a new command can be implemented only by qxl and not by
> > > anything else (although I guess that would be a NACK?)
> > 
> > Is there anything specific to qlx in the command? If there's, then you
> > should also consider making this a QOM device property. The downside is
> > that QOM commands are not going to be stable for 1.1.
> 
> qxl is the only one that needs the async stuff since it needs to ask
> spice-server to do the rendering, which is done in another thread
> outside of qemu control. The rest of the screendump implementations
> afaict don't need any such complexity.

In theory, any file I/O should be asynchronous.

> The command itself would not be specific to qxl.
> 
> I thought I just need a QAPI command, and docs/ say QMP doesn't use that
> yet, so what's the connection to QMP?

Not sure I can follow you here, "that" what?