Re: [Qemu-devel] [PATCH 19/22] qapi: add QMP put-event command

[Avi Kivity <avi@redhat.com>]

On 03/09/2011 04:26 PM, Anthony Liguori wrote:
> On 03/09/2011 07:58 AM, Avi Kivity wrote:
>> On 03/09/2011 03:48 PM, Anthony Liguori wrote:
>>>>> +[ 'put-event', {'tag': 'int'}, {}, 'none' ]
>>>>
>>>> Why is tag an int?
>>> +##
>>>
>>> It's a handle so the type doesn't matter as long as I can make sure 
>>> values are unique.  ints are easier to work with because they don't 
>>> require memory allocation.
>>
>> I think it's nicer for the client to use a string.  Instead of a 
>> global ID allocator, it can use unique IDs or unique prefixes + local 
>> IDs.  Should also aid a little in debugging.
>
> handle's are opaque to clients.
>
> I don't have a huge objection to using strings and it may make sense 
> to do a prefix like you're suggesting.  I won't do that initially but 
> since handles are opaque, we can make that change later.

What I mean is that the client should specify the handle, like it does 
for everything else it gives a name (netdevs, blockdevs, SCM_RIGHT fds, 
etc).

   { execute: listen-event, arguments: { event: blah, id: blah00001 } }
   { execute: unlisten-event arguments: { id: blah00001 } }

>
>>>>   don't we use strings for command ids and similar?
>>>
>>> id's can be any valid JSON value.
>>>
>>> But a handle is not the same thing as an id.
>>
>> Why not?
>>
>> I hope handles are client-provided?
>
> No, they are generated by the server.  It makes sense because really a 
> handle is a marshalled version of a signal.  The signal accessor looks 
> something like this:
>
> { 'BLOCK_IO_ERROR': { 'device': 'str', 'action': 'str', 'operation': 
> 'str' } }
> [ 'get-block-io-error-event': {}, 'BLOCK_IO_ERROR' }
>
> The way we marshal a 'BLOCK_IO_ERROR' type is by generating a unique 
> handle and returning that.

I don't follow at all.  Where's the handle here?  Why don't we return 
the BLOCK_IO_ERROR as an object, on the wire?

>
> While this looks like an int on the wire, at both the server and 
> libqmp level, it looks like a BlockIoErrorEvent object.  So in QEMU:
>
> BlockIoErrorEvent *qmp_get_block_io_error_event(Error **errp)
> {
> }
>
> And in libqmp:
>
> BlockIoErrorEvent *libqmp_get_block_io_error_event(QmpSession *sess, 
> Error **errp)
> {
> }

What would the wire exchange look like?

>
>>>> Also could be better named, disconnect-event or unlisten-event.
>>>
>>> I was going for symmetry with the signal accessors which are 
>>> typically in the format 'get-block-io-error-event'.
>>>
>>> Maybe it would be better to do 'connect-block-io-error-event' and 
>>> 'disconnect-event'?
>>
>> Yes.
>>
>> But I'm confused, do we have a per-event command on the wire?  Or 
>> just C stubs?
>
> Ignoring default events, you'll never see an event until you execute a 
> signal accessor function.  When you execute this function, you will 
> start receiving the events and those events will carry a tag 
> containing the handle returned by the signal accessor.

A "signal accessor" is a command to start listening to a signal?

So why not have the signal accessor provide the tag?  Like execute: blah 
provides a tag?

>
> Within libqmp, any time you execute a signal accessor, a new signal 
> object is created of the appropriate type.  When that object is 
> destroyed, you send a put-event to stop receiving the signal.
>
> When you connect to a signal object (via libqmp), you don't execute 
> the signal accessor because the object is already receiving the signal.
>
> Default events (which exist to preserve compatibility) are a set of 
> events that are automatically connected to after qmp_capabilities is 
> executed.  Because these connections are implicit, they arrive without 
> a handle in the event object.
>
> At this point, libqmp just ignores default events.  In the future, I'd 
> like to add a command that can be executed before qmp_capabilities 
> that will avoid connecting to default events.

I'm really confused.  Part of that is because the conversation mixes 
libqmp, server API, and wire protocol.  I'd like to understand the wire 
protocol first, everything else follows from that.

-- 
error compiling committee.c: too many arguments to function