| Message ID | 1343308701-29632-3-git-send-email-benoit@irqsave.net |
|---|---|
| State | New |
| Headers | show |
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> > --- > block.c | 3 +++ > qapi-schema.json | 9 ++++++--- > 2 files changed, 9 insertions(+), 3 deletions(-) > > diff --git a/block.c b/block.c > index 22ebe49..a6fba1d 100644 > --- a/block.c > +++ b/block.c > @@ -2450,6 +2450,9 @@ BlockInfoList *qmp_query_block(Error **errp) > info->value->inserted->backing_file = g_strdup(bs->backing_file); > } > > + info->value->inserted->backing_file_depth = > + bdrv_get_backing_file_depth(bs); > + > if (bs->io_limits_enabled) { > info->value->inserted->bps = > bs->io_limits.bps[BLOCK_IO_LIMIT_TOTAL]; > diff --git a/qapi-schema.json b/qapi-schema.json > index a92adb1..2b50f22 100644 > --- a/qapi-schema.json > +++ 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. Also, the next patch and this can be squashed together. Otherwise series looks good to me now. > + 'encrypted': 'bool', 'bps': 'int', 'bps_rd': 'int', > + 'bps_wr': 'int', 'iops': 'int', 'iops_rd': 'int', > + 'iops_wr': 'int'} } > > ## > # @BlockDeviceIoStatus:
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. 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), but for the new field, we have no back-compat constraints, and I would prefer backing-file-depth. 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).
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). >
diff --git a/block.c b/block.c index 22ebe49..a6fba1d 100644 --- a/block.c +++ b/block.c @@ -2450,6 +2450,9 @@ BlockInfoList *qmp_query_block(Error **errp) info->value->inserted->backing_file = g_strdup(bs->backing_file); } + info->value->inserted->backing_file_depth = + bdrv_get_backing_file_depth(bs); + if (bs->io_limits_enabled) { info->value->inserted->bps = bs->io_limits.bps[BLOCK_IO_LIMIT_TOTAL]; diff --git a/qapi-schema.json b/qapi-schema.json index a92adb1..2b50f22 100644 --- a/qapi-schema.json +++ 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', + 'encrypted': 'bool', 'bps': 'int', 'bps_rd': 'int', + 'bps_wr': 'int', 'iops': 'int', 'iops_rd': 'int', + 'iops_wr': 'int'} } ## # @BlockDeviceIoStatus: