Re: [Qemu-devel] [PATCH v7 19/20] qcow2: report encryption specific image information

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3579 bytes --]

On 05/25/2017 11:38 AM, Daniel P. Berrange wrote:
> Currently 'qemu-img info' reports a simple "encrypted: yes"
> field. This is not very useful now that qcow2 can support
> multiple encryption formats. Users want to know which format
> is in use and some data related to it.
> 
> Wire up usage of the qcrypto_block_get_info() method so that
> 'qemu-img info' can report about the encryption format
> and parameters in use
> 

> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  block/qcow2.c        | 35 ++++++++++++++++++++++++++++++++++-
>  qapi/block-core.json | 24 +++++++++++++++++++++++-
>  2 files changed, 57 insertions(+), 2 deletions(-)
> 
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 38f9eb5..cb321a2 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -3196,8 +3196,17 @@ static int qcow2_get_info(BlockDriverState *bs, BlockDriverInfo *bdi)
>  static ImageInfoSpecific *qcow2_get_specific_info(BlockDriverState *bs)
>  {
>      BDRVQcow2State *s = bs->opaque;
> -    ImageInfoSpecific *spec_info = g_new(ImageInfoSpecific, 1);
> +    ImageInfoSpecific *spec_info;
> +    QCryptoBlockInfo *encrypt_info = NULL;
>  
> +    if (s->crypto != NULL) {
> +        encrypt_info = qcrypto_block_get_info(s->crypto, NULL);

Worth using &error_abort instead of silently ignoring the error?  Is an
error even possible in our output visitor [adding Markus for reference]?

> +        if (!encrypt_info) {
> +            return NULL;
> +        }

If you use &error_abort, this is a dead check.

> +    }
> +
> +    spec_info = g_new(ImageInfoSpecific, 1);
>      *spec_info = (ImageInfoSpecific){
>          .type  = IMAGE_INFO_SPECIFIC_KIND_QCOW2,
>          .u.qcow2.data = g_new(ImageInfoSpecificQCow2, 1),
> @@ -3224,6 +3233,30 @@ static ImageInfoSpecific *qcow2_get_specific_info(BlockDriverState *bs)
>          assert(false);
>      }
>  
> +    if (encrypt_info) {
> +        ImageInfoSpecificQCow2Encryption *qencrypt =
> +            g_new(ImageInfoSpecificQCow2Encryption, 1);
> +        switch (encrypt_info->format) {
> +        case Q_CRYPTO_BLOCK_FORMAT_QCOW:
> +            qencrypt->format = BLOCKDEV_QCOW2_ENCRYPTION_FORMAT_AES;
> +            qencrypt->u.aes = encrypt_info->u.qcow;
> +            break;
> +        case Q_CRYPTO_BLOCK_FORMAT_LUKS:
> +            qencrypt->format = BLOCKDEV_QCOW2_ENCRYPTION_FORMAT_LUKS;
> +            qencrypt->u.luks = encrypt_info->u.luks;
> +            break;
> +        default:
> +            assert(false);
> +        }
> +        /* Since we did shallow copy above, erase any pointers
> +         * in the original info */
> +        memset(&encrypt_info->u, 0, sizeof(encrypt_info->u));
> +        qapi_free_QCryptoBlockInfo(encrypt_info);

Does QAPI_CLONE_MEMBERS (commit 4626a19c) make this code any easier to
write?  Then again, malloc'ing duplicates to then free the unchanged
original is a bit slower than stealing references from the original then
making sure the original can be freed without problem.

>  # @ImageInfoSpecificQCow2:
>  #
>  # @compat: compatibility level
> @@ -51,7 +72,8 @@
>        'compat': 'str',
>        '*lazy-refcounts': 'bool',
>        '*corrupt': 'bool',
> -      'refcount-bits': 'int'
> +      'refcount-bits': 'int',
> +      '*encrypt': 'ImageInfoSpecificQCow2Encryption'

Missing documentation of the @encrypt field.

Close!

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]