From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53842) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZlvQb-0005g7-1x for qemu-devel@nongnu.org; Tue, 13 Oct 2015 04:59:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZlvQa-00042J-6M for qemu-devel@nongnu.org; Tue, 13 Oct 2015 04:59:24 -0400 Date: Tue, 13 Oct 2015 10:59:14 +0200 From: Kevin Wolf Message-ID: <20151013085914.GC4906@noname.str.redhat.com> References: <1444392941-28704-1-git-send-email-kwolf@redhat.com> <1444392941-28704-6-git-send-email-kwolf@redhat.com> <20151013013305.GL11943@localhost.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20151013013305.GL11943@localhost.localdomain> Subject: Re: [Qemu-devel] [PATCH v3 05/16] block: Convert bs->file to BdrvChild List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jeff Cody Cc: berto@igalia.com, qemu-block@nongnu.org, qemu-devel@nongnu.org, armbru@redhat.com, stefanha@redhat.com, famz@redhat.com, mreitz@redhat.com Am 13.10.2015 um 03:33 hat Jeff Cody geschrieben: > On Fri, Oct 09, 2015 at 02:15:30PM +0200, 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 > > Reviewed-by: Max Reitz > > Reviewed-by: Alberto Garcia > > Reviewed-by: Fam Zheng > > --- > > block.c | 63 ++++++++++++++++++++++------------------------- > > 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, 378 insertions(+), 344 deletions(-) > > > > [snip lots of code] > > > diff --git a/block/blkdebug.c b/block/blkdebug.c > > index bc247f4..117fce6 100644 > > --- a/block/blkdebug.c > > +++ b/block/blkdebug.c > > @@ -427,10 +427,10 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags, > > s->state = 1; > > > > /* Open the backing file */ > > - assert(bs->file == NULL); > > - ret = bdrv_open_image(&bs->file, qemu_opt_get(opts, "x-image"), options, "image", > > - bs, &child_file, false, &local_err); > > - if (ret < 0) { > > + bs->file = bdrv_open_child(qemu_opt_get(opts, "x-image"), options, "image", > > + bs, &child_file, false, &local_err); > > + if (local_err) { > > + ret = -EINVAL; > > error_propagate(errp, local_err); > > goto out; > > } > > @@ -449,7 +449,7 @@ static int blkdebug_open(BlockDriverState *bs, QDict *options, int flags, > > goto out; > > > > fail_unref: > > - bdrv_unref(bs->file); > > + bdrv_unref(bs->file->bs); > > > Would it be better to use bdrv_unref_child() here? Good catch, without it the BdrvChild object is leaked here. (And blkverify already leaks the whole BDS on .bdrv_open failure. I'll send a separate patch on top of this series for that.) I'll just fix this (and your coding style points) while merging the series; unless something major comes up, I'm not going to send another version. Kevin