From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=49516 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Pov2q-0007uR-VU for qemu-devel@nongnu.org; Mon, 14 Feb 2011 04:48:38 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Pov2p-0000aQ-Ne for qemu-devel@nongnu.org; Mon, 14 Feb 2011 04:48:36 -0500 Received: from mx1.redhat.com ([209.132.183.28]:64731) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Pov2p-0000aH-E1 for qemu-devel@nongnu.org; Mon, 14 Feb 2011 04:48:35 -0500 Message-ID: <4D58FADB.3010005@redhat.com> Date: Mon, 14 Feb 2011 10:50:19 +0100 From: Kevin Wolf MIME-Version: 1.0 References: <4D581E04.1020901@codemonkey.ws> In-Reply-To: <4D581E04.1020901@codemonkey.ws> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: [Qemu-devel] Re: [RFC] qapi: events in QMP List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: Markus Armbruster , qemu-devel , Luiz Capitulino 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 would probably be much easier to review if you had a real patch that contains the actual changes instead of just one big addition of the duplicated 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)? By the way, we're not that short on disk space. Let's call it 'string' instead of 'str'. > > 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. So would an event look like a response to a command that was never issued? > 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? Kevin