From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37190) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XUyrp-0005sW-3G for qemu-devel@nongnu.org; Fri, 19 Sep 2014 10:09:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XUyri-0006YR-TP for qemu-devel@nongnu.org; Fri, 19 Sep 2014 10:08:57 -0400 Received: from dew.nodalink.com ([95.130.14.197]:57707) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XUyri-0006WJ-Mm for qemu-devel@nongnu.org; Fri, 19 Sep 2014 10:08:50 -0400 Date: Fri, 19 Sep 2014 14:08:52 +0000 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20140919140852.GA8433@nodalink.com> References: <1410790880-21861-1-git-send-email-benoit.canet@nodalink.com> <20140919043536.GC3813@localhost.localdomain> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <20140919043536.GC3813@localhost.localdomain> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 0/2] Handle interferences between multiple children drivers and stream and commit List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jeff Cody Cc: kwolf@redhat.com, stefanha@redhat.com, qemu-devel@nongnu.org, =?iso-8859-1?Q?Beno=EEt?= Canet On Fri, Sep 19, 2014 at 12:35:36AM -0400, Jeff Cody wrote: > On Mon, Sep 15, 2014 at 04:21:18PM +0200, Beno=EEt Canet wrote: > > We do not want to try to stream or commit with a base argument throug= h > > a multiple children driver. > >=20 > > Handle this case. > >=20 > > Beno=EEt Canet (2): > > block: Introduce a BlockDriver field to flag drivers supporting > > multiple children > > block: commit and stream return an error when a subtree is found > >=20 > > block.c | 17 +++++++++++++++-- > > block/blkverify.c | 1 + > > block/quorum.c | 1 + > > block/vmdk.c | 1 + > > blockdev.c | 18 ++++++++++++------ > > include/block/block.h | 3 ++- > > include/block/block_int.h | 6 ++++++ > > 7 files changed, 38 insertions(+), 9 deletions(-) > >=20 > > --=20 > > 2.1.0 > > >=20 >=20 > I had some specific comments on the implementation, but once I stepped > back some I am not sure this series is needed. >=20 > When we talked on irc, you mentioned it was needed because your > recursive blocker code would assert in quorum/blkverify/vmdk - but I > think that might mean the recursive code needs refactoring. >=20 > Here is why I am not sure this is needed: we should be able to commit > through/to an image that has multiple child nodes, if we can treat > that image as an actual BDS. >=20 > I view these as a black box, like so: >=20 > (imageE) > ------------ > | [childA] | > | [childB] | > [base] <---- | | <----- [imageF] <--- [active] > | [childC] | > ^ | [childD] | > | ------------ > | > | > (Perhaps base isn't support by the format, in which case it will always > be NULL anyway) >=20 >=20 > ImageE may have multiple children (and perhaps each of those children > are an entire chain themselves), but ultimately imageE is a 'BDS' > entry. >=20 > Perhaps the format, like quorum, will not have a backing_hd pointer. > But in that case, backing_hd will be NULL. >=20 > Thinking about your recursive blocker code, why assert when the 'base' > termination argument is NULL? If the multi-child format does not > support a backing_hd, then bdrv_find_base_image() will just find it as > the end. >=20 > The .bdrv_op_recursive_block() in that case could still navigate down > each private child node, because if the original 'base' blocker > applied to the multi-child parent (imageE), then that same blocker > should apply to each private child (and their children), regardless of > 'base' - because they are essentially part of the pseudo-atomic entity > of (imageE). >=20 > Does this make sense, or am I missing a piece? Thanks for the reply Jeff. I think I'll drop the entry I CCed you, remove the assert then work to po= st the new recursive blocker code. Best regards Beno=EEt >=20 > Thanks, > Jeff >=20