On 01/21/2014 08:30 PM, Fam Zheng wrote: > Currently there is no way to query BlockStats of the backing chain. This > adds "backing" and "file" fields into BlockStats to make it possible. > > The old optional field "parent", which has the information for "file", > is kept there for backward compatibility, as an alias for "file".
Eww. BlockStats is a big struct (and getting bigger), which means this
is a lot of JSON duplication to send over the wire just to get discarded
(no matter whether the client uses the old or the new name).
> However field name "file" is more consistent with "file" option name in
> the block device configuration interface.
Consistency is nice, but back-compat and efficiency take higher priority
in this case. You are better off documenting that 'parent' refers to
the details of the backing file, without adding a 'file' field.
>
> Signed-off-by: Fam Zheng <f...@redhat.com>
> ---
> block/qapi.c | 7 +++++++
> qapi-schema.json | 15 +++++++++++----
> 2 files changed, 18 insertions(+), 4 deletions(-)
>
> +++ b/block/qapi.c
> @@ -316,6 +316,13 @@ BlockStats *bdrv_query_stats(const BlockDriverState *bs)
> if (bs->file) {
> s->has_parent = true;
> s->parent = bdrv_query_stats(bs->file);
> + s->has_file = true;
> + s->file = bdrv_query_stats(bs->file);
> + }
Drop this portion.
> +
> + if (bs->backing_hd) {
> + s->has_backing = true;
> + s->backing = bdrv_query_stats(bs->backing_hd);
> }
But this one is fine.
>
> return s;
> diff --git a/qapi-schema.json b/qapi-schema.json
> index f27c48a..c3c8aec 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -1022,15 +1022,22 @@
> #
> # @stats: A @BlockDeviceStats for the device.
> #
> -# @parent: #optional This may point to the backing block device if this is a
> -# a virtual block device. If it's a backing block, this will point
> -# to the backing file is one is present.
I'm okay if you improve the wording here, but keep this field untouched
with just a better description.
> +# @file: #optional This may point to the file block device if present.
> +# (Since 2.0)
Don't add this field.
> +#
> +# @backing: #optional This may point to the backing block device if present.
> +# (Since 2.0)
> +#
s/may point/describes/
> +# @parent: #optional An alias of @file for backward comatibility. (Since
> +# 0.14.0)
s/comatibility/compatibility/ - except that I don't think you want to
keep it as an alias. Also, the (Since 0.14.0) is bogus, since the field
has existed for as long as the command has.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
