From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37474) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1buFZC-0002HI-Fz for qemu-devel@nongnu.org; Wed, 12 Oct 2016 05:11:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1buFZA-0000ah-Aj for qemu-devel@nongnu.org; Wed, 12 Oct 2016 05:11:13 -0400 Date: Wed, 12 Oct 2016 11:11:02 +0200 From: Kevin Wolf Message-ID: <20161012091102.GD5544@noname.redhat.com> References: <1c8e6e73084e968514b6522576cfacd1bf69609e.1475757437.git.berto@igalia.com> <48b78d7e-3f52-79d3-27a1-19047100161b@redhat.com> <20161011145712.GF6334@noname.redhat.com> <5a38945a-a764-1dcf-2a5b-495b6d131f9b@redhat.com> <871szm96ek.fsf@dusky.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <871szm96ek.fsf@dusky.pond.sub.org> Subject: Re: [Qemu-devel] [PATCH v10 09/16] block: Add QMP support for streaming to an intermediate layer List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: Eric Blake , Alberto Garcia , Max Reitz , Stefan Hajnoczi , qemu-devel@nongnu.org, qemu-block@nongnu.org Am 11.10.2016 um 18:50 hat Markus Armbruster geschrieben: > Eric Blake writes: > > > On 10/11/2016 09:57 AM, Kevin Wolf wrote: > >> Should we introduce a new, clean blockdev-stream command that fixes this > >> and matches the common name pattern? Of course, block-stream vs. > >> blockdev-stream could be a bit confusing, too... > >> > > > > A new command is easy to introspect (query-commands), lets us get rid of > > cruft, and makes it obvious that we support everything from the get-go. > > I'm favoring that option, even if it leads to slightly confusing names > > of the deprecated vs. the new command. > > Let's take a step back and consider extending old commands vs. adding > new commands. > > A new command is trivial to detect in introspection. > > Command extensions are not as trivial to detect in introspection. Many > extensions are exposed in query-qmp-schema, but not all. > > Back when QMP was young, Anthony argued for always adding new commands, > never extend existing ones. I opposed it, because it would lead to a > confusing mess of related commands, unreadable or incomplete > documentation, and abysmal test coverage. > > However, the other extreme is also unwise: we shouldn't shoehorn new > functionality into existing commands just because we can. We should ask > ourselves questions like these: > > * Is the extended command still a sane interface? If writing clear > documentation for it is hard, it perhaps isn't. Pay special attention > to failure modes. Overloaded arguments are prone to confusing errors. It has never been a sane interface in the first place (identifying a backing file node by its filename). We ended up having two versions of all block job commands anyway (one that creates an image file, and later one that just takes a node-name of an existing node), except for image streaming so far. So it would be consistent (and enable consistent naming for the preferred commands) to have it here, too. > * How will the command's users use the extension? If it requires new > code paths, a new command may be more convenient for them. Kevin