All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>,
	Fam Zheng <famz@redhat.com>,
	kwolf@redhat.com, qemu-block@nongnu.org, jcody@redhat.com,
	qemu-devel@nongnu.org, mreitz@redhat.com
Subject: Re: [Qemu-devel] Block layer complexity: what to do to keep it under control?
Date: Wed, 29 Nov 2017 19:58:18 +0000	[thread overview]
Message-ID: <20171129195818.GA32030@work-vm> (raw)
In-Reply-To: <7ccb7f4a-b576-349f-655c-f741ec3a0dff@redhat.com>

* Paolo Bonzini (pbonzini@redhat.com) wrote:
> 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.
> 
> 2) block jobs and coroutines.  Block jobs were too clever about
> coroutines.  Using a simplified API is going to fix this problem.
> Ideally, if you're not in a coroutine "co", the only coroutine APIs you
> should use on "co" are:
> 
> - aio_co_enter/qemu_coroutine_enter (start a coroutine, respectively on
> another AioContext or this context);
> 
> - aio_co_schedule/aio_co_wake (restart a coroutine that has yielded,
> respectively on a given AioContext or its own original.
> 
> 3) block jobs and drain.  This is related to (1) because drain can
> terminate jobs and in turn that can cause block graph modifications.
> I'm not even sure it's a separate issue.

Block and migration has been having a rough time for a while, generally
around whether block devices should be inactivated at particular points.
While we've got some changes recently, we've still got at least one
known failure.

Dave

> Regarding documentation, the include file documentation is good for
> coroutines and block jobs.  But it's bad for block graph modification
> APIs, and even for coroutines + block jobs the docs/devel documentation
> could be improved *and* it's ugly that we're not generating anything
> readable from include file documentation, to go with docs/devel.
> 
> Paolo
> 
> > A little progress has been made with defining higher-level APIs for
> > block drivers and block jobs.  This way they either don't deal with
> > low-level details of the concurrency and event loop models (e.g.
> > bdrv_coroutine_enter()) or there is an interface that prompts them to
> > integrate properly like bdrv_attach/detach_aio_context().
> > 
> > Event loops and coroutines are good but they should not be used directly
> > by block drivers and block jobs.  We need safe, high-level APIs that
> > implement commonly-used operations.
> 
> 


--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

  parent reply	other threads:[~2017-11-29 19:58 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
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 [this message]
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=20171129195818.GA32030@work-vm \
    --to=dgilbert@redhat.com \
    --cc=famz@redhat.com \
    --cc=jcody@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.