Re: [Qemu-devel] [PATCH V5 2/4] block: Use bdrv_get_backing_file_depth()

[Luiz Capitulino <lcapitulino@redhat.com>]

On Mon, 30 Jul 2012 10:48:49 -0600
Eric Blake <eblake@redhat.com> wrote:

> On 07/30/2012 10:32 AM, Luiz Capitulino wrote:
> > On Thu, 26 Jul 2012 15:18:19 +0200
> > benoit.canet@gmail.com wrote:
> > 
> >> From: Benoît Canet <benoit@irqsave.net>
> >>
> >> Use the dedicated counting function in qmp_query_block in order to
> >> propagate the backing file depth to HMP.
> >>
> >> Signed-off-by: Benoit Canet <benoit@irqsave.net>
> 
> >> +++ b/qapi-schema.json
> >> @@ -398,6 +398,8 @@
> >>  #
> >>  # @backing_file: #optional the name of the backing file (for copy-on-write)
> >>  #
> >> +# @backing_file_depth: number of files in the backing file chain (since: 1.2)
> >> +#
> >>  # @encrypted: true if the backing device is encrypted
> >>  #
> >>  # @bps: total throughput limit in bytes per second is specified
> >> @@ -418,9 +420,10 @@
> >>  ##
> >>  { 'type': 'BlockDeviceInfo',
> >>    'data': { 'file': 'str', 'ro': 'bool', 'drv': 'str',
> >> -            '*backing_file': 'str', 'encrypted': 'bool',
> >> -            'bps': 'int', 'bps_rd': 'int', 'bps_wr': 'int',
> >> -            'iops': 'int', 'iops_rd': 'int', 'iops_wr': 'int'} }
> >> +            '*backing_file': 'str', 'backing-file-depth': 'int',
> > 
> > Should use underscores, ie. should be backing_file_depth.
> 
> Really?  I thought we _want_ new interfaces to use '-', not '_', in QMP.

The problem is mixing them. BlockDeviceInfo already uses "_", and it's
not only for backing_file.

For new commands or existing commands that don't use "_", we certainly should
use "-", but mixing them for the same command is probably a bad thing.

>  For 'backing_file', we are stuck due to back-compat (unless Anthony's
> proposed patch to parse names case-insensitively with '-' and '_' folded
> together is taken),

We can't take Anthony's patch for what we emit. We could use it for input,
but I don't think it's worth it.

> but for the new field, we have no back-compat
> constraints, and I would prefer backing-file-depth.

It's like not I can't be convinced, but I think we shouldn't mix "-" and
"_" for the same command.

> 
> At any rate, you definitely need to make sure you agree between the docs
> above and the JSON statement below (as written, you have both spellings
> in the same patch).
>