On Tue, Sep 03, 2013 at 06:24:49PM +0200, Benoît Canet wrote: > -$user want to do Copy On Read
This feature is currently implemented in the read code path in block.c. Putting it into a separate, stackable module is fine but may require a per-device request queue. Today every BDS has its own request queue and I think that starts to become inefficient if every small stackable feature duplicates all request metadata. > Moreover it would have no way to manipulate easily this tree of > BlockDriverState > has each one is encapsulated in it's parent. > > Also there no generic way to tell the block layer that two or more > BlockDriverState > are siblings. vmdk also has this issue. block.c provides bs->file and bs->backing_hd but it doesn't support multi-file VMDK's natively. So the vmdk.c has to implement this internally. Perhaps VMDK could be simplified by using generic infrastructure for siblings. > When doing a snapshot blockdev.c would access > BlockBackend->top_node_below_filter and make a snapshot of the bs contained in > this node and it's sibblings. I'm not a fan of ->top_node_below_filter. It seems ad-hoc to me. The snapshot operation should be part of the function interface that block drivers can implement. Filters would forward it to their only child in the simple case. Quorum would perform the snapshot on each of its children. Image formats would allow the snapshot to happen. If we need slightly different semantics from ->top_node_below_filter then we'll need to introduce more ad-hoc fields. I think the cleanest solution is simply to ask the block drivers to perform the operation and provide default implementations for the common case. > After each individual snapshot the linked lists and the hash/arrays would be > updated to point to the new top bsn. > The snapshot operation can be done without violating any of the top block > filter BlockDriverState. I think we should add bs->parent and bs->children[] fields to BDS and APIs to manipulate the tree. Propagate operations like snapshot down the tree. block.c is designed for bs->file/bs->backing_hd kind of BlockDrivers, perhaps it needs to become a bit more generic to support other types of BlockDrivers properly. BTW I don't see a need to create a separate tree structure. Stefan
