Re: [Qemu-devel] BlockDriverState stack and BlockListeners

[Stefan Hajnoczi <stefanha@gmail.com>]

On Tue, Feb 21, 2012 at 11:36 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 02/21/2012 11:51 AM, Kevin Wolf wrote:
>> And even protocols and protocols don't. Compare file to blkdebug, for
>> example. In fact, blkdebug and blkverify are already very close to what
>> BlockListeners would be.
>
> Yes, and I think considering blkdebug and blkverify help in the design.
> They provide the difference between views and listeners: listeners can
> be applied to both a protocol and a view, while views can only be
> applied to a format.
>
>> > - Protocols -- Protocols tell you where to get the raw bits.
>> > - Formats -- Formats transform those raw bits into a block device.
>> > - Views -- Views can move from a format to another.  A format can use a
>> > default view implementation, or provide its own (e.g. to access
>> > different snapshots).
>> > - Listeners -- I think a view can have-a listener?
>>
>> Where protocols, formats and listeners are-a image (Not the best name,
>> I'm open for suggestions. Something like "BDS stack building block"
>> would be most accurate...). Or actually not is-a in terms of
>> inheritance, but I think it would be the very same thing without any
>> subclassing, implemented by a BlockDriver.
>>
>> > with the following relationship:
>> >
>> > - A format has-a protocol for the raw bits and has-a view for the
>> > backing file.
>>
>> An image has-a image from which it takes its data (bs->file).
>
> No. A protocol has neither an image below it, nor a backing file.  I
> think a view has no backing file either (except as provided by the
> format).  I'm not sure that a listener can have a backing file, either;
> you could say that blkverify has one, but it could just as well have
> three or four, so it's not the same thing.
>
> A format does not do much more than create views, do snapshots, and hold
> state that is common to all of its views.
>
> So, let's put aside listeners for a moment, and consider this
> alternative hierarchy:
>
> BlockDriver
>    Protocol
>        FileProtocol
>        ...
>    View
>        QCOW2View
>        RawView
>        ...
> BlockFormat
>    QCOW2Format
>    RawFormat
>    ...
>
> Now we have to figure out how to fit listeners in this picture.
>
>> And it has-a view for the backing file, yes. Both could be a listener.
>>
>>> > - A view has-a format, a device has-a view.
>>> > - A view can have-a listener?  Or is it formats?
>> A view has-a image. This may happen to be a listener, which in turn
>> has-a image (could be another listener, a format or a protocol).
>>
>> The question is what the semantics is with live snapshots (there are
>> probably similar problems, but this is the obvious one). For example we
>> could now have:
>>
>> mirror -> qcow2 -> blkdebug -> file
>
> Let's be precise here:
>
> mirror -> currentSnapshot -> qcow2 -> blkdebug -> file
>
> - file is a protocol.
>
> - blkdebug is a listener
>
> - qcow2 is a format
>
> - currentSnapshot is a view
>
> - mirror is a listener
>
> The difference between blkdebug/mirror on one side, and currentSnapshot
> on the other, is that (as you said) blkdebug/mirror are always stacked
> on top of something else. A driver that references currentSnapshot
> actually gets mirror.
>
> So we get to the actual hierarchy I'm proposing:
>
> BlockDriver
>    BlockSource (see below)
>    Protocol (bdrv_file_open)
>        FileProtocol
>        ...
>    View (has-a BlockFormat)
>        QCOW2View
>        RawView
>        ...
>    BlockListener (has-a BlockDriver)
>        MirrorListener
>        BlkDebugListener
> BlockFormat (has-a BlockSource)
>    QCOW2Format
>    RawFormat
>    ...
>
> Protocols and views are only internal.  Formats and devices in practice
> will only ever see BlockSources.  A BlockSource is a reference a stack of
> BlockDrivers, where the base must be a protocol or view and there can
> be a number of listeners stacked on top of it.  Listeners can be
> added or removed from the stack, and the bottom driver can be swapped
> for another (for snapshots).
>
> So, here is how it would look:
>
>  .== BlockSource ==.                   .== BlockSource ===.
>  | MirrorListener  |                   | BlkDebugListener |
>  | QCOW2View ------+--> QCOW2Format -> | FileProtocol     |
>  '================='                   '=================='
>
>
>> There are two listeners here, mirror and blkdebug. (Things like blkdebug
>> are why view has-a listener isn't enough). After creating an external
>> snapshot, we expect the graph to look like this (the arrow down is the
>> backing file):
>>
>> mirror -> qcow2 -> file
>>             |
>>             +-> qcow2 -> blkdebug -> file
>
> And here:
>
>  .== BlockSource ==.
>  | MirrorListener  |                   .== BlockSource ==.
>  | QCOW2View ------+--> QCOW2Format -> | FileProtocol    |
>  '================='    |              '================='
>                         |                                          .== BlockSource ===.
>                         |   .== BlockSource ==.                    | BlkDebugListener |
>                         '-> | QCOW2View ------+--> QCOW2Format --> | FileProtocol     |
>                             '================='                    '=================='
>
> Does it seem sane?

This is a good discussion because BlockDriverState has become bloated
and complex.  A lot of fields only apply to sub-cases and we should
really split this struct.

Fields like "backing_file" *should* be in generic code, not duplicated
in each Format.  But BlockDriverState is too generic since it also
encompasses Protocols and Listeners.

You mentioned that some of these classes would be "internal".  I think
everything should be exposed in the QOM just like Linux exposes kernel
objects in sysfs.  It makes troubleshooting and debugging easier.

As has been pointed out, "Listener" suggests a passive role.  Perhaps
BlockFilter, BlockProcessor, or BlockModule is a better name.

Ideally Formats can be isolated from the rest of the block layer so
that it becomes easy to create a libimageformat.  If we bake
coroutines, I/O APIs, and memory allocation functions too deeply into
Formats then they are hard to test and impossible to use outside of
QEMU.

Stefan