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

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

[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.

Events in QMP Today

QMP events are asynchronous messages.  They are not tied explicitly to 
any type of context, do not have a well defined format, and are have no 
mechanism to mask or filter events.  As of right now, we have somewhere 
around a dozen events.

Goals of QAPI

1) Make all interfaces consumable in C such that we can use the 
interfaces in QEMU

2) Make all interfaces exposed through a library using code generation 
from static introspection

3) Make all interfaces well specified in a formal schema

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);

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' ]

A function definition that returns an Event is a signal accessor.  
Within QEMU, we implement a signal accessor like so:

BlockIOEvent *qmp_block_io_event(Error **errp)
{
      return &global_block_io_event;
}

For QMP and libqmp, signal accessors are handled specially.  A signal 
accessor is expanded into two QMP functions:

[ 'block-io-event-connect', {}, {}, 'str' ]
[ 'block-io-event-disconnect', {'tag' : 'str'}, {}, 'none' ]

Connect returns a handle to the connection as an opaque string.  
Disconnect takes that handle.  Both functions will also take any 
arguments that the signal accessor takes and under the cover uses the 
signal accessor to access the object.

In libqmp, the following interfaces will be generated:

int libqmp_block_io_event_connect(QmpSession *sess, BlockIOEventFunc 
*cb, void *opaque, Error **errp);
void libqmp_block_io_event_disconnect(QmpSession *sess, int handle, 
Error **errp);

Again, if the signal accessor takes additional arguments, they will be 
passed here.

All existing events will be converted to signals with signal accessors 
that take no arguments.  However, BlockIOEvent is a good example of a 
signal that we'll add a new version of that takes a 'const char *device' 
as an argument for the signal accessor.  This will let clients register 
for block io events on specific block devices.

Backwards Compatibility

Right now, QMP events to not have any type of context information which 
means that there's no way to communicate the connection handle.  We'll 
solve this by adding a new 'tag' field to events that are registered 
through one of the connect interfaces.  Since this is only present on 
events registered through this new interface, backwards compatibility is 
preserved.

Upon initial connection, we'll treat all existing signals as having a 
'default signal connection'.  We'll introduce a new QMP command that can 
be executed while in capabilities mode to disconnect default signal 
connections.  The effect is that new clients can use the explicit 
register/unregister interface while old clients will continue to see the 
old style events.

We'll deprecate the 'default signal connections' and make sure to never 
add a new one post 0.15.  Old clients will need to be updated to 
explicitly register for events.

Another Example

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

# qmp-schema.json
{ 'BlockDeviceAction': [ 'report', 'stop', 'none' ] }
{ 'BlockDeviceOperation': [ 'read', 'write' ] }
{ 'BlockDeviceActionReason': [ 'nospc', 'io', 'other' ] }
{ 'BlockDeviceEvent': { 'action': 'BlockDeviceAction', 'operation': 
'BlockDeviceOperation', 'reason': 'BlockDeviceActionReason' } }

[ 'block-device-event', { 'device': 'str' }, {}, 'BlockDeviceEvent' ]

# qemu/block.c (Signal Accessor)
BlockDeviceEvent *qmp_block_device_event(const char *str, Error **errp)'

# qemu/qmp-types.h (Generated QMP interfaces)
typedef enum BlockDeviceAction {
     BDA_REPORT = 0,
     BDA_STOP,
     BDA_NONE,
} BlockDeviceAction;

typedef enum BlockDeviceOperation {
     BDO_READ = 0,
     BDO_WRITE,
} BlockDeviceOperation;

typedef enum BlockDeviceActionReason {
    BDAR_NOSPC = 0,
    BDAR_IO,
    BDAR_OTHER,
} BlockDeviceActionReason;

typedef struct BlockDeviceEvent {
     /* private */
} BlockDeviceEvent;

typedef void (BlockDeviceEventFunc)(BlockDeviceAction action, 
BlockDeviceOperation operator, BlockDeviceActionReason reason);

int block_device_event_connect(BlockDeviceEvent *event, 
BlockDeviceEventFunc *cb, void *opaque);
void block_device_event_disconnect(BlockDeviceEvent *event, int handle);

# qemu/libqmp.h (Generated libqmp interfaces)

int libqmp_block_device_event_connect(QmpSession *sess, const char 
*device, BlockDeviceEventFunc *cb, void *opaque);

void libqmp_block_device_event_disconnect(QmpSession *sess, const char 
*device, int handle);

Regards,

Anthony Liguori

[1] http://repo.or.cz/w/qemu/aliguori.git/shortlog/refs/heads/glib

[2] http://en.wikipedia.org/wiki/Signals_and_slots

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

[Anthony Liguori]

On 02/13/2011 12:08 PM, Anthony Liguori wrote:
> 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.
>
> Events in QMP Today
>
> QMP events are asynchronous messages.  They are not tied explicitly to 
> any type of context, do not have a well defined format, and are have 
> no mechanism to mask or filter events.  As of right now, we have 
> somewhere around a dozen events.
>
> Goals of QAPI
>
> 1) Make all interfaces consumable in C such that we can use the 
> interfaces in QEMU
>
> 2) Make all interfaces exposed through a library using code generation 
> from static introspection
>
> 3) Make all interfaces well specified in a formal schema
>
> 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);

One thing I'm on the fence about, is making the slot callback take a 
BlockIOEvent *event as the first argument.  If we did this, the signals 
would be compatible with GObject signals.

It's hard to map that to a libqmp though so I'm not sure.

Regards,

Anthony Liguori

>
> 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' ]
>
> A function definition that returns an Event is a signal accessor.  
> Within QEMU, we implement a signal accessor like so:
>
> BlockIOEvent *qmp_block_io_event(Error **errp)
> {
>      return &global_block_io_event;
> }
>
> For QMP and libqmp, signal accessors are handled specially.  A signal 
> accessor is expanded into two QMP functions:
>
> [ 'block-io-event-connect', {}, {}, 'str' ]
> [ 'block-io-event-disconnect', {'tag' : 'str'}, {}, 'none' ]
>
> Connect returns a handle to the connection as an opaque string.  
> Disconnect takes that handle.  Both functions will also take any 
> arguments that the signal accessor takes and under the cover uses the 
> signal accessor to access the object.
>
> In libqmp, the following interfaces will be generated:
>
> int libqmp_block_io_event_connect(QmpSession *sess, BlockIOEventFunc 
> *cb, void *opaque, Error **errp);
> void libqmp_block_io_event_disconnect(QmpSession *sess, int handle, 
> Error **errp);
>
> Again, if the signal accessor takes additional arguments, they will be 
> passed here.
>
> All existing events will be converted to signals with signal accessors 
> that take no arguments.  However, BlockIOEvent is a good example of a 
> signal that we'll add a new version of that takes a 'const char 
> *device' as an argument for the signal accessor.  This will let 
> clients register for block io events on specific block devices.
>
> Backwards Compatibility
>
> Right now, QMP events to not have any type of context information 
> which means that there's no way to communicate the connection handle.  
> We'll solve this by adding a new 'tag' field to events that are 
> registered through one of the connect interfaces.  Since this is only 
> present on events registered through this new interface, backwards 
> compatibility is preserved.
>
> Upon initial connection, we'll treat all existing signals as having a 
> 'default signal connection'.  We'll introduce a new QMP command that 
> can be executed while in capabilities mode to disconnect default 
> signal connections.  The effect is that new clients can use the 
> explicit register/unregister interface while old clients will continue 
> to see the old style events.
>
> We'll deprecate the 'default signal connections' and make sure to 
> never add a new one post 0.15.  Old clients will need to be updated to 
> explicitly register for events.
>
> Another Example
>
> Here's an example of BlockDeviceEvent, the successor to BlockIOEvent.  
> It makes use of signal accessor arguments and QAPI enumeration support:
>
> # qmp-schema.json
> { 'BlockDeviceAction': [ 'report', 'stop', 'none' ] }
> { 'BlockDeviceOperation': [ 'read', 'write' ] }
> { 'BlockDeviceActionReason': [ 'nospc', 'io', 'other' ] }
> { 'BlockDeviceEvent': { 'action': 'BlockDeviceAction', 'operation': 
> 'BlockDeviceOperation', 'reason': 'BlockDeviceActionReason' } }
>
> [ 'block-device-event', { 'device': 'str' }, {}, 'BlockDeviceEvent' ]
>
> # qemu/block.c (Signal Accessor)
> BlockDeviceEvent *qmp_block_device_event(const char *str, Error **errp)'
>
> # qemu/qmp-types.h (Generated QMP interfaces)
> typedef enum BlockDeviceAction {
>     BDA_REPORT = 0,
>     BDA_STOP,
>     BDA_NONE,
> } BlockDeviceAction;
>
> typedef enum BlockDeviceOperation {
>     BDO_READ = 0,
>     BDO_WRITE,
> } BlockDeviceOperation;
>
> typedef enum BlockDeviceActionReason {
>    BDAR_NOSPC = 0,
>    BDAR_IO,
>    BDAR_OTHER,
> } BlockDeviceActionReason;
>
> typedef struct BlockDeviceEvent {
>     /* private */
> } BlockDeviceEvent;
>
> typedef void (BlockDeviceEventFunc)(BlockDeviceAction action, 
> BlockDeviceOperation operator, BlockDeviceActionReason reason);
>
> int block_device_event_connect(BlockDeviceEvent *event, 
> BlockDeviceEventFunc *cb, void *opaque);
> void block_device_event_disconnect(BlockDeviceEvent *event, int handle);
>
> # qemu/libqmp.h (Generated libqmp interfaces)
>
> int libqmp_block_device_event_connect(QmpSession *sess, const char 
> *device, BlockDeviceEventFunc *cb, void *opaque);
>
> void libqmp_block_device_event_disconnect(QmpSession *sess, const char 
> *device, int handle);
>
> Regards,
>
> Anthony Liguori
>
> [1] http://repo.or.cz/w/qemu/aliguori.git/shortlog/refs/heads/glib
>
> [2] http://en.wikipedia.org/wiki/Signals_and_slots
>
>

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

[Kevin Wolf]

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

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

[Anthony Liguori]

On 02/14/2011 03:50 AM, Kevin Wolf wrote:
> 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's just temporary.  It makes it very easy to test code side-by-side.   
By the time I submit, I'll remove the old 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)?
>    

A signal is a function call.  You can pass a structure as a parameter is 
you so desire but the natural thing to do is pass position arguments.

If you've got a ton of signal arguments, it's probably an indication 
that you're doing something wrong.

>> 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.'
>    

Sorry, first is the name of the function, second is required arguments, 
third is optional arguments, last is return value.

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

 From a protocol perspective, events look like this today:

{ "event": "BlockIOError", "data": { "device": "ide1-cd0", "operation": 
"read", "action": "report" } }

The only change to the protocol I'm proposing is adding a "tag" field 
which would give us:

{ "event": "BlockIOError", tag: "event023234", "data": { "device": 
"ide1-cd0", "operation": "read", "action": "report" } }

>> 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?
>    

Both have to exist.  We can't drop old events without breaking backwards 
compatibility.

Regards,

Anthony Liguori

> Kevin
>
>    

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

[Kevin Wolf]

Am 14.02.2011 13:03, schrieb Anthony Liguori:
> On 02/14/2011 03:50 AM, Kevin Wolf wrote:
>> Am 13.02.2011 19:08, schrieb Anthony Liguori:
>>> 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)?
>>    
> 
> A signal is a function call.  You can pass a structure as a parameter is 
> you so desire but the natural thing to do is pass position arguments.
> 
> If you've got a ton of signal arguments, it's probably an indication 
> that you're doing something wrong.

Yes. For example, you're taking tons of independent arguments for
something that is logically a single entity, namely a block error event. ;-)

I'm almost sure that we'll want to add more things to this specific
event, for example a more detailed error description (Luiz once
suggested using errno here, but seems it hasn't made its way into
upstream). And I would be surprised if we never wanted to add more
information to other events, too.

>>> 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.'
>>    
> 
> Sorry, first is the name of the function, second is required arguments, 
> third is optional arguments, last is return value.
> 
>> So would an event look like a response to a command that was never issued?
>>    
> 
>  From a protocol perspective, events look like this today:
> 
> { "event": "BlockIOError", "data": { "device": "ide1-cd0", "operation": 
> "read", "action": "report" } }
> 
> The only change to the protocol I'm proposing is adding a "tag" field 
> which would give us:
> 
> { "event": "BlockIOError", tag: "event023234", "data": { "device": 
> "ide1-cd0", "operation": "read", "action": "report" } }

Right, I was more referring to this schema thing. I didn't quite
understand yet if/why functions and events are the same thing from that
perspective.

They seem to be some kind of function that is called from within qemu,
gets its arguments from the event_connect call and returns its return
value to the QMP client.

