Kevin Wolf <kw...@redhat.com> writes: > Am 10.09.2014 um 10:13 hat Markus Armbruster geschrieben: >> On BlockBackend destruction, unref its BlockDriverState. Replaces the >> callers' unrefs. >> >> Signed-off-by: Markus Armbruster <arm...@redhat.com> >> --- >> block/block-backend.c | 9 ++------- >> blockdev.c | 11 +++-------- >> hw/block/xen_disk.c | 6 +++--- >> qemu-img.c | 35 +---------------------------------- >> qemu-io.c | 5 ----- >> 5 files changed, 9 insertions(+), 57 deletions(-) >> >> diff --git a/block/block-backend.c b/block/block-backend.c >> index 2a22660..ae51f7f 100644 >> --- a/block/block-backend.c >> +++ b/block/block-backend.c >> @@ -58,10 +58,7 @@ BlockBackend *blk_new(const char *name, Error **errp) >> * @errp: return location for an error to be set on failure, or %NULL >> * >> * Create a new BlockBackend, with a reference count of one, and >> - * attach a new BlockDriverState to it, also with a reference count of >> - * one. Caller owns *both* references. >> - * TODO Let caller own only the BlockBackend reference >> - * Fail if @name already exists. >> + * a new BlockDriverState attached. Fail if @name already exists. >> * >> * Returns: the BlockBackend on success, %NULL on error >> */ >> @@ -88,6 +85,7 @@ BlockBackend *blk_new_with_bs(const char *name, Error >> **errp) >> static void blk_delete(BlockBackend *blk) >> { >> assert(!blk->refcnt); >> + bdrv_unref(blk->bs); >> blk_detach_bs(blk); > > I think the bdrv_unref() should really be part of blk_detach_bs(). > > The same way it would be more logical to have bdrv_ref() as part of > blk_attach_bs(). For blk_new_with_bs() this might mean bdrv_new, > blk_attach_bs, bdrv_unref, which looks a bit odd, but if blk_attach_bs() > is ever called from somewhere else, it probably makes more sense (if it > isn't, it should be static).
My thinking was that you usually want to acquire a reference when you detach, and you're usually ready to give yours up when you attach. However, I now think I got a use-after-free hidden around there. I'll look into it tomorrow with a fresh mind. Might lead to me accepting your suggestion without further ado :)