From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=34899 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PpiRC-0006i1-Md for qemu-devel@nongnu.org; Wed, 16 Feb 2011 09:33:03 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PpiRA-0005iC-C1 for qemu-devel@nongnu.org; Wed, 16 Feb 2011 09:33:02 -0500 Received: from mail-qy0-f173.google.com ([209.85.216.173]:55302) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PpiRA-0005hm-8Q for qemu-devel@nongnu.org; Wed, 16 Feb 2011 09:33:00 -0500 Received: by qyl38 with SMTP id 38so3842626qyl.4 for ; Wed, 16 Feb 2011 06:32:59 -0800 (PST) Message-ID: <4D5BE007.40106@codemonkey.ws> Date: Wed, 16 Feb 2011 08:32:39 -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> <4D591A01.4030105@codemonkey.ws> <4D5920ED.6020104@redhat.com> <20110214104517.32b77291@doriath> <4D593E8F.7050306@codemonkey.ws> <20110214163443.57ad8a37@doriath> <4D5983B3.5010902@codemonkey.ws> <4D5A4541.3080906@redhat.com> <20110215113831.6d647b95@doriath> <4D5B217A.40202@codemonkey.ws> <4D5B8FCC.3050303@redhat.com> <4D5BD48B.6010705@codemonkey.ws> <4D5BDBF1.8020501@redhat.com> In-Reply-To: <4D5BDBF1.8020501@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: Markus Armbruster , qemu-devel , Luiz Capitulino On 02/16/2011 08:15 AM, Kevin Wolf wrote: > Am 16.02.2011 14:43, schrieb Anthony Liguori: > >> On 02/16/2011 02:50 AM, Kevin Wolf wrote: >> >>> Am 16.02.2011 01:59, schrieb Anthony Liguori: >>> >>> >>>> On 02/15/2011 07:38 AM, Luiz Capitulino wrote: >>>> >>>> >>>>> On Tue, 15 Feb 2011 10:20:01 +0100 >>>>> Kevin Wolf wrote: >>>>> >>>>> >>>>> >>>>> >>>>>> Am 14.02.2011 20:34, schrieb Anthony Liguori: >>>>>> >>>>>> >>>>>> >>>>>>> On 02/14/2011 12:34 PM, Luiz Capitulino wrote: >>>>>>> >>>>>>> >>>>>>> >>>>>>>> On Mon, 14 Feb 2011 08:39:11 -0600 >>>>>>>> Anthony Liguori wrote: >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>>> On 02/14/2011 06:45 AM, Luiz Capitulino wrote: >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>>> So the question is: how does the schema based design support extending >>>>>>>>>> commands or events? Does it require adding new commands/events? >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>> Well, let me ask you, how do we do that today? >>>>>>>>> >>>>>>>>> Let's say that I want to add a new parameter to the `change' function so >>>>>>>>> that I can include a salt parameter as part of the password. >>>>>>>>> >>>>>>>>> The way we'd do this today is by checking for the 'salt' parameter in >>>>>>>>> qdict, and if it's not present, use a random salt or something like that. >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>> You likely want to do what you did before. Of course that you have to >>>>>>>> consider if what you're doing is extending an existing command or badly >>>>>>>> overloading it (like change is today), in this case you'll want to add >>>>>>>> a new command instead. >>>>>>>> >>>>>>>> But yes, the use-case here is extending an existing command. >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>>> However, if I'm a QMP client, how can I tell whether you're going to >>>>>>>>> ignore my salt parameter or actually use it? Nothing in QMP tells me >>>>>>>>> this today. If I set the salt parameter in the `change' command, I'll >>>>>>>>> just get a success message. >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>> I'm sorry? >>>>>>>> >>>>>>>> { "execute": "change", "arguments": { "device": "vnc", "target": "password", "arg": "1234", "salt": "r1" } } >>>>>>>> {"error": {"class": "InvalidParameter", "desc": "Invalid parameter 'salt'", "data": {"name": "salt"}}} >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>> So I'm supposed to execute the command, and if execution fails, drop the >>>>>>> new parameter? If we add a few optional parameters, does that mean I >>>>>>> have to try every possible combination of parameters? >>>>>>> >>>>>>> >>>>>>> >>>>>> How is that different from trying out multiple commands? In the end, you >>>>>> always need some meta information like a schema in order to avoid trying >>>>>> out which parameters the server supports. >>>>>> >>>>>> Anyway, I think there's a second interesting point: Adding parameters >>>>>> does cause these problems, but it's different for data sent from qemu to >>>>>> the client (return values and events). If we add more information there, >>>>>> an older client can just ignore it, without even looking at a schema. >>>>>> >>>>>> So I think we should consider this for return values and definitely do >>>>>> it for events. Sending out five different messages for a single event >>>>>> that are completely redundant and only differ in the number of fields is >>>>>> just insane (okay, they wouldn't actually get on the wire because a >>>>>> client registers only for one of them, but the code for generating them >>>>>> must exist). >>>>>> >>>>>> >>>>>> >>>>> That's my point when I asked about events in the other thread. >>>>> >>>>> >>>>> >>>> Okay, I had confused myself about this. It's not quite as bad as I had >>>> been saying. >>>> >>>> One of the reasons to have generated allocation function is so that we >>>> can make sure to always pad structures. Since all optional fields has a >>>> bool to indicate the fields presence, by setting the allocated structure >>>> to zero, we can support forwards compatibility for structures. >>>> >>>> >>> I think in most cases we would even get away with a default value >>> instead of the bool. For example for strings, NULL would be a very clear >>> indication that the field wasn't there in the JSON message. >>> >> In order to support forwards compatibility, we need to have a zero-value >> for non-presence. For strings or pointers, NULL would work very well. >> > What's the kind of compatibility you're concerned about? I was mainly > considering older clients communicating with newer qemu, i.e. > compatibility on the protocol level. The has_XX fields are not sent over the wire. > The library can set default values > for fields that are not present in JSON messages it receives. > > Your point is older applications using a newer library? Compiled against a new library, but running against an old library. This has to be supported in order to avoid bumping the library version. >> That's not entirely true. For human-monitor-command, we return a bare >> string. For all other commands, we return structures specifically to >> enable better forwards compatibility. >> >> I do agree that for most of our events, we should be using a structure >> for passing information. That's not what we're doing today but there's >> only a couple events that are even doing that so fixing it won't be that >> bad. >> > Right, you could still have something like this (although I'm not sure > if it's very useful): > > [ 'block-io-event', {}, {}, 'string' ] > > What I think isn't a good idea is that the following definition doesn't > generate a structure in your original proposal. This should really > generate a structure: > > { 'BlockIOEvent': {'device': 'string', 'action': 'string', 'operation': > 'string'} } > [ 'block-io-event', {}, {}, 'BlockIOEvent' ] > Actually, I think it's better to explicitly call out the structure name so that you can do things like: { 'VncConnectedEvent': {'info': 'VncClientInfo'} } Which happens to be the same structure used by 'query-vnc' Regards, Anthony Liguori > Kevin > >