Re: [Qemu-devel] [PATCH v11 1/3] bdrv_query_image_info Error parameter added

[Kevin Wolf <kwolf@redhat.com>]

Am 31.01.2019 um 14:46 hat Andrey Shinkevich geschrieben:
> Inform a user in case qcow2_get_specific_info fails to obtain
> QCOW2 image specific information. This patch is preliminary to
> the print of bitmap information in the 'qemu-img info' output.
> 
> Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>

> diff --git a/block/crypto.c b/block/crypto.c
> index f0a5f6b..4ede833 100644
> --- a/block/crypto.c
> +++ b/block/crypto.c
> @@ -594,13 +594,13 @@ static int block_crypto_get_info_luks(BlockDriverState *bs,
>  }
>  
>  static ImageInfoSpecific *
> -block_crypto_get_specific_info_luks(BlockDriverState *bs)
> +block_crypto_get_specific_info_luks(BlockDriverState *bs, Error **errp)
>  {
>      BlockCrypto *crypto = bs->opaque;
>      ImageInfoSpecific *spec_info;
>      QCryptoBlockInfo *info;
>  
> -    info = qcrypto_block_get_info(crypto->block, NULL);
> +    info = qcrypto_block_get_info(crypto->block, errp);
>      if (!info) {
>          return NULL;
>      }

In v9 of the series I suggested to have another patch to remove errp
from qcrypto_block_get_info() because it never returns errors anyway.

Don't you think that would be better? But it's your decision, I'm fine
with either way.

> diff --git a/block/qapi.c b/block/qapi.c
> index c66f949..f53f100 100644
> --- a/block/qapi.c
> +++ b/block/qapi.c
> @@ -282,7 +282,12 @@ void bdrv_query_image_info(BlockDriverState *bs,
>          info->dirty_flag = bdi.is_dirty;
>          info->has_dirty_flag = true;
>      }
> -    info->format_specific     = bdrv_get_specific_info(bs);
> +    info->format_specific     = bdrv_get_specific_info(bs, &err);

The spacing looks odd now. I'd either keep the assignment for
info->has_format_specific above the if block so that both are aligned to
the same column, or remove the additional spaces.

With that fixed:

Reviewed-by: Kevin Wolf <kwolf@redhat.com>