Re: [Qemu-devel] Block layer complexity: what to do to keep it under control?

[Kevin Wolf <kwolf@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2106 bytes --]

Am 29.11.2017 um 13:24 hat Paolo Bonzini geschrieben:
> On 29/11/2017 13:00, Stefan Hajnoczi wrote:
> > We are at a point where code review isn't finding certain bugs because
> > no single person knows all the assumptions.  Previously the problem was
> > contained because maintainers spotted problems before patches were
> > merged.
> > 
> > This is not primarily a documentation problem though.  We cannot
> > document our way out of this because no single person (patch author or
> > code reviewer) can know or check everything anymore due to the scale.
> > 
> > I think it's a (lack of) design problem because we have many incomplete
> > abstractions like block jobs, IOThreads, block graph, image locking,
> > etc.  They do not cover all possibly states and interactions today.
> > Extending them leads to complex bugs.
> 
> I think the main interactions are:
> 
> 1) block graph modifications and drain.  This has always been a carnage.
>  Implementing BlockBackend isolation instead of drain would probably be
> a starting point to fix it, because IIRC there are extremely few cases
> where we really need "drain" semantics.

I think it's not just specifically drain, but nested event loops in
general. Drain is just more prominent because it recursively affects the
whole tree and actively waits for callbacks, so if anything can go
wrong, it will certainly affect drain, too.

The big problem I see here is that we have never defined in which places
or under which conditions it's allowed to make changes to the graph.
This means that callers never know when to use an extra bdrv_ref/unref
pair, when to expect that child references change in the middle of the
operation etc.

Maybe what we need there is some coroutine locks that make sure that
e.g. a block job completion simply has to wait until a drain has
completed before the graph change is actually executed. We need to make
sure that these locks don't deadlock the drain operation, but as long as
these things run in a separate coroutine (like the block job coroutine),
it should be okay.

Kevin

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]