From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54040) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wxii8-00056v-QD for qemu-devel@nongnu.org; Thu, 19 Jun 2014 16:13:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wxii4-0002tX-4X for qemu-devel@nongnu.org; Thu, 19 Jun 2014 16:13:28 -0400 Received: from mx1.redhat.com ([209.132.183.28]:11492) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wxii3-0002tK-Sz for qemu-devel@nongnu.org; Thu, 19 Jun 2014 16:13:24 -0400 Message-ID: <53A34460.8010302@redhat.com> Date: Thu, 19 Jun 2014 14:13:20 -0600 From: Eric Blake MIME-Version: 1.0 References: <1403208081-18247-1-git-send-email-benoit.canet@irqsave.net> <1403208081-18247-2-git-send-email-benoit.canet@irqsave.net> In-Reply-To: <1403208081-18247-2-git-send-email-benoit.canet@irqsave.net> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="wAOKLIANUgPUcsUoilTI6GtMtUmCdDWRn" Subject: Re: [Qemu-devel] [PATCH] block: Make op blocker recursive List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?UTF-8?B?QmVub8OudCBDYW5ldA==?= , qemu-devel@nongnu.org Cc: kwolf@redhat.com, jcody@redhat.com, Benoit Canet , stefanha@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --wAOKLIANUgPUcsUoilTI6GtMtUmCdDWRn Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On 06/19/2014 02:01 PM, Beno=C3=AEt Canet wrote: > As the code will start to operate on arbitratry nodes we need the op bl= ocker s/arbitratry/arbitrary/ > to recursively block or unblock whole BDS subtrees. >=20 > Also add a function to reset all blocker from a BDS. >=20 > This patch also take care of changing blocker user so they are not brok= en. >=20 > Signed-off-by: Benoit Canet > --- > + > +/* This remove unconditionally all blockers of type op of the subtree = */ This unconditionally removes all blockers of type op of the subtree Yikes - is that really what we want? Or do we need to start doing blocker reference counting? Consider: base <- snap1 <- active Looking at Jeff's proposal of making blockers based on access patterns rather than operations, we want the mere act of being a backing file to automatically put a guest_write block on base and snap1 (we must not modify the backing chain out of underneath active). But now suppose we do two operations in parallel - we take a fleecing export of active, and we start a drive-mirror on active. base <- snap1 <- active | \-- fleecing \-- copy Both of those actions should be doable in parallel, and both of them probably put additional blocker restrictions on the chain. But if we unconditionally clear those additional restrictions on the first of the two jobs ending, that would inappropriately stop blocking that action from the still on-going second action. The only way I see around that is via reference-counted blocking. Definitely not 2.1 material (but good to be thinking about it now, so we can get it in early in the 2.2 cycle). > =20 > +/* This remove unconditionally all blockers */ Unconditionally remove all blockers > =20 > +/* Used to prevent recursion loop. A case exists in block commit mirro= r usage */ > +static BlockDriverState *recurse_op_bs =3D NULL; > +/* take note of the recursion depth to allow assigning recurse_op_bs o= nce */ > +static uint64_t recurse_op_depth =3D 0; The '=3D 0' is redundant; the C language guarantees that all static variables are 0-initialized. > + > +/* set or unset an op blocker to a BDS whether set is true or false */= > +void bdrv_op_block_action(BlockDriverState *bs, BlockOpType op, > + BlockerAction action, Error *reason) > +{ Not sure I follow that comment, since 'set' is not one of the parameter names. > + > +/* Recursively set or unset an op block to a BDS tree whether set is t= rue or > + * false > + */ > +void bdrv_recurse_op_block(BlockDriverState *bs, BlockOpType op, > + BlockerAction action, Error *reason) and again > +{ > + /* If recursion is detected simply return */ > + if (recurse_op_bs =3D=3D bs) { > + return; > + } > + > + /* if we are starting recursion remeber the bs for later compariso= n */ s/remeber/remember/ > + if (!recurse_op_depth) { > + recurse_op_bs =3D bs; > + } > + > + /* try to set or unset on bs->file and bs->backing_hd first */ > + bdrv_op_block_action(bs->file, op, action, reason); > + bdrv_op_block_action(bs->backing_hd, op, action, reason); > + > + /* if the BDS is a filter with multiple childs ask the driver to r= ecurse */ s/childs/children/ > +static void blkverify_recurse_op_block(BlockDriverState *bs, BlockOpTy= pe op, > + BlockerAction action, Error *re= ason) > +{ > + BDRVBlkverifyState *s =3D bs->opaque; > + bdrv_op_block_action(bs->file, op , action, reason); > + bdrv_op_block_action(s->test_file, op , action, reason); s/ ,/,/ twice > +++ b/include/block/block_int.h > @@ -86,6 +86,11 @@ struct BlockDriver { > */ > bool (*bdrv_recurse_is_first_non_filter)(BlockDriverState *bs, > BlockDriverState *candida= te); > + /* In order to be able to recursively block operation on BDS trees= filter > + * like quorum can implement this callback s/trees filter/trees, a filter/ --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --wAOKLIANUgPUcsUoilTI6GtMtUmCdDWRn Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJTo0RgAAoJEKeha0olJ0NqV+4IAIA/npLwdAKca5aU3NrHXrbp zutfnkDgUHXPWB8Hvp743m7s05Jz+HJ9+4XM6Cs/xmrtmbvDL5Dsw85FnFLJ6RfS Jl39aWVqyoIIrwx/hcHRXEp4Jp1mSd1K50MUqtdXZwO6oqgJZ9TEYO3U3DiVHqYI fydzrpkpXd1SdIXBuFSN/ikCbsC7QrerTY5JpuAda/JSsSeS9qwBfw6+0f4lCQ9B QqGBWD9pMLj7WvW4IDeEo4Tn7X19B/JUp0mVyGi0ZvYH+LarYEtG0lkfPvFyAZJR ejZylv7OXPTeqo6qPR94KglEfUWZlexSAdCUbI0wIUM5Kbg9p/i/xAidx5f/qT0= =/8Fx -----END PGP SIGNATURE----- --wAOKLIANUgPUcsUoilTI6GtMtUmCdDWRn--