[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH 05/16] block: Convert bs->file to BdrvChild
From: |
Kevin Wolf |
Subject: |
Re: [Qemu-block] [PATCH 05/16] block: Convert bs->file to BdrvChild |
Date: |
Wed, 23 Sep 2015 15:13:00 +0200 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
Am 22.09.2015 um 20:36 hat Max Reitz geschrieben:
> On 17.09.2015 15:48, Kevin Wolf wrote:
> > This patch removes the temporary duplication between bs->file and
> > bs->file_child by converting everything to BdrvChild.
> >
> > Signed-off-by: Kevin Wolf <address@hidden>
> > ---
> > block.c | 61
> > ++++++++++++++++++++++------------------------
> > block/blkdebug.c | 32 ++++++++++++------------
> > block/blkverify.c | 33 ++++++++++++++-----------
> > block/bochs.c | 8 +++---
> > block/cloop.c | 10 ++++----
> > block/dmg.c | 20 +++++++--------
> > block/io.c | 50 +++++++++++++++++++-------------------
> > block/parallels.c | 38 +++++++++++++++--------------
> > block/qapi.c | 2 +-
> > block/qcow.c | 42 +++++++++++++++++---------------
> > block/qcow2-cache.c | 11 +++++----
> > block/qcow2-cluster.c | 38 +++++++++++++++++------------
> > block/qcow2-refcount.c | 45 ++++++++++++++++++----------------
> > block/qcow2-snapshot.c | 30 ++++++++++++-----------
> > block/qcow2.c | 62
> > ++++++++++++++++++++++++-----------------------
> > block/qed-table.c | 4 +--
> > block/qed.c | 32 ++++++++++++------------
> > block/raw_bsd.c | 40 +++++++++++++++---------------
> > block/snapshot.c | 12 ++++-----
> > block/vdi.c | 17 +++++++------
> > block/vhdx-log.c | 25 ++++++++++---------
> > block/vhdx.c | 36 ++++++++++++++-------------
> > block/vmdk.c | 27 +++++++++++----------
> > block/vpc.c | 34 ++++++++++++++------------
> > include/block/block.h | 8 +++++-
> > include/block/block_int.h | 3 +--
> > 26 files changed, 377 insertions(+), 343 deletions(-)
>
> Reviewed-by: Max Reitz <address@hidden>
>
> > diff --git a/block.c b/block.c
> > index 2e9e5e2..93d713b 100644
> > --- a/block.c
> > +++ b/block.c
>
> [...]
>
> > @@ -1929,6 +1925,11 @@ void bdrv_close(BlockDriverState *bs)
> > bdrv_unref(backing_hd);
> > }
> >
> > + if (bs->file != NULL) {
> > + bdrv_unref(bs->file->bs);
>
> I think we can use bdrv_unref_child(bs->file) here. I'd personally like
> it more, at least (because using bdrv_unref() and relying on the loop
> below is basically what the comment inside of the loop advises against).
>
> Yes, I know, eventually, we want to drop this block anyway and let the
> loop below handle everything using bdrv_unref_child(). But when we do
> that conversion, it'll be obvious to drop a bdrv_unref_child() call,
> whereas dropping bdrv_unref() alone isn't so obvious.
Seems safe to do, and reads a bit nicer, so I'll change the patch
accordingly. Thanks.
Kevin
> > + bs->file = NULL;
> > + }
> > +
> > QLIST_FOREACH_SAFE(child, &bs->children, next, next) {
> > /* TODO Remove bdrv_unref() from drivers' close function and
> > use
> > * bdrv_unref_child() here */
> > @@ -1953,11 +1954,6 @@ void bdrv_close(BlockDriverState *bs)
> > bs->options = NULL;
> > QDECREF(bs->full_open_options);
> > bs->full_open_options = NULL;
> > -
> > - if (bs->file != NULL) {
> > - bdrv_unref(bs->file);
> > - bs->file = NULL;
> > - }
> > }
> >
> > if (bs->blk) {
>
pgp36qEcyc0tu.pgp
Description: PGP signature
- [Qemu-block] [PATCH 08/16] block: Manage backing file references in bdrv_set_backing_hd(), (continued)
- [Qemu-block] [PATCH 08/16] block: Manage backing file references in bdrv_set_backing_hd(), Kevin Wolf, 2015/09/17
- [Qemu-block] [PATCH 09/16] block: Split bdrv_move_feature_fields(), Kevin Wolf, 2015/09/17
- [Qemu-block] [PATCH 07/16] block: Convert bs->backing_hd to BdrvChild, Kevin Wolf, 2015/09/17
- [Qemu-block] [PATCH 10/16] block/io: Make bdrv_requests_pending() public, Kevin Wolf, 2015/09/17
- [Qemu-block] [PATCH 05/16] block: Convert bs->file to BdrvChild, Kevin Wolf, 2015/09/17
- [Qemu-block] [PATCH 11/16] block-backend: Add blk_set_bs(), Kevin Wolf, 2015/09/17
- [Qemu-block] [PATCH 13/16] block: Implement bdrv_append() without bdrv_swap(), Kevin Wolf, 2015/09/17