Re: [Qemu-block] [PATCH v1 15/15] block: remove all encryption handling APIs

[Max Reitz]

On 03.01.2017 19:28, Daniel P. Berrange wrote:
> Now that all encryption keys must be provided upfront via
> the QCryptoSecret API and associated block driver properties
> there is no need for any explicit encryption handling APIs
> in the block layer. Encryption can be handled transparently
> within the block driver. We only retain an API for querying
> whether an image is encrypted or not, since that is a
> potentially useful piece of metadata to report to the user.
> 
> Signed-off-by: Daniel P. Berrange <address@hidden>
> ---
>  block.c                   | 77 
> +----------------------------------------------
>  block/crypto.c            |  1 -
>  block/qapi.c              |  2 +-
>  block/qcow.c              |  1 -
>  block/qcow2.c             |  1 -
>  blockdev.c                | 37 ++---------------------
>  include/block/block.h     |  3 --
>  include/block/block_int.h |  1 -
>  include/qapi/error.h      |  1 -
>  qapi/common.json          |  5 +--
>  10 files changed, 5 insertions(+), 124 deletions(-)

It would probably make sense to replace the description of
BlockDeviceInfo's @encryption_key_missing in qapi/block-core.json by
"Deprecated; always false".

[...]

> diff --git a/blockdev.c b/blockdev.c
> index 245e1e1..dfeba0c 100644
> --- a/blockdev.c
> +++ b/blockdev.c

[...]

> @@ -2244,24 +2240,8 @@ void qmp_block_passwd(bool has_device, const char 
> *device,
>                        bool has_node_name, const char *node_name,
>                        const char *password, Error **errp)
>  {
> -    Error *local_err = NULL;
> -    BlockDriverState *bs;
> -    AioContext *aio_context;
> -
> -    bs = bdrv_lookup_bs(has_device ? device : NULL,
> -                        has_node_name ? node_name : NULL,
> -                        &local_err);
> -    if (local_err) {
> -        error_propagate(errp, local_err);
> -        return;
> -    }
> -
> -    aio_context = bdrv_get_aio_context(bs);
> -    aio_context_acquire(aio_context);
> -
> -    bdrv_add_key(bs, password, errp);
> -
> -    aio_context_release(aio_context);
> +    error_setg_errno(errp, -ENOSYS,
> +                     "Setting block passwords directly is no longer 
> supported");

A plain error_setg() without _errno should be sufficient.

>  }
>  

I'll leave it up to you whether you want to follow the suggestions I've
given, so:

Reviewed-by: Max Reitz <address@hidden>

signature.asc
Description: OpenPGP digital signature