Re: [Qemu-devel] Block Filters

[Stefan Hajnoczi]

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