From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=41410 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Pp4bw-0000DO-E5 for qemu-devel@nongnu.org; Mon, 14 Feb 2011 15:01:30 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Pp4bu-00080d-AB for qemu-devel@nongnu.org; Mon, 14 Feb 2011 15:01:28 -0500 Received: from mx1.redhat.com ([209.132.183.28]:2888) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Pp4bu-00080R-25 for qemu-devel@nongnu.org; Mon, 14 Feb 2011 15:01:26 -0500 Date: Mon, 14 Feb 2011 18:01:21 -0200 From: Luiz Capitulino Subject: Re: [Qemu-devel] Re: [RFC] qapi: events in QMP Message-ID: <20110214180121.7b969efa@doriath> In-Reply-To: <20110214175800.060d4204@doriath> 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> <20110214175800.060d4204@doriath> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII 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 Mon, 14 Feb 2011 17:58:00 -0200 Luiz Capitulino wrote: > On Mon, 14 Feb 2011 13:34:11 -0600 > Anthony Liguori wrote: > > > 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? > > No, of course not, our plan has always been to do this via an schema, > the only reason we don't do this today is lack of time/help. > > > >> 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. > > Commands and events, you haven't commented on events yet and that seems > a bit worse than commands. > > > 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). > > So, let's agree we disagree. > > > >> 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. > > I disagree. Let's say we have added three new arguments to the command foo, > and now we have foo1, foo2 and foo3. I'm a quite old distro and only > have foo, which command should I backport? All of them? Only the latest? > > I can't see how easier this is. Backporting APIs will almost always suck. > > > >> 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. > > For C, yes. But one of the main goals of a high level protocol is to be > language independent, isn't it? > > > > 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. > > I wouldn't say it's impossible, but anyway, the important point here is > that we disagree about the side effects QAPI is going to introduce in QMP, > I don't know how to solve this, maybe we can discuss this upstream, but I'm > not sure the situation will change much. Oh, it's upstream, let's vote? :)