From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36282) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZejzG-00034z-B3 for qemu-devel@nongnu.org; Wed, 23 Sep 2015 09:21:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZejzA-0005zz-BF for qemu-devel@nongnu.org; Wed, 23 Sep 2015 09:21:30 -0400 Date: Wed, 23 Sep 2015 15:13:00 +0200 From: Kevin Wolf Message-ID: <20150923131300.GC4557@noname.redhat.com> References: <1442497700-2536-1-git-send-email-kwolf@redhat.com> <1442497700-2536-6-git-send-email-kwolf@redhat.com> <56019F9D.9020800@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="rS8CxjVDS/+yyDmU" Content-Disposition: inline In-Reply-To: <56019F9D.9020800@redhat.com> Subject: Re: [Qemu-devel] [PATCH 05/16] block: Convert bs->file to BdrvChild List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Max Reitz Cc: berto@igalia.com, qemu-block@nongnu.org, jcody@redhat.com, qemu-devel@nongnu.org, armbru@redhat.com, stefanha@redhat.com --rS8CxjVDS/+yyDmU Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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. > >=20 > > Signed-off-by: Kevin Wolf > > --- > > 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(-) >=20 > Reviewed-by: Max Reitz >=20 > > diff --git a/block.c b/block.c > > index 2e9e5e2..93d713b 100644 > > --- a/block.c > > +++ b/block.c >=20 > [...] >=20 > > @@ -1929,6 +1925,11 @@ void bdrv_close(BlockDriverState *bs) > > bdrv_unref(backing_hd); > > } > > =20 > > + if (bs->file !=3D NULL) { > > + bdrv_unref(bs->file->bs); >=20 > 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). >=20 > 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 =3D NULL; > > + } > > + > > QLIST_FOREACH_SAFE(child, &bs->children, next, next) { > > /* TODO Remove bdrv_unref() from drivers' close function a= nd use > > * bdrv_unref_child() here */ > > @@ -1953,11 +1954,6 @@ void bdrv_close(BlockDriverState *bs) > > bs->options =3D NULL; > > QDECREF(bs->full_open_options); > > bs->full_open_options =3D NULL; > > - > > - if (bs->file !=3D NULL) { > > - bdrv_unref(bs->file); > > - bs->file =3D NULL; > > - } > > } > > =20 > > if (bs->blk) { >=20 --rS8CxjVDS/+yyDmU Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.22 (GNU/Linux) iQIcBAEBAgAGBQJWAqVbAAoJEH8JsnLIjy/WeVsQAL06aAa7e0c+UkLPp6zcutKx fuqbOyDYNArPU15SFZtD7o1JaVp/tAQ94p1i6AzCz3cp9n8OLNZsySofqEaHHQiU dY4yGxuowisUoBlB1bBpz045u0oU4qQZWrIKRSV53iIeDi1JPYue+v5Iy5RWY3qy zSOAs8rb/RebzjGDM58Qw4OVryAoTOToL/H6OrN4C4eeMiarg7TwfujxkgSAhCPQ /fMErWovb7Gj9fwjCGRd51W5ZFa1+BAl8Qy10CdeQaErifSPQdRha2B5YGEYCf1A K8UPArkk1Ohzm9wZdPvEGVi9+NFTkXg9cPJNoG5a/ZMiLKrKF0tPL8eS48+2YAmH K8GRB1owxBwG/Xp/N9fLMoFl6AThHUScXegihBf3WM79sL4TQt+R8u6Vm5Zc9oIG 1gZHOySgRCqf7X/Gu/fm3suRwO9GKkqNvJaOwQbk7rQFzURYGdDDoWnaVkyS0W3W QqXiLsFJyQ1um/g8NhosyTIroZjjucp5e8GtRWyEjNRPEkDL3imPo4gJWex0XTVK 5frLHV23lRAFOMLOrqzgeiuGBG08eNURjhNrBzascxRPrJdody2MiMp9ipK7xKie 2zHh1tLQ5qNHqxOXqonZK35WoNA2dUKb1zIf27OAI7rjWzGAQ366PaWlzVIWD1hP K1yGNomR6pUV1G2lh8Xt =OwHD -----END PGP SIGNATURE----- --rS8CxjVDS/+yyDmU--