|
From: | Manos Pitsidianakis |
Subject: | Re: [Qemu-block] [PATCH RFC v3 3/8] block: add throttle block filter driver |
Date: | Tue, 27 Jun 2017 16:34:22 +0300 |
User-agent: | NeoMutt/20170609-57-1e93be (1.8.3) |
On Tue, Jun 27, 2017 at 01:45:40PM +0100, Stefan Hajnoczi wrote:
On Mon, Jun 26, 2017 at 07:26:41PM +0300, Manos Pitsidianakis wrote:On Mon, Jun 26, 2017 at 03:30:55PM +0100, Stefan Hajnoczi wrote: > > + bs->file = bdrv_open_child(NULL, options, "file", > > + bs, &child_file, false, &local_err); > > + > > + if (local_err) { > > + error_propagate(errp, local_err); > > + return -EINVAL; > > + } > > + > > + qdict_flatten(options); > > + return throttle_configure_tgm(bs, tgm, options, errp); > > Who destroys bs->file on error? It is reaped by bdrv_open_inherit() on failure, if I'm not mistaken. That's how other drivers handle this as well. Some (eg block/qcow2.c) check if bs->file is NULL instead of the error pointer they pass, so this is not not very consistent.Maybe I'm missing it but I don't see relevant bs->file cleanup in bdrv_open_inherit() or bdrv_open_common(). Please post the exact line where it happens. Stefan
Relevant commit: de234897b60e034ba94b307fc289e2dc692c9251 block: Do not unref bs->file on error in BD's open
bdrv_open_inherit() does this on failure: fail: blk_unref(file); if (bs->file != NULL) { bdrv_unref_child(bs, bs->file); }While looking into this I noticed bdrv_new_open_driver() doesn't handle bs->file on failure. It simply unrefs the bs but because its child's ref still remains, it is leaked.
signature.asc
Description: PGP signature
[Prev in Thread] | Current Thread | [Next in Thread] |