On Tue 21 Feb 2017 12:55:07 PM CET, Daniel P. Berrange wrote:
> static int qcow2_set_up_encryption(BlockDriverState *bs, QemuOpts *opts,
> - Error **errp)
> + const char *fmtstr, Error **errp)
> {
> BDRVQcow2State *s = bs->opaque;
> QCryptoBlockCreateOptions *cryptoopts = NULL;
> QCryptoBlock *crypto = NULL;
> int ret = -EINVAL;
> + int fmt = qcow2_crypt_method_from_format(fmtstr);
>
> - cryptoopts = block_crypto_create_opts_init(
> - Q_CRYPTO_BLOCK_FORMAT_QCOW, opts, "aes-", errp);
> + switch (fmt) {
> + case QCOW_CRYPT_LUKS:
> + cryptoopts = block_crypto_create_opts_init(
> + Q_CRYPTO_BLOCK_FORMAT_LUKS, opts, "luks-", errp);
> + break;
> + case QCOW_CRYPT_AES:
> + cryptoopts = block_crypto_create_opts_init(
> + Q_CRYPTO_BLOCK_FORMAT_QCOW, opts, "aes-", errp);
> + break;
> + default:
> + error_setg(errp, "Unknown encryption format %s", fmtstr);
> + break;
> + }
> + s->crypt_method_header = fmt;
> if (!cryptoopts) {
> ret = -EINVAL;
> goto out;
> }
I don't believe it has any practical effect in this case, but I think
it's cleaner to set s->crypt_method_header = fmt after checking that
everything went well. Otherwise an incorrect format string will result
in s->crypt_method_header = -EINVAL, which doesn't make sense (it's not
even the correct type: the variable is unsigned).
Other than that, the patch looks good to me.
Reviewed-by: Alberto Garcia <be...@igalia.com>
Berto
