On Tue, Feb 21, 2017 at 02:19:46PM +0100, Alberto Garcia wrote:
> On Tue 21 Feb 2017 12:55:03 PM CET, Daniel P. Berrange wrote:
> > @@ -175,8 +185,31 @@ static int qcow_open(BlockDriverState *bs, QDict
> > *options, int flags,
> > ret = -ENOSYS;
> > goto fail;
> > }
> > + if (s->crypt_method_header == QCOW_CRYPT_AES) {
> > + crypto_opts = block_crypto_open_opts_init(
> > + Q_CRYPTO_BLOCK_FORMAT_QCOW, opts, "aes-", &local_err);
> > + if (local_err) {
> > + error_propagate(errp, local_err);
> > + ret = -EINVAL;
> > + goto fail;
> > + }
> >
> > + if (flags & BDRV_O_NO_IO) {
> > + cflags |= QCRYPTO_BLOCK_OPEN_NO_IO;
> > + }
> > + s->crypto = qcrypto_block_open(crypto_opts, NULL, NULL,
> > + cflags, errp);
>
> You don't call qcrypto_block_free() if qcow_open() eventually fails.
> Although qcow_close() takes care of that, a failure to open the image
> sets bs->drv = NULL in bdrv_open_common(), preventing qcow_close() from
> being called.
Yes, I'll fix that leak.
>
> > @@ -260,14 +293,17 @@ static int qcow_open(BlockDriverState *bs, QDict
> > *options, int flags,
> > goto fail;
> > }
> >
> > + qapi_free_QCryptoBlockOpenOptions(crypto_opts);
> > qemu_co_mutex_init(&s->lock);
> > return 0;
> >
> > fail:
> > + qemu_opts_del(opts);
>
> You need to delete opts as well if this function succeeds, don't you?
New version will avoid the use of QemuOpts entirely - using the QDict
APIs instead, but I'll have equiv cleanup todo.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
