31.01.2019 16:46, Andrey Shinkevich wrote:
> 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.shinkev...@virtuozzo.com>
> Reviewed-by: Eric Blake <ebl...@redhat.com>
> ---
[...]
> --- 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;
> }
more context:
if (info->format != Q_CRYPTO_BLOCK_FORMAT_LUKS) {
qapi_free_QCryptoBlockInfo(info);
return NULL;
}
for a fast look, I think it should be assertion, not if, Daniel, am I right?
Also, I think we don't have block/crypto.c in Cryptography section of
MAINTAINERS
by mistake, so you were not CC'ed.
> 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);
while being here, let's drop these extra whitespaces
> + if (err) {
> + error_propagate(errp, err);
> + qapi_free_ImageInfo(info);
> + goto out;
> + }
> info->has_format_specific = info->format_specific != NULL;
>
> backing_filename = bs->backing_file;
So, I'm unsure about crypto, but it's safe to keep it as is, so for this patch:
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
--
Best regards,
Vladimir
