[v2,6/6] block: Remove BB options from blockdev-add

werror/rerror are now available as qdev options. The stats-* options are
removed without an existing replacement; they should probably be
configurable with a separate QMP command like I/O throttling settings.

Removing id is left for another day because this involves updating
qemu-iotests cases to use node-name for everything. Before we can do
that, however, all QMP commands must support node-name.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 qapi/block-core.json | 18 +-----------------
 1 file changed, 1 insertion(+), 17 deletions(-)

On 30.06.2016 16:13, Kevin Wolf wrote:
> werror/rerror are now available as qdev options. The stats-* options are
> removed without an existing replacement; they should probably be
> configurable with a separate QMP command like I/O throttling settings.

I'm not sure I agree with removing the stats-* options without a
replacement. If we'd get rid of @id in the process, fine. But we won't
get a pure blockdev-add before 2.7 anyway, so I'm not sure removing
stats-* now is necessary.

Max

> 
> Removing id is left for another day because this involves updating
> qemu-iotests cases to use node-name for everything. Before we can do
> that, however, all QMP commands must support node-name.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  qapi/block-core.json | 18 +-----------------
>  1 file changed, 1 insertion(+), 17 deletions(-)
> 
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 6008986..88b78a7 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -2051,20 +2051,8 @@
>  # @discard:       #optional discard-related options (default: ignore)
>  # @cache:         #optional cache-related options
>  # @aio:           #optional AIO backend (default: threads)
> -# @rerror:        #optional how to handle read errors on the device
> -#                 (default: report)
> -# @werror:        #optional how to handle write errors on the device
> -#                 (default: enospc)
>  # @read-only:     #optional whether the block device should be read-only
>  #                 (default: false)
> -# @stats-account-invalid: #optional whether to include invalid
> -#                         operations when computing last access statistics
> -#                         (default: true) (Since 2.5)
> -# @stats-account-failed: #optional whether to include failed
> -#                         operations when computing latency and last
> -#                         access statistics (default: true) (Since 2.5)
> -# @stats-intervals: #optional list of intervals for collecting I/O
> -#                   statistics, in seconds (default: none) (Since 2.5)
>  # @detect-zeroes: #optional detect and optimize zero writes (Since 2.1)
>  #                 (default: off)
>  #
> @@ -2074,17 +2062,13 @@
>  ##
>  { 'union': 'BlockdevOptions',
>    'base': { 'driver': 'BlockdevDriver',
> +# TODO 'id' is a BB-level option, remove it
>              '*id': 'str',
>              '*node-name': 'str',
>              '*discard': 'BlockdevDiscardOptions',
>              '*cache': 'BlockdevCacheOptions',
>              '*aio': 'BlockdevAioOptions',
> -            '*rerror': 'BlockdevOnError',
> -            '*werror': 'BlockdevOnError',
>              '*read-only': 'bool',
> -            '*stats-account-invalid': 'bool',
> -            '*stats-account-failed': 'bool',
> -            '*stats-intervals': ['int'],
>              '*detect-zeroes': 'BlockdevDetectZeroesOptions' },
>    'discriminator': 'driver',
>    'data': {
>

Am 02.07.2016 um 18:28 hat Max Reitz geschrieben:
> On 30.06.2016 16:13, Kevin Wolf wrote:
> > werror/rerror are now available as qdev options. The stats-* options are
> > removed without an existing replacement; they should probably be
> > configurable with a separate QMP command like I/O throttling settings.
> 
> I'm not sure I agree with removing the stats-* options without a
> replacement. If we'd get rid of @id in the process, fine. But we won't
> get a pure blockdev-add before 2.7 anyway, so I'm not sure removing
> stats-* now is necessary.

Actually, I'm not so sure about removing id only after 2.7. Basically
all that's missing is a conversion of block job commands to accept
node-name so that qemu-iotests cases can be converted. This is a mostly
mechanical conversion and I have part of it ready.

About keeping stats-*, what good is it to keep an option that we know
will go away sooner or later? For id there is a good reason, removing it
now would break test cases. But stats-*? I don't see a reason.

Kevin

On 04.07.2016 12:58, Kevin Wolf wrote:
> Am 02.07.2016 um 18:28 hat Max Reitz geschrieben:
>> On 30.06.2016 16:13, Kevin Wolf wrote:
>>> werror/rerror are now available as qdev options. The stats-* options are
>>> removed without an existing replacement; they should probably be
>>> configurable with a separate QMP command like I/O throttling settings.
>>
>> I'm not sure I agree with removing the stats-* options without a
>> replacement. If we'd get rid of @id in the process, fine. But we won't
>> get a pure blockdev-add before 2.7 anyway, so I'm not sure removing
>> stats-* now is necessary.
> 
> Actually, I'm not so sure about removing id only after 2.7. Basically
> all that's missing is a conversion of block job commands to accept
> node-name so that qemu-iotests cases can be converted. This is a mostly
> mechanical conversion and I have part of it ready.
> 
> About keeping stats-*, what good is it to keep an option that we know
> will go away sooner or later? For id there is a good reason, removing it
> now would break test cases. But stats-*? I don't see a reason.

The reason would be "If someone is insane enough to use blockdev-add and
then wants to make use of those options".

I guess your reason for removing it would be "Don't encourage such
people further". Fine with me, then.

Max