[Qemu-devel] Re: [RFC] qapi: events in QMP

[Kevin Wolf <kwolf@redhat.com>]

Am 13.02.2011 19:08, schrieb Anthony Liguori:
> Hi,
> 
> In my QAPI branch[1], I've now got almost every existing QMP command 
> converted with (hopefully) all of the hard problems solved.  There is 
> only one remaining thing to attack before posting for inclusion and 
> that's events.  Here's my current thinking about what to do.

I've had a quick look at your branch and I have one general question: Is
there a specific reason why you duplicate existing monitor handlers
instead of converting the existing ones? Are the old ones still used?

It would probably be much easier to review if you had a real patch that
contains the actual changes instead of just one big addition of the
duplicated code.

> Proposal for events in QAPI
> 
> For QAPI, I'd like to model events on the notion of signals and 
> slots[2].  A client would explicitly connect to a signal through a QMP 
> interface which would result in a slot being added that then generates 
> an event.  Internally in QEMU, we could also connect slots to the same 
> signals.  Since we don't have an object API in QMP, we'd use a pair of 
> connect/disconnect functions that had enough information to identify the 
> signal.
> 
> Example:
> 
> We would define QEVENT_BLOCK_IO_EVENT as:
> 
> # qmp-schema.json
> { 'BlockIOEvent': {'device': 'str', 'action': 'str', 'operation': 'str'} }
> 
> The 'Event' suffix is used to determine that the type is an event and 
> not a structure.  This would generate the following structures for QEMU:
> 
> typedef void (BlockIOEventFunc)(const char *device, const char *action, 
> const char *operation, void *opaque);

Why is an event not a structure? For one it makes things inconsistent
(you have this 'Event' suffix magic), and it's not even convenient. The
parameter list of the BlockIOEventFunc might become very long. At the
moment you have three const char* there and I think it's only going to
grow - who is supposed to remember the right order of arguments?

So why not make the event a struct and have a typedef void
(BlockIOEventFunc)(BlockIOEvent *evt)?

By the way, we're not that short on disk space. Let's call it 'string'
instead of 'str'.

> 
> typedef struct BlockIOEvent {
>      /* contents are private */
> } BlockIOEvent;
> 
> /* Connect a slot to a BlockIOEvent signal and return a handle to the 
> connection */
> int block_io_event_connect(BlockIOEvent *event, BlockIOEventFunc *cb, 
> void *opaque);
> 
> /* Signal any connect slots of a BlockIOEvent */
> void block_io_event_signal(BlockIOEvent *event, const char *device, 
> const char *action, const char *operation);
> 
> /* Disconnect a slot from a signal based on a connection handle */
> void block_io_event_disconnect(BlockIOEvent *event, int handle);
> 
> In the case of BlockIOEvent, this is a global signal that is not tied to 
> any specific object so there would be a global BlockIOEvent object 
> within QEMU.
> 
>  From a QMP perspective, we would have the following function in the schema:
> 
> [ 'block-io-event', {}, {}, 'BlockIOEvent' ]

Not sure what this list is supposed to tell me...

I see that you use the same for command, so I guess the first one is
something like an command/event name, the second seems to be the
arguments, last could be the type of the return value, the third no idea.

So would an event look like a response to a command that was never issued?

> Another Example
> 
> Here's an example of BlockDeviceEvent, the successor to BlockIOEvent.  
> It makes use of signal accessor arguments and QAPI enumeration support:

So are we going to drop BlockIOEvent (or any other event that will be
extended) or will both old and new version continue to exist and we
always signal both of them?

Kevin