From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38796) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VdKYz-0001Ja-LB for qemu-devel@nongnu.org; Mon, 04 Nov 2013 08:51:35 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VdKYt-0008NE-1q for qemu-devel@nongnu.org; Mon, 04 Nov 2013 08:51:29 -0500 Received: from mx1.redhat.com ([209.132.183.28]:53367) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VdKYs-0008N2-QI for qemu-devel@nongnu.org; Mon, 04 Nov 2013 08:51:22 -0500 Date: Mon, 4 Nov 2013 08:51:16 -0500 From: Luiz Capitulino Message-ID: <20131104085116.55cc3673@redhat.com> In-Reply-To: <20131104111359.GD4199@dhcp-200-207.str.redhat.com> References: <20131028154038.GG2890@irqsave.net> <87eh72eumr.fsf@blackfin.pond.sub.org> <52715D2F.8000109@redhat.com> <20131101105139.3635bd82@redhat.com> <5273C1C8.4010601@redhat.com> <20131101111235.381c8373@redhat.com> <20131104111359.GD4199@dhcp-200-207.str.redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] How to introduce bs->node_name ? List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: =?UTF-8?B?QmVub8OudA==?= Canet , Markus Armbruster , stefanha@redhat.com, qemu-devel@nongnu.org On Mon, 4 Nov 2013 12:13:59 +0100 Kevin Wolf wrote: > Am 01.11.2013 um 16:12 hat Luiz Capitulino geschrieben: > > On Fri, 01 Nov 2013 08:59:20 -0600 > > Eric Blake wrote: > > > > > On 11/01/2013 08:51 AM, Luiz Capitulino wrote: > > > > On Wed, 30 Oct 2013 13:25:35 -0600 > > > > Eric Blake wrote: > > > > > > > >> On 10/30/2013 07:49 AM, Markus Armbruster wrote: > > > >> > > > >>> > > > >>> The first proposal is to add another parameter, say "id". Users can > > > >>> then refer either to an arbitrary BDS by "id", or (for backward > > > >>> compatibility) to the root BDS by "device". When the code sees > > > >>> "device", it'll look up the BB, then fetch its root BDS. > > > >>> > > > >>> CON: Existing parameter "device" becomes compatibility cruft. > > > >>> > > > >>> PRO: Clean and obvious semantics (in my opinion). > > > >> > > > >> I like this one as well. > > > > > > > > Does this proposal makes "device" optional for existing commands? If it > > > > does then I'm afraid it breaks compatibility because if you don't > > > > specify a device you'll get an error today. > > > > > > Changing from error to success is not backwards-incompatible. Old > > > applications will ALWAYS supply device (because it used to be > > > mandatory). That is, a management application that was intentionally > > > omitting 'device' and expecting an error is so unlikely to exist that we > > > can consider such a management app as buggy. > > > > Doing such changes makes me nervous nevertheless. In my mind a stable > > API doesn't change. Of course that there might exceptions, but 99.9% > > of those exceptions should be bug fixes not deliberate API extensions. > > Stable API means that whatever was working before keeps working. Nothing > less, but also nothing more. > > If an extension that changes from error to success is breaking the API, > adding a new QMP command is breaking the API as well. Because sending an > unknown command means returning an error... Let's not turn this into a surreal discussion, I'm sure we all want the best for our users. There's another problem, btw. We're not doing command extensions for input arguments before heaving a way to discover them. Even if there's consensus that extending the command is okay, we need the query-qmp-schema command merged first. IMO, this a blocker requirement (although it shouldn't be difficult to finalize what has been posted already). > > A more compelling argument against it is the quality of the resulting > > command. I'm sure it's going to be anything but a simple, clean API. > > I consider one command with an argument made optional a higher quality > API than having two different commands that do almost the same, except > that the older one has a mandatory argument where the newer one has an > optional argument. I wouldn't expect the new command to be just a duplication of the old one with a new argument. I'm expecting it to have a one-cover all argument or to have a dict or something that makes more sense for the new capabilities. But yes, we need a schema example to see how it would look like.