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

[Anthony Liguori <anthony@codemonkey.ws>]

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.

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

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)
{
}

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

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.

Regards,

Anthony Liguori