[2/3] file: Expose some protocol-specific information

When analyzing performance, it might be nice to let 'qemu-img info'
expose details that it learned from the underlying file or block
device.  Start the process by picking a few useful items determined
during raw_open_common().

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 qapi/block-core.json | 24 +++++++++++++++++++++++-
 block/file-posix.c   | 21 +++++++++++++++++++++
 2 files changed, 44 insertions(+), 1 deletion(-)

17.01.2019 18:33, Eric Blake wrote:
> When analyzing performance, it might be nice to let 'qemu-img info'
> expose details that it learned from the underlying file or block
> device.  Start the process by picking a few useful items determined
> during raw_open_common().
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> ---
>   qapi/block-core.json | 24 +++++++++++++++++++++++-
>   block/file-posix.c   | 21 +++++++++++++++++++++
>   2 files changed, 44 insertions(+), 1 deletion(-)
> 
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index f28d5f5fc76..296c22a1003 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -101,6 +101,25 @@
>         'extents': ['ImageInfo']
>     } }
> 
> +##
> +# @ImageInfoSpecificFile:
> +#
> +# Details about a file protocol BDS.
> +#
> +# @align: Alignment in use
> +#
> +# @discard: True if discard operations can be attempted
> +#
> +# @write-zero: True if efficient write zero operations can be attempted
> +#
> +# @discard-zero: True if discarding is known to read back as zero
> +#
> +# Since: 4.0
> +##
> +{ 'struct': 'ImageInfoSpecificFile',
> +  'data': { 'align': 'int', 'discard': 'bool', 'write-zero': 'bool',
> +            '*discard-zero': 'bool' } }

Not sure if this works, as I remember raw-posix will adjust these fields
on first failed write, and without this first write, we will not actually
know are they supported.

> +
>   ##
>   # @ImageInfoSpecific:
>   #
> @@ -110,12 +129,15 @@
>   ##
>   { 'union': 'ImageInfoSpecific',
>     'data': {
> +      # Format drivers:
>         'qcow2': 'ImageInfoSpecificQCow2',
>         'vmdk': 'ImageInfoSpecificVmdk',
>         # If we need to add block driver specific parameters for
>         # LUKS in future, then we'll subclass QCryptoBlockInfoLUKS
>         # to define a ImageInfoSpecificLUKS
> -      'luks': 'QCryptoBlockInfoLUKS'
> +      'luks': 'QCryptoBlockInfoLUKS',
> +      # Protocol drivers:
> +      'file': 'ImageInfoSpecificFile'
>     } }
> 
>   ##
> diff --git a/block/file-posix.c b/block/file-posix.c
> index 8aee7a3fb8b..d4ce0e9116c 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -1079,6 +1079,23 @@ static void raw_refresh_limits(BlockDriverState *bs, Error **errp)
>       bs->bl.opt_mem_alignment = MAX(s->buf_align, getpagesize());
>   }
> 
> +static ImageInfoSpecific *raw_get_specific_info(BlockDriverState *bs)
> +{
> +    BDRVRawState *s = bs->opaque;
> +    ImageInfoSpecific *info = g_new0(ImageInfoSpecific, 1);
> +    ImageInfoSpecificFile *infofile = g_new0(ImageInfoSpecificFile, 1);
> +
> +    info->type = IMAGE_INFO_SPECIFIC_KIND_FILE;
> +    info->u.file.data = infofile;
> +    infofile->align = s->buf_align;
> +    infofile->discard = s->has_discard;
> +    infofile->write_zero = s->has_write_zeroes;
> +    infofile->has_discard_zero = s->has_discard;
> +    infofile->discard_zero = s->discard_zeroes;
> +
> +    return info;
> +}
> +
>   static int check_for_dasd(int fd)
>   {
>   #ifdef BIODASDINFO2
> @@ -2765,6 +2782,7 @@ BlockDriver bdrv_file = {
>       .bdrv_co_copy_range_from = raw_co_copy_range_from,
>       .bdrv_co_copy_range_to  = raw_co_copy_range_to,
>       .bdrv_refresh_limits = raw_refresh_limits,
> +    .bdrv_get_specific_info = raw_get_specific_info,
>       .bdrv_io_plug = raw_aio_plug,
>       .bdrv_io_unplug = raw_aio_unplug,
>       .bdrv_attach_aio_context = raw_aio_attach_aio_context,
> @@ -3240,6 +3258,7 @@ static BlockDriver bdrv_host_device = {
>       .bdrv_co_copy_range_from = raw_co_copy_range_from,
>       .bdrv_co_copy_range_to  = raw_co_copy_range_to,
>       .bdrv_refresh_limits = raw_refresh_limits,
> +    .bdrv_get_specific_info = raw_get_specific_info,
>       .bdrv_io_plug = raw_aio_plug,
>       .bdrv_io_unplug = raw_aio_unplug,
>       .bdrv_attach_aio_context = raw_aio_attach_aio_context,
> @@ -3363,6 +3382,7 @@ static BlockDriver bdrv_host_cdrom = {
>       .bdrv_co_pwritev        = raw_co_pwritev,
>       .bdrv_co_flush_to_disk  = raw_co_flush_to_disk,
>       .bdrv_refresh_limits = raw_refresh_limits,
> +    .bdrv_get_specific_info = raw_get_specific_info,
>       .bdrv_io_plug = raw_aio_plug,
>       .bdrv_io_unplug = raw_aio_unplug,
>       .bdrv_attach_aio_context = raw_aio_attach_aio_context,
> @@ -3494,6 +3514,7 @@ static BlockDriver bdrv_host_cdrom = {
>       .bdrv_co_pwritev        = raw_co_pwritev,
>       .bdrv_co_flush_to_disk  = raw_co_flush_to_disk,
>       .bdrv_refresh_limits = raw_refresh_limits,
> +    .bdrv_get_specific_info = raw_get_specific_info,
>       .bdrv_io_plug = raw_aio_plug,
>       .bdrv_io_unplug = raw_aio_unplug,
>       .bdrv_attach_aio_context = raw_aio_attach_aio_context,
>

On 1/18/19 8:08 AM, Vladimir Sementsov-Ogievskiy wrote:

>>
>> +##
>> +# @ImageInfoSpecificFile:
>> +#
>> +# Details about a file protocol BDS.
>> +#
>> +# @align: Alignment in use
>> +#
>> +# @discard: True if discard operations can be attempted
>> +#
>> +# @write-zero: True if efficient write zero operations can be attempted
>> +#
>> +# @discard-zero: True if discarding is known to read back as zero
>> +#
>> +# Since: 4.0
>> +##
>> +{ 'struct': 'ImageInfoSpecificFile',
>> +  'data': { 'align': 'int', 'discard': 'bool', 'write-zero': 'bool',
>> +            '*discard-zero': 'bool' } }
> 
> Not sure if this works, as I remember raw-posix will adjust these fields
> on first failed write, and without this first write, we will not actually
> know are they supported.

Indeed. We could make them a tri-state enum instead of a bool (unknown,
supported, unsupported) - but it would require code tweaks to use the
tri-state in the code too (basically, treat unknown and supported as a
request to try on the first write that needs it; and change unknown to
supported or unsupported after that first write).  Then again, I worded
it as "can be attempted", not "known to work", so while it is indeed
non-helpful for 'qemu-img info' (because the attempt wasn't resolved
into something known for sure), it DOES help a long-running qemu process
when querying the same information over QMP (where such writes have
probably been attempted in the past).

Again, this series is more of an RFC on what do we really want to expose
to the user, and when; and not necessarily a hard commitment that this
is the best struct.