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). Kevin