On Thu 25 May 2017 06:38:40 PM CEST, Daniel P. Berrange wrote:
> @@ -105,6 +116,13 @@ static int qcow_open(BlockDriverState *bs, QDict
> *options, int flags,
> int ret;
> QCowHeader header;
> Error *local_err = NULL;
> + QCryptoBlockOpenOptions *crypto_opts = NULL;
> + unsigned int cflags = 0;
> + QDict *encryptopts = NULL;
> + const char *encryptfmt;
> +
> + qdict_extract_subqdict(options, &encryptopts, "encrypt.");
> + encryptfmt = qdict_get_try_str(encryptopts, "format");
>
> bs->file = bdrv_open_child(NULL, options, "file", bs, &child_file,
> false, errp);
> if (!bs->file) {
> return -EINVAL;
> }
You're leaking encryptopts if the function returns here.
> @@ -873,6 +850,20 @@ static int qcow_create(const char *filename, QemuOpts
> *opts, Error **errp)
> goto exit;
> }
> header.crypt_method = cpu_to_be32(QCOW_CRYPT_AES);
> +
> + crypto_opts = block_crypto_create_opts_init(
> + Q_CRYPTO_BLOCK_FORMAT_QCOW, encryptopts, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + ret = -EINVAL;
> + goto exit;
> + }
Not very important, and my fault for not having pointed it out in my
previous review, but you can spare the error_propagate() call if you
pass errp directly to block_crypto_create_opts_init() and then check if
crypto_opts is NULL.
Actually none of the error_propagate() calls in qcow_create() is really
necessary, but that could be fixed in a separate patch, if at all (it's
not so important).
The leak however needs to be fixed. With that,
Reviewed-by: Alberto Garcia <be...@igalia.com>
Berto
