[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [Qemu-devel] [PATCH v10 09/16] block: Add QMP support f

From: Kevin Wolf
Subject: Re: [Qemu-block] [Qemu-devel] [PATCH v10 09/16] block: Add QMP support for streaming to an intermediate layer
Date: Wed, 12 Oct 2016 11:11:02 +0200
User-agent: Mutt/1.5.21 (2010-09-15)

Am 11.10.2016 um 18:50 hat Markus Armbruster geschrieben:
> Eric Blake <address@hidden> 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.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]