[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.
Kevin
- [Qemu-block] [PATCH v10 00/16] Support streaming to an intermediate layer, Alberto Garcia, 2016/10/06
- [Qemu-block] [PATCH v10 02/16] block: Add block_job_add_bdrv(), Alberto Garcia, 2016/10/06
- [Qemu-block] [PATCH v10 09/16] block: Add QMP support for streaming to an intermediate layer, Alberto Garcia, 2016/10/06
- Re: [Qemu-block] [Qemu-devel] [PATCH v10 09/16] block: Add QMP support for streaming to an intermediate layer, Eric Blake, 2016/10/10
- Re: [Qemu-block] [Qemu-devel] [PATCH v10 09/16] block: Add QMP support for streaming to an intermediate layer, Alberto Garcia, 2016/10/11
- Re: [Qemu-block] [Qemu-devel] [PATCH v10 09/16] block: Add QMP support for streaming to an intermediate layer, Kevin Wolf, 2016/10/11
- Re: [Qemu-block] [Qemu-devel] [PATCH v10 09/16] block: Add QMP support for streaming to an intermediate layer, Eric Blake, 2016/10/11
- Re: [Qemu-block] [Qemu-devel] [PATCH v10 09/16] block: Add QMP support for streaming to an intermediate layer, Markus Armbruster, 2016/10/11
- Re: [Qemu-block] [Qemu-devel] [PATCH v10 09/16] block: Add QMP support for streaming to an intermediate layer,
Kevin Wolf <=
- Re: [Qemu-block] [Qemu-devel] [PATCH v10 09/16] block: Add QMP support for streaming to an intermediate layer, Alberto Garcia, 2016/10/12
- Re: [Qemu-block] [Qemu-devel] [PATCH v10 09/16] block: Add QMP support for streaming to an intermediate layer, Markus Armbruster, 2016/10/11
- Re: [Qemu-block] [Qemu-devel] [PATCH v10 09/16] block: Add QMP support for streaming to an intermediate layer, Alberto Garcia, 2016/10/12
- Re: [Qemu-block] [Qemu-devel] [PATCH v10 09/16] block: Add QMP support for streaming to an intermediate layer, Markus Armbruster, 2016/10/12
Re: [Qemu-block] [PATCH v10 09/16] block: Add QMP support for streaming to an intermediate layer, Kevin Wolf, 2016/10/12
[Qemu-block] [PATCH v10 06/16] block: Block all nodes involved in the block-commit operation, Alberto Garcia, 2016/10/06
[Qemu-block] [PATCH v10 03/16] block: Use block_job_add_bdrv() in mirror_start_job(), Alberto Garcia, 2016/10/06