[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 05/23] block: Make BlockBackend own its BlockDri
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-devel] [PATCH 05/23] block: Make BlockBackend own its BlockDriverState |
Date: |
Wed, 10 Sep 2014 12:14:18 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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 <address@hidden>
> ---
> 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
[Qemu-devel] [PATCH 05/23] block: Make BlockBackend own its BlockDriverState, Markus Armbruster, 2014/09/10
- Re: [Qemu-devel] [PATCH 05/23] block: Make BlockBackend own its BlockDriverState,
Kevin Wolf <=
[Qemu-devel] [PATCH 10/23] block: Eliminate DriveInfo member bdrv, use blk_by_legacy_dinfo(), Markus Armbruster, 2014/09/10
- Re: [Qemu-devel] [PATCH 10/23] block: Eliminate DriveInfo member bdrv, use blk_by_legacy_dinfo(), Benoît Canet, 2014/09/11
- Re: [Qemu-devel] [PATCH 10/23] block: Eliminate DriveInfo member bdrv, use blk_by_legacy_dinfo(), Benoît Canet, 2014/09/11
- Re: [Qemu-devel] [PATCH 10/23] block: Eliminate DriveInfo member bdrv, use blk_by_legacy_dinfo(), Markus Armbruster, 2014/09/11
- Re: [Qemu-devel] [PATCH 10/23] block: Eliminate DriveInfo member bdrv, use blk_by_legacy_dinfo(), Benoît Canet, 2014/09/11
- Re: [Qemu-devel] [PATCH 10/23] block: Eliminate DriveInfo member bdrv, use blk_by_legacy_dinfo(), Eric Blake, 2014/09/11
- Re: [Qemu-devel] [PATCH 10/23] block: Eliminate DriveInfo member bdrv, use blk_by_legacy_dinfo(), Markus Armbruster, 2014/09/12
[Qemu-devel] [PATCH 12/23] virtio-blk: Drop redundant VirtIOBlock member conf, Markus Armbruster, 2014/09/10