qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Stefan Hajnoczi <stefanha@redhat.com>
To: qemu-devel@nongnu.org
Cc: Max Reitz <mreitz@redhat.com>, Kevin Wolf <kwolf@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Stefan Hajnoczi <stefanha@redhat.com>
Subject: [Qemu-devel] [RFC for-2.7] block: keep AioContext pointer in BlockBackend
Date: Mon, 11 Apr 2016 15:14:16 +0100	[thread overview]
Message-ID: <1460384056-29494-1-git-send-email-stefanha@redhat.com> (raw)

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(-)

diff --git a/block/block-backend.c b/block/block-backend.c
index d74f670..e39c583 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -30,6 +30,7 @@ static AioContext *blk_aiocb_get_aio_context(BlockAIOCB *acb);
 struct BlockBackend {
     char *name;
     int refcnt;
+    AioContext *aio_context;
     BdrvChild *root;
     DriveInfo *legacy_dinfo;    /* null unless created by drive_new() */
     QTAILQ_ENTRY(BlockBackend) link;         /* for block_backends */
@@ -106,6 +107,7 @@ BlockBackend *blk_new(Error **errp)
 
     blk = g_new0(BlockBackend, 1);
     blk->refcnt = 1;
+    blk->aio_context = qemu_get_aio_context();
     notifier_list_init(&blk->remove_bs_notifiers);
     notifier_list_init(&blk->insert_bs_notifiers);
     QTAILQ_INSERT_TAIL(&block_backends, blk, link);
@@ -433,6 +435,7 @@ void blk_insert_bs(BlockBackend *blk, BlockDriverState *bs)
 {
     assert(!blk->root && !bs->blk);
     bdrv_ref(bs);
+    bdrv_set_aio_context(bs, blk->aio_context);
     blk->root = bdrv_root_attach_child(bs, "root", &child_root);
     bs->blk = blk;
 
@@ -1348,13 +1351,7 @@ void blk_op_unblock_all(BlockBackend *blk, Error *reason)
 
 AioContext *blk_get_aio_context(BlockBackend *blk)
 {
-    BlockDriverState *bs = blk_bs(blk);
-
-    if (bs) {
-        return bdrv_get_aio_context(bs);
-    } else {
-        return qemu_get_aio_context();
-    }
+    return blk->aio_context;
 }
 
 static AioContext *blk_aiocb_get_aio_context(BlockAIOCB *acb)
@@ -1367,6 +1364,8 @@ void blk_set_aio_context(BlockBackend *blk, AioContext *new_context)
 {
     BlockDriverState *bs = blk_bs(blk);
 
+    blk->aio_context = new_context;
+
     if (bs) {
         bdrv_set_aio_context(bs, new_context);
     }
-- 
2.5.5

             reply	other threads:[~2016-04-11 14:14 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-11 14:14 Stefan Hajnoczi [this message]
2016-05-09 16:48 ` [Qemu-devel] [RFC for-2.7] block: keep AioContext pointer in BlockBackend 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
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=1460384056-29494-1-git-send-email-stefanha@redhat.com \
    --to=stefanha@redhat.com \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /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).