qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Kevin Wolf
Subject: Re: [Qemu-devel] Re: [RFC] qapi: events in QMP
Date: Mon, 14 Feb 2011 13:32:45 +0100
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.1.15) Gecko/20101027 Fedora/3.0.10-1.fc12 Thunderbird/3.0.10

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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]