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

[Alon Levy]

On Wed, Feb 22, 2012 at 11:49:27AM -0200, Luiz Capitulino wrote:
> On Wed, 22 Feb 2012 14:22:50 +0100
> Alon Levy <address@hidden> wrote:
> 
> > On Wed, Feb 22, 2012 at 11:17:17AM -0200, Luiz Capitulino wrote:
> > > On Tue, 21 Feb 2012 19:40:16 +0200
> > > Alon Levy <address@hidden> wrote:
> > > 
> > > > On Tue, Feb 21, 2012 at 09:15:45AM -0700, Eric Blake wrote:
> > > > > On 02/21/2012 01:19 AM, Alon Levy wrote:
> > > > > 
> > > > > >>>  (2) Async monitor command.  Keeps interface and works nicely.  A 
> > > > > >>> bunch
> > > > > >>>      of QAPI bits tickled into master meanwhile, so we could look 
> > > > > >>> at
> > > > > >>>      this again.  Luiz?  What is the status here?
> > > 
> > > The qapi infra is already in place for sometime now, but it doesn't 
> > > support
> > > async commands yet. However, we're accepting a combination of command + 
> > > async
> > > event on completion for commands that have to be async.
> > > 
> > > We'll eventually have a good interface for async support, but it's not 
> > > likely
> > > we'll have it for 1.1 (possible, but unlikely).
> > > 
> > > I think item 2 above is a good way to go, considering it will add a new 
> > > command,
> > > of course.
> > > 
> > 
> > Ok, so that sounds good: I'll add an event, and later libvirt/autotest
> > can use it. But in that case I'll need to introduce a connection between
> > the command and the event, some id. That id will have to be generated by
> > the command issuer, so a new command with event id + complete event?
> 
> That's a good question.
> 
> The only events we have today which are actually a response to an asynchronous
> command are the block streaming API ones and they don't include an id.
> 
> 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.
> 
> 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?

I was thinking to add a new command since I'll want to add the id, and
if I'm already adding a new command I'll put in a display number too.