[1/3] block: Expose protocol-specific data to 'qemu-img info'

'qemu-img info' is useful for showing additional information
about an image - but sometimes the interesting information is
specific to the protocol rather than to the format layer.  Set
the groundwork for showing this information; further patches
will then enable specific pieces of information.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 qapi/block-core.json | 6 +++++-
 block/qapi.c         | 7 +++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

Am 17.01.2019 um 16:33 hat Eric Blake geschrieben:
> 'qemu-img info' is useful for showing additional information
> about an image - but sometimes the interesting information is
> specific to the protocol rather than to the format layer.  Set
> the groundwork for showing this information; further patches
> will then enable specific pieces of information.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>

Isn't this making invalid assumptions about the structure of the block
graph? Who says that I don't have a quorum node below the format layer
with five different protocol layer children? Some of which may have
filter nodes on top?

Kevin

On 1/17/19 10:39 AM, Kevin Wolf wrote:
> Am 17.01.2019 um 16:33 hat Eric Blake geschrieben:
>> 'qemu-img info' is useful for showing additional information
>> about an image - but sometimes the interesting information is
>> specific to the protocol rather than to the format layer.  Set
>> the groundwork for showing this information; further patches
>> will then enable specific pieces of information.
>>
>> Signed-off-by: Eric Blake <eblake@redhat.com>
> 
> Isn't this making invalid assumptions about the structure of the block
> graph? Who says that I don't have a quorum node below the format layer
> with five different protocol layer children? Some of which may have
> filter nodes on top?

Does quorum have bs->file?  If not, then this field is not populated for
quorum, so it doesn't make invalid assumptions.

You do have a point about bs->file pointing to filters being an
interesting case, though.  Max had a series proposal back in August that
tries to do smarter role-based descent through the graph rather than
just hard-coding assumptions based on bs->file:

https://lists.gnu.org/archive/html/qemu-devel/2018-08/msg01644.html

Maybe we need to get that in place first, at which point it then becomes
easier to reason about what related BDS information to visit in addition
to the format layer.

17.01.2019 18:33, Eric Blake wrote:
> 'qemu-img info' is useful for showing additional information
> about an image - but sometimes the interesting information is
> specific to the protocol rather than to the format layer.  Set
> the groundwork for showing this information; further patches
> will then enable specific pieces of information.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>   qapi/block-core.json | 6 +++++-
>   block/qapi.c         | 7 +++++++
>   2 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 91685be6c29..f28d5f5fc76 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -152,6 +152,9 @@
>   # @format-specific: structure supplying additional format-specific
>   # information (since 1.7)
>   #
> +# @protocol-specific: structure supplying additional protocol-specific
> +# information (since 4.0)
> +#
>   # Since: 1.3
>   #
>   ##
> @@ -162,7 +165,8 @@
>              '*backing-filename': 'str', '*full-backing-filename': 'str',
>              '*backing-filename-format': 'str', '*snapshots': ['SnapshotInfo'],
>              '*backing-image': 'ImageInfo',
> -           '*format-specific': 'ImageInfoSpecific' } }
> +           '*format-specific': 'ImageInfoSpecific',
> +           '*protocol-specific': 'ImageInfoSpecific' } }
> 
>   ##
>   # @ImageCheck:
> diff --git a/block/qapi.c b/block/qapi.c
> index c66f949db83..a8d104ba8ce 100644
> --- a/block/qapi.c
> +++ b/block/qapi.c
> @@ -284,6 +284,9 @@ void bdrv_query_image_info(BlockDriverState *bs,
>       }
>       info->format_specific     = bdrv_get_specific_info(bs);
>       info->has_format_specific = info->format_specific != NULL;
> +    info->protocol_specific =
> +        bs->file ? bdrv_get_specific_info(bs->file->bs) : NULL;
> +    info->has_protocol_specific = info->protocol_specific != NULL;
> 
>       backing_filename = bs->backing_file;
>       if (backing_filename[0] != '\0') {
> @@ -870,4 +873,8 @@ void bdrv_image_info_dump(fprintf_function func_fprintf, void *f,
>           func_fprintf(f, "Format specific information:\n");
>           bdrv_image_info_specific_dump(func_fprintf, f, info->format_specific);
>       }
> +    if (info->has_protocol_specific) {
> +        func_fprintf(f, "Protocol specific information:\n");
> +        bdrv_image_info_specific_dump(func_fprintf, f, info->protocol_specific);
> +    }
>   }
> 


A bit related question:

Why do we always have nbd node as a protocol node? Can't we use nbd node directly,
without raw layer?