qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Max Reitz <mreitz@redhat.com>
To: Stefan Hajnoczi <stefanha@redhat.com>, qemu-devel@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [Qemu-devel] [RFC for-2.7] block: keep AioContext pointer in BlockBackend
Date: Tue, 17 May 2016 12:52:54 +0200	[thread overview]
Message-ID: <e88589c1-a57f-8910-ac44-0bc288e9ff25@redhat.com> (raw)
In-Reply-To: <1460384056-29494-1-git-send-email-stefanha@redhat.com>

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

On 11.04.2016 16:14, Stefan Hajnoczi wrote:
> blk_get/set_aio_context() delegate to BlockDriverState without storing
> the AioContext pointer in BlockBackend.
> 
> There are two flaws:
> 
> 1. BlockBackend falls back to the QEMU main loop AioContext when there
>    is no root BlockDriverState.  This means the drive loses its
>    AioContext during media change and would break dataplane.
> 
> 2. BlockBackend state used from multiple threads has no lock.  Race
>    conditions will creep in as functionality is moved from
>    BlockDriverState to BlockBackend due to the absense of a lock.  The
>    monitor cannot access BlockBackend state safely while an IOThread is
>    also accessing the state.
> 
> Both issue #1 and #2 are mostly theoretical at the moment.  I haven't
> figured out a way to trigger #1 with virtio-blk (does not support media
> change) or virtio-scsi (blocks the eject operation).  #2 may be possible
> with block accounting statistics in BlockBackend but I'm not aware of a
> crash that can be triggered.
> 
> This patch stores the AioContext pointer in BlockBackend and puts newly
> inserted BlockDriverStates into the AioContext.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  block/block-backend.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)

We can have multiple BBs per BDS tree, and in the future (after Kevin's
"block: Remove BlockDriverState.blk" series) we may even have multiple
BBs per BDS. Therefore, we have to make sure to propagate the AIO
context to all BBs attached to the BDS tree, just as the AIO context is
propagated to all BDSs in the BDS tree.

A trivial way to do this would be to query the BDS's AIO context in
blk_get_aio_context() and update the BB's context if they don't match.
The more correct way would probably be to make bdrv_attach_aio_context()
update its BB's (or BBs', in the future) AIO context.

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

  parent reply	other threads:[~2016-05-17 10:53 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-11 14:14 [Qemu-devel] [RFC for-2.7] block: keep AioContext pointer in BlockBackend Stefan Hajnoczi
2016-05-09 16:48 ` Paolo Bonzini
2016-05-10  9:33   ` Stefan Hajnoczi
2016-05-10 10:38     ` Paolo Bonzini
2016-05-11 13:24       ` Stefan Hajnoczi
2016-05-17 10:52 ` Max Reitz [this message]
2016-05-17 11:07   ` Paolo Bonzini
2016-05-17 11:14     ` Max Reitz
2016-05-17 11:37       ` Paolo Bonzini

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=e88589c1-a57f-8910-ac44-0bc288e9ff25@redhat.com \
    --to=mreitz@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=pbonzini@redhat.com \
    --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).