All of lore.kernel.org
 help / color / mirror / Atom feed
From: Luiz Capitulino <lcapitulino@redhat.com>
To: Kevin Wolf <kwolf@redhat.com>
Cc: "Benoît Canet" <benoit.canet@irqsave.net>,
	"Markus Armbruster" <armbru@redhat.com>,
	stefanha@redhat.com, qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] How to introduce bs->node_name ?
Date: Mon, 4 Nov 2013 08:51:16 -0500	[thread overview]
Message-ID: <20131104085116.55cc3673@redhat.com> (raw)
In-Reply-To: <20131104111359.GD4199@dhcp-200-207.str.redhat.com>

On Mon, 4 Nov 2013 12:13:59 +0100
Kevin Wolf <kwolf@redhat.com> wrote:

> Am 01.11.2013 um 16:12 hat Luiz Capitulino geschrieben:
> > On Fri, 01 Nov 2013 08:59:20 -0600
> > Eric Blake <eblake@redhat.com> wrote:
> > 
> > > On 11/01/2013 08:51 AM, Luiz Capitulino wrote:
> > > > On Wed, 30 Oct 2013 13:25:35 -0600
> > > > Eric Blake <eblake@redhat.com> 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.

  reply	other threads:[~2013-11-04 13:51 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-10-28 15:40 [Qemu-devel] How to introduce bs->node_name ? Benoît Canet
2013-10-29  1:03 ` Fam Zheng
2013-10-30 13:49 ` Markus Armbruster
2013-10-30 19:25   ` Eric Blake
2013-11-01 14:51     ` Luiz Capitulino
2013-11-01 14:59       ` Eric Blake
2013-11-01 15:12         ` Luiz Capitulino
2013-11-04 11:13           ` Kevin Wolf
2013-11-04 13:51             ` Luiz Capitulino [this message]
2013-11-04  9:31   ` Stefan Hajnoczi
2013-11-04  9:48     ` Fam Zheng
2013-11-04 11:06       ` Kevin Wolf
2013-11-04 11:33         ` Fam Zheng
2013-11-07 18:50           ` Benoît Canet

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20131104085116.55cc3673@redhat.com \
    --to=lcapitulino@redhat.com \
    --cc=armbru@redhat.com \
    --cc=benoit.canet@irqsave.net \
    --cc=kwolf@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.