From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39518) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZenXH-0001NR-Dl for qemu-devel@nongnu.org; Wed, 23 Sep 2015 13:08:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZenXG-0007eD-Ao for qemu-devel@nongnu.org; Wed, 23 Sep 2015 13:08:51 -0400 References: <1442497700-2536-1-git-send-email-kwolf@redhat.com> <1442497700-2536-16-git-send-email-kwolf@redhat.com> From: Max Reitz Message-ID: <5602DC99.5040204@redhat.com> Date: Wed, 23 Sep 2015 19:08:41 +0200 MIME-Version: 1.0 In-Reply-To: <1442497700-2536-16-git-send-email-kwolf@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="5vFljDb0rO44XpPMFuHAqNvhPAqG0sgqn" Subject: Re: [Qemu-devel] [PATCH 15/16] block: Add and use bdrv_replace_in_backing_chain() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf , qemu-block@nongnu.org Cc: jcody@redhat.com, berto@igalia.com, armbru@redhat.com, stefanha@redhat.com, qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --5vFljDb0rO44XpPMFuHAqNvhPAqG0sgqn Content-Type: text/plain; charset=iso-8859-15 Content-Transfer-Encoding: quoted-printable On 17.09.2015 15:48, Kevin Wolf wrote: > This cleans up the mess we left behind in the mirror code after the > previous patch. Instead of using bdrv_swap(), just change pointers. >=20 > The interface change of the mirror job that callers must consider is > that after job completion, their local BDS pointers still point to the > same node now. qemu-img must change its code accordingly (which makes i= t > easier to understand); the other callers stays unchanged because after > completion they don't do anything with the BDS, but just with the job, > and the job is still owned by the source BDS. >=20 > Signed-off-by: Kevin Wolf > --- > block.c | 32 +++++++++++++++++++++++++++++++- > block/mirror.c | 23 +++++++---------------- > include/block/block.h | 4 +++- > qemu-img.c | 16 ++++++++-------- > 4 files changed, 49 insertions(+), 26 deletions(-) >=20 > diff --git a/block.c b/block.c > index 98fc17c..7c21659 100644 > --- a/block.c > +++ b/block.c > @@ -1095,7 +1095,7 @@ static BdrvChild *bdrv_attach_child(BlockDriverSt= ate *parent_bs, > return child; > } > =20 > -void bdrv_detach_child(BdrvChild *child) > +static void bdrv_detach_child(BdrvChild *child) > { > QLIST_REMOVE(child, next); > QLIST_REMOVE(child, next_parent); > @@ -2228,6 +2228,36 @@ void bdrv_append(BlockDriverState *bs_new, Block= DriverState *bs_top) > bdrv_unref(bs_new); > } > =20 > +void bdrv_replace_in_backing_chain(BlockDriverState *old, BlockDriverS= tate *new) > +{ > + assert(!bdrv_requests_pending(old)); > + assert(!bdrv_requests_pending(new)); > + > + bdrv_ref(old); > + > + if (old->blk) { > + /* As long as these fields aren't in BlockBackend, but in the = top-level > + * BlockDriverState, it's not possible for a BDS to have two B= Bs. > + * > + * We really want to copy the fields from old to new, but we g= o for a > + * swap instead so that pointers aren't duplicated and cause t= rouble. > + * (Also, bdrv_swap() used to do the same.) */ > + assert(!new->blk); > + swap_feature_fields(old, new); > + } > + change_parent_backing_link(old, new); > + > + /* Change backing files if a previously independent node is added = to the > + * chain. For active commit, we replace top by its own (indirect) = backing > + * file and don't do anything here so we don't build a loop. */ > + if (new->backing =3D=3D NULL && !bdrv_chain_contains(backing_bs(ol= d), new)) { > + bdrv_set_backing_hd(new, backing_bs(old)); > + bdrv_set_backing_hd(old, NULL); Wouldn't we want @old to keep its backing file? Then bdrv_append() would basically be a special case of this function, with it additionally decrementing the refcount of @bs_new. Max > + } > + > + bdrv_unref(old); > +} > + > static void bdrv_delete(BlockDriverState *bs) > { > assert(!bs->job); --5vFljDb0rO44XpPMFuHAqNvhPAqG0sgqn Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAEBCAAGBQJWAtyZAAoJEDuxQgLoOKytjIEH/i3v4GpckgqsYhhQtXHAkBMi wQS56ILIhzMU12tMw/r+fVHIftRsspnReM1N+QFebbiGu0KrLsx52b2ALQWW0wAw shmjote0htrX2uoTjtuROhqb75SSGCWYbQwIXw9nzXIqbDCycNViAoonzyE4reU9 DcwmIIRbwhbRjcsRemga7tGecPIZobSjncQIzHMDgqbKCYdkWJCp5i2vBzJWyAuz cpQLDn5yM8dz74dFpfuCkemryrJaXO5FWrgGPAAJQ6Qx4ewdrTJLhYiYpzI6VMb/ 2H6NjRjTNI2kfLU/AnI3daiXkT4sdnYbZZjcQpoF6SLC2TmjouSaGRYrKURsAJk= =8usm -----END PGP SIGNATURE----- --5vFljDb0rO44XpPMFuHAqNvhPAqG0sgqn--