Is that about right?

>>> 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?
> 
> Both have to exist.  We can't drop old events without breaking backwards 
> compatibility.

Right. So a second event doesn't sound like something we'd love to
have... Maybe extending the existing ones with optional arguments would
be better?

Kevin

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

[Luiz Capitulino]

On Mon, 14 Feb 2011 13:32:45 +0100
Kevin Wolf <kwolf@redhat.com> wrote:

> Am 14.02.2011 13:03, schrieb Anthony Liguori:
> > On 02/14/2011 03:50 AM, Kevin Wolf wrote:
> >> Am 13.02.2011 19:08, schrieb Anthony Liguori:
> >>> 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)?
> >>    
> > 
> > A signal is a function call.  You can pass a structure as a parameter is 
> > you so desire but the natural thing to do is pass position arguments.
> > 
> > If you've got a ton of signal arguments, it's probably an indication 
> > that you're doing something wrong.
> 
> Yes. For example, you're taking tons of independent arguments for
> something that is logically a single entity, namely a block error event. ;-)
> 
> I'm almost sure that we'll want to add more things to this specific
> event, for example a more detailed error description (Luiz once
> suggested using errno here, but seems it hasn't made its way into
> upstream). And I would be surprised if we never wanted to add more
> information to other events, too.

So the question is: how does the schema based design support extending
commands or events? Does it require adding new commands/events?

While the current code is in really in bad shape currently, I'm not sure that
having this disadvantage will pay off the new design.

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

[Luiz Capitulino]

On Sun, 13 Feb 2011 12:08:04 -0600
Anthony Liguori <anthony@codemonkey.ws> wrote:

> 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.
> 
> Events in QMP Today
> 
> QMP events are asynchronous messages.  They are not tied explicitly to 
> any type of context, do not have a well defined format, and are have no 
> mechanism to mask or filter events.  As of right now, we have somewhere 
> around a dozen events.
> 
> Goals of QAPI
> 
> 1) Make all interfaces consumable in C such that we can use the 
> interfaces in QEMU
> 
> 2) Make all interfaces exposed through a library using code generation 
> from static introspection
> 
> 3) Make all interfaces well specified in a formal schema
> 
> 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.

This seems to be the right way to do this in C, but I wonder if it will
get complex/bloated to require this on the wire protocol.

In the initial discussions on QMP events, we decided that it was
simpler/easier to just send all events and let the client do the masking if it
wants to. Later on, Daniel commented that it would be useful to able to mask
events early on.. Now, this proposal will require registration.

We don't seem to make our mind..

Daniel, what do you think?

> Example:
> 
> We would define QEVENT_BLOCK_IO_EVENT as:

I won't comment on the code right now, I want to read it in detail in your
branch, so that I can do a better review. Will try to do it in the next
few days.

My only immediate concern is that, as I commented on the other email, this
new model will require us to add new commands/events when extending existing
commands/events, right?

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

[Daniel P. Berrange]

On Mon, Feb 14, 2011 at 11:28:52AM -0200, Luiz Capitulino wrote:
> On Sun, 13 Feb 2011 12:08:04 -0600
> Anthony Liguori <anthony@codemonkey.ws> wrote:
> 
> > 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.
> > 
> > Events in QMP Today
> > 
> > QMP events are asynchronous messages.  They are not tied explicitly to 
> > any type of context, do not have a well defined format, and are have no 
> > mechanism to mask or filter events.  As of right now, we have somewhere 
> > around a dozen events.
> > 
> > Goals of QAPI
> > 
> > 1) Make all interfaces consumable in C such that we can use the 
> > interfaces in QEMU
> > 
> > 2) Make all interfaces exposed through a library using code generation 
> > from static introspection
> > 
> > 3) Make all interfaces well specified in a formal schema
> > 
> > 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.
> 
> This seems to be the right way to do this in C, but I wonder if it will
> get complex/bloated to require this on the wire protocol.
> 
> In the initial discussions on QMP events, we decided that it was
> simpler/easier to just send all events and let the client do the masking if it
> wants to. Later on, Daniel commented that it would be useful to able to mask
> events early on.. Now, this proposal will require registration.
> 
> We don't seem to make our mind..
> 
> Daniel, what do you think?

I've always thought it was bad to have all events enabled all
the time, because it is not scalable as the number & frequency
of events increases. Requiring the app register for each event
it cares about is more scalable & also allows the app to discover
if a particular event is actually supported by that QEMU version.
eg The initial 'qmp_capabilities' greeting from the client could
include the list of all desired event names from the client,
and the server could reply with the list of those event it can
actually support & has enabled.


Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|

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

[Anthony Liguori]

On 02/14/2011 07:33 AM, Daniel P. Berrange wrote:
> On Mon, Feb 14, 2011 at 11:28:52AM -0200, Luiz Capitulino wrote:
>    
>> On Sun, 13 Feb 2011 12:08:04 -0600
>> Anthony Liguori<anthony@codemonkey.ws>  wrote:
>>
>>      
>>> 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.
>>>
>>> Events in QMP Today
>>>
>>> QMP events are asynchronous messages.  They are not tied explicitly to
>>> any type of context, do not have a well defined format, and are have no
>>> mechanism to mask or filter events.  As of right now, we have somewhere
>>> around a dozen events.
>>>
>>> Goals of QAPI
>>>
>>> 1) Make all interfaces consumable in C such that we can use the
>>> interfaces in QEMU
>>>
>>> 2) Make all interfaces exposed through a library using code generation
>>> from static introspection
>>>
>>> 3) Make all interfaces well specified in a formal schema
>>>
>>> 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.
>>>        
>> This seems to be the right way to do this in C, but I wonder if it will
>> get complex/bloated to require this on the wire protocol.
>>
>> In the initial discussions on QMP events, we decided that it was
>> simpler/easier to just send all events and let the client do the masking if it
>> wants to. Later on, Daniel commented that it would be useful to able to mask
>> events early on.. Now, this proposal will require registration.
>>
>> We don't seem to make our mind..
>>
>> Daniel, what do you think?
>>      
> I've always thought it was bad to have all events enabled all
> the time, because it is not scalable as the number&  frequency
> of events increases. Requiring the app register for each event
> it cares about is more scalable&  also allows the app to discover
> if a particular event is actually supported by that QEMU version.
>    

Exactly.

> eg The initial 'qmp_capabilities' greeting from the client could
> include the list of all desired event names from the client,
> and the server could reply with the list of those event it can
> actually support&  has enabled.
>    

The trouble is that if we want to have events associated with particular 
objects, then this method doesn't really work well.

For instance, if we had some high frequency event for block drivers, I 
may want to register the event for a specific device only for a short 
period of time.

Having a simple mask/unmask global event doesn't really do that well.

I think a key to success in QMP is to make the API usable in QEMU and I 
think we need that ability for it to be useful in QEMU.

Regards,

Anthony Liguori
> Daniel
>    

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

[Anthony Liguori]

On 02/14/2011 07:28 AM, Luiz Capitulino wrote:
> On Sun, 13 Feb 2011 12:08:04 -0600
> Anthony Liguori<anthony@codemonkey.ws>  wrote:
>
>    
>> 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.
>>
>> Events in QMP Today
>>
>> QMP events are asynchronous messages.  They are not tied explicitly to
>> any type of context, do not have a well defined format, and are have no
>> mechanism to mask or filter events.  As of right now, we have somewhere
>> around a dozen events.
>>
>> Goals of QAPI
>>
>> 1) Make all interfaces consumable in C such that we can use the
>> interfaces in QEMU
>>
>> 2) Make all interfaces exposed through a library using code generation
>> from static introspection
>>
>> 3) Make all interfaces well specified in a formal schema
>>
>> 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.
>>      
> This seems to be the right way to do this in C, but I wonder if it will
> get complex/bloated to require this on the wire protocol.
>    

It adds a bunch of new RPC functions, but I don't see a better way.

> In the initial discussions on QMP events, we decided that it was
> simpler/easier to just send all events and let the client do the masking if it
> wants to. Later on, Daniel commented that it would be useful to able to mask
> events early on.. Now, this proposal will require registration.
>
> We don't seem to make our mind..
>
> Daniel, what do you think?
>
>    
>> Example:
>>
>> We would define QEVENT_BLOCK_IO_EVENT as:
>>      
> I won't comment on the code right now, I want to read it in detail in your
> branch, so that I can do a better review. Will try to do it in the next
> few days.
>
> My only immediate concern is that, as I commented on the other email, this
> new model will require us to add new commands/events when extending existing
> commands/events, right?
>    

No, optional parameters are supported.  This changes the structure size 
and function signature which means that libqmp has to bump it's version 
number but from a wire protocol perspective, a new and/or libqmp will 
work just fine with all versions of QEMU that support QMP.

The version bump of libqmp is surely undesirable though so we should 
restrict these type of changes.  It's very hard to make this type of 
change in a compatible way regardless of C though so that's generally true.

Regards,

Anthony Liguori

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

[Anthony Liguori]

On 02/14/2011 06:45 AM, Luiz Capitulino wrote:
> So the question is: how does the schema based design support extending
> commands or events? Does it require adding new commands/events?
>    

Well, let me ask you, how do we do that today?

Let's say that I want to add a new parameter to the `change' function so 
that I can include a salt parameter as part of the password.

The way we'd do this today is by checking for the 'salt' parameter in 
qdict, and if it's not present, use a random salt or something like that.

However, if I'm a QMP client, how can I tell whether you're going to 
ignore my salt parameter or actually use it?  Nothing in QMP tells me 
this today.  If I set the salt parameter in the `change' command, I'll 
just get a success message.

Even if we expose a schema, but leave things as-is, having to parse the 
schema as part of a function call is pretty horrible, particularly if 
distros do silly things like backport some optional parameters and not 
others.  If those optional parameters are deeply nested in a structure, 
it's even worse.

OTOH, if we introduce a new command to set the password with a salt, it 
becomes very easy for the client to support.  The do something as simple as:

if qmp.has_command("vnc-set-password-with-salt"):
     qmp.vnc_set_password_with_salt('foobar', 'X*')
else:
     window.set_weak_security_icon(True)
     qmp.vnc_set_password('foobar')

Now you could answer, hey, we can add capabilities then those 
capabilities can quickly get out of hand.

Regards,

Anthony Liguori

> While the current code is in really in bad shape currently, I'm not sure that
> having this disadvantage will pay off the new design.
>
>    

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

[Luiz Capitulino]

On Mon, 14 Feb 2011 08:39:11 -0600
Anthony Liguori <anthony@codemonkey.ws> wrote:

> On 02/14/2011 06:45 AM, Luiz Capitulino wrote:
> > So the question is: how does the schema based design support extending
> > commands or events? Does it require adding new commands/events?
> >    
> 
> Well, let me ask you, how do we do that today?
> 
> Let's say that I want to add a new parameter to the `change' function so 
> that I can include a salt parameter as part of the password.
> 
> The way we'd do this today is by checking for the 'salt' parameter in 
> qdict, and if it's not present, use a random salt or something like that.

You likely want to do what you did before. Of course that you have to
consider if what you're doing is extending an existing command or badly
overloading it (like change is today), in this case you'll want to add
a new command instead.

But yes, the use-case here is extending an existing command.

