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

[Avi Kivity]

On 03/10/2011 04:12 PM, Anthony Liguori wrote:
> On 03/10/2011 06:39 AM, Avi Kivity wrote:
> > 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 } }
>
> Yeah, I understand, it just doesn't fit the model quite as well of 
> signal accessors.
>
> Normally, in a signal/slot event model, you'd have some notion of an 
> object model and/or hierarchy.  For instance, with dbus, you'd do 
> something like:
>
> bus = dbus.SystemBus()
> hal = # magic to get a hal object
> device = hal.FindDeviceByCapability('media.storage')
>
> device.connect_to_signal('media-changed', fn)
>
> We don't have objects and I don't mean to introduce them, but I like 
> the idea of treating signals as objects and returning them via an 
> accessor function.
>
> So the idea is that the handle is a marshalled form of the signal object.

It's not a marshalled form, it's an opaque handle.  A marshalled form 
doesn't destroy information.

Anyway it would work with a client-provided tag just as well.  
connect_to_signal() could manufacture one and provide it to the server.

> > > { '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?
>
> How we marshal an object depends on the RPC layer.
>
> We marshal events on the wire as an integer handle.  That is only a 
> concept within the wire protocol.

I don't think it's an accurate description.  We marshall an event as a 
json object according to the schema above (BLOCK_IO_ERROR).  How we 
marshall a subscription to an event, or an unsubscription to an event, 
depends on how we specify the protocol.  It has nothing to do with 
client or server internal rpc stubs.

> We could just as easily return an object but without diving into JSON 
> class hinting, it'd be pretty meaningless because we'd just return "{ 
> 'handle': 32}" instead of "32".

Right, I suggest we return nothing at all.  Instead the client provides 
the handle.

> > > 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?
>
> > { 'execute': 'get-block-io-error-event' }
> < { 'return' : 32 }
> ...
> < { 'event': 'BLOCK_IO_ERROR', 'data': { 'action': 'stop', 'device': 
> 'ide0-hd0', 'operation': 'read' }, 'tag': 32 }
> ...
> > { 'execute': 'put-event', 'arguments': { 'tag': 32 } }

Well, I may be biased, but I prefer my variant.

btw, it's good to decree that a subscription is immediately followed by 
an event with the current state (which means events have to provide 
state and be idempotent); so the subscribe-and-query pattern is provided 
automatically.

btw2, I now nominate subscribe and unsubscribe as replacements for get 
and put.

> > > 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?
>
> Yes, it basically enables the signal for the session.

Okay, the subscription command.

> > So why not have the signal accessor provide the tag?  Like execute: 
> > blah provides a tag?
>
> How would this map to a C API?  You'd either have to completely drop 
> the notion of signal objects and use a separate mechanism to register 
> callbacks against a tag (and lose type safety) or do some major 
> munging to have the C API take a radically different signature than 
> the wire protocol.

A C API could create and maintain tags under the covers (an int that 
keeps increasing would do).

> > > 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.
>
> No, it's the opposite for me.  We design a good C API and then figure 
> out how to make it work well as a wire protocol.  The whole point of 
> this effort is to build an API that we can consume within QEMU such 
> that we can start breaking large chunks of code out of the main 
> executable.

That makes a C centric wire protocol.  Clients don't have to be C.

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