From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37864) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a2MU0-0006wL-GZ for qemu-devel@nongnu.org; Fri, 27 Nov 2015 12:06:57 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a2MTz-0003gR-Ba for qemu-devel@nongnu.org; Fri, 27 Nov 2015 12:06:52 -0500 References: <1448294400-476-1-git-send-email-kwolf@redhat.com> <1448294400-476-4-git-send-email-kwolf@redhat.com> From: Max Reitz Message-ID: <56588D9D.6040208@redhat.com> Date: Fri, 27 Nov 2015 18:06:37 +0100 MIME-Version: 1.0 In-Reply-To: <1448294400-476-4-git-send-email-kwolf@redhat.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="SaGwB0EgijD7rprltpHtHBHRvBQshuhGU" Subject: Re: [Qemu-devel] [PATCH v2 03/21] mirror: Error out when a BDS would get two BBs List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf , qemu-block@nongnu.org Cc: qemu-devel@nongnu.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --SaGwB0EgijD7rprltpHtHBHRvBQshuhGU Content-Type: text/plain; charset=iso-8859-15 Content-Transfer-Encoding: quoted-printable On 23.11.2015 16:59, Kevin Wolf wrote: > bdrv_replace_in_backing_chain() asserts that not both old and new > BlockDdriverState have a BlockBackend attached to them because both > would have to end up pointing to the new BDS and we don't support more > than one BB per BDS yet. >=20 > Before we can safely allow references to existing nodes as backing > files, we need to make sure that even if a backing file has a BB on it,= > this doesn't crash qemu. >=20 > There are probably also some cases with the 'replaces' option set where= > drive-mirror could fail this assertion today. They are fixed with this > error check as well. >=20 > Signed-off-by: Kevin Wolf > --- > block/mirror.c | 28 ++++++++++++++++++++++++++++ > 1 file changed, 28 insertions(+) >=20 > diff --git a/block/mirror.c b/block/mirror.c > index 52c9abf..0620068 100644 > --- a/block/mirror.c > +++ b/block/mirror.c > @@ -18,6 +18,7 @@ > #include "qapi/qmp/qerror.h" > #include "qemu/ratelimit.h" > #include "qemu/bitmap.h" > +#include "qemu/error-report.h" > =20 > #define SLICE_TIME 100000000ULL /* ns */ > #define MAX_IN_FLIGHT 16 > @@ -370,11 +371,22 @@ static void mirror_exit(BlockJob *job, void *opaq= ue) > if (s->to_replace) { > to_replace =3D s->to_replace; > } > + > + /* This was checked in mirror_start_job(), but meanwhile one o= f the > + * nodes could have been newly attached to a BlockBackend. */ > + if (to_replace->blk && s->target->blk) { > + error_report("block job: Can't create node with two BlockB= ackends"); > + data->ret =3D -EINVAL; > + goto out; > + } > + > if (bdrv_get_flags(s->target) !=3D bdrv_get_flags(to_replace))= { > bdrv_reopen(s->target, bdrv_get_flags(to_replace), NULL); > } > bdrv_replace_in_backing_chain(to_replace, s->target); > } > + > +out: > if (s->to_replace) { > bdrv_op_unblock_all(s->to_replace, s->replace_blocker); > error_free(s->replace_blocker); > @@ -701,6 +713,7 @@ static void mirror_start_job(BlockDriverState *bs, = BlockDriverState *target, > bool is_none_mode, BlockDriverState *base= ) > { > MirrorBlockJob *s; > + BlockDriverState *replaced_bs; > =20 > if (granularity =3D=3D 0) { > granularity =3D bdrv_get_default_bitmap_granularity(target); > @@ -724,6 +737,21 @@ static void mirror_start_job(BlockDriverState *bs,= BlockDriverState *target, > buf_size =3D DEFAULT_MIRROR_BUF_SIZE; > } > =20 > + /* We can't support this case as long as the block layer can't han= dle > + * multple BlockBackends per BlockDriverState. */ *multiple With that fixed: Reviewed-by: Max Reitz > + if (replaces) { > + replaced_bs =3D bdrv_lookup_bs(replaces, replaces, errp); > + if (replaced_bs =3D=3D NULL) { > + return; > + } > + } else { > + replaced_bs =3D bs; > + } > + if (replaced_bs->blk && target->blk) { > + error_setg(errp, "Can't create node with two BlockBackends"); > + return; > + } > + > s =3D block_job_create(driver, bs, speed, cb, opaque, errp); > if (!s) { > return; >=20 --SaGwB0EgijD7rprltpHtHBHRvBQshuhGU 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 iQEcBAEBCAAGBQJWWI2dAAoJEDuxQgLoOKytQx0H/3nobjxweaTFfdn0FasjYpwp 2VKc5D1uH4Hx9A1/N7WkeFfGPwoyjjsdDmhLFwuwZXHCU98VToizkL+kkovRzZY2 UmDcFD32JxEFP0Vh7xMa1yVJCOAtktrbeuLLzexBMtcSRi9aXugDDFNNpHgo6Qji zgOb7+oDEft1xvKfn2EnFmC/n5kvQWS35coHO0OuaZqpW8ag0WFBoOCR8R83IP3q 515dhaGcXOTOS2vjFSbSwAS4CC3BqaOOyBo0+iOpDTnRJuJ4jRBXeuIlfZ0nxzFR xuj/OqNgWYd1sN59Dt6+Xhco9W6LjZYeCAjs8JmivqokauuyOqsJMbwjbjowS4M= =fxbH -----END PGP SIGNATURE----- --SaGwB0EgijD7rprltpHtHBHRvBQshuhGU--