Re: [Qemu-block] [PATCH v1 02/15] block: add ability to set a prefix for opt names

[Max Reitz]

On 03.01.2017 19:27, Daniel P. Berrange wrote:
> When integrating the crypto support with qcow/qcow2, we don't
> want to use the bare LUKS option names "hash-alg", "key-secret",
> etc. We want to namespace them "luks-hash-alg", "luks-key-secret"
> so that they don't clash with any general qcow options at a later
> date.
> 
> Signed-off-by: Daniel P. Berrange <address@hidden>
> ---
>  block/crypto.c | 110 
> +++++++++++++++++++++++++++++++++++++++++++++++++--------
>  block/crypto.h |  42 +++++++++++-----------
>  2 files changed, 118 insertions(+), 34 deletions(-)
> 
> diff --git a/block/crypto.c b/block/crypto.c
> index d281de6..1037c70 100644
> --- a/block/crypto.c
> +++ b/block/crypto.c

[...]

> +static int block_crypto_copy_value(void *opaque, const char *name,
> +                                   const char *value, Error **errp)
> +{
> +    struct BlockCryptoCopyData *data = opaque;
> +
> +    if (g_str_has_prefix(name, data->prefix)) {
> +        Error *local_err = NULL;
> +        const char *newname = name + strlen(data->prefix);

strstart() would be shorter:

const char *newname;

if (strstart(name, data->prefix, &newname)) {
    /* ... */
}

> +
> +        qemu_opt_set(data->opts, newname, value, &local_err);
> +        if (local_err) {
> +            error_propagate(errp, local_err);
> +            return 1;

I'd prefer -1, because 0/1 looks more like false/true to me, which in
turn looks like failure/success.

Both optional suggestions, so either way:

Reviewed-by: Max Reitz <address@hidden>

> +        }
> +    }
> +
> +    return 0;
> +}

[...]

signature.asc
Description: OpenPGP digital signature