From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=57679 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Pp4Bl-0005Xr-VU for qemu-devel@nongnu.org; Mon, 14 Feb 2011 14:34:27 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Pp4Bk-0008TO-DQ for qemu-devel@nongnu.org; Mon, 14 Feb 2011 14:34:25 -0500 Received: from mail-iw0-f173.google.com ([209.85.214.173]:48121) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Pp4Bk-0008TK-7f for qemu-devel@nongnu.org; Mon, 14 Feb 2011 14:34:24 -0500 Received: by iwc10 with SMTP id 10so5594480iwc.4 for ; Mon, 14 Feb 2011 11:34:23 -0800 (PST) Message-ID: <4D5983B3.5010902@codemonkey.ws> Date: Mon, 14 Feb 2011 13:34:11 -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> In-Reply-To: <20110214163443.57ad8a37@doriath> 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: Luiz Capitulino Cc: Kevin Wolf , Markus Armbruster , qemu-devel 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? > >> Even if we expose a schema, but leave things as-is, having to parse the >> schema as part of a function call is pretty horrible, >> > That's a client implementation detail, they are not required to do it > as part of a function call. > > But let me ask, if we don't expose a schema, how will clients be able to > query available commands/events and their parameters? > We need to expose the schema, I'm not saying we shouldn't. But we don't today. You're arguing that we should extend commands by adding new parameters. I'm saying that's a bad interface. If we need to change a command, we should introduce a new command. It's a well understood mechanism for maintaining compatibility (just about every C library does exactly this). >> particularly if >> distros do silly things like backport some optional parameters and not >> others. If those optional parameters are deeply nested in a structure, >> it's even worse. >> > Why would they do this? I mean, if distros (or anyone else shipping qemu) > goes that deep on changing the wire protocol they are on their own, why > would we want to solve this problem? > It's not at all unreasonable for a distro to backport a new QMP command. If all modifications are discrete commands, compatibility is easy to preserve, however if a distro does backporting and we end up with a frankenstein command, compatibility will be an issue. >> OTOH, if we introduce a new command to set the password with a salt, it >> becomes very easy for the client to support. The do something as simple as: >> >> if qmp.has_command("vnc-set-password-with-salt"): >> qmp.vnc_set_password_with_salt('foobar', 'X*') >> else: >> window.set_weak_security_icon(True) >> qmp.vnc_set_password('foobar') >> >> Now you could answer, hey, we can add capabilities then those >> capabilities can quickly get out of hand. >> > Adding one command per new argument has its problems too and it's even > worse with events, as clients will have to be changed to handle a > new event just because of a parameter addition. > Yes, but it's an extremely well understood way to design compatible APIs. > Look, although I did _not_ check any code yet, your description of the QAPI > looks really exciting. I'm not against it, what bothers me though is this > number of small limitations we're imposing to the wire protocol. > > Why don't we make libqmp internal only? This way we're free to change it > whatever we want. > libqmp is a test of how easy it is to use QMP from an external application. If we can't keep libqmp stable, then that means tools like libvirt will always have a hard time using QMP. Proper C support is important. We cannot make it impossible to write a useful C client API. Regards, Anthony Liguori