Re: [PATCH v3 2/4] luks: implement .bdrv_measure()

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1403 bytes --]

On 11.02.20 17:03, Stefan Hajnoczi wrote:
> Add qemu-img measure support in the "luks" block driver.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  block/crypto.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 62 insertions(+)
> 
> diff --git a/block/crypto.c b/block/crypto.c
> index 24823835c1..453119875e 100644
> --- a/block/crypto.c
> +++ b/block/crypto.c
> @@ -484,6 +484,67 @@ static int64_t block_crypto_getlength(BlockDriverState *bs)

[...]

> +    cryptoopts = qemu_opts_to_qdict_filtered(opts, NULL,
> +            &block_crypto_create_opts_luks, true);
> +    qdict_put_str(cryptoopts, "format", "luks");
> +    create_opts = block_crypto_create_opts_init(cryptoopts, errp);

It looks a bit weird to me to use errp here...

> +    qobject_unref(cryptoopts);
> +    if (!create_opts) {
> +        goto err;
> +    }
> +
> +    if (!qcrypto_block_calculate_payload_offset(create_opts, NULL,
> +                                                &luks_payload_size,
> +                                                &local_err)) {

...and local_err here.  Either works, but consistent style would be a
bit nicer.

But not more correct, so:

Reviewed-by: Max Reitz <mreitz@redhat.com>

> +        goto err;
> +    }

[...]

> +err:
> +    error_propagate(errp, local_err);
> +    return NULL;
> +}


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