Re: [Qemu-block] [PATCH v8 07/20] block: deprecate "encryption=on" in favor of "encrypt.format=aes"

[Max Reitz]

On 2017-06-01 19:27, Daniel P. Berrange wrote:
> Historically the qcow & qcow2 image formats supported a property
> "encryption=on" to enable their built-in AES encryption. We'll
> soon be supporting LUKS for qcow2, so need a more general purpose
> way to enable encryption, with a choice of formats.
> 
> This introduces an "encrypt.format" option, which will later be
> joined by a number of other "encrypt.XXX" options. The use of
> a "encrypt." prefix instead of "encrypt-" is done to facilitate
> mapping to a nested QAPI schema at later date.
> 
> e.g. the preferred syntax is now
> 
>   qemu-img create -f qcow2 -o encrypt.format=aes demo.qcow2
> 
> Reviewed-by: Eric Blake <address@hidden>
> Reviewed-by: Alberto Garcia <address@hidden>
> Signed-off-by: Daniel P. Berrange <address@hidden>
> ---
>  block/qcow.c               | 30 ++++++++++++++---
>  block/qcow2.c              | 33 +++++++++++++++----
>  include/block/block_int.h  |  2 +-
>  qemu-img.c                 |  4 ++-
>  tests/qemu-iotests/082.out | 81 
> ++++++++++++++++++++++++++++++----------------
>  5 files changed, 110 insertions(+), 40 deletions(-)
> 
> diff --git a/block/qcow.c b/block/qcow.c
> index 6738bc7..42f83b2 100644
> --- a/block/qcow.c
> +++ b/block/qcow.c

[...]

> @@ -818,8 +818,16 @@ static int qcow_create(const char *filename, QemuOpts 
> *opts, Error **errp)
>      }
>  
>      backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
> -    if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, false)) {
> -        flags |= BLOCK_FLAG_ENCRYPT;
> +    encryptfmt = qemu_opt_get_del(opts, BLOCK_OPT_ENCRYPT_FORMAT);
> +    if (encryptfmt) {
> +        if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, false)) {

You should probably just use qemu_opt_get_del(opts, BLOCK_OPT_ENCRYPT)
here, because otherwise you can do this:

$ ./qemu-img create -f qcow -o encryption=off,encrypt.format=aes \
    foo.qcow 64M
Formatting 'foo.qcow', fmt=qcow size=67108864 encryption=off
encrypt.format=aes

> +            error_setg(errp, "Options " BLOCK_OPT_ENCRYPT " and "
> +                       BLOCK_OPT_ENCRYPT_FORMAT " are mutually exclusive");
> +            ret = -EINVAL;
> +            goto cleanup;
> +        }
> +    } else if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, false)) {
> +        encryptfmt = "aes";
>      }
>  
>      ret = bdrv_create_file(filename, opts, &local_err);

[...]

> diff --git a/block/qcow2.c b/block/qcow2.c
> index b3ba5da..19dfcd1 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c

[...]

> @@ -2366,8 +2373,16 @@ static int qcow2_create(const char *filename, QemuOpts 
> *opts, Error **errp)
>                      BDRV_SECTOR_SIZE);
>      backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
>      backing_fmt = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FMT);
> -    if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, false)) {
> -        flags |= BLOCK_FLAG_ENCRYPT;
> +    encryptfmt = qemu_opt_get_del(opts, BLOCK_OPT_ENCRYPT_FORMAT);
> +    if (encryptfmt) {
> +        if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, false)) {

Same here.

With these places fixed:

Reviewed-by: Max Reitz <address@hidden>

> +            error_setg(errp, "Options " BLOCK_OPT_ENCRYPT " and "
> +                       BLOCK_OPT_ENCRYPT_FORMAT " are mutually exclusive");
> +            ret = -EINVAL;
> +            goto finish;
> +        }
> +    } else if (qemu_opt_get_bool_del(opts, BLOCK_OPT_ENCRYPT, false)) {
> +        encryptfmt = "aes";
>      }
>      cluster_size = qemu_opt_get_size_del(opts, BLOCK_OPT_CLUSTER_SIZE,
>                                           DEFAULT_CLUSTER_SIZE);

signature.asc
Description: OpenPGP digital signature