From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=32879 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PxhsP-0003ZH-8C for qemu-devel@nongnu.org; Thu, 10 Mar 2011 10:34:10 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PxhsM-0002FB-4l for qemu-devel@nongnu.org; Thu, 10 Mar 2011 10:34:07 -0500 Received: from mail-yi0-f45.google.com ([209.85.218.45]:51823) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PxhsM-0002F4-0q for qemu-devel@nongnu.org; Thu, 10 Mar 2011 10:34:06 -0500 Received: by yib19 with SMTP id 19so824546yib.4 for ; Thu, 10 Mar 2011 07:34:05 -0800 (PST) Message-ID: <4D78EF67.4060009@codemonkey.ws> Date: Thu, 10 Mar 2011 09:33:59 -0600 From: Anthony Liguori 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> <4D78DF3A.9000600@redhat.com> In-Reply-To: <4D78DF3A.9000600@redhat.com> 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: Avi Kivity Cc: Luiz Capitulino , Markus Armbruster , qemu-devel@nongnu.org On 03/10/2011 08:24 AM, Avi Kivity wrote: > 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. It's all just bits, right? :-) We pretty much need to keep the QEMU signature the same. That would mean an internal signature of: BlockIoErrorEvent *qmp_connect_block_io_error_event(Error **errp) { } So the marshal function would then need to do something like: void qmp_marshal_connect_block_io_error_event(QmpState *state, QDict *args, QObject **ret_data, Error **errp) { BlockIoErrorEvent *ev; QObject *tag = qdict_get_obj(args, "tag"); ev = qmp_connect_block_io_error_event(errp); qmp_state_add_connection(state, ev->signal, tag); } That's not too bad, but would the schema be: [ 'connect-block-io-error-event', {}, 'BLOCK_IO_ERROR' ] Or would it be: [ 'connect-block-io-error-event', { 'tag': 'variant' }, 'none' ] I'm really opposed to adding a variant type to the schema. I'm also not a big fan of there not being a 1-1 relationship to the wire protocol and the C API. I think it's easy to rationalize 'this is how events are marshalled' vs. 'for events, a totally different declaration is generated'. >>>> { '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's not really a subscription to an event. It is an event. Maybe signal accessor is the wrong word. Maybe signal factory conveys a better notion of what it is. >> >> > { '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. Not all events are updates of data. BLOCK_IO_ERROR is a great example of this. There is no useful information that can be sent after a connection. > btw2, I now nominate subscribe and unsubscribe as replacements for get > and put. Subscribe implies sub/pub in my mind and we're not publishing events so I don't think it fits the model. A pub/sub event model would be interesting to think through but without a global namespace and object model, I don't think we can make it fit well. That's why I'm using signal/slots. It's much more conducive to a procedural model. 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. But a C client is by far the most important of all clients--QEMU. If we use QMP extensively internally, then we guarantee that the API is expressive and robust. If we build the API only for third-party clients, we end up with pretty much what we have today. An API with good intentions but that's more or less impossible to use in practice. Regards, Anthony Liguori