From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39471) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WzSgh-0001CC-S9 for qemu-devel@nongnu.org; Tue, 24 Jun 2014 11:31:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WzSga-0006PQ-I8 for qemu-devel@nongnu.org; Tue, 24 Jun 2014 11:31:11 -0400 Received: from lputeaux-656-01-25-125.w80-12.abo.wanadoo.fr ([80.12.84.125]:33747 helo=paradis.irqsave.net) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WzSga-0006P0-8c for qemu-devel@nongnu.org; Tue, 24 Jun 2014 11:31:04 -0400 Date: Tue, 24 Jun 2014 17:30:53 +0200 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20140624153053.GA20264@irqsave.net> References: <20140619091716.GS21236@stefanha-thinkpad.redhat.com> <20140619162600.GB6096@localhost.localdomain> <20140623130809.GB26269@stefanha-thinkpad.redhat.com> <20140624024852.GD26197@T430.redhat.com> <20140624133259.GA5491@localhost.localdomain> <20140624140857.GI3458@noname.redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <20140624140857.GI3458@noname.redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v6 for 2.1 00/10] Modify block jobs to use node-names List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: benoit.canet@irqsave.net, pkrempa@redhat.com, Fam Zheng , Stefan Hajnoczi , Jeff Cody , qemu-devel@nongnu.org, stefanha@redhat.com The Tuesday 24 Jun 2014 =E0 16:08:57 (+0200), Kevin Wolf wrote : > Am 24.06.2014 um 15:32 hat Jeff Cody geschrieben: > > On Tue, Jun 24, 2014 at 10:48:52AM +0800, Fam Zheng wrote: > > > On Mon, 06/23 21:08, Stefan Hajnoczi wrote: > > > > On Thu, Jun 19, 2014 at 12:26:00PM -0400, Jeff Cody wrote: > > > > > On Thu, Jun 19, 2014 at 05:17:16PM +0800, Stefan Hajnoczi wrote= : > > > > > > On Tue, Jun 17, 2014 at 05:53:48PM -0400, Jeff Cody wrote: > > > > > > Let's discuss this topic in a sub-thread and figure out what = to do for > > > > > > QEMU 2.1. This is an important issue to solve before the rel= ease > > > > > > because we can't change QMP command semantics easily later. > > > > > >=20 > > > > > > My questions are: > > > > > > a. How do we fix resize, snapshot-sync, etc? It seems like w= e need to > > > > > > propagate child op blockers. > > > > > >=20 > > > > > > b. Is it a good idea to perform op blocker checks on the root= node? > > > > > > It's inconsistent with resize, snapshot-sync, etc. Permis= sions in > > > > > > BDS graphs with multiple root nodes (e.g. guest device and= NBD > > > > > > run-time server) will be different depending on which root= you > > > > > > specify. > > > > >=20 > > > > > I don't think (b) is the ultimate solution. It is used as a st= op-gap > > > > > because op blockers in the current implementation is essentiall= y > > > > > analogous to the in-use flag. But is it good enough for 2.1? = If > > > > > *everything* checks the topmost node in 2.1, then I think we ar= e OK in > > > > > all cases except where images files share a common BDS. > > > >=20 > > > > Checking op blockers on the root node as a stop-gap is a good ide= a. > > > > Let's apply it across all commands (e.g. snapshot-sync, resize). > > > >=20 > > > > Fam pointed out that this approach is vulnerable to blockdev-add,= where > > > > blockers could be set/checked on an incomplete BDS graph (since y= ou can > > > > add new nodes on top). Do we need to move the blockers up the gr= aph if > > > > a new root node is inserted? > > >=20 > > > My concern is if we allow adding new root on top, it's not easy to = know the > > > real root then. > > >=20 > > > To give an example: > > >=20 > > > If we have > > >=20 > > > [base id=3D""] <- [active id=3D"drive0" blockers=3D...] > > >=20 > > > When user does > > >=20 > > > (QMP) block-commit device=3D"drive0" ... > > >=20 > > > We should check drive0, which is OK. > > >=20 > > > Then, assume user adds a new root on top, we would take care of mov= ing the > > > blockers: > > >=20 > > > [base id=3D""] <- [active id=3D"drive0"] <- [active id=3D"drive= 1" blockers=3D] > > >=20 > > > At this point, what if user does something on drive0 again? > > >=20 > > > (QMP) block-commit device=3D"drive0" ... > > >=20 > > > The right thing to do is to check blockers on "drive1", since it's = the real > > > root now. But how do we know? Do we need to add a back reference p= ointer > > > ->overlap_hd in BDS, or do we maintain a look up table, or do we se= arch all BDS > > > graphs to figure out? > > >=20 > > > None is easier than if we put the blockers in the bottom BDS, in th= e first > > > place: > > >=20 > > > [base id=3D"" blockers=3D...] <- [active id=3D"drive0"] > > > ^^^^^^^^^^^^ > > >=20 > >=20 > > I think you are right. If we place the blocker at the bottom-most > > BDS, then that would be a more restrictive blocker. This may end up > > being more restrictive than needed, but more importantly it should > > make everything safe. > >=20 > > Also, it is an easy change for 2.1 - just call bdrv_find_base(bs), an= d > > set/check/clear blockers on the returned BDS. >=20 > What does bdrv_find_base() return for e.g. quorum? This will not work when unblocking a BDS loop like the one formed by driv= e-mirror when replacing an arbitrary node. >=20 > Kevin >=20 > > > Even if user adds a new root, we don't need to worry about moving b= lockers, > > > because the bottom is not changed. > > >=20 > > > [base id=3D"" blockers=3D...] <- [active id=3D"drive0"] <- [act= ive id=3D"drive1"] > > >=20 > > > Checking the blockers are easy, either for drive0 or drive1: just f= ollow the > > > backing chain until getting to the end. > > >=20 > > > Fam >=20