From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=53244 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Pxgna-00067u-KL for qemu-devel@nongnu.org; Thu, 10 Mar 2011 09:25:08 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PxgnZ-0002yO-2a for qemu-devel@nongnu.org; Thu, 10 Mar 2011 09:25:06 -0500 Received: from mx1.redhat.com ([209.132.183.28]:13235) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PxgnY-0002y7-QD for qemu-devel@nongnu.org; Thu, 10 Mar 2011 09:25:05 -0500 Message-ID: <4D78DF3A.9000600@redhat.com> Date: Thu, 10 Mar 2011 16:24:58 +0200 From: Avi Kivity MIME-Version: 1.0 Subject: Re: [Qemu-devel] [PATCH 19/22] qapi: add QMP put-event command References: <1299460984-15849-1-git-send-email-aliguori@us.ibm.com> <1299460984-15849-20-git-send-email-aliguori@us.ibm.com> <4D778117.9060505@redhat.com> <4D778545.7040403@codemonkey.ws> <4D778797.9090602@redhat.com> <4D778E07.5070408@codemonkey.ws> <4D78C69F.6010003@redhat.com> <4D78DC5A.3000601@codemonkey.ws> In-Reply-To: <4D78DC5A.3000601@codemonkey.ws> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: Markus Armbruster , qemu-devel@nongnu.org, Luiz Capitulino 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