qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Jeff Cody <jcody@redhat.com>
To: Fam Zheng <famz@redhat.com>
Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org,
	pbonzini@redhat.com, kwolf@redhat.com, stefanha@redhat.com,
	mreitz@redhat.com, eblake@redhat.com
Subject: Re: [Qemu-devel] Block layer complexity: what to do to keep it under control?
Date: Wed, 29 Nov 2017 01:30:06 -0500	[thread overview]
Message-ID: <20171129063006.GD18521@localhost.localdomain> (raw)
In-Reply-To: <20171129035502.GD8889@lemon>

On Wed, Nov 29, 2017 at 11:55:02AM +0800, Fam Zheng wrote:
> Hi all,
> 
> As we move forwards with new features in the block layer, the chances of tricky
> bugs happening have been increasing alongside - block jobs, coroutines,
> throttling, AioContext, op blockers and image locking combined together make a
> large and complex picture that is hard to fully understand and work with. Some
> bugs we've encountered are quite challenging already.  Examples are:
> 
> - segfault in parallel blockjobs (iotest 30)
>   https://lists.gnu.org/archive/html/qemu-devel/2017-11/msg01144.html
> 
> - Intermittent hang of iotest 194 (bdrv_drain_all after non-shared storage
>   migration)
>   https://lists.gnu.org/archive/html/qemu-devel/2017-11/msg01626.html
> 
> - Drainage in bdrv_replace_child_noperm()
>   https://lists.gnu.org/archive/html/qemu-devel/2017-11/msg00868.html
> 
> - Regression from 2.8: stuck in bdrv_drain()
>   https://lists.gnu.org/archive/html/qemu-devel/2017-04/msg02193.html
> 

I agree, it seems the complexity is growing by quite a bit.

> So in principle, what should we do to make the block layer easy to understand,
> develop with and debug? I think we have opportunities in these aspects:
> 
> - Documentation
> 
>   There is no central developer doc about block layer, especially how all pieces
>   fit together. Having one will make it a lot easier for new contributors to
>   understand better. Of course, we're facing the old problem: the code is
>   moving, maintaining an updated document needs effort.
> 
>   Idea: add ./doc/deve/block.txt?
> 

There are some bits of brilliance in what is already there; for instance,
devel/atomics.txt is very thorough.  But I agree that a major piece missing
is an overall design document, that provides the "why" to the "what".

Even given the cost of maintaining a higher level design document, I
think your suggestion here is probably the one that can help mitigate the
complexity the most; the more we (developers) can keep a coherent design
model in mind, the better we are able to do your _other_ suggestions: create
effective tests, simplify code, and enhance debuggability.

> - Tests
> 
>   Writing tests is a great way not only to exercise code, verify new features
>   work as expected and catch regression bugs, but also a way to show how the
>   feature can be used. There is this trend that the QEMU user interface
>   gradually moves from high level commands and args to small and flexible
>   building blocks, therefore demostrating the usage in iotests is meaningful.
> 
>   Idea: Add tests to simulate how libvirt uses block layer, or how we expect it
>   to. This would be a long term investment. We could reuse iotests, or create a
>   new test framework specifically, if it's easier (for example, use docker/vm
>   tests that just uses libvirt).
> 
>   Idea: Patchew already tests the quick group of iotests for a few
>   formats/protocols, but we should really add it to "make check".
> 

Perhaps higher level testing (like your example of how libvirt uses the
block layer) is a good candidate for avocado?


> - Simplified code, or more orthogonal/modularized architecture.
> 
>   Each aspect of block layer is complex enough so isolating them as much as
>   possible is a reasonable approach to control the complexity. Block jobs and
>   throttling becoming block filters is a good example, we should identify more.
> 
>   Idea: rethink event loops. Create coroutines ubiquitously (for example for
>   each fd handler, BH and timer), so that many nested aio_poll() can be removed.
> 
>   Crazy idea: move the whole block layer to a vhost process, and implement
>   existing features differently, especially in terms of multi-threading (hint:
>   rust?).
> 
> - Debuggability.
> 
>   Working with backtraces when coroutine is used is pretty hard, it would be
>   nice if ./scripts/qemugdb/coroutine.py could work with core files (i.e.
>   without a process to debug), and trace back to co->caller automatically.
> 

IIRC, this used to work, right?

>   It's always useful to dump block graph. Maybe we should add a helper function
>   in block layer that dumps all node graphs in graphviz DOT format, and even
>   make it available in QMP as x-dump-block-graph?
> 
>   Of course gdb scripts to dump various lists are also nice little things to
>   have.
> 
>   Idea: write more ./scripts/qemugdb/<scriptlet>.py.

More qemugdb macros would be great, especially for dumping the block chain
and making coroutines less opaque.

-Jeff

  reply	other threads:[~2017-11-29  6:30 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-29  3:55 [Qemu-devel] Block layer complexity: what to do to keep it under control? Fam Zheng
2017-11-29  6:30 ` Jeff Cody [this message]
2017-11-29 12:16   ` Stefan Hajnoczi
2017-11-29 12:22     ` Paolo Bonzini
2017-11-29 12:00 ` Stefan Hajnoczi
2017-11-29 12:24   ` Paolo Bonzini
2017-11-29 13:24     ` [Qemu-devel] [Qemu-block] " Stefan Hajnoczi
2017-11-29 13:41     ` [Qemu-devel] " Kevin Wolf
2017-11-29 19:58     ` Dr. David Alan Gilbert
2017-11-30  9:47   ` Fam Zheng
2017-11-30 14:19     ` Stefan Hajnoczi
2017-12-01 10:16       ` Fam Zheng
2017-12-01 14:08         ` Stefan Hajnoczi
2017-12-01 15:00           ` Fam Zheng
2017-12-01 17:03           ` Paolo Bonzini
2017-12-01 19:03             ` Peter Maydell
2017-12-04 10:41               ` Stefan Hajnoczi
2017-12-01 19:27             ` Eric Blake
2017-12-04 10:16               ` Stefan Hajnoczi
2017-12-04 10:32                 ` Peter Maydell
2017-11-29 12:32 ` Daniel P. Berrange

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171129063006.GD18521@localhost.localdomain \
    --to=jcody@redhat.com \
    --cc=eblake@redhat.com \
    --cc=famz@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=stefanha@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).