Re: [Qemu-devel] [PATCH v6 09/18] qcow: convert QCow to use QCryptoBlock for encryption

[Alberto Garcia]

On Tue 25 Apr 2017 05:38:49 PM CEST, Daniel P. Berrange wrote:
> @@ -181,8 +188,39 @@ static int qcow_open(BlockDriverState *bs, QDict 
> *options, int flags,
  [...]
> +            crypto_opts = block_crypto_open_opts_init(
> +                Q_CRYPTO_BLOCK_FORMAT_QCOW, encryptopts, &local_err);
> +            if (local_err) {
> +                error_propagate(errp, local_err);
> +                ret = -EINVAL;
> +                goto fail;
> +            }

Not very important, but if you check !crypto_opts for errors instead you
can pass errp directly and avoid that error_propagate() call. Exactly
the same that you do here in qcrypto_block_open():

> +            if (flags & BDRV_O_NO_IO) {
> +                cflags |= QCRYPTO_BLOCK_OPEN_NO_IO;
> +            }
> +            s->crypto = qcrypto_block_open(crypto_opts, NULL, NULL,
> +                                           cflags, errp);
> +            if (!s->crypto) {
> +                ret = -EINVAL;
> +                goto fail;
> +            }


> @@ -792,6 +762,10 @@ static int qcow_create(const char *filename, QemuOpts 
> *opts, Error **errp)
>      int ret;
>      BlockBackend *qcow_blk;
>      const char *encryptfmt = NULL;
> +    QDict *options;
> +    QDict *encryptopts = NULL;
> +    QCryptoBlockCreateOptions *crypto_opts = NULL;
> +    QCryptoBlock *crypto = NULL;
>  
>      /* Read out options */
>      total_size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
> @@ -865,6 +839,10 @@ static int qcow_create(const char *filename, QemuOpts 
> *opts, Error **errp)
>      l1_size = (total_size + (1LL << shift) - 1) >> shift;
>  
>      header.l1_table_offset = cpu_to_be64(header_size);
> +
> +    options = qemu_opts_to_qdict(opts, NULL);
> +    qdict_extract_subqdict(options, &encryptopts, "encrypt.");
> +    QDECREF(options);

I think you're leaking encryptopts in this function.

>  ##
> +# @BlockdevQcowEncryptionFormat:
> +# @qcow: AES-CBC with plain64 initialization venctors
> +#
> +# Since: 2.10
> +##
> +{ 'enum': 'BlockdevQcowEncryptionFormat',
> +  'data': [ 'qcow' ] }

Shouldn't this be 'aes' instead of 'qcow' ??

Berto