> However, if I'm a QMP client, how can I tell whether you're going to 
> ignore my salt parameter or actually use it?  Nothing in QMP tells me 
> this today.  If I set the salt parameter in the `change' command, I'll 
> just get a success message.

I'm sorry?

{ "execute": "change", "arguments": { "device": "vnc", "target": "password", "arg": "1234", "salt": "r1" } }
{"error": {"class": "InvalidParameter", "desc": "Invalid parameter 'salt'", "data": {"name": "salt"}}}

> Even if we expose a schema, but leave things as-is, having to parse the 
> schema as part of a function call is pretty horrible,

That's a client implementation detail, they are not required to do it
as part of a function call.

But let me ask, if we don't expose a schema, how will clients be able to
query available commands/events and their parameters?

> particularly if 
> distros do silly things like backport some optional parameters and not 
> others.  If those optional parameters are deeply nested in a structure, 
> it's even worse.

Why would they do this? I mean, if distros (or anyone else shipping qemu)
goes that deep on changing the wire protocol they are on their own, why
would we want to solve this problem?

Note that this is different from having a modular design where we can offer
the possibility of disabling features, say, in configure. That should be
possible, but it's different from choosing random parameters in commands.

> OTOH, if we introduce a new command to set the password with a salt, it 
> becomes very easy for the client to support.  The do something as simple as:
> 
> if qmp.has_command("vnc-set-password-with-salt"):
>      qmp.vnc_set_password_with_salt('foobar', 'X*')
> else:
>      window.set_weak_security_icon(True)
>      qmp.vnc_set_password('foobar')
> 
> Now you could answer, hey, we can add capabilities then those 
> capabilities can quickly get out of hand.

Adding one command per new argument has its problems too and it's even
worse with events, as clients will have to be changed to handle a
new event just because of a parameter addition.

Look, although I did _not_ check any code yet, your description of the QAPI
looks really exciting. I'm not against it, what bothers me though is this
number of small limitations we're imposing to the wire protocol.

Why don't we make libqmp internal only? This way we're free to change it
whatever we want.

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

[Anthony Liguori]

On 02/14/2011 12:34 PM, Luiz Capitulino wrote:
> On Mon, 14 Feb 2011 08:39:11 -0600
> Anthony Liguori<anthony@codemonkey.ws>  wrote:
>
>    
>> On 02/14/2011 06:45 AM, Luiz Capitulino wrote:
>>      
>>> So the question is: how does the schema based design support extending
>>> commands or events? Does it require adding new commands/events?
>>>
>>>        
>> Well, let me ask you, how do we do that today?
>>
>> Let's say that I want to add a new parameter to the `change' function so
>> that I can include a salt parameter as part of the password.
>>
>> The way we'd do this today is by checking for the 'salt' parameter in
>> qdict, and if it's not present, use a random salt or something like that.
>>      
> You likely want to do what you did before. Of course that you have to
> consider if what you're doing is extending an existing command or badly
> overloading it (like change is today), in this case you'll want to add
> a new command instead.
>
> But yes, the use-case here is extending an existing command.
>
>    
>> However, if I'm a QMP client, how can I tell whether you're going to
>> ignore my salt parameter or actually use it?  Nothing in QMP tells me
>> this today.  If I set the salt parameter in the `change' command, I'll
>> just get a success message.
>>      
> I'm sorry?
>
> { "execute": "change", "arguments": { "device": "vnc", "target": "password", "arg": "1234", "salt": "r1" } }
> {"error": {"class": "InvalidParameter", "desc": "Invalid parameter 'salt'", "data": {"name": "salt"}}}
>    

So I'm supposed to execute the command, and if execution fails, drop the 
new parameter?  If we add a few optional parameters, does that mean I 
have to try every possible combination of parameters?

>    
>> Even if we expose a schema, but leave things as-is, having to parse the
>> schema as part of a function call is pretty horrible,
>>      
> That's a client implementation detail, they are not required to do it
> as part of a function call.
>
> But let me ask, if we don't expose a schema, how will clients be able to
> query available commands/events and their parameters?
>    

We need to expose the schema, I'm not saying we shouldn't.  But we don't 
today.

You're arguing that we should extend commands by adding new parameters.  
I'm saying that's a bad interface.  If we need to change a command, we 
should introduce a new command.  It's a well understood mechanism for 
maintaining compatibility (just about every C library does exactly this).

>> particularly if
>> distros do silly things like backport some optional parameters and not
>> others.  If those optional parameters are deeply nested in a structure,
>> it's even worse.
>>      
> Why would they do this? I mean, if distros (or anyone else shipping qemu)
> goes that deep on changing the wire protocol they are on their own, why
> would we want to solve this problem?
>    

It's not at all unreasonable for a distro to backport a new QMP 
command.  If all modifications are discrete commands, compatibility is 
easy to preserve, however if a distro does backporting and we end up 
with a frankenstein command, compatibility will be an issue.

>> OTOH, if we introduce a new command to set the password with a salt, it
>> becomes very easy for the client to support.  The do something as simple as:
>>
>> if qmp.has_command("vnc-set-password-with-salt"):
>>       qmp.vnc_set_password_with_salt('foobar', 'X*')
>> else:
>>       window.set_weak_security_icon(True)
>>       qmp.vnc_set_password('foobar')
>>
>> Now you could answer, hey, we can add capabilities then those
>> capabilities can quickly get out of hand.
>>      
> Adding one command per new argument has its problems too and it's even
> worse with events, as clients will have to be changed to handle a
> new event just because of a parameter addition.
>    

Yes, but it's an extremely well understood way to design compatible APIs.

> Look, although I did _not_ check any code yet, your description of the QAPI
> looks really exciting. I'm not against it, what bothers me though is this
> number of small limitations we're imposing to the wire protocol.
>
> Why don't we make libqmp internal only? This way we're free to change it
> whatever we want.
>    

libqmp is a test of how easy it is to use QMP from an external 
application.  If we can't keep libqmp stable, then that means tools like 
libvirt will always have a hard time using QMP.

Proper C support is important.  We cannot make it impossible to write a 
useful C client API.

Regards,

Anthony Liguori

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

[Luiz Capitulino]

On Mon, 14 Feb 2011 13:34:11 -0600
Anthony Liguori <anthony@codemonkey.ws> wrote:

> On 02/14/2011 12:34 PM, Luiz Capitulino wrote:
> > On Mon, 14 Feb 2011 08:39:11 -0600
> > Anthony Liguori<anthony@codemonkey.ws>  wrote:
> >
> >    
> >> On 02/14/2011 06:45 AM, Luiz Capitulino wrote:
> >>      
> >>> So the question is: how does the schema based design support extending
> >>> commands or events? Does it require adding new commands/events?
> >>>
> >>>        
> >> Well, let me ask you, how do we do that today?
> >>
> >> Let's say that I want to add a new parameter to the `change' function so
> >> that I can include a salt parameter as part of the password.
> >>
> >> The way we'd do this today is by checking for the 'salt' parameter in
> >> qdict, and if it's not present, use a random salt or something like that.
> >>      
> > You likely want to do what you did before. Of course that you have to
> > consider if what you're doing is extending an existing command or badly
> > overloading it (like change is today), in this case you'll want to add
> > a new command instead.
> >
> > But yes, the use-case here is extending an existing command.
> >
> >    
> >> However, if I'm a QMP client, how can I tell whether you're going to
> >> ignore my salt parameter or actually use it?  Nothing in QMP tells me
> >> this today.  If I set the salt parameter in the `change' command, I'll
> >> just get a success message.
> >>      
> > I'm sorry?
> >
> > { "execute": "change", "arguments": { "device": "vnc", "target": "password", "arg": "1234", "salt": "r1" } }
> > {"error": {"class": "InvalidParameter", "desc": "Invalid parameter 'salt'", "data": {"name": "salt"}}}
> >    
> 
> So I'm supposed to execute the command, and if execution fails, drop the 
> new parameter?  If we add a few optional parameters, does that mean I 
> have to try every possible combination of parameters?

No, of course not, our plan has always been to do this via an schema,
the only reason we don't do this today is lack of time/help.

> >> Even if we expose a schema, but leave things as-is, having to parse the
> >> schema as part of a function call is pretty horrible,
> >>      
> > That's a client implementation detail, they are not required to do it
> > as part of a function call.
> >
> > But let me ask, if we don't expose a schema, how will clients be able to
> > query available commands/events and their parameters?
> >    
> 
> We need to expose the schema, I'm not saying we shouldn't.  But we don't 
> today.
> 
> You're arguing that we should extend commands by adding new parameters.  

Commands and events, you haven't commented on events yet and that seems
a bit worse than commands.

> I'm saying that's a bad interface.  If we need to change a command, we 
> should introduce a new command.  It's a well understood mechanism for 
> maintaining compatibility (just about every C library does exactly this).

So, let's agree we disagree.

> >> particularly if
> >> distros do silly things like backport some optional parameters and not
> >> others.  If those optional parameters are deeply nested in a structure,
> >> it's even worse.
> >>      
> > Why would they do this? I mean, if distros (or anyone else shipping qemu)
> > goes that deep on changing the wire protocol they are on their own, why
> > would we want to solve this problem?
> >    
> 
> It's not at all unreasonable for a distro to backport a new QMP 
> command.  If all modifications are discrete commands, compatibility is 
> easy to preserve, however if a distro does backporting and we end up 
> with a frankenstein command, compatibility will be an issue.

I disagree. Let's say we have added three new arguments to the command foo,
and now we have foo1, foo2 and foo3. I'm a quite old distro and only
have foo, which command should I backport? All of them? Only the latest?

I can't see how easier this is. Backporting APIs will almost always suck.

> >> OTOH, if we introduce a new command to set the password with a salt, it
> >> becomes very easy for the client to support.  The do something as simple as:
> >>
> >> if qmp.has_command("vnc-set-password-with-salt"):
> >>       qmp.vnc_set_password_with_salt('foobar', 'X*')
> >> else:
> >>       window.set_weak_security_icon(True)
> >>       qmp.vnc_set_password('foobar')
> >>
> >> Now you could answer, hey, we can add capabilities then those
> >> capabilities can quickly get out of hand.
> >>      
> > Adding one command per new argument has its problems too and it's even
> > worse with events, as clients will have to be changed to handle a
> > new event just because of a parameter addition.
> >    
> 
> Yes, but it's an extremely well understood way to design compatible APIs.

For C, yes. But one of the main goals of a high level protocol is to be
language independent, isn't it?

> > Look, although I did _not_ check any code yet, your description of the QAPI
> > looks really exciting. I'm not against it, what bothers me though is this
> > number of small limitations we're imposing to the wire protocol.
> >
> > Why don't we make libqmp internal only? This way we're free to change it
> > whatever we want.
> >    
> 
> libqmp is a test of how easy it is to use QMP from an external 
> application.  If we can't keep libqmp stable, then that means tools like 
> libvirt will always have a hard time using QMP.
> 
> Proper C support is important.  We cannot make it impossible to write a 
> useful C client API.

I wouldn't say it's impossible, but anyway, the important point here is
that we disagree about the side effects QAPI is going to introduce in QMP,
I don't know how to solve this, maybe we can discuss this upstream, but I'm
not sure the situation will change much.

Should we vote? :)

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

[Luiz Capitulino]

On Mon, 14 Feb 2011 17:58:00 -0200
Luiz Capitulino <lcapitulino@redhat.com> wrote:

> On Mon, 14 Feb 2011 13:34:11 -0600
> Anthony Liguori <anthony@codemonkey.ws> wrote:
> 
> > On 02/14/2011 12:34 PM, Luiz Capitulino wrote:
> > > On Mon, 14 Feb 2011 08:39:11 -0600
> > > Anthony Liguori<anthony@codemonkey.ws>  wrote:
> > >
> > >    
> > >> On 02/14/2011 06:45 AM, Luiz Capitulino wrote:
> > >>      
> > >>> So the question is: how does the schema based design support extending
> > >>> commands or events? Does it require adding new commands/events?
> > >>>
> > >>>        
> > >> Well, let me ask you, how do we do that today?
> > >>
> > >> Let's say that I want to add a new parameter to the `change' function so
> > >> that I can include a salt parameter as part of the password.
> > >>
> > >> The way we'd do this today is by checking for the 'salt' parameter in
> > >> qdict, and if it's not present, use a random salt or something like that.
> > >>      
> > > You likely want to do what you did before. Of course that you have to
> > > consider if what you're doing is extending an existing command or badly
> > > overloading it (like change is today), in this case you'll want to add
> > > a new command instead.
> > >
> > > But yes, the use-case here is extending an existing command.
> > >
> > >    
> > >> However, if I'm a QMP client, how can I tell whether you're going to
> > >> ignore my salt parameter or actually use it?  Nothing in QMP tells me
> > >> this today.  If I set the salt parameter in the `change' command, I'll
> > >> just get a success message.
> > >>      
> > > I'm sorry?
> > >
> > > { "execute": "change", "arguments": { "device": "vnc", "target": "password", "arg": "1234", "salt": "r1" } }
> > > {"error": {"class": "InvalidParameter", "desc": "Invalid parameter 'salt'", "data": {"name": "salt"}}}
> > >    
> > 
> > So I'm supposed to execute the command, and if execution fails, drop the 
> > new parameter?  If we add a few optional parameters, does that mean I 
> > have to try every possible combination of parameters?
> 
> No, of course not, our plan has always been to do this via an schema,
> the only reason we don't do this today is lack of time/help.
> 
> > >> Even if we expose a schema, but leave things as-is, having to parse the
> > >> schema as part of a function call is pretty horrible,
> > >>      
> > > That's a client implementation detail, they are not required to do it
> > > as part of a function call.
> > >
> > > But let me ask, if we don't expose a schema, how will clients be able to
> > > query available commands/events and their parameters?
> > >    
> > 
> > We need to expose the schema, I'm not saying we shouldn't.  But we don't 
> > today.
> > 
> > You're arguing that we should extend commands by adding new parameters.  
> 
> Commands and events, you haven't commented on events yet and that seems
> a bit worse than commands.
> 
> > I'm saying that's a bad interface.  If we need to change a command, we 
> > should introduce a new command.  It's a well understood mechanism for 
> > maintaining compatibility (just about every C library does exactly this).
> 
> So, let's agree we disagree.
> 
> > >> particularly if
> > >> distros do silly things like backport some optional parameters and not
> > >> others.  If those optional parameters are deeply nested in a structure,
> > >> it's even worse.
> > >>      
> > > Why would they do this? I mean, if distros (or anyone else shipping qemu)
> > > goes that deep on changing the wire protocol they are on their own, why
> > > would we want to solve this problem?
> > >    
> > 
> > It's not at all unreasonable for a distro to backport a new QMP 
> > command.  If all modifications are discrete commands, compatibility is 
> > easy to preserve, however if a distro does backporting and we end up 
> > with a frankenstein command, compatibility will be an issue.
> 
> I disagree. Let's say we have added three new arguments to the command foo,
> and now we have foo1, foo2 and foo3. I'm a quite old distro and only
> have foo, which command should I backport? All of them? Only the latest?
> 
> I can't see how easier this is. Backporting APIs will almost always suck.
> 
> > >> OTOH, if we introduce a new command to set the password with a salt, it
> > >> becomes very easy for the client to support.  The do something as simple as:
> > >>
> > >> if qmp.has_command("vnc-set-password-with-salt"):
> > >>       qmp.vnc_set_password_with_salt('foobar', 'X*')
> > >> else:
> > >>       window.set_weak_security_icon(True)
> > >>       qmp.vnc_set_password('foobar')
> > >>
> > >> Now you could answer, hey, we can add capabilities then those
> > >> capabilities can quickly get out of hand.
> > >>      
> > > Adding one command per new argument has its problems too and it's even
> > > worse with events, as clients will have to be changed to handle a
> > > new event just because of a parameter addition.
> > >    
> > 
> > Yes, but it's an extremely well understood way to design compatible APIs.
> 
> For C, yes. But one of the main goals of a high level protocol is to be
> language independent, isn't it?
> 
> > > Look, although I did _not_ check any code yet, your description of the QAPI
> > > looks really exciting. I'm not against it, what bothers me though is this
> > > number of small limitations we're imposing to the wire protocol.
> > >
> > > Why don't we make libqmp internal only? This way we're free to change it
> > > whatever we want.
> > >    
> > 
> > libqmp is a test of how easy it is to use QMP from an external 
> > application.  If we can't keep libqmp stable, then that means tools like 
> > libvirt will always have a hard time using QMP.
> > 
> > Proper C support is important.  We cannot make it impossible to write a 
> > useful C client API.
> 
> I wouldn't say it's impossible, but anyway, the important point here is
> that we disagree about the side effects QAPI is going to introduce in QMP,
> I don't know how to solve this, maybe we can discuss this upstream, but I'm
> not sure the situation will change much.

Oh, it's upstream, let's vote? :)

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

[Anthony Liguori]

On 02/14/2011 01:58 PM, Luiz Capitulino wrote:
> No, of course not, our plan has always been to do this via an schema,
> the only reason we don't do this today is lack of time/help.
>
>    

Understood--I'm here to help now :-)

>> We need to expose the schema, I'm not saying we shouldn't.  But we don't
>> today.
>>
>> You're arguing that we should extend commands by adding new parameters.
>>      
> Commands and events, you haven't commented on events yet and that seems
> a bit worse than commands.
>    

Events are just commands that never return a value and are posted.

IOW:

client -> server message == command
server -> client message == event

However, we limit events to have no return value and semantically, when 
an event is invoked, the message is only posted (we're not guaranteed 
that the client has seen it).

The reason for the lack of symmetry is to simplify the programming 
model.  If we had to wait for an event to be ack'd and receive a return 
value, it would involve a lot of ugly callback registrations.

But beyond those few limitations, events and commands should be treated 
exactly the same IMHO.

> I disagree. Let's say we have added three new arguments to the command foo,
> and now we have foo1, foo2 and foo3. I'm a quite old distro and only
> have foo, which command should I backport? All of them? Only the latest?
>
> I can't see how easier this is. Backporting APIs will almost always suck.
>    

1) if we have to add three arguments to a command, we've failed in our 
API design after an initial release

2) it depends on what level of functionality the distro needs.  The more 
complicated we make the API, the harder it will be to write clients 
against it.  If we have four ways to do the same thing (even if that's 
via one command or four separate ones), writing a client is going to 
suck no matter what.

> For C, yes. But one of the main goals of a high level protocol is to be
> language independent, isn't it?
>    

Yes, which means first-class support for a variety of languages with C 
being a very important one.

>>> Look, although I did _not_ check any code yet, your description of the QAPI
>>> looks really exciting. I'm not against it, what bothers me though is this
>>> number of small limitations we're imposing to the wire protocol.
>>>
>>> Why don't we make libqmp internal only? This way we're free to change it
>>> whatever we want.
>>>
>>>        
>> libqmp is a test of how easy it is to use QMP from an external
>> application.  If we can't keep libqmp stable, then that means tools like
>> libvirt will always have a hard time using QMP.
>>
>> Proper C support is important.  We cannot make it impossible to write a
>> useful C client API.
>>      
> I wouldn't say it's impossible, but anyway, the important point here is
> that we disagree about the side effects QAPI is going to introduce in QMP,
> I don't know how to solve this, maybe we can discuss this upstream, but I'm
> not sure the situation will change much.
>
> Should we vote? :)
>    

Let's wait until I actually post patches...  My purpose of this thread 
was to get some feedback on using signal/slots as an abstraction in QEMU.

The QMP conversion is almost done, I'll be able to post patches very soon.

Regards,

Anthony Liguori

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

[Anthony Liguori]

On 02/14/2011 06:32 AM, Kevin Wolf wrote:
> Am 14.02.2011 13:03, schrieb Anthony Liguori:
>    
>> On 02/14/2011 03:50 AM, Kevin Wolf wrote:
>>      
>>> Am 13.02.2011 19:08, schrieb Anthony Liguori:
>>>        
>>>> 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)?
>>>
>>>        
>> A signal is a function call.  You can pass a structure as a parameter is
>> you so desire but the natural thing to do is pass position arguments.
>>
>> If you've got a ton of signal arguments, it's probably an indication
>> that you're doing something wrong.
>>      
> Yes. For example, you're taking tons of independent arguments for
> something that is logically a single entity, namely a block error event. ;-)
>
> I'm almost sure that we'll want to add more things to this specific
> event, for example a more detailed error description (Luiz once
> suggested using errno here, but seems it hasn't made its way into
> upstream). And I would be surprised if we never wanted to add more
> information to other events, too.
>    

You can pass structures as part of events.  For instance:

{ 'BlockIOEventInfo': { 'action': 'str', 'operation': 'str' } }
{ 'BlockIOEvent': { 'data': 'BlockIOEventInfo' } }

Which generates:

typedef struct BlockIOEventInfo
{
      const char *action;
      const char *operation;
} BlockIOEventInfo;

typedef void (BlockIOEventFunc)(BlockIOEventInfo *data, void *opaque);

There are some cases where this might make sense.  For instance, device 
hotplug events might want to pass quite a bit of device info in the event.

It's just like designing any normal API.  Sometimes you need to pass 
structs and sometimes adding more args is the right thing to do.

>>    From a protocol perspective, events look like this today:
>>
>> { "event": "BlockIOError", "data": { "device": "ide1-cd0", "operation":
>> "read", "action": "report" } }
>>
>> The only change to the protocol I'm proposing is adding a "tag" field
>> which would give us:
>>
>> { "event": "BlockIOError", tag: "event023234", "data": { "device":
>> "ide1-cd0", "operation": "read", "action": "report" } }
>>      
> Right, I was more referring to this schema thing. I didn't quite
> understand yet if/why functions and events are the same thing from that
> perspective.
>
> They seem to be some kind of function that is called from within qemu,
> gets its arguments from the event_connect call and returns its return
> value to the QMP client.
>
> Is that about right?
>    

Close.  It's a function that's called from within QEMU whereas the 
arguments to the function end up being sent as the payload for the event.

The arguments to connect allow the client to connect to very specific 
signals.  For example, each BlockDriverState may have it's own 
BlockIOEvent signal.  So:

struct BlockDriverState {
      // ...
      BlockIOEvent ioevent;
};

And then the various places would do:

block_io_event_signal(&bs->ioevent, "report", "read");

The connect operation is used to figure out which bs->ioevent to connect 
to.  So it would look like:

block_io_event_connect("ide0-hd0", my_io_event_handler, &mystate);

We could eliminate the need for individual connect/disconnect operations 
if we had a global object model where you could describe objects in a 
unified hierarchy.  For instance:

signal_connect("/blockdev/ide0-hd0", my_ioevent_handler, &mystate);

But this is a pretty big change that I fear is overengineering.

>>>> 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?
>>>        
>> Both have to exist.  We can't drop old events without breaking backwards
>> compatibility.
>>      
> Right. So a second event doesn't sound like something we'd love to
> have... Maybe extending the existing ones with optional arguments would
> be better?
>    

See the other thread about using optional arguments to extend.  I think 
it's extremely hard to write a client if we use this model to do 
extensions.  It basically forces you to work with variant types instead 
of native types.

Regards,

Anthony Liguori

> Kevin
>
>    

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

[Kevin Wolf]

Am 14.02.2011 20:34, schrieb Anthony Liguori:
> On 02/14/2011 12:34 PM, Luiz Capitulino wrote:
>> On Mon, 14 Feb 2011 08:39:11 -0600
>> Anthony Liguori<anthony@codemonkey.ws>  wrote:
>>
>>    
>>> On 02/14/2011 06:45 AM, Luiz Capitulino wrote:
>>>      
>>>> So the question is: how does the schema based design support extending
>>>> commands or events? Does it require adding new commands/events?
>>>>
>>>>        
>>> Well, let me ask you, how do we do that today?
>>>
>>> Let's say that I want to add a new parameter to the `change' function so
>>> that I can include a salt parameter as part of the password.
>>>
>>> The way we'd do this today is by checking for the 'salt' parameter in
>>> qdict, and if it's not present, use a random salt or something like that.
>>>      
>> You likely want to do what you did before. Of course that you have to
>> consider if what you're doing is extending an existing command or badly
>> overloading it (like change is today), in this case you'll want to add
>> a new command instead.
>>
>> But yes, the use-case here is extending an existing command.
>>
>>    
>>> However, if I'm a QMP client, how can I tell whether you're going to
>>> ignore my salt parameter or actually use it?  Nothing in QMP tells me
>>> this today.  If I set the salt parameter in the `change' command, I'll
>>> just get a success message.
>>>      
>> I'm sorry?
>>
>> { "execute": "change", "arguments": { "device": "vnc", "target": "password", "arg": "1234", "salt": "r1" } }
>> {"error": {"class": "InvalidParameter", "desc": "Invalid parameter 'salt'", "data": {"name": "salt"}}}
>>    
> 
> So I'm supposed to execute the command, and if execution fails, drop the 
> new parameter?  If we add a few optional parameters, does that mean I 
> have to try every possible combination of parameters?

How is that different from trying out multiple commands? In the end, you
always need some meta information like a schema in order to avoid trying
out which parameters the server supports.

Anyway, I think there's a second interesting point: Adding parameters
does cause these problems, but it's different for data sent from qemu to
the client (return values and events). If we add more information there,
an older client can just ignore it, without even looking at a schema.

So I think we should consider this for return values and definitely do
it for events. Sending out five different messages for a single event
that are completely redundant and only differ in the number of fields is
just insane (okay, they wouldn't actually get on the wire because a
client registers only for one of them, but the code for generating them
must exist).

> You're arguing that we should extend commands by adding new parameters.  
> I'm saying that's a bad interface.  If we need to change a command, we 
> should introduce a new command.  It's a well understood mechanism for 
> maintaining compatibility (just about every C library does exactly this).

I'm yet undecided about adding parameters. I have a feeling that you
might be right here.

Kevin

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

[Luiz Capitulino]

On Mon, 14 Feb 2011 14:15:27 -0600
Anthony Liguori <anthony@codemonkey.ws> wrote:

> On 02/14/2011 01:58 PM, Luiz Capitulino wrote:
> > No, of course not, our plan has always been to do this via an schema,
> > the only reason we don't do this today is lack of time/help.
> >
> >    
> 
> Understood--I'm here to help now :-)
> 
> >> We need to expose the schema, I'm not saying we shouldn't.  But we don't
> >> today.
> >>
> >> You're arguing that we should extend commands by adding new parameters.
> >>      
> > Commands and events, you haven't commented on events yet and that seems
> > a bit worse than commands.
> >    
> 
> Events are just commands that never return a value and are posted.
> 
> IOW:
> 
> client -> server message == command
> server -> client message == event
> 
> However, we limit events to have no return value and semantically, when 
> an event is invoked, the message is only posted (we're not guaranteed 
> that the client has seen it).
> 
> The reason for the lack of symmetry is to simplify the programming 
> model.  If we had to wait for an event to be ack'd and receive a return 
> value, it would involve a lot of ugly callback registrations.
> 
> But beyond those few limitations, events and commands should be treated 
> exactly the same IMHO.
> 
> > I disagree. Let's say we have added three new arguments to the command foo,
> > and now we have foo1, foo2 and foo3. I'm a quite old distro and only
> > have foo, which command should I backport? All of them? Only the latest?
> >
> > I can't see how easier this is. Backporting APIs will almost always suck.
> >    
> 
> 1) if we have to add three arguments to a command, we've failed in our 
> API design after an initial release

Maybe. Still, having to add a new command because we wanted to make a simple
extension seems a bit overkill to me. I'm not sure how common this is going
to be, however when we discussed QMP originally there were a lot on emphasis
on this kind of feature.

What makes it pretty hard to work on this project is that we are always
changing our mind in a number of details.

> 2) it depends on what level of functionality the distro needs.  The more 
> complicated we make the API, the harder it will be to write clients 
> against it.  If we have four ways to do the same thing (even if that's 
> via one command or four separate ones), writing a client is going to 
> suck no matter what.

Yes, but I still do not see how easier it's going to be for a client
writer if s/he has to choose among four commands that do the same thing.

Please, note that I do see the internal benefits, as always, the disagreement
is about the wire protocol.

> > For C, yes. But one of the main goals of a high level protocol is to be
> > language independent, isn't it?
> >    
> 
> Yes, which means first-class support for a variety of languages with C 
> being a very important one.
>
> >>> Look, although I did _not_ check any code yet, your description of the QAPI
> >>> looks really exciting. I'm not against it, what bothers me though is this
> >>> number of small limitations we're imposing to the wire protocol.
> >>>
> >>> Why don't we make libqmp internal only? This way we're free to change it
> >>> whatever we want.
> >>>
> >>>        
> >> libqmp is a test of how easy it is to use QMP from an external
> >> application.  If we can't keep libqmp stable, then that means tools like
> >> libvirt will always have a hard time using QMP.
> >>
> >> Proper C support is important.  We cannot make it impossible to write a
> >> useful C client API.
> >>      
> > I wouldn't say it's impossible, but anyway, the important point here is
> > that we disagree about the side effects QAPI is going to introduce in QMP,
> > I don't know how to solve this, maybe we can discuss this upstream, but I'm
> > not sure the situation will change much.
> >
> > Should we vote? :)
> >    
> 
> Let's wait until I actually post patches...  My purpose of this thread 
> was to get some feedback on using signal/slots as an abstraction in QEMU.

Ok.

> 
> The QMP conversion is almost done, I'll be able to post patches very soon.
> 
> Regards,
> 
> Anthony Liguori
> 

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

[Luiz Capitulino]

On Tue, 15 Feb 2011 10:20:01 +0100
Kevin Wolf <kwolf@redhat.com> wrote:

> Am 14.02.2011 20:34, schrieb Anthony Liguori:
> > On 02/14/2011 12:34 PM, Luiz Capitulino wrote:
> >> On Mon, 14 Feb 2011 08:39:11 -0600
> >> Anthony Liguori<anthony@codemonkey.ws>  wrote:
> >>
> >>    
> >>> On 02/14/2011 06:45 AM, Luiz Capitulino wrote:
> >>>      
> >>>> So the question is: how does the schema based design support extending
> >>>> commands or events? Does it require adding new commands/events?
> >>>>
> >>>>        
> >>> Well, let me ask you, how do we do that today?
> >>>
> >>> Let's say that I want to add a new parameter to the `change' function so
> >>> that I can include a salt parameter as part of the password.
> >>>
> >>> The way we'd do this today is by checking for the 'salt' parameter in
> >>> qdict, and if it's not present, use a random salt or something like that.
> >>>      
> >> You likely want to do what you did before. Of course that you have to
> >> consider if what you're doing is extending an existing command or badly
> >> overloading it (like change is today), in this case you'll want to add
> >> a new command instead.
> >>
> >> But yes, the use-case here is extending an existing command.
> >>
> >>    
> >>> However, if I'm a QMP client, how can I tell whether you're going to
> >>> ignore my salt parameter or actually use it?  Nothing in QMP tells me
> >>> this today.  If I set the salt parameter in the `change' command, I'll
> >>> just get a success message.
> >>>      
> >> I'm sorry?
> >>
> >> { "execute": "change", "arguments": { "device": "vnc", "target": "password", "arg": "1234", "salt": "r1" } }
> >> {"error": {"class": "InvalidParameter", "desc": "Invalid parameter 'salt'", "data": {"name": "salt"}}}
> >>    
> > 
> > So I'm supposed to execute the command, and if execution fails, drop the 
> > new parameter?  If we add a few optional parameters, does that mean I 
> > have to try every possible combination of parameters?
> 
> How is that different from trying out multiple commands? In the end, you
> always need some meta information like a schema in order to avoid trying
> out which parameters the server supports.
> 
> Anyway, I think there's a second interesting point: Adding parameters
> does cause these problems, but it's different for data sent from qemu to
> the client (return values and events). If we add more information there,
> an older client can just ignore it, without even looking at a schema.
> 
> So I think we should consider this for return values and definitely do
> it for events. Sending out five different messages for a single event
> that are completely redundant and only differ in the number of fields is
> just insane (okay, they wouldn't actually get on the wire because a
> client registers only for one of them, but the code for generating them
> must exist).

That's my point when I asked about events in the other thread.

> > You're arguing that we should extend commands by adding new parameters.  
> > I'm saying that's a bad interface.  If we need to change a command, we 
> > should introduce a new command.  It's a well understood mechanism for 
> > maintaining compatibility (just about every C library does exactly this).
> 
> I'm yet undecided about adding parameters. I have a feeling that you
> might be right here.
> 
> Kevin
> 

What's QAPI? (was: [Qemu-devel] [RFC] qapi: events in QMP)

[Markus Armbruster]

Anthony Liguori <anthony@codemonkey.ws> writes:

> 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.

Have you put your thinking behind the QAPI branch in writing anywhere?
Ideally in comparable detail as your discussion of events in QAPI (which
I snipped).

[...]

[Qemu-devel] Re: What's QAPI?

[Anthony Liguori]

On 02/15/2011 08:07 AM, Markus Armbruster wrote:
> Anthony Liguori<anthony@codemonkey.ws>  writes:
>
>    
>> 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.
>>      
> Have you put your thinking behind the QAPI branch in writing anywhere?
> Ideally in comparable detail as your discussion of events in QAPI (which
> I snipped).
>    

It's an evolution of http://wiki.qemu.org/Features/QMP2 which grew out 
of our discussions at KVM Forum.  I need to update the page considerably 
and we try to do that before the community call.

Regards,

Anthony Liguori


> [...]
>
>    

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

[Markus Armbruster]

Luiz Capitulino <lcapitulino@redhat.com> writes:

> On Mon, 14 Feb 2011 13:34:11 -0600
> Anthony Liguori <anthony@codemonkey.ws> wrote:
>
>> On 02/14/2011 12:34 PM, Luiz Capitulino wrote:
>> > On Mon, 14 Feb 2011 08:39:11 -0600
>> > Anthony Liguori<anthony@codemonkey.ws>  wrote:
>> >
>> >    
>> >> On 02/14/2011 06:45 AM, Luiz Capitulino wrote:
>> >>      
>> >>> So the question is: how does the schema based design support extending
>> >>> commands or events? Does it require adding new commands/events?
>> >>>
>> >>>        
>> >> Well, let me ask you, how do we do that today?
>> >>
>> >> Let's say that I want to add a new parameter to the `change' function so
>> >> that I can include a salt parameter as part of the password.
>> >>
>> >> The way we'd do this today is by checking for the 'salt' parameter in
>> >> qdict, and if it's not present, use a random salt or something like that.
>> >>      
>> > You likely want to do what you did before. Of course that you have to
>> > consider if what you're doing is extending an existing command or badly
>> > overloading it (like change is today), in this case you'll want to add
>> > a new command instead.
>> >
>> > But yes, the use-case here is extending an existing command.
>> >
>> >    
>> >> However, if I'm a QMP client, how can I tell whether you're going to
>> >> ignore my salt parameter or actually use it?  Nothing in QMP tells me
>> >> this today.  If I set the salt parameter in the `change' command, I'll
>> >> just get a success message.
>> >>      
>> > I'm sorry?
>> >
>> > { "execute": "change", "arguments": { "device": "vnc", "target": "password", "arg": "1234", "salt": "r1" } }
>> > {"error": {"class": "InvalidParameter", "desc": "Invalid parameter 'salt'", "data": {"name": "salt"}}}
>> >    
>> 
>> So I'm supposed to execute the command, and if execution fails, drop the 
>> new parameter?  If we add a few optional parameters, does that mean I 
>> have to try every possible combination of parameters?
>
> No, of course not, our plan has always been to do this via an schema,

Correct.

> the only reason we don't do this today is lack of time/help.

Disagree on "only".  QMP didn't just suffer from lack of pushing power.
On the contrary, it suffered from too much pushing in opposite
directions.

[...]

[Qemu-devel] Re: What's QAPI?

[Anthony Liguori]

On 02/15/2011 08:07 AM, Markus Armbruster wrote:
> Anthony Liguori<anthony@codemonkey.ws>  writes:
>
>    
>> 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.
>>      
> Have you put your thinking behind the QAPI branch in writing anywhere?
> Ideally in comparable detail as your discussion of events in QAPI (which
> I snipped).
>    

http://wiki.qemu.org/Features/QAPI

Inlined here in case anyone wants to discuss:

== Implementation ==

QAPI uses a schema/IDL written in JSON to automatically generate the 
types and
marshalling information for QMP commands.  It is used to generate both the
server side marshalling interfaces and a client library.

A typical function definition looks like this:

  ##
  # @change:
  #
  # This command is multiple commands multiplexed together.  Generally 
speaking,
  # it should not be used in favor of the single purpose alternatives 
such as
  # @change-vnc-listen, @change-vnc-password, and @change-blockdev.
  #
  # @device: This is normally the name of a block device but it may also 
be 'vnc'.
  #          when it's 'vnc', then sub command depends on @target
  #
  # @target: If @device is a block device, then this is the new filename.
  #          If @device is 'vnc', then if the value 'password' selects 
the vnc
  #          change password command.   Otherwise, this specifies a new 
server URI
  #          address to listen to for VNC connections.
  #
  # @arg:    If @device is a block device, then this is an optional 
format to open
  #          the device with.
  #          If @device is 'vnc' and @target is 'password', this is the 
new VNC
  #          password to set.  If this argument is an empty string, then 
no future
  #          logins will be allowed.
  #
  # Returns: Nothing on success.
  #          If @device is not a valid block device, DeviceNotFound
  #          If @format is not a valid block format, InvalidBlockFormat
  #          If the new block device is encrypted, DeviceEncrypted.  
Note that
  #          if this error is returned, the device has been opened 
successfully
  #          and an additional call to @block_passwd is required to set the
  #          device's password.  The behavior of reads and writes to the 
block
  #          device between when these calls are executed is undefined.
  #
  # Notes:  It is strongly recommended that this interface is not used 
especially
  #         for changing block devices.
  #
  # Since: 0.14.0
  ##
  [ 'change', {'device': 'str', 'target': 'str'}, {'arg': 'str'}, 'none' ]

The comments above the function are written in gtk-doc format and meant 
to be
extracted to generate both protocol documentation and libqmp documentation.

The first element of the list is the command name, in this case, 'change'.

The second element of the list is a dictionary containing the required 
arguments
for the function.  The key is the argument name and the value is the 
type.  The
type can be a string or a complex type (see section on Typing).

The third element is a dictionary of optional arguments that follows the 
same
rules as the required arguments.

The final list element is the return type of the function.  'none' is a 
special
type representing a void return value.

== QMP Server ==

This definition will generate the following signature in qemu/qmp.h:

  void qmp_change(const char *device, const char *target, bool has_arg,
                  const char *arg, Error **errp);

Which is then implemented in the appropriate place in QEMU.  The optional
arguments always are prefixed with a boolean argument to indicate 
whether the
option has been specified.  The final argument is a pointer to an Error 
object
in the fashion of GError.  The caller of this function is will check for a
non-NULL error object to determine if the function failed.  errp can be 
set to
NULL if the caller does not care about failure.

Currently, Error is roughly identical to QError with the exception that 
it only
supports simple key/value arguments.

== Complex Types ==

Some commands take or return complex types.  It's usually a good idea to 
define
complex types outside of the function definition.  An example of a command
that returns a complex type is below:

  ##
  # @VersionInfo:
  #
  # A description of QEMU's version.
  #
  # @qemu.major:  The major version of QEMU
  #
  # @qemu.minor:  The minor version of QEMU
  #
  # @qemu.micro:  The micro version of QEMU.  By current convention, a micro
  #               version of 50 signifies a development branch.  A micro 
version
  #               greater than or equal to 90 signifies a release 
candidate for
  #               the next minor version.  A micro version of less than 50
  #               signifies a stable release.
  #
  # @package:     QEMU will always set this field to an empty string.  
Downstream
  #               versions of QEMU should set this to a non-empty 
string.  The
  #               exact format depends on the downstream however it highly
  #               recommended that a unique name is used.
  #
  # Since: 0.14.0
  ##
  { 'VersionInfo': {'qemu': {'major': 'int', 'minor': 'int', 'micro': 
'int'},
                    'package': 'str'} }

  ##
  # @query-version:
  #
  # Returns the current version of QEMU.
  #
  # Returns:  A @VersionInfo object describing the current version of QEMU.
  #
  # Since: 0.14.0
  ##
  [ 'query-version', {}, {}, 'VersionInfo' ]

The syntax the use of a dictionary instead of a list is an indication of a
typedef.  Within QEMU,  This will generate the following code in
qemu/qmp-types.h:

  typedef struct VersionInfo
  {
      struct {
          int64_t major;
          int64_t minor;
          int64_t micro;
      } qemu;
      const char *package;
      struct VersionInfo *next;
  } VersionInfo;

The use of a next pointer is to enable support for returning lists of 
complex
types.  The query-version command will generate the following signature:

  // qemu/qmp-types.h
  VersionInfo *qmp_alloc_version_info(void);
  void qmp_free_version_info(VersionInfo *obj);

  // qemu/qmp.h
  VersionInfo *qmp_query_version(Error **errp);

A typical implementation might look something like:

  VersionInfo *qmp_query_version(Error **errp)
  {
      VersionInfo *info = qmp_alloc_version_info();
      info->qemu.major = 0;
      info->qemu.minor = 14;
      info->qemu.micro = 92;
      info->package = qemu_strdup(" (qemu-kvm)");
      return info;
  }

== Optional Structure Members ==

Optional structure members can be specified by using a '*' as a prefix 
to the
member name.  For example:

  ##
  # @BlockDeviceInfo:
  #
  # Information about the backing device for a block device.
  #
  # @file: the filename of the backing device
  #
  # @ro: true if the backing device was open read-only
  #
  # @drv: the name of the block format used to open the backing dvice
  #
  # @backing_file: #optional the name of the backing file (for 
copy-on-write)
  #
  # @encrypted: true if the backing device is encrypted
  #
  # Since: 0.14.0
  #
  # Notes: This interface is only found in @BlockInfo.
  ##
  { 'BlockDeviceInfo': { 'file': 'str', 'ro': 'bool', 'drv': 'str',
                         '*backing_file': 'str', 'encrypted': 'bool' } }

A typical implementation may look like:

  BlockDeviceInfo *qmp_query_block_device_info(const char *device, Error 
**errp)
  {
      BlockDeviceInfo *info;
      BlockDriverState *bs;
      Error *local_err = NULL;

      bs = bdrv_find(device, &local_err);
      if (local_err) {
          error_propagate(errp, local_err);
          return NULL;
      }

      info->file = qemu_strdup(bs->filename);
      info->ro = bs->readonly;
      info->drv = qemu_strdup(bs->drv);
      info->encrypted = bs->encrypted;
      if (bs->backing_file[0]) {
          info->has_backing_file = true;
          info->backing_file = qemu_strdup(info->backing_file);
      }

      return info;
  }

== Enumeration Types ==

QAPI also supports enumeration types.  The following syntax is used:

  { 'VirtioNetTxStrategy': ['bh', 'timer'] }

A list of strings signifies a enumeration type.  The enumeration type will
generate the following code in qemu/qmp-types.h:

  typedef enum VirtioNetTxStrategy
  {
      VNTS_BH = 0,
      VNTS_TIMER,
  } VirtioNetTxStrategy;

Any use of the type in QEMU or in libqmp will use the enumeration.  At the
moment, the integer values of the enumeration are sent over the wire in 
order to
better support languages like Python that don't have enumeration types.

String to enumeration conversion functions will also be generated.

  // qemu/qmp-types.h
  VirtioNetTxStrategy qmp_virtio_net_tx_strategy_from_str(const char *str,
                                                          Error **errp);
  char *qmp_virtio_net_tx_strategy_to_str(VirtioNetTxStrategy value,
                                          Error **errp);

== Client Library ==

A client library will also be generated that makes use of qmp-types.h.  The
client library functions are very similar to the QEMU functions except they
reside in libqmp.h, use a different prefix, and take a QmpSession argument.
For instance, 'query-version' will generate:

  VersionInfo *libqmp_query_version(QmpSession *sess, Error **errp);

== QMP Server Discovery ==

QAPI has a standard mechanism to discover QMP servers.  By default, a QMP
session is always created in ~/.qemu/name-$name.sock or 
~/.qemu/pid-$pid.sock.
libqmp provides functions to enumerate the running guests and connect to 
a guest
by name or pid.

Regards,

Anthony Liguori

> [...]
>
>    

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

[Anthony Liguori]

On 02/15/2011 07:38 AM, Luiz Capitulino wrote:
> On Tue, 15 Feb 2011 10:20:01 +0100
> Kevin Wolf<kwolf@redhat.com>  wrote:
>
>    
>> Am 14.02.2011 20:34, schrieb Anthony Liguori:
>>      
>>> On 02/14/2011 12:34 PM, Luiz Capitulino wrote:
>>>        
>>>> On Mon, 14 Feb 2011 08:39:11 -0600
>>>> Anthony Liguori<anthony@codemonkey.ws>   wrote:
>>>>
>>>>
>>>>          
>>>>> On 02/14/2011 06:45 AM, Luiz Capitulino wrote:
>>>>>
>>>>>            
>>>>>> So the question is: how does the schema based design support extending
>>>>>> commands or events? Does it require adding new commands/events?
>>>>>>
>>>>>>
>>>>>>              
>>>>> Well, let me ask you, how do we do that today?
>>>>>
>>>>> Let's say that I want to add a new parameter to the `change' function so
>>>>> that I can include a salt parameter as part of the password.
>>>>>
>>>>> The way we'd do this today is by checking for the 'salt' parameter in
>>>>> qdict, and if it's not present, use a random salt or something like that.
>>>>>
>>>>>            
>>>> You likely want to do what you did before. Of course that you have to
>>>> consider if what you're doing is extending an existing command or badly
>>>> overloading it (like change is today), in this case you'll want to add
>>>> a new command instead.
>>>>
>>>> But yes, the use-case here is extending an existing command.
>>>>
>>>>
>>>>          
>>>>> However, if I'm a QMP client, how can I tell whether you're going to
>>>>> ignore my salt parameter or actually use it?  Nothing in QMP tells me
>>>>> this today.  If I set the salt parameter in the `change' command, I'll
>>>>> just get a success message.
>>>>>
>>>>>            
>>>> I'm sorry?
>>>>
>>>> { "execute": "change", "arguments": { "device": "vnc", "target": "password", "arg": "1234", "salt": "r1" } }
>>>> {"error": {"class": "InvalidParameter", "desc": "Invalid parameter 'salt'", "data": {"name": "salt"}}}
>>>>
>>>>          
>>> So I'm supposed to execute the command, and if execution fails, drop the
>>> new parameter?  If we add a few optional parameters, does that mean I
>>> have to try every possible combination of parameters?
>>>        
>> How is that different from trying out multiple commands? In the end, you
>> always need some meta information like a schema in order to avoid trying
>> out which parameters the server supports.
>>
>> Anyway, I think there's a second interesting point: Adding parameters
>> does cause these problems, but it's different for data sent from qemu to
>> the client (return values and events). If we add more information there,
>> an older client can just ignore it, without even looking at a schema.
>>
>> So I think we should consider this for return values and definitely do
>> it for events. Sending out five different messages for a single event
>> that are completely redundant and only differ in the number of fields is
>> just insane (okay, they wouldn't actually get on the wire because a
>> client registers only for one of them, but the code for generating them
>> must exist).
>>      
> That's my point when I asked about events in the other thread.
>    

Okay, I had confused myself about this.  It's not quite as bad as I had 
been saying.

One of the reasons to have generated allocation function is so that we 
can make sure to always pad structures.  Since all optional fields has a 
bool to indicate the fields presence, by setting the allocated structure 
to zero, we can support forwards compatibility for structures.

So we can add new members to structures with no libqmp compatibility 
problem.  It's only if we add new arguments to signals or commands that 
we will break the ABI.

If we expect to add fields later, we just have to make sure we use a 
structure to encapsulate things.

Regards,

Anthony Liguori

>>> You're arguing that we should extend commands by adding new parameters.
>>> I'm saying that's a bad interface.  If we need to change a command, we
>>> should introduce a new command.  It's a well understood mechanism for
>>> maintaining compatibility (just about every C library does exactly this).
>>>        
>> I'm yet undecided about adding parameters. I have a feeling that you
>> might be right here.
>>
>> Kevin
>>
>>      
>
>    

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

[Kevin Wolf]

Am 16.02.2011 01:59, schrieb Anthony Liguori:
> On 02/15/2011 07:38 AM, Luiz Capitulino wrote:
>> On Tue, 15 Feb 2011 10:20:01 +0100
>> Kevin Wolf<kwolf@redhat.com>  wrote:
>>
>>    
>>> Am 14.02.2011 20:34, schrieb Anthony Liguori:
>>>      
>>>> On 02/14/2011 12:34 PM, Luiz Capitulino wrote:
>>>>        
>>>>> On Mon, 14 Feb 2011 08:39:11 -0600
>>>>> Anthony Liguori<anthony@codemonkey.ws>   wrote:
>>>>>
>>>>>
>>>>>          
>>>>>> On 02/14/2011 06:45 AM, Luiz Capitulino wrote:
>>>>>>
>>>>>>            
>>>>>>> So the question is: how does the schema based design support extending
>>>>>>> commands or events? Does it require adding new commands/events?
>>>>>>>
>>>>>>>
>>>>>>>              
>>>>>> Well, let me ask you, how do we do that today?
>>>>>>
>>>>>> Let's say that I want to add a new parameter to the `change' function so
>>>>>> that I can include a salt parameter as part of the password.
>>>>>>
>>>>>> The way we'd do this today is by checking for the 'salt' parameter in
>>>>>> qdict, and if it's not present, use a random salt or something like that.
>>>>>>
>>>>>>            
>>>>> You likely want to do what you did before. Of course that you have to
>>>>> consider if what you're doing is extending an existing command or badly
>>>>> overloading it (like change is today), in this case you'll want to add
>>>>> a new command instead.
>>>>>
>>>>> But yes, the use-case here is extending an existing command.
>>>>>
>>>>>
>>>>>          
>>>>>> However, if I'm a QMP client, how can I tell whether you're going to
>>>>>> ignore my salt parameter or actually use it?  Nothing in QMP tells me
>>>>>> this today.  If I set the salt parameter in the `change' command, I'll
>>>>>> just get a success message.
>>>>>>
>>>>>>            
>>>>> I'm sorry?
>>>>>
>>>>> { "execute": "change", "arguments": { "device": "vnc", "target": "password", "arg": "1234", "salt": "r1" } }
>>>>> {"error": {"class": "InvalidParameter", "desc": "Invalid parameter 'salt'", "data": {"name": "salt"}}}
>>>>>
>>>>>          
>>>> So I'm supposed to execute the command, and if execution fails, drop the
>>>> new parameter?  If we add a few optional parameters, does that mean I
>>>> have to try every possible combination of parameters?
>>>>        
>>> How is that different from trying out multiple commands? In the end, you
>>> always need some meta information like a schema in order to avoid trying
>>> out which parameters the server supports.
>>>
>>> Anyway, I think there's a second interesting point: Adding parameters
>>> does cause these problems, but it's different for data sent from qemu to
>>> the client (return values and events). If we add more information there,
>>> an older client can just ignore it, without even looking at a schema.
>>>
>>> So I think we should consider this for return values and definitely do
>>> it for events. Sending out five different messages for a single event
>>> that are completely redundant and only differ in the number of fields is
>>> just insane (okay, they wouldn't actually get on the wire because a
>>> client registers only for one of them, but the code for generating them
>>> must exist).
>>>      
>> That's my point when I asked about events in the other thread.
>>    
> 
> Okay, I had confused myself about this.  It's not quite as bad as I had 
> been saying.
> 
> One of the reasons to have generated allocation function is so that we 
> can make sure to always pad structures.  Since all optional fields has a 
> bool to indicate the fields presence, by setting the allocated structure 
> to zero, we can support forwards compatibility for structures.

I think in most cases we would even get away with a default value
instead of the bool. For example for strings, NULL would be a very clear
indication that the field wasn't there in the JSON message. I think it
works also for most (if not all) integer fields, but maybe having a bool
has_* field is safer there.

Or maybe we can add an indication to the schema that specifies if a
default value should be generated or (for the rare cases where it's
needed) a has_* field is added.

> So we can add new members to structures with no libqmp compatibility 
> problem.  It's only if we add new arguments to signals or commands that 
> we will break the ABI.

Yeah, that makes sense.

> If we expect to add fields later, we just have to make sure we use a 
> structure to encapsulate things.

As stated before, I think we should use structures for all events. I
still don't understand why we should have an exception for events. Any
other command returns structures, too, and you don't automagically pull
their fields up one level anywhere except for events.

Kevin

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

[Anthony Liguori]

On 02/16/2011 02:50 AM, Kevin Wolf wrote:
> Am 16.02.2011 01:59, schrieb Anthony Liguori:
>    
>> On 02/15/2011 07:38 AM, Luiz Capitulino wrote:
>>      
>>> On Tue, 15 Feb 2011 10:20:01 +0100
>>> Kevin Wolf<kwolf@redhat.com>   wrote:
>>>
>>>
>>>        
>>>> Am 14.02.2011 20:34, schrieb Anthony Liguori:
>>>>
>>>>          
>>>>> On 02/14/2011 12:34 PM, Luiz Capitulino wrote:
>>>>>
>>>>>            
>>>>>> On Mon, 14 Feb 2011 08:39:11 -0600
>>>>>> Anthony Liguori<anthony@codemonkey.ws>    wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>>              
>>>>>>> On 02/14/2011 06:45 AM, Luiz Capitulino wrote:
>>>>>>>
>>>>>>>
>>>>>>>                
>>>>>>>> So the question is: how does the schema based design support extending
>>>>>>>> commands or events? Does it require adding new commands/events?
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>                  
>>>>>>> Well, let me ask you, how do we do that today?
>>>>>>>
>>>>>>> Let's say that I want to add a new parameter to the `change' function so
>>>>>>> that I can include a salt parameter as part of the password.
>>>>>>>
>>>>>>> The way we'd do this today is by checking for the 'salt' parameter in
>>>>>>> qdict, and if it's not present, use a random salt or something like that.
>>>>>>>
>>>>>>>
>>>>>>>                
>>>>>> You likely want to do what you did before. Of course that you have to
>>>>>> consider if what you're doing is extending an existing command or badly
>>>>>> overloading it (like change is today), in this case you'll want to add
>>>>>> a new command instead.
>>>>>>
>>>>>> But yes, the use-case here is extending an existing command.
>>>>>>
>>>>>>
>>>>>>
>>>>>>              
>>>>>>> However, if I'm a QMP client, how can I tell whether you're going to
>>>>>>> ignore my salt parameter or actually use it?  Nothing in QMP tells me
>>>>>>> this today.  If I set the salt parameter in the `change' command, I'll
>>>>>>> just get a success message.
>>>>>>>
>>>>>>>
>>>>>>>                
>>>>>> I'm sorry?
>>>>>>
>>>>>> { "execute": "change", "arguments": { "device": "vnc", "target": "password", "arg": "1234", "salt": "r1" } }
>>>>>> {"error": {"class": "InvalidParameter", "desc": "Invalid parameter 'salt'", "data": {"name": "salt"}}}
>>>>>>
>>>>>>
>>>>>>              
>>>>> So I'm supposed to execute the command, and if execution fails, drop the
>>>>> new parameter?  If we add a few optional parameters, does that mean I
>>>>> have to try every possible combination of parameters?
>>>>>
>>>>>            
>>>> How is that different from trying out multiple commands? In the end, you
>>>> always need some meta information like a schema in order to avoid trying
>>>> out which parameters the server supports.
>>>>
>>>> Anyway, I think there's a second interesting point: Adding parameters
>>>> does cause these problems, but it's different for data sent from qemu to
>>>> the client (return values and events). If we add more information there,
>>>> an older client can just ignore it, without even looking at a schema.
>>>>
>>>> So I think we should consider this for return values and definitely do
>>>> it for events. Sending out five different messages for a single event
>>>> that are completely redundant and only differ in the number of fields is
>>>> just insane (okay, they wouldn't actually get on the wire because a
>>>> client registers only for one of them, but the code for generating them
>>>> must exist).
>>>>
>>>>          
>>> That's my point when I asked about events in the other thread.
>>>
>>>        
>> Okay, I had confused myself about this.  It's not quite as bad as I had
>> been saying.
>>
>> One of the reasons to have generated allocation function is so that we
>> can make sure to always pad structures.  Since all optional fields has a
>> bool to indicate the fields presence, by setting the allocated structure
>> to zero, we can support forwards compatibility for structures.
>>      
> I think in most cases we would even get away with a default value
> instead of the bool. For example for strings, NULL would be a very clear
> indication that the field wasn't there in the JSON message.

In order to support forwards compatibility, we need to have a zero-value 
for non-presence.  For strings or pointers, NULL would work very well.

But for integers, I'm not sure we can reasonably use 0 as a universal 
default value.  We could just use has_ fields for non-pointers but I 
figured consistency would make the code more robust since it's hard to 
tell that a field is really optional vs. required.  For instance:

typedef struct BlockInfo {
     const char *device_name;
     bool has_backing_file;
     const char *backing_file;
} BlockInfo;

It's very obvious that backing_file is optional.  If you don't set 
has_backing_file (because you're incorrectly treating backing_file is 
required), it will fail immediately as the field won't be marshalled.

OTOH:

typedef struct BlockInfo {
      const char *device_name;
      // optional
      const char *backing_file;
} BlockInfo;

Is a bit easier to screw up.  If you happen to not do the NULL check and 
work with a client that's sending it, you can end up with a NULL pointer 
dereference pretty easily.

>> If we expect to add fields later, we just have to make sure we use a
>> structure to encapsulate things.
>>      
> As stated before, I think we should use structures for all events. I
> still don't understand why we should have an exception for events. Any
> other command returns structures, too, and you don't automagically pull
> their fields up one level anywhere except for events.
>    

That's not entirely true.   For human-monitor-command, we return a bare 
string.  For all other commands, we return structures specifically to 
enable better forwards compatibility.

I do agree that for most of our events, we should be using a structure 
for passing information.  That's not what we're doing today but there's 
only a couple events that are even doing that so fixing it won't be that 
bad.

Regards,

Anthony Liguori

> Kevin
>
>    

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

[Kevin Wolf]

Am 16.02.2011 14:43, schrieb Anthony Liguori:
> On 02/16/2011 02:50 AM, Kevin Wolf wrote:
>> Am 16.02.2011 01:59, schrieb Anthony Liguori:
>>    
>>> On 02/15/2011 07:38 AM, Luiz Capitulino wrote:
>>>      
>>>> On Tue, 15 Feb 2011 10:20:01 +0100
>>>> Kevin Wolf<kwolf@redhat.com>   wrote:
>>>>
>>>>
>>>>        
>>>>> Am 14.02.2011 20:34, schrieb Anthony Liguori:
>>>>>
>>>>>          
>>>>>> On 02/14/2011 12:34 PM, Luiz Capitulino wrote:
>>>>>>
>>>>>>            
>>>>>>> On Mon, 14 Feb 2011 08:39:11 -0600
>>>>>>> Anthony Liguori<anthony@codemonkey.ws>    wrote:
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>              
>>>>>>>> On 02/14/2011 06:45 AM, Luiz Capitulino wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>                
>>>>>>>>> So the question is: how does the schema based design support extending
>>>>>>>>> commands or events? Does it require adding new commands/events?
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>                  
>>>>>>>> Well, let me ask you, how do we do that today?
>>>>>>>>
>>>>>>>> Let's say that I want to add a new parameter to the `change' function so
>>>>>>>> that I can include a salt parameter as part of the password.
>>>>>>>>
>>>>>>>> The way we'd do this today is by checking for the 'salt' parameter in
>>>>>>>> qdict, and if it's not present, use a random salt or something like that.
>>>>>>>>
>>>>>>>>
>>>>>>>>                
>>>>>>> You likely want to do what you did before. Of course that you have to
>>>>>>> consider if what you're doing is extending an existing command or badly
>>>>>>> overloading it (like change is today), in this case you'll want to add
>>>>>>> a new command instead.
>>>>>>>
>>>>>>> But yes, the use-case here is extending an existing command.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>              
>>>>>>>> However, if I'm a QMP client, how can I tell whether you're going to
>>>>>>>> ignore my salt parameter or actually use it?  Nothing in QMP tells me
>>>>>>>> this today.  If I set the salt parameter in the `change' command, I'll
>>>>>>>> just get a success message.
>>>>>>>>
>>>>>>>>
>>>>>>>>                
>>>>>>> I'm sorry?
>>>>>>>
>>>>>>> { "execute": "change", "arguments": { "device": "vnc", "target": "password", "arg": "1234", "salt": "r1" } }
>>>>>>> {"error": {"class": "InvalidParameter", "desc": "Invalid parameter 'salt'", "data": {"name": "salt"}}}
>>>>>>>
>>>>>>>
>>>>>>>              
>>>>>> So I'm supposed to execute the command, and if execution fails, drop the
>>>>>> new parameter?  If we add a few optional parameters, does that mean I
>>>>>> have to try every possible combination of parameters?
>>>>>>
>>>>>>            
>>>>> How is that different from trying out multiple commands? In the end, you
>>>>> always need some meta information like a schema in order to avoid trying
>>>>> out which parameters the server supports.
>>>>>
>>>>> Anyway, I think there's a second interesting point: Adding parameters
>>>>> does cause these problems, but it's different for data sent from qemu to
>>>>> the client (return values and events). If we add more information there,
>>>>> an older client can just ignore it, without even looking at a schema.
>>>>>
>>>>> So I think we should consider this for return values and definitely do
>>>>> it for events. Sending out five different messages for a single event
>>>>> that are completely redundant and only differ in the number of fields is
>>>>> just insane (okay, they wouldn't actually get on the wire because a
>>>>> client registers only for one of them, but the code for generating them
>>>>> must exist).
>>>>>
>>>>>          
>>>> That's my point when I asked about events in the other thread.
>>>>
>>>>        
>>> Okay, I had confused myself about this.  It's not quite as bad as I had
>>> been saying.
>>>
>>> One of the reasons to have generated allocation function is so that we
>>> can make sure to always pad structures.  Since all optional fields has a
>>> bool to indicate the fields presence, by setting the allocated structure
>>> to zero, we can support forwards compatibility for structures.
>>>      
>> I think in most cases we would even get away with a default value
>> instead of the bool. For example for strings, NULL would be a very clear
>> indication that the field wasn't there in the JSON message.
> 
> In order to support forwards compatibility, we need to have a zero-value 
> for non-presence.  For strings or pointers, NULL would work very well.

What's the kind of compatibility you're concerned about? I was mainly
considering older clients communicating with newer qemu, i.e.
compatibility on the protocol level. The library can set default values
for fields that are not present in JSON messages it receives.

Your point is older applications using a newer library? Or newer ones
using an older lib (this one doesn't make sense, imho)? Or something
entirely different?

For older applications the exact value shouldn't matter, because they
don't know the field and don't even look at it.

> But for integers, I'm not sure we can reasonably use 0 as a universal 
> default value.  We could just use has_ fields for non-pointers but I 
> figured consistency would make the code more robust since it's hard to 
> tell that a field is really optional vs. required.  For instance:
> 
> typedef struct BlockInfo {
>      const char *device_name;
>      bool has_backing_file;
>      const char *backing_file;
> } BlockInfo;
> 
> It's very obvious that backing_file is optional.  If you don't set 
> has_backing_file (because you're incorrectly treating backing_file is 
> required), it will fail immediately as the field won't be marshalled.
> 
> OTOH:
> 
> typedef struct BlockInfo {
>       const char *device_name;
>       // optional
>       const char *backing_file;
> } BlockInfo;
> 
> Is a bit easier to screw up.  If you happen to not do the NULL check and 
> work with a client that's sending it, you can end up with a NULL pointer 
> dereference pretty easily.

You can also forget to check has_backing_file, so I don't think there's
a real difference between the both. Either you know and consider that
it's optional or you don't.

>>> If we expect to add fields later, we just have to make sure we use a
>>> structure to encapsulate things.
>>>      
>> As stated before, I think we should use structures for all events. I
>> still don't understand why we should have an exception for events. Any
>> other command returns structures, too, and you don't automagically pull
>> their fields up one level anywhere except for events.
>>    
> 
> That's not entirely true.   For human-monitor-command, we return a bare 
> string.  For all other commands, we return structures specifically to 
> enable better forwards compatibility.
> 
> I do agree that for most of our events, we should be using a structure 
> for passing information.  That's not what we're doing today but there's 
> only a couple events that are even doing that so fixing it won't be that 
> bad.

Right, you could still have something like this (although I'm not sure
if it's very useful):

[ 'block-io-event', {}, {}, 'string' ]

What I think isn't a good idea is that the following definition doesn't
generate a structure in your original proposal. This should really
generate a structure:

{ 'BlockIOEvent': {'device': 'string', 'action': 'string', 'operation':
'string'} }
[ 'block-io-event', {}, {}, 'BlockIOEvent' ]

Kevin

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

[Anthony Liguori]

On 02/16/2011 08:15 AM, Kevin Wolf wrote:
> Am 16.02.2011 14:43, schrieb Anthony Liguori:
>    
>> On 02/16/2011 02:50 AM, Kevin Wolf wrote:
>>      
>>> Am 16.02.2011 01:59, schrieb Anthony Liguori:
>>>
>>>        
>>>> On 02/15/2011 07:38 AM, Luiz Capitulino wrote:
>>>>
>>>>          
>>>>> On Tue, 15 Feb 2011 10:20:01 +0100
>>>>> Kevin Wolf<kwolf@redhat.com>    wrote:
>>>>>
>>>>>
>>>>>
>>>>>            
>>>>>> Am 14.02.2011 20:34, schrieb Anthony Liguori:
>>>>>>
>>>>>>
>>>>>>              
>>>>>>> On 02/14/2011 12:34 PM, Luiz Capitulino wrote:
>>>>>>>
>>>>>>>
>>>>>>>                
>>>>>>>> On Mon, 14 Feb 2011 08:39:11 -0600
>>>>>>>> Anthony Liguori<anthony@codemonkey.ws>     wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>                  
>>>>>>>>> On 02/14/2011 06:45 AM, Luiz Capitulino wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>                    
>>>>>>>>>> So the question is: how does the schema based design support extending
>>>>>>>>>> commands or events? Does it require adding new commands/events?
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>                      
>>>>>>>>> Well, let me ask you, how do we do that today?
>>>>>>>>>
>>>>>>>>> Let's say that I want to add a new parameter to the `change' function so
>>>>>>>>> that I can include a salt parameter as part of the password.
>>>>>>>>>
>>>>>>>>> The way we'd do this today is by checking for the 'salt' parameter in
>>>>>>>>> qdict, and if it's not present, use a random salt or something like that.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>                    
>>>>>>>> You likely want to do what you did before. Of course that you have to
>>>>>>>> consider if what you're doing is extending an existing command or badly
>>>>>>>> overloading it (like change is today), in this case you'll want to add
>>>>>>>> a new command instead.
>>>>>>>>
>>>>>>>> But yes, the use-case here is extending an existing command.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>                  
>>>>>>>>> However, if I'm a QMP client, how can I tell whether you're going to
>>>>>>>>> ignore my salt parameter or actually use it?  Nothing in QMP tells me
>>>>>>>>> this today.  If I set the salt parameter in the `change' command, I'll
>>>>>>>>> just get a success message.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>                    
>>>>>>>> I'm sorry?
>>>>>>>>
>>>>>>>> { "execute": "change", "arguments": { "device": "vnc", "target": "password", "arg": "1234", "salt": "r1" } }
>>>>>>>> {"error": {"class": "InvalidParameter", "desc": "Invalid parameter 'salt'", "data": {"name": "salt"}}}
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>                  
>>>>>>> So I'm supposed to execute the command, and if execution fails, drop the
>>>>>>> new parameter?  If we add a few optional parameters, does that mean I
>>>>>>> have to try every possible combination of parameters?
>>>>>>>
>>>>>>>
>>>>>>>                
>>>>>> How is that different from trying out multiple commands? In the end, you
>>>>>> always need some meta information like a schema in order to avoid trying
>>>>>> out which parameters the server supports.
>>>>>>
>>>>>> Anyway, I think there's a second interesting point: Adding parameters
>>>>>> does cause these problems, but it's different for data sent from qemu to
>>>>>> the client (return values and events). If we add more information there,
>>>>>> an older client can just ignore it, without even looking at a schema.
>>>>>>
>>>>>> So I think we should consider this for return values and definitely do
>>>>>> it for events. Sending out five different messages for a single event
>>>>>> that are completely redundant and only differ in the number of fields is
>>>>>> just insane (okay, they wouldn't actually get on the wire because a
>>>>>> client registers only for one of them, but the code for generating them
>>>>>> must exist).
>>>>>>
>>>>>>
>>>>>>              
>>>>> That's my point when I asked about events in the other thread.
>>>>>
>>>>>
>>>>>            
>>>> Okay, I had confused myself about this.  It's not quite as bad as I had
>>>> been saying.
>>>>
>>>> One of the reasons to have generated allocation function is so that we
>>>> can make sure to always pad structures.  Since all optional fields has a
>>>> bool to indicate the fields presence, by setting the allocated structure
>>>> to zero, we can support forwards compatibility for structures.
>>>>
>>>>          
>>> I think in most cases we would even get away with a default value
>>> instead of the bool. For example for strings, NULL would be a very clear
>>> indication that the field wasn't there in the JSON message.
>>>        
>> In order to support forwards compatibility, we need to have a zero-value
>> for non-presence.  For strings or pointers, NULL would work very well.
>>      
> What's the kind of compatibility you're concerned about? I was mainly
> considering older clients communicating with newer qemu, i.e.
> compatibility on the protocol level.

The has_XX fields are not sent over the wire.

>   The library can set default values
> for fields that are not present in JSON messages it receives.
>
> Your point is older applications using a newer library?

Compiled against a new library, but running against an old library.

This has to be supported in order to avoid bumping the library version.

>> That's not entirely true.   For human-monitor-command, we return a bare
>> string.  For all other commands, we return structures specifically to
>> enable better forwards compatibility.
>>
>> I do agree that for most of our events, we should be using a structure
>> for passing information.  That's not what we're doing today but there's
>> only a couple events that are even doing that so fixing it won't be that
>> bad.
>>      
> Right, you could still have something like this (although I'm not sure
> if it's very useful):
>
> [ 'block-io-event', {}, {}, 'string' ]
>
> What I think isn't a good idea is that the following definition doesn't
> generate a structure in your original proposal. This should really
> generate a structure:
>
> { 'BlockIOEvent': {'device': 'string', 'action': 'string', 'operation':
> 'string'} }
> [ 'block-io-event', {}, {}, 'BlockIOEvent' ]
>    

Actually, I think it's better to explicitly call out the structure name 
so that you can do things like:

{ 'VncConnectedEvent': {'info': 'VncClientInfo'} }

Which happens to be the same structure used by 'query-vnc'

Regards,

Anthony Liguori

> Kevin
>
>    

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

[Anthony Liguori]

On 02/16/2011 08:15 AM, Kevin Wolf wrote:
> Am 16.02.2011 14:43, schrieb Anthony Liguori:
>    
>> On 02/16/2011 02:50 AM, Kevin Wolf wrote:
>>      
>>> Am 16.02.2011 01:59, schrieb Anthony Liguori:
>>>
>>>        
>>>> On 02/15/2011 07:38 AM, Luiz Capitulino wrote:
>>>>
>>>>          
>>>>> On Tue, 15 Feb 2011 10:20:01 +0100
>>>>> Kevin Wolf<kwolf@redhat.com>    wrote:
>>>>>
>>>>>
>>>>>
>>>>>            
>>>>>> Am 14.02.2011 20:34, schrieb Anthony Liguori:
>>>>>>
>>>>>>
>>>>>>              
>>>>>>> On 02/14/2011 12:34 PM, Luiz Capitulino wrote:
>>>>>>>
>>>>>>>
>>>>>>>                
>>>>>>>> On Mon, 14 Feb 2011 08:39:11 -0600
>>>>>>>> Anthony Liguori<anthony@codemonkey.ws>     wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>                  
>>>>>>>>> On 02/14/2011 06:45 AM, Luiz Capitulino wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>                    
>>>>>>>>>> So the question is: how does the schema based design support extending
>>>>>>>>>> commands or events? Does it require adding new commands/events?
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>                      
>>>>>>>>> Well, let me ask you, how do we do that today?
>>>>>>>>>
>>>>>>>>> Let's say that I want to add a new parameter to the `change' function so
>>>>>>>>> that I can include a salt parameter as part of the password.
>>>>>>>>>
>>>>>>>>> The way we'd do this today is by checking for the 'salt' parameter in
>>>>>>>>> qdict, and if it's not present, use a random salt or something like that.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>                    
>>>>>>>> You likely want to do what you did before. Of course that you have to
>>>>>>>> consider if what you're doing is extending an existing command or badly
>>>>>>>> overloading it (like change is today), in this case you'll want to add
>>>>>>>> a new command instead.
>>>>>>>>
>>>>>>>> But yes, the use-case here is extending an existing command.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>                  
>>>>>>>>> However, if I'm a QMP client, how can I tell whether you're going to
>>>>>>>>> ignore my salt parameter or actually use it?  Nothing in QMP tells me
>>>>>>>>> this today.  If I set the salt parameter in the `change' command, I'll
>>>>>>>>> just get a success message.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>                    
>>>>>>>> I'm sorry?
>>>>>>>>
>>>>>>>> { "execute": "change", "arguments": { "device": "vnc", "target": "password", "arg": "1234", "salt": "r1" } }
>>>>>>>> {"error": {"class": "InvalidParameter", "desc": "Invalid parameter 'salt'", "data": {"name": "salt"}}}
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>                  
>>>>>>> So I'm supposed to execute the command, and if execution fails, drop the
>>>>>>> new parameter?  If we add a few optional parameters, does that mean I
>>>>>>> have to try every possible combination of parameters?
>>>>>>>
>>>>>>>
>>>>>>>                
>>>>>> How is that different from trying out multiple commands? In the end, you
>>>>>> always need some meta information like a schema in order to avoid trying
>>>>>> out which parameters the server supports.
>>>>>>
>>>>>> Anyway, I think there's a second interesting point: Adding parameters
>>>>>> does cause these problems, but it's different for data sent from qemu to
>>>>>> the client (return values and events). If we add more information there,
>>>>>> an older client can just ignore it, without even looking at a schema.
>>>>>>
>>>>>> So I think we should consider this for return values and definitely do
>>>>>> it for events. Sending out five different messages for a single event
>>>>>> that are completely redundant and only differ in the number of fields is
>>>>>> just insane (okay, they wouldn't actually get on the wire because a
>>>>>> client registers only for one of them, but the code for generating them
>>>>>> must exist).
>>>>>>
>>>>>>
>>>>>>              
>>>>> That's my point when I asked about events in the other thread.
>>>>>
>>>>>
>>>>>            
>>>> Okay, I had confused myself about this.  It's not quite as bad as I had
>>>> been saying.
>>>>
>>>> One of the reasons to have generated allocation function is so that we
>>>> can make sure to always pad structures.  Since all optional fields has a
>>>> bool to indicate the fields presence, by setting the allocated structure
>>>> to zero, we can support forwards compatibility for structures.
>>>>
>>>>          
>>> I think in most cases we would even get away with a default value
>>> instead of the bool. For example for strings, NULL would be a very clear
>>> indication that the field wasn't there in the JSON message.
>>>        
>> In order to support forwards compatibility, we need to have a zero-value
>> for non-presence.  For strings or pointers, NULL would work very well.
>>      
> What's the kind of compatibility you're concerned about? I was mainly
> considering older clients communicating with newer qemu, i.e.
> compatibility on the protocol level.

The has_XX fields are not sent over the wire.

>   The library can set default values
> for fields that are not present in JSON messages it receives.
>
> Your point is older applications using a newer library?

Compiled against a new library, but running against an old library.

This has to be supported in order to avoid bumping the library version.

>> That's not entirely true.   For human-monitor-command, we return a bare
>> string.  For all other commands, we return structures specifically to
>> enable better forwards compatibility.
>>
>> I do agree that for most of our events, we should be using a structure
>> for passing information.  That's not what we're doing today but there's
>> only a couple events that are even doing that so fixing it won't be that
>> bad.
>>      
> Right, you could still have something like this (although I'm not sure
> if it's very useful):
>
> [ 'block-io-event', {}, {}, 'string' ]
>
> What I think isn't a good idea is that the following definition doesn't
> generate a structure in your original proposal. This should really
> generate a structure:
>
> { 'BlockIOEvent': {'device': 'string', 'action': 'string', 'operation':
> 'string'} }
> [ 'block-io-event', {}, {}, 'BlockIOEvent' ]
>    

Actually, I think it's better to explicitly call out the structure name 
so that you can do things like:

{ 'VncConnectedEvent': {'info': 'VncClientInfo'} }

Which happens to be the same structure used by 'query-vnc'

Regards,

Anthony Liguori

> Kevin
>
>