From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58324) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XPds5-00056h-MZ for qemu-devel@nongnu.org; Thu, 04 Sep 2014 16:43:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XPdrw-0008L4-L2 for qemu-devel@nongnu.org; Thu, 04 Sep 2014 16:43:09 -0400 Received: from mail-wi0-x22d.google.com ([2a00:1450:400c:c05::22d]:47563) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XPdrw-0008J0-AJ for qemu-devel@nongnu.org; Thu, 04 Sep 2014 16:43:00 -0400 Received: by mail-wi0-f173.google.com with SMTP id cc10so1851070wib.6 for ; Thu, 04 Sep 2014 13:42:59 -0700 (PDT) Date: Thu, 4 Sep 2014 21:42:56 +0100 From: Stefan Hajnoczi Message-ID: <20140904204256.GF25226@stefanha-thinkpad.redhat.com> References: <1408723870-7826-1-git-send-email-benoit.canet@nodalink.com> <1408723870-7826-2-git-send-email-benoit.canet@nodalink.com> <20140825060424.GA17482@T430.nay.redhat.com> <20140825090611.GA18202@nodalink.com> <20140825093737.GA25434@T430.nay.redhat.com> <20140825121219.GB18202@nodalink.com> <20140826044204.GB2517@T430.nay.redhat.com> <20140826064554.GA13982@nodalink.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="+ts6NCQ4mrNQIV8p" Content-Disposition: inline In-Reply-To: <20140826064554.GA13982@nodalink.com> Subject: Re: [Qemu-devel] [PATCH] block: Make op blockers recursive List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: =?iso-8859-1?Q?Beno=EEt?= Canet Cc: kwolf@redhat.com, jcody@redhat.com, Fam Zheng , qemu-devel@nongnu.org, stefanha@redhat.com --+ts6NCQ4mrNQIV8p Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Aug 26, 2014 at 06:45:54AM +0000, Beno=EEt Canet wrote: > On Tue, Aug 26, 2014 at 12:42:04PM +0800, Fam Zheng wrote: > > On Mon, 08/25 12:12, Beno=EEt Canet wrote: > > > On Mon, Aug 25, 2014 at 05:37:37PM +0800, Fam Zheng wrote: > > > > On Mon, 08/25 09:06, Beno=EEt Canet wrote: > > > > > On Mon, Aug 25, 2014 at 02:04:24PM +0800, Fam Zheng wrote: > > > > > > On Fri, 08/22 18:11, Beno=EEt Canet wrote: > > > > > > > Since the block layer code is starting to modify the BDS grap= h right in the > > > > > > > middle of BDS chains (block-mirror's replace parameter for ex= ample) QEMU needs > > > > > > > to properly block and unblock whole BDS subtrees; recursion i= s a neat way to > > > > > > > achieve this task. > > > > > > >=20 > > > > > > > This patch also takes care of modifying the op blockers users. > > > > > >=20 > > > > > > Is this going to replace backing_blocker? > > > > > >=20 > > > > > > I think it is too general an approach to control the operation = properly, > > > > > > because the op blocker may not work in the same way for all typ= es of BDS > > > > > > connections. In other words, the choosing of op blockers are l= ikely > > > > > > conditional on graph edge types, that's why backing_blocker was= added here. For > > > > > > example, A VMDK extent connection will probably need a differen= t set of > > > > > > blockers than bs->file connection. > > > > > >=20 > > > > > > So could you explain in which cases is the recursive blocking/u= nblocking > > > > > > useful? > > > > >=20 > > > > > It's designed for the new crop of block operations operating on B= DS located in > > > > > the middle of the backing chain: Jeff's patches, intermediate liv= e streaming or > > > > > intermediate mirroring. > > > > > Recursively blocking BDS allows to do these operations safely. > > > >=20 > > > > Sorry I may be slow on this, but it's still not clear to me. > > > >=20 > > > > That doesn't immediately show how backing_blocker doesn't work. The= se > > > > operations are in the category of operations that update graph topo= logy, > > > > meaning that they drop, add or swap some nodes in the middle of the= chain. It > > > > is not safe because there are used by the other nodes, but they are= supposed to > > > > be protected by backing_blocker. Could you be more specific? > > >=20 > > > I don't know particularly about the backing blocker case. > > >=20 > > > >=20 > > > > I can think of something more than backing_hd: there are also link = types other > > > > than backing_hd, for example ->file, (vmdk)->extents or (quorum)->q= crs, etc. > > > > They should be protected as well. > > >=20 > > > This patch takes cares of recursing everywhere. > > >=20 > > > I can give you an example for quorum. > > >=20 > > > If a streaming operation is running on a quorum block backend the rec= ursive > > > blocking will help to block any operation done directly on any of the= children. > >=20 > > At what points should block layer recursively block/unblock the operati= ons in > > this quorum case? >=20 > When the streaming starts it should block all the top bs children. > So after when an operation tries to operate on a child of the top bs it w= ill be > forbidden. >=20 > The beauty of it is that recursive blockers can easily replace regular bl= ockers. Let's think of a situation that recursive blockers protect but backing_blocker does not: a <- b <- c <- d c is the backing file and is therefore protected by the op blocker. The block-commit command works with node-names, however, so we can manipulate any nodes in the graph, not just the topmost one. Try this: block-commit d block-commit b I haven't checked yet but I suspect it will launch two block-commit jobs on the same partial chain (that's a bad thing because it can lead to corruption). With recursive blockers, not just c but also a and b are protected. To me, this demonstrates that recursive op blockers are safer than non-recursive backing_blocker and will prevent real-world cases that could lead to corruption. BTW I'm fairly happy with the patch itself, but like Fam I'm still thinking through different cases to convince myself the design is sound. Stefan --+ts6NCQ4mrNQIV8p Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJUCM7QAAoJEJykq7OBq3PIFIkH/1NU+F+l2WT6xb+3GF7hWMhT qDl7NV61AsENjSoMgPEzOweIRF0rKtiwx3OOmOqlVLssmEYeSbvf8n+maDoGVSa7 mxqg8XUn2mdE1VcorgCTdqL/qgU/XtEnYbIqQuD36JPiVRIwiyrxSrP/8z63H4AR RFDQ27F4Guad1Tp2smwivoxOG8Q8SDyfB298kC9B2ODtpUQfFdCsWR1WPk3gdM/w QSQg7x4PwqRPx2bucdIGI1jwFNVa4aXaQ3YQBG1n3JUdoxd9E9jGtehDsJI+cr8z OvMz2ghf3GAJiNBFSZyDjST7Li3OQwmPwXgY3hnCU3tBt1g42kxBCXzxwL+KFF8= =A766 -----END PGP SIGNATURE----- --+ts6NCQ4mrNQIV8p--