On Fri, Sep 19, 2014 at 12:35:36AM -0400, Jeff Cody wrote: > On Mon, Sep 15, 2014 at 04:21:18PM +0200, Benoît Canet wrote: > > We do not want to try to stream or commit with a base argument through > > a multiple children driver. > > > > Handle this case. > > > > Benoît 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 > > > > 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(-) > > > > -- > > 2.1.0 > > > > > I had some specific comments on the implementation, but once I stepped > back some I am not sure this series is needed. > > 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. > > 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. > > I view these as a black box, like so: > > (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) > > > ImageE may have multiple children (and perhaps each of those children > are an entire chain themselves), but ultimately imageE is a 'BDS' > entry. > > Perhaps the format, like quorum, will not have a backing_hd pointer. > But in that case, backing_hd will be NULL. > > 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. > > 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). > > 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 post the new recursive blocker code. Best regards Benoît > > Thanks, > Jeff >
