From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=59972 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Pox9L-0000lq-WC for qemu-devel@nongnu.org; Mon, 14 Feb 2011 07:03:28 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Pox9K-0003XF-QV for qemu-devel@nongnu.org; Mon, 14 Feb 2011 07:03:27 -0500 Received: from mail-yi0-f45.google.com ([209.85.218.45]:43050) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Pox9K-0003Wu-Dk for qemu-devel@nongnu.org; Mon, 14 Feb 2011 07:03:26 -0500 Received: by yie21 with SMTP id 21so2236427yie.4 for ; Mon, 14 Feb 2011 04:03:22 -0800 (PST) Message-ID: <4D591A01.4030105@codemonkey.ws> Date: Mon, 14 Feb 2011 06:03:13 -0600 From: Anthony Liguori MIME-Version: 1.0 Subject: Re: [Qemu-devel] Re: [RFC] qapi: events in QMP References: <4D581E04.1020901@codemonkey.ws> <4D58FADB.3010005@redhat.com> In-Reply-To: <4D58FADB.3010005@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: Kevin Wolf Cc: Luiz Capitulino , Markus Armbruster , qemu-devel On 02/14/2011 03:50 AM, Kevin Wolf wrote: > Am 13.02.2011 19:08, schrieb Anthony Liguori: > >> Hi, >> >> In my QAPI branch[1], I've now got almost every existing QMP command >> converted with (hopefully) all of the hard problems solved. There is >> only one remaining thing to attack before posting for inclusion and >> that's events. Here's my current thinking about what to do. >> > I've had a quick look at your branch and I have one general question: Is > there a specific reason why you duplicate existing monitor handlers > instead of converting the existing ones? Are the old ones still used? > It's just temporary. It makes it very easy to test code side-by-side. By the time I submit, I'll remove the old code. > >> Proposal for events in QAPI >> >> For QAPI, I'd like to model events on the notion of signals and >> slots[2]. A client would explicitly connect to a signal through a QMP >> interface which would result in a slot being added that then generates >> an event. Internally in QEMU, we could also connect slots to the same >> signals. Since we don't have an object API in QMP, we'd use a pair of >> connect/disconnect functions that had enough information to identify the >> signal. >> >> Example: >> >> We would define QEVENT_BLOCK_IO_EVENT as: >> >> # qmp-schema.json >> { 'BlockIOEvent': {'device': 'str', 'action': 'str', 'operation': 'str'} } >> >> The 'Event' suffix is used to determine that the type is an event and >> not a structure. This would generate the following structures for QEMU: >> >> typedef void (BlockIOEventFunc)(const char *device, const char *action, >> const char *operation, void *opaque); >> > Why is an event not a structure? For one it makes things inconsistent > (you have this 'Event' suffix magic), and it's not even convenient. The > parameter list of the BlockIOEventFunc might become very long. At the > moment you have three const char* there and I think it's only going to > grow - who is supposed to remember the right order of arguments? > > So why not make the event a struct and have a typedef void > (BlockIOEventFunc)(BlockIOEvent *evt)? > A signal is a function call. You can pass a structure as a parameter is you so desire but the natural thing to do is pass position arguments. If you've got a ton of signal arguments, it's probably an indication that you're doing something wrong. >> typedef struct BlockIOEvent { >> /* contents are private */ >> } BlockIOEvent; >> >> /* Connect a slot to a BlockIOEvent signal and return a handle to the >> connection */ >> int block_io_event_connect(BlockIOEvent *event, BlockIOEventFunc *cb, >> void *opaque); >> >> /* Signal any connect slots of a BlockIOEvent */ >> void block_io_event_signal(BlockIOEvent *event, const char *device, >> const char *action, const char *operation); >> >> /* Disconnect a slot from a signal based on a connection handle */ >> void block_io_event_disconnect(BlockIOEvent *event, int handle); >> >> In the case of BlockIOEvent, this is a global signal that is not tied to >> any specific object so there would be a global BlockIOEvent object >> within QEMU. >> >> From a QMP perspective, we would have the following function in the schema: >> >> [ 'block-io-event', {}, {}, 'BlockIOEvent' ] >> > Not sure what this list is supposed to tell me... > > I see that you use the same for command, so I guess the first one is > something like an command/event name, the second seems to be the > arguments, last could be the type of the return value, the third no idea.' > Sorry, first is the name of the function, second is required arguments, third is optional arguments, last is return value. > So would an event look like a response to a command that was never issued? > From a protocol perspective, events look like this today: { "event": "BlockIOError", "data": { "device": "ide1-cd0", "operation": "read", "action": "report" } } The only change to the protocol I'm proposing is adding a "tag" field which would give us: { "event": "BlockIOError", tag: "event023234", "data": { "device": "ide1-cd0", "operation": "read", "action": "report" } } >> Another Example >> >> Here's an example of BlockDeviceEvent, the successor to BlockIOEvent. >> It makes use of signal accessor arguments and QAPI enumeration support: >> > So are we going to drop BlockIOEvent (or any other event that will be > extended) or will both old and new version continue to exist and we > always signal both of them? > Both have to exist. We can't drop old events without breaking backwards compatibility. Regards, Anthony Liguori > Kevin > >