* [Qemu-devel] [PATCH 01/10] block: Add bdrv_try_set_aio_context()
2019-05-06 17:17 [Qemu-devel] [PATCH 00/10] block: AioContext management, part 1 Kevin Wolf
@ 2019-05-06 17:17 ` Kevin Wolf
2019-05-06 17:17 ` [Qemu-devel] [PATCH 02/10] block: Make bdrv_attach/detach_aio_context() static Kevin Wolf
` (9 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Kevin Wolf @ 2019-05-06 17:17 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, mreitz
Eventually, we want to make sure that all parents and all children of a
node are in the same AioContext as the node itself. This means that
changing the AioContext may fail because one of the other involved
parties (e.g. a guest device that was configured with an iothread)
cannot allow switching to a different AioContext.
Introduce a set of functions that allow to first check whether all
involved nodes can switch to a new context and only then do the actual
switch. The check recursively covers children and parents.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
include/block/block.h | 8 ++++
include/block/block_int.h | 3 ++
block.c | 92 +++++++++++++++++++++++++++++++++++++++
3 files changed, 103 insertions(+)
diff --git a/include/block/block.h b/include/block/block.h
index c7a26199aa..649aa5b86d 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -590,6 +590,14 @@ void bdrv_coroutine_enter(BlockDriverState *bs, Coroutine *co);
* This function must be called with iothread lock held.
*/
void bdrv_set_aio_context(BlockDriverState *bs, AioContext *new_context);
+int bdrv_try_set_aio_context(BlockDriverState *bs, AioContext *ctx,
+ Error **errp);
+int bdrv_child_try_set_aio_context(BlockDriverState *bs, AioContext *ctx,
+ BdrvChild *ignore_child, Error **errp);
+bool bdrv_child_can_set_aio_context(BdrvChild *c, AioContext *ctx,
+ GSList **ignore, Error **errp);
+bool bdrv_can_set_aio_context(BlockDriverState *bs, AioContext *ctx,
+ GSList **ignore, Error **errp);
int bdrv_probe_blocksizes(BlockDriverState *bs, BlockSizes *bsz);
int bdrv_probe_geometry(BlockDriverState *bs, HDGeometry *geo);
diff --git a/include/block/block_int.h b/include/block/block_int.h
index 94d45c9708..b150c5f047 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -691,6 +691,9 @@ struct BdrvChildRole {
* can update its reference. */
int (*update_filename)(BdrvChild *child, BlockDriverState *new_base,
const char *filename, Error **errp);
+
+ bool (*can_set_aio_ctx)(BdrvChild *child, AioContext *ctx,
+ GSList **ignore, Error **errp);
};
extern const BdrvChildRole child_file;
diff --git a/block.c b/block.c
index 9ae5c0ed2f..a70c01effb 100644
--- a/block.c
+++ b/block.c
@@ -936,6 +936,13 @@ static int bdrv_child_cb_inactivate(BdrvChild *child)
return 0;
}
+static bool bdrv_child_cb_can_set_aio_ctx(BdrvChild *child, AioContext *ctx,
+ GSList **ignore, Error **errp)
+{
+ BlockDriverState *bs = child->opaque;
+ return bdrv_can_set_aio_context(bs, ctx, ignore, errp);
+}
+
/*
* Returns the options and flags that a temporary snapshot should get, based on
* the originally requested flags (the originally requested image will have
@@ -1003,6 +1010,7 @@ const BdrvChildRole child_file = {
.attach = bdrv_child_cb_attach,
.detach = bdrv_child_cb_detach,
.inactivate = bdrv_child_cb_inactivate,
+ .can_set_aio_ctx = bdrv_child_cb_can_set_aio_ctx,
};
/*
@@ -1029,6 +1037,7 @@ const BdrvChildRole child_format = {
.attach = bdrv_child_cb_attach,
.detach = bdrv_child_cb_detach,
.inactivate = bdrv_child_cb_inactivate,
+ .can_set_aio_ctx = bdrv_child_cb_can_set_aio_ctx,
};
static void bdrv_backing_attach(BdrvChild *c)
@@ -1152,6 +1161,7 @@ const BdrvChildRole child_backing = {
.drained_end = bdrv_child_cb_drained_end,
.inactivate = bdrv_child_cb_inactivate,
.update_filename = bdrv_backing_update_filename,
+ .can_set_aio_ctx = bdrv_child_cb_can_set_aio_ctx,
};
static int bdrv_open_flags(BlockDriverState *bs, int flags)
@@ -5751,6 +5761,88 @@ void bdrv_set_aio_context(BlockDriverState *bs, AioContext *new_context)
aio_context_release(new_context);
}
+static bool bdrv_parent_can_set_aio_context(BdrvChild *c, AioContext *ctx,
+ GSList **ignore, Error **errp)
+{
+ if (g_slist_find(*ignore, c)) {
+ return true;
+ }
+ *ignore = g_slist_prepend(*ignore, c);
+
+ /* A BdrvChildRole that doesn't handle AioContext changes cannot
+ * tolerate any AioContext changes */
+ if (!c->role->can_set_aio_ctx) {
+ char *user = bdrv_child_user_desc(c);
+ error_setg(errp, "Changing iothreads is not supported by %s", user);
+ g_free(user);
+ return false;
+ }
+ if (!c->role->can_set_aio_ctx(c, ctx, ignore, errp)) {
+ assert(!errp || *errp);
+ return false;
+ }
+ return true;
+}
+
+bool bdrv_child_can_set_aio_context(BdrvChild *c, AioContext *ctx,
+ GSList **ignore, Error **errp)
+{
+ if (g_slist_find(*ignore, c)) {
+ return true;
+ }
+ *ignore = g_slist_prepend(*ignore, c);
+ return bdrv_can_set_aio_context(c->bs, ctx, ignore, errp);
+}
+
+/* @ignore will accumulate all visited BdrvChild object. The caller is
+ * responsible for freeing the list afterwards. */
+bool bdrv_can_set_aio_context(BlockDriverState *bs, AioContext *ctx,
+ GSList **ignore, Error **errp)
+{
+ BdrvChild *c;
+
+ if (bdrv_get_aio_context(bs) == ctx) {
+ return true;
+ }
+
+ QLIST_FOREACH(c, &bs->parents, next_parent) {
+ if (!bdrv_parent_can_set_aio_context(c, ctx, ignore, errp)) {
+ return false;
+ }
+ }
+ QLIST_FOREACH(c, &bs->children, next) {
+ if (!bdrv_child_can_set_aio_context(c, ctx, ignore, errp)) {
+ return false;
+ }
+ }
+
+ return true;
+}
+
+int bdrv_child_try_set_aio_context(BlockDriverState *bs, AioContext *ctx,
+ BdrvChild *ignore_child, Error **errp)
+{
+ GSList *ignore;
+ bool ret;
+
+ ignore = ignore_child ? g_slist_prepend(NULL, ignore_child) : NULL;
+ ret = bdrv_can_set_aio_context(bs, ctx, &ignore, errp);
+ g_slist_free(ignore);
+
+ if (!ret) {
+ return -EPERM;
+ }
+
+ bdrv_set_aio_context(bs, ctx);
+ return 0;
+}
+
+int bdrv_try_set_aio_context(BlockDriverState *bs, AioContext *ctx,
+ Error **errp)
+{
+ return bdrv_child_try_set_aio_context(bs, ctx, NULL, errp);
+}
+
void bdrv_add_aio_context_notifier(BlockDriverState *bs,
void (*attached_aio_context)(AioContext *new_context, void *opaque),
void (*detach_aio_context)(void *opaque), void *opaque)
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH 02/10] block: Make bdrv_attach/detach_aio_context() static
2019-05-06 17:17 [Qemu-devel] [PATCH 00/10] block: AioContext management, part 1 Kevin Wolf
2019-05-06 17:17 ` [Qemu-devel] [PATCH 01/10] block: Add bdrv_try_set_aio_context() Kevin Wolf
@ 2019-05-06 17:17 ` Kevin Wolf
2019-05-06 17:17 ` [Qemu-devel] [PATCH 03/10] block: Move recursion to bdrv_set_aio_context() Kevin Wolf
` (8 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Kevin Wolf @ 2019-05-06 17:17 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, mreitz
Since commit b97511c7bc8, there is no reason for block drivers any more
to call these functions (see the function comment in block_int.h). They
are now just internal helper functions for bdrv_set_aio_context()
and can be made static.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
include/block/block_int.h | 21 ---------------------
block.c | 6 +++---
2 files changed, 3 insertions(+), 24 deletions(-)
diff --git a/include/block/block_int.h b/include/block/block_int.h
index b150c5f047..aa2c638b02 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -965,27 +965,6 @@ void bdrv_parse_filename_strip_prefix(const char *filename, const char *prefix,
void bdrv_add_before_write_notifier(BlockDriverState *bs,
NotifierWithReturn *notifier);
-/**
- * bdrv_detach_aio_context:
- *
- * May be called from .bdrv_detach_aio_context() to detach children from the
- * current #AioContext. This is only needed by block drivers that manage their
- * own children. Both ->file and ->backing are automatically handled and
- * block drivers should not call this function on them explicitly.
- */
-void bdrv_detach_aio_context(BlockDriverState *bs);
-
-/**
- * bdrv_attach_aio_context:
- *
- * May be called from .bdrv_attach_aio_context() to attach children to the new
- * #AioContext. This is only needed by block drivers that manage their own
- * children. Both ->file and ->backing are automatically handled and block
- * drivers should not call this function on them explicitly.
- */
-void bdrv_attach_aio_context(BlockDriverState *bs,
- AioContext *new_context);
-
/**
* bdrv_add_aio_context_notifier:
*
diff --git a/block.c b/block.c
index a70c01effb..00332c1eb4 100644
--- a/block.c
+++ b/block.c
@@ -5677,7 +5677,7 @@ static void bdrv_do_remove_aio_context_notifier(BdrvAioNotifier *ban)
g_free(ban);
}
-void bdrv_detach_aio_context(BlockDriverState *bs)
+static void bdrv_detach_aio_context(BlockDriverState *bs)
{
BdrvAioNotifier *baf, *baf_tmp;
BdrvChild *child;
@@ -5709,8 +5709,8 @@ void bdrv_detach_aio_context(BlockDriverState *bs)
bs->aio_context = NULL;
}
-void bdrv_attach_aio_context(BlockDriverState *bs,
- AioContext *new_context)
+static void bdrv_attach_aio_context(BlockDriverState *bs,
+ AioContext *new_context)
{
BdrvAioNotifier *ban, *ban_tmp;
BdrvChild *child;
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH 03/10] block: Move recursion to bdrv_set_aio_context()
2019-05-06 17:17 [Qemu-devel] [PATCH 00/10] block: AioContext management, part 1 Kevin Wolf
2019-05-06 17:17 ` [Qemu-devel] [PATCH 01/10] block: Add bdrv_try_set_aio_context() Kevin Wolf
2019-05-06 17:17 ` [Qemu-devel] [PATCH 02/10] block: Make bdrv_attach/detach_aio_context() static Kevin Wolf
@ 2019-05-06 17:17 ` Kevin Wolf
2019-05-06 17:17 ` [Qemu-devel] [PATCH 04/10] block: Propagate AioContext change to parents Kevin Wolf
` (7 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Kevin Wolf @ 2019-05-06 17:17 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, mreitz
Instead of having two recursions, in bdrv_attach_aio_context() and in
bdrv_detach_aio_context(), just having one recursion is enough. Said
functions are only about a single node now.
It is important that the recursion doesn't happen between detaching and
attaching a context to the current node because the nested call will
drain the node, and draining with a NULL context would segfault.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/block.c b/block.c
index 00332c1eb4..e36c72e297 100644
--- a/block.c
+++ b/block.c
@@ -5680,7 +5680,6 @@ static void bdrv_do_remove_aio_context_notifier(BdrvAioNotifier *ban)
static void bdrv_detach_aio_context(BlockDriverState *bs)
{
BdrvAioNotifier *baf, *baf_tmp;
- BdrvChild *child;
assert(!bs->walking_aio_notifiers);
bs->walking_aio_notifiers = true;
@@ -5699,9 +5698,6 @@ static void bdrv_detach_aio_context(BlockDriverState *bs)
if (bs->drv && bs->drv->bdrv_detach_aio_context) {
bs->drv->bdrv_detach_aio_context(bs);
}
- QLIST_FOREACH(child, &bs->children, next) {
- bdrv_detach_aio_context(child->bs);
- }
if (bs->quiesce_counter) {
aio_enable_external(bs->aio_context);
@@ -5713,7 +5709,6 @@ static void bdrv_attach_aio_context(BlockDriverState *bs,
AioContext *new_context)
{
BdrvAioNotifier *ban, *ban_tmp;
- BdrvChild *child;
if (bs->quiesce_counter) {
aio_disable_external(new_context);
@@ -5721,9 +5716,6 @@ static void bdrv_attach_aio_context(BlockDriverState *bs,
bs->aio_context = new_context;
- QLIST_FOREACH(child, &bs->children, next) {
- bdrv_attach_aio_context(child->bs, new_context);
- }
if (bs->drv && bs->drv->bdrv_attach_aio_context) {
bs->drv->bdrv_attach_aio_context(bs, new_context);
}
@@ -5745,11 +5737,18 @@ static void bdrv_attach_aio_context(BlockDriverState *bs,
* the same as the current context of bs). */
void bdrv_set_aio_context(BlockDriverState *bs, AioContext *new_context)
{
+ BdrvChild *child;
+
if (bdrv_get_aio_context(bs) == new_context) {
return;
}
bdrv_drained_begin(bs);
+
+ QLIST_FOREACH(child, &bs->children, next) {
+ bdrv_set_aio_context(child->bs, new_context);
+ }
+
bdrv_detach_aio_context(bs);
/* This function executes in the old AioContext so acquire the new one in
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH 04/10] block: Propagate AioContext change to parents
2019-05-06 17:17 [Qemu-devel] [PATCH 00/10] block: AioContext management, part 1 Kevin Wolf
` (2 preceding siblings ...)
2019-05-06 17:17 ` [Qemu-devel] [PATCH 03/10] block: Move recursion to bdrv_set_aio_context() Kevin Wolf
@ 2019-05-06 17:17 ` Kevin Wolf
2019-05-06 17:18 ` [Qemu-devel] [PATCH 05/10] test-block-iothread: Test AioContext propagation through the tree Kevin Wolf
` (6 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Kevin Wolf @ 2019-05-06 17:17 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, mreitz
All block nodes and users in any connected component of the block graph
must be in the same AioContext, so changing the AioContext of one node
must not only change all of its children, but all of its parents (and
in turn their children etc.) as well.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
include/block/block.h | 2 ++
include/block/block_int.h | 1 +
block.c | 48 ++++++++++++++++++++++++++++++++++-----
3 files changed, 45 insertions(+), 6 deletions(-)
diff --git a/include/block/block.h b/include/block/block.h
index 649aa5b86d..b72110f315 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -590,6 +590,8 @@ void bdrv_coroutine_enter(BlockDriverState *bs, Coroutine *co);
* This function must be called with iothread lock held.
*/
void bdrv_set_aio_context(BlockDriverState *bs, AioContext *new_context);
+void bdrv_set_aio_context_ignore(BlockDriverState *bs,
+ AioContext *new_context, GSList **ignore);
int bdrv_try_set_aio_context(BlockDriverState *bs, AioContext *ctx,
Error **errp);
int bdrv_child_try_set_aio_context(BlockDriverState *bs, AioContext *ctx,
diff --git a/include/block/block_int.h b/include/block/block_int.h
index aa2c638b02..1eebc7c8f3 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -694,6 +694,7 @@ struct BdrvChildRole {
bool (*can_set_aio_ctx)(BdrvChild *child, AioContext *ctx,
GSList **ignore, Error **errp);
+ void (*set_aio_ctx)(BdrvChild *child, AioContext *ctx, GSList **ignore);
};
extern const BdrvChildRole child_file;
diff --git a/block.c b/block.c
index e36c72e297..b8bd20a234 100644
--- a/block.c
+++ b/block.c
@@ -943,6 +943,13 @@ static bool bdrv_child_cb_can_set_aio_ctx(BdrvChild *child, AioContext *ctx,
return bdrv_can_set_aio_context(bs, ctx, ignore, errp);
}
+static void bdrv_child_cb_set_aio_ctx(BdrvChild *child, AioContext *ctx,
+ GSList **ignore)
+{
+ BlockDriverState *bs = child->opaque;
+ return bdrv_set_aio_context_ignore(bs, ctx, ignore);
+}
+
/*
* Returns the options and flags that a temporary snapshot should get, based on
* the originally requested flags (the originally requested image will have
@@ -1011,6 +1018,7 @@ const BdrvChildRole child_file = {
.detach = bdrv_child_cb_detach,
.inactivate = bdrv_child_cb_inactivate,
.can_set_aio_ctx = bdrv_child_cb_can_set_aio_ctx,
+ .set_aio_ctx = bdrv_child_cb_set_aio_ctx,
};
/*
@@ -1038,6 +1046,7 @@ const BdrvChildRole child_format = {
.detach = bdrv_child_cb_detach,
.inactivate = bdrv_child_cb_inactivate,
.can_set_aio_ctx = bdrv_child_cb_can_set_aio_ctx,
+ .set_aio_ctx = bdrv_child_cb_set_aio_ctx,
};
static void bdrv_backing_attach(BdrvChild *c)
@@ -1162,6 +1171,7 @@ const BdrvChildRole child_backing = {
.inactivate = bdrv_child_cb_inactivate,
.update_filename = bdrv_backing_update_filename,
.can_set_aio_ctx = bdrv_child_cb_can_set_aio_ctx,
+ .set_aio_ctx = bdrv_child_cb_set_aio_ctx,
};
static int bdrv_open_flags(BlockDriverState *bs, int flags)
@@ -5732,10 +5742,10 @@ static void bdrv_attach_aio_context(BlockDriverState *bs,
bs->walking_aio_notifiers = false;
}
-/* The caller must own the AioContext lock for the old AioContext of bs, but it
- * must not own the AioContext lock for new_context (unless new_context is
- * the same as the current context of bs). */
-void bdrv_set_aio_context(BlockDriverState *bs, AioContext *new_context)
+/* @ignore will accumulate all visited BdrvChild object. The caller is
+ * responsible for freeing the list afterwards. */
+void bdrv_set_aio_context_ignore(BlockDriverState *bs,
+ AioContext *new_context, GSList **ignore)
{
BdrvChild *child;
@@ -5746,7 +5756,20 @@ void bdrv_set_aio_context(BlockDriverState *bs, AioContext *new_context)
bdrv_drained_begin(bs);
QLIST_FOREACH(child, &bs->children, next) {
- bdrv_set_aio_context(child->bs, new_context);
+ if (g_slist_find(*ignore, child)) {
+ continue;
+ }
+ *ignore = g_slist_prepend(*ignore, child);
+ bdrv_set_aio_context_ignore(child->bs, new_context, ignore);
+ }
+ QLIST_FOREACH(child, &bs->parents, next_parent) {
+ if (g_slist_find(*ignore, child)) {
+ continue;
+ }
+ if (child->role->set_aio_ctx) {
+ *ignore = g_slist_prepend(*ignore, child);
+ child->role->set_aio_ctx(child, new_context, ignore);
+ }
}
bdrv_detach_aio_context(bs);
@@ -5760,6 +5783,16 @@ void bdrv_set_aio_context(BlockDriverState *bs, AioContext *new_context)
aio_context_release(new_context);
}
+/* The caller must own the AioContext lock for the old AioContext of bs, but it
+ * must not own the AioContext lock for new_context (unless new_context is
+ * the same as the current context of bs). */
+void bdrv_set_aio_context(BlockDriverState *bs, AioContext *new_context)
+{
+ GSList *ignore_list = NULL;
+ bdrv_set_aio_context_ignore(bs, new_context, &ignore_list);
+ g_slist_free(ignore_list);
+}
+
static bool bdrv_parent_can_set_aio_context(BdrvChild *c, AioContext *ctx,
GSList **ignore, Error **errp)
{
@@ -5832,7 +5865,10 @@ int bdrv_child_try_set_aio_context(BlockDriverState *bs, AioContext *ctx,
return -EPERM;
}
- bdrv_set_aio_context(bs, ctx);
+ ignore = ignore_child ? g_slist_prepend(NULL, ignore_child) : NULL;
+ bdrv_set_aio_context_ignore(bs, ctx, &ignore);
+ g_slist_free(ignore);
+
return 0;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH 05/10] test-block-iothread: Test AioContext propagation through the tree
2019-05-06 17:17 [Qemu-devel] [PATCH 00/10] block: AioContext management, part 1 Kevin Wolf
` (3 preceding siblings ...)
2019-05-06 17:17 ` [Qemu-devel] [PATCH 04/10] block: Propagate AioContext change to parents Kevin Wolf
@ 2019-05-06 17:18 ` Kevin Wolf
2019-05-06 17:18 ` [Qemu-devel] [PATCH 06/10] block: Implement .(can_)set_aio_ctx for BlockBackend Kevin Wolf
` (5 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Kevin Wolf @ 2019-05-06 17:18 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, mreitz
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/test-block-iothread.c | 131 ++++++++++++++++++++++++++++++++++++
1 file changed, 131 insertions(+)
diff --git a/tests/test-block-iothread.c b/tests/test-block-iothread.c
index 036ed9a3b3..938831c9bd 100644
--- a/tests/test-block-iothread.c
+++ b/tests/test-block-iothread.c
@@ -27,6 +27,7 @@
#include "block/blockjob_int.h"
#include "sysemu/block-backend.h"
#include "qapi/error.h"
+#include "qapi/qmp/qdict.h"
#include "iothread.h"
static int coroutine_fn bdrv_test_co_prwv(BlockDriverState *bs,
@@ -459,6 +460,134 @@ static void test_attach_blockjob(void)
blk_unref(blk);
}
+/*
+ * Test that changing the AioContext for one node in a tree (here through blk)
+ * changes all other nodes as well:
+ *
+ * blk
+ * |
+ * | bs_verify [blkverify]
+ * | / \
+ * | / \
+ * bs_a [bdrv_test] bs_b [bdrv_test]
+ *
+ */
+static void test_propagate_basic(void)
+{
+ IOThread *iothread = iothread_new();
+ AioContext *ctx = iothread_get_aio_context(iothread);
+ BlockBackend *blk;
+ BlockDriverState *bs_a, *bs_b, *bs_verify;
+ QDict *options;
+
+ /* Create bs_a and its BlockBackend */
+ blk = blk_new(BLK_PERM_ALL, BLK_PERM_ALL);
+ bs_a = bdrv_new_open_driver(&bdrv_test, "bs_a", BDRV_O_RDWR, &error_abort);
+ blk_insert_bs(blk, bs_a, &error_abort);
+
+ /* Create bs_b */
+ bs_b = bdrv_new_open_driver(&bdrv_test, "bs_b", BDRV_O_RDWR, &error_abort);
+
+ /* Create blkverify filter that references both bs_a and bs_b */
+ options = qdict_new();
+ qdict_put_str(options, "driver", "blkverify");
+ qdict_put_str(options, "test", "bs_a");
+ qdict_put_str(options, "raw", "bs_b");
+
+ bs_verify = bdrv_open(NULL, NULL, options, BDRV_O_RDWR, &error_abort);
+
+ /* Switch the AioContext */
+ blk_set_aio_context(blk, ctx);
+ g_assert(blk_get_aio_context(blk) == ctx);
+ g_assert(bdrv_get_aio_context(bs_a) == ctx);
+ g_assert(bdrv_get_aio_context(bs_verify) == ctx);
+ g_assert(bdrv_get_aio_context(bs_b) == ctx);
+
+ /* Switch the AioContext back */
+ ctx = qemu_get_aio_context();
+ blk_set_aio_context(blk, ctx);
+ g_assert(blk_get_aio_context(blk) == ctx);
+ g_assert(bdrv_get_aio_context(bs_a) == ctx);
+ g_assert(bdrv_get_aio_context(bs_verify) == ctx);
+ g_assert(bdrv_get_aio_context(bs_b) == ctx);
+
+ bdrv_unref(bs_verify);
+ bdrv_unref(bs_b);
+ bdrv_unref(bs_a);
+ blk_unref(blk);
+}
+
+/*
+ * Test that diamonds in the graph don't lead to endless recursion:
+ *
+ * blk
+ * |
+ * bs_verify [blkverify]
+ * / \
+ * / \
+ * bs_b [raw] bs_c[raw]
+ * \ /
+ * \ /
+ * bs_a [bdrv_test]
+ */
+static void test_propagate_diamond(void)
+{
+ IOThread *iothread = iothread_new();
+ AioContext *ctx = iothread_get_aio_context(iothread);
+ BlockBackend *blk;
+ BlockDriverState *bs_a, *bs_b, *bs_c, *bs_verify;
+ QDict *options;
+
+ /* Create bs_a */
+ bs_a = bdrv_new_open_driver(&bdrv_test, "bs_a", BDRV_O_RDWR, &error_abort);
+
+ /* Create bs_b and bc_c */
+ options = qdict_new();
+ qdict_put_str(options, "driver", "raw");
+ qdict_put_str(options, "file", "bs_a");
+ qdict_put_str(options, "node-name", "bs_b");
+ bs_b = bdrv_open(NULL, NULL, options, BDRV_O_RDWR, &error_abort);
+
+ options = qdict_new();
+ qdict_put_str(options, "driver", "raw");
+ qdict_put_str(options, "file", "bs_a");
+ qdict_put_str(options, "node-name", "bs_c");
+ bs_c = bdrv_open(NULL, NULL, options, BDRV_O_RDWR, &error_abort);
+
+ /* Create blkverify filter that references both bs_b and bs_c */
+ options = qdict_new();
+ qdict_put_str(options, "driver", "blkverify");
+ qdict_put_str(options, "test", "bs_b");
+ qdict_put_str(options, "raw", "bs_c");
+
+ bs_verify = bdrv_open(NULL, NULL, options, BDRV_O_RDWR, &error_abort);
+ blk = blk_new(BLK_PERM_ALL, BLK_PERM_ALL);
+ blk_insert_bs(blk, bs_verify, &error_abort);
+
+ /* Switch the AioContext */
+ blk_set_aio_context(blk, ctx);
+ g_assert(blk_get_aio_context(blk) == ctx);
+ g_assert(bdrv_get_aio_context(bs_verify) == ctx);
+ g_assert(bdrv_get_aio_context(bs_a) == ctx);
+ g_assert(bdrv_get_aio_context(bs_b) == ctx);
+ g_assert(bdrv_get_aio_context(bs_c) == ctx);
+
+ /* Switch the AioContext back */
+ ctx = qemu_get_aio_context();
+ blk_set_aio_context(blk, ctx);
+ g_assert(blk_get_aio_context(blk) == ctx);
+ g_assert(bdrv_get_aio_context(bs_verify) == ctx);
+ g_assert(bdrv_get_aio_context(bs_a) == ctx);
+ g_assert(bdrv_get_aio_context(bs_b) == ctx);
+ g_assert(bdrv_get_aio_context(bs_c) == ctx);
+
+ blk_unref(blk);
+ bdrv_unref(bs_verify);
+ bdrv_unref(bs_c);
+ bdrv_unref(bs_b);
+ bdrv_unref(bs_a);
+}
+
int main(int argc, char **argv)
{
int i;
@@ -474,6 +603,8 @@ int main(int argc, char **argv)
}
g_test_add_func("/attach/blockjob", test_attach_blockjob);
+ g_test_add_func("/propagate/basic", test_propagate_basic);
+ g_test_add_func("/propagate/diamond", test_propagate_diamond);
return g_test_run();
}
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH 06/10] block: Implement .(can_)set_aio_ctx for BlockBackend
2019-05-06 17:17 [Qemu-devel] [PATCH 00/10] block: AioContext management, part 1 Kevin Wolf
` (4 preceding siblings ...)
2019-05-06 17:18 ` [Qemu-devel] [PATCH 05/10] test-block-iothread: Test AioContext propagation through the tree Kevin Wolf
@ 2019-05-06 17:18 ` Kevin Wolf
2019-05-06 17:18 ` [Qemu-devel] [PATCH 07/10] block: Add blk_set_allow_aio_context_change() Kevin Wolf
` (4 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Kevin Wolf @ 2019-05-06 17:18 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, mreitz
bdrv_try_set_aio_context() currently fails if a BlockBackend is attached
to a node because it doesn't implement the BdrvChildRole callbacks for
AioContext management.
We can allow changing the AioContext of monitor-owned BlockBackends as
long as no device is attached to them.
When setting the AioContext of the root node of a BlockBackend, we now
need to pass blk->root as an ignored child because we don't want the
root node to recursively call back into BlockBackend and execute
blk_do_set_aio_context() a second time.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/block-backend.c | 45 +++++++++++++++++++++++++++++++++++++++++--
1 file changed, 43 insertions(+), 2 deletions(-)
diff --git a/block/block-backend.c b/block/block-backend.c
index f78e82a707..0e75fc8849 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -124,6 +124,11 @@ static void blk_root_drained_end(BdrvChild *child);
static void blk_root_change_media(BdrvChild *child, bool load);
static void blk_root_resize(BdrvChild *child);
+static bool blk_root_can_set_aio_ctx(BdrvChild *child, AioContext *ctx,
+ GSList **ignore, Error **errp);
+static void blk_root_set_aio_ctx(BdrvChild *child, AioContext *ctx,
+ GSList **ignore);
+
static char *blk_root_get_parent_desc(BdrvChild *child)
{
BlockBackend *blk = child->opaque;
@@ -300,6 +305,9 @@ static const BdrvChildRole child_root = {
.attach = blk_root_attach,
.detach = blk_root_detach,
+
+ .can_set_aio_ctx = blk_root_can_set_aio_ctx,
+ .set_aio_ctx = blk_root_set_aio_ctx,
};
/*
@@ -1852,7 +1860,8 @@ static AioContext *blk_aiocb_get_aio_context(BlockAIOCB *acb)
return blk_get_aio_context(blk_acb->blk);
}
-void blk_set_aio_context(BlockBackend *blk, AioContext *new_context)
+static void blk_do_set_aio_context(BlockBackend *blk, AioContext *new_context,
+ bool update_root_node)
{
BlockDriverState *bs = blk_bs(blk);
ThrottleGroupMember *tgm = &blk->public.throttle_group_member;
@@ -1864,10 +1873,42 @@ void blk_set_aio_context(BlockBackend *blk, AioContext *new_context)
throttle_group_attach_aio_context(tgm, new_context);
bdrv_drained_end(bs);
}
- bdrv_set_aio_context(bs, new_context);
+ if (update_root_node) {
+ GSList *ignore = g_slist_prepend(NULL, blk->root);
+ bdrv_set_aio_context_ignore(bs, new_context, &ignore);
+ g_slist_free(ignore);
+ }
}
}
+void blk_set_aio_context(BlockBackend *blk, AioContext *new_context)
+{
+ blk_do_set_aio_context(blk, new_context, true);
+}
+
+static bool blk_root_can_set_aio_ctx(BdrvChild *child, AioContext *ctx,
+ GSList **ignore, Error **errp)
+{
+ BlockBackend *blk = child->opaque;
+
+ /* Only manually created BlockBackends that are not attached to anything
+ * can change their AioContext without updating their user. */
+ if (!blk->name || blk->dev) {
+ /* TODO Add BB name/QOM path */
+ error_setg(errp, "Cannot change iothread of active block backend");
+ return false;
+ }
+
+ return true;
+}
+
+static void blk_root_set_aio_ctx(BdrvChild *child, AioContext *ctx,
+ GSList **ignore)
+{
+ BlockBackend *blk = child->opaque;
+ blk_do_set_aio_context(blk, ctx, false);
+}
+
void blk_add_aio_context_notifier(BlockBackend *blk,
void (*attached_aio_context)(AioContext *new_context, void *opaque),
void (*detach_aio_context)(void *opaque), void *opaque)
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH 07/10] block: Add blk_set_allow_aio_context_change()
2019-05-06 17:17 [Qemu-devel] [PATCH 00/10] block: AioContext management, part 1 Kevin Wolf
` (5 preceding siblings ...)
2019-05-06 17:18 ` [Qemu-devel] [PATCH 06/10] block: Implement .(can_)set_aio_ctx for BlockBackend Kevin Wolf
@ 2019-05-06 17:18 ` Kevin Wolf
2019-05-06 17:18 ` [Qemu-devel] [PATCH 08/10] blockjob: Propagate AioContext change to all job nodes Kevin Wolf
` (3 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Kevin Wolf @ 2019-05-06 17:18 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, mreitz
Some users (like block jobs) can tolerate an AioContext change for their
BlockBackend. Add a function that tells the BlockBackend that it can
allow changes.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
include/sysemu/block-backend.h | 1 +
block/block-backend.c | 10 ++++++++++
2 files changed, 11 insertions(+)
diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h
index 5be6224226..938de34fe9 100644
--- a/include/sysemu/block-backend.h
+++ b/include/sysemu/block-backend.h
@@ -103,6 +103,7 @@ int blk_set_perm(BlockBackend *blk, uint64_t perm, uint64_t shared_perm,
void blk_get_perm(BlockBackend *blk, uint64_t *perm, uint64_t *shared_perm);
void blk_set_allow_write_beyond_eof(BlockBackend *blk, bool allow);
+void blk_set_allow_aio_context_change(BlockBackend *blk, bool allow);
void blk_iostatus_enable(BlockBackend *blk);
bool blk_iostatus_is_enabled(const BlockBackend *blk);
BlockDeviceIoStatus blk_iostatus(const BlockBackend *blk);
diff --git a/block/block-backend.c b/block/block-backend.c
index 0e75fc8849..4c0a8ef88d 100644
--- a/block/block-backend.c
+++ b/block/block-backend.c
@@ -71,6 +71,7 @@ struct BlockBackend {
uint64_t shared_perm;
bool disable_perm;
+ bool allow_aio_context_change;
bool allow_write_beyond_eof;
NotifierList remove_bs_notifiers, insert_bs_notifiers;
@@ -1092,6 +1093,11 @@ void blk_set_allow_write_beyond_eof(BlockBackend *blk, bool allow)
blk->allow_write_beyond_eof = allow;
}
+void blk_set_allow_aio_context_change(BlockBackend *blk, bool allow)
+{
+ blk->allow_aio_context_change = allow;
+}
+
static int blk_check_byte_request(BlockBackend *blk, int64_t offset,
size_t size)
{
@@ -1891,6 +1897,10 @@ static bool blk_root_can_set_aio_ctx(BdrvChild *child, AioContext *ctx,
{
BlockBackend *blk = child->opaque;
+ if (blk->allow_aio_context_change) {
+ return true;
+ }
+
/* Only manually created BlockBackends that are not attached to anything
* can change their AioContext without updating their user. */
if (!blk->name || blk->dev) {
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH 08/10] blockjob: Propagate AioContext change to all job nodes
2019-05-06 17:17 [Qemu-devel] [PATCH 00/10] block: AioContext management, part 1 Kevin Wolf
` (6 preceding siblings ...)
2019-05-06 17:18 ` [Qemu-devel] [PATCH 07/10] block: Add blk_set_allow_aio_context_change() Kevin Wolf
@ 2019-05-06 17:18 ` Kevin Wolf
2019-05-06 17:18 ` [Qemu-devel] [PATCH 09/10] blockjob: Remove AioContext notifiers Kevin Wolf
` (2 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Kevin Wolf @ 2019-05-06 17:18 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, mreitz
Block jobs require that all of the nodes the job is using are in the
same AioContext. Therefore all BdrvChild objects of the job propagate
.(can_)set_aio_context to all other job nodes, so that the switch is
checked and performed consistently even if both nodes are in different
subtrees.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/backup.c | 8 --------
block/mirror.c | 10 +---------
blockjob.c | 34 ++++++++++++++++++++++++++++++++++
3 files changed, 35 insertions(+), 17 deletions(-)
diff --git a/block/backup.c b/block/backup.c
index 910ed764aa..916817d8b1 100644
--- a/block/backup.c
+++ b/block/backup.c
@@ -300,13 +300,6 @@ static void backup_clean(Job *job)
s->target = NULL;
}
-static void backup_attached_aio_context(BlockJob *job, AioContext *aio_context)
-{
- BackupBlockJob *s = container_of(job, BackupBlockJob, common);
-
- blk_set_aio_context(s->target, aio_context);
-}
-
void backup_do_checkpoint(BlockJob *job, Error **errp)
{
BackupBlockJob *backup_job = container_of(job, BackupBlockJob, common);
@@ -558,7 +551,6 @@ static const BlockJobDriver backup_job_driver = {
.abort = backup_abort,
.clean = backup_clean,
},
- .attached_aio_context = backup_attached_aio_context,
.drain = backup_drain,
};
diff --git a/block/mirror.c b/block/mirror.c
index ff15cfb197..ec4bd9f404 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -1142,13 +1142,6 @@ static bool mirror_drained_poll(BlockJob *job)
return !!s->in_flight;
}
-static void mirror_attached_aio_context(BlockJob *job, AioContext *new_context)
-{
- MirrorBlockJob *s = container_of(job, MirrorBlockJob, common);
-
- blk_set_aio_context(s->target, new_context);
-}
-
static void mirror_drain(BlockJob *job)
{
MirrorBlockJob *s = container_of(job, MirrorBlockJob, common);
@@ -1178,7 +1171,6 @@ static const BlockJobDriver mirror_job_driver = {
.complete = mirror_complete,
},
.drained_poll = mirror_drained_poll,
- .attached_aio_context = mirror_attached_aio_context,
.drain = mirror_drain,
};
@@ -1196,7 +1188,6 @@ static const BlockJobDriver commit_active_job_driver = {
.complete = mirror_complete,
},
.drained_poll = mirror_drained_poll,
- .attached_aio_context = mirror_attached_aio_context,
.drain = mirror_drain,
};
@@ -1612,6 +1603,7 @@ static void mirror_start_job(const char *job_id, BlockDriverState *bs,
* ensure that. */
blk_set_force_allow_inactivate(s->target);
}
+ blk_set_allow_aio_context_change(s->target, true);
s->replaces = g_strdup(replaces);
s->on_source_error = on_source_error;
diff --git a/blockjob.c b/blockjob.c
index 730101d282..24e6093a9c 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -183,11 +183,44 @@ static void child_job_drained_end(BdrvChild *c)
job_resume(&job->job);
}
+static bool child_job_can_set_aio_ctx(BdrvChild *c, AioContext *ctx,
+ GSList **ignore, Error **errp)
+{
+ BlockJob *job = c->opaque;
+ GSList *l;
+
+ for (l = job->nodes; l; l = l->next) {
+ BdrvChild *sibling = l->data;
+ if (!bdrv_child_can_set_aio_context(sibling, ctx, ignore, errp)) {
+ return false;
+ }
+ }
+ return true;
+}
+
+static void child_job_set_aio_ctx(BdrvChild *c, AioContext *ctx,
+ GSList **ignore)
+{
+ BlockJob *job = c->opaque;
+ GSList *l;
+
+ for (l = job->nodes; l; l = l->next) {
+ BdrvChild *sibling = l->data;
+ if (g_slist_find(*ignore, sibling)) {
+ continue;
+ }
+ *ignore = g_slist_prepend(*ignore, sibling);
+ bdrv_set_aio_context_ignore(sibling->bs, ctx, ignore);
+ }
+}
+
static const BdrvChildRole child_job = {
.get_parent_desc = child_job_get_parent_desc,
.drained_begin = child_job_drained_begin,
.drained_poll = child_job_drained_poll,
.drained_end = child_job_drained_end,
+ .can_set_aio_ctx = child_job_can_set_aio_ctx,
+ .set_aio_ctx = child_job_set_aio_ctx,
.stay_at_node = true,
};
@@ -440,6 +473,7 @@ void *block_job_create(const char *job_id, const BlockJobDriver *driver,
blk_add_aio_context_notifier(blk, block_job_attached_aio_context,
block_job_detach_aio_context, job);
+ blk_set_allow_aio_context_change(blk, true);
/* Only set speed when necessary to avoid NotSupported error */
if (speed != 0) {
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH 09/10] blockjob: Remove AioContext notifiers
2019-05-06 17:17 [Qemu-devel] [PATCH 00/10] block: AioContext management, part 1 Kevin Wolf
` (7 preceding siblings ...)
2019-05-06 17:18 ` [Qemu-devel] [PATCH 08/10] blockjob: Propagate AioContext change to all job nodes Kevin Wolf
@ 2019-05-06 17:18 ` Kevin Wolf
2019-05-06 17:18 ` [Qemu-devel] [PATCH 10/10] test-block-iothread: Test AioContext propagation for block jobs Kevin Wolf
2019-05-20 11:08 ` [Qemu-devel] [PATCH 00/10] block: AioContext management, part 1 Kevin Wolf
10 siblings, 0 replies; 12+ messages in thread
From: Kevin Wolf @ 2019-05-06 17:18 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, mreitz
The notifiers made sure that the job is quiesced and that the
job->aio_context field is updated. The first part is unnecessary today
since bdrv_set_aio_context_ignore() drains the block node, and this
means drainig the block job, too. The second part can be done in the
.set_aio_ctx callback of the block job BdrvChildRole.
The notifiers were problematic because they poll the AioContext while
the graph is in an inconsistent state with some nodes already in the new
context, but others still in the old context. So removing the notifiers
not only simplifies the code, but actually makes the code safer.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
blockjob.c | 43 ++-----------------------------------------
1 file changed, 2 insertions(+), 41 deletions(-)
diff --git a/blockjob.c b/blockjob.c
index 24e6093a9c..9ca942ba01 100644
--- a/blockjob.c
+++ b/blockjob.c
@@ -81,10 +81,6 @@ BlockJob *block_job_get(const char *id)
}
}
-static void block_job_attached_aio_context(AioContext *new_context,
- void *opaque);
-static void block_job_detach_aio_context(void *opaque);
-
void block_job_free(Job *job)
{
BlockJob *bjob = container_of(job, BlockJob, job);
@@ -92,28 +88,10 @@ void block_job_free(Job *job)
bs->job = NULL;
block_job_remove_all_bdrv(bjob);
- blk_remove_aio_context_notifier(bjob->blk,
- block_job_attached_aio_context,
- block_job_detach_aio_context, bjob);
blk_unref(bjob->blk);
error_free(bjob->blocker);
}
-static void block_job_attached_aio_context(AioContext *new_context,
- void *opaque)
-{
- BlockJob *job = opaque;
- const JobDriver *drv = job->job.driver;
- BlockJobDriver *bjdrv = container_of(drv, BlockJobDriver, job_driver);
-
- job->job.aio_context = new_context;
- if (bjdrv->attached_aio_context) {
- bjdrv->attached_aio_context(job, new_context);
- }
-
- job_resume(&job->job);
-}
-
void block_job_drain(Job *job)
{
BlockJob *bjob = container_of(job, BlockJob, job);
@@ -126,23 +104,6 @@ void block_job_drain(Job *job)
}
}
-static void block_job_detach_aio_context(void *opaque)
-{
- BlockJob *job = opaque;
-
- /* In case the job terminates during aio_poll()... */
- job_ref(&job->job);
-
- job_pause(&job->job);
-
- while (!job->job.paused && !job_is_completed(&job->job)) {
- job_drain(&job->job);
- }
-
- job->job.aio_context = NULL;
- job_unref(&job->job);
-}
-
static char *child_job_get_parent_desc(BdrvChild *c)
{
BlockJob *job = c->opaque;
@@ -212,6 +173,8 @@ static void child_job_set_aio_ctx(BdrvChild *c, AioContext *ctx,
*ignore = g_slist_prepend(*ignore, sibling);
bdrv_set_aio_context_ignore(sibling->bs, ctx, ignore);
}
+
+ job->job.aio_context = ctx;
}
static const BdrvChildRole child_job = {
@@ -471,8 +434,6 @@ void *block_job_create(const char *job_id, const BlockJobDriver *driver,
bdrv_op_unblock(bs, BLOCK_OP_TYPE_DATAPLANE, job->blocker);
- blk_add_aio_context_notifier(blk, block_job_attached_aio_context,
- block_job_detach_aio_context, job);
blk_set_allow_aio_context_change(blk, true);
/* Only set speed when necessary to avoid NotSupported error */
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH 10/10] test-block-iothread: Test AioContext propagation for block jobs
2019-05-06 17:17 [Qemu-devel] [PATCH 00/10] block: AioContext management, part 1 Kevin Wolf
` (8 preceding siblings ...)
2019-05-06 17:18 ` [Qemu-devel] [PATCH 09/10] blockjob: Remove AioContext notifiers Kevin Wolf
@ 2019-05-06 17:18 ` Kevin Wolf
2019-05-20 11:08 ` [Qemu-devel] [PATCH 00/10] block: AioContext management, part 1 Kevin Wolf
10 siblings, 0 replies; 12+ messages in thread
From: Kevin Wolf @ 2019-05-06 17:18 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, qemu-devel, mreitz
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/test-block-iothread.c | 71 +++++++++++++++++++++++++++++++++++++
1 file changed, 71 insertions(+)
diff --git a/tests/test-block-iothread.c b/tests/test-block-iothread.c
index 938831c9bd..59f692892e 100644
--- a/tests/test-block-iothread.c
+++ b/tests/test-block-iothread.c
@@ -588,6 +588,76 @@ static void test_propagate_diamond(void)
bdrv_unref(bs_a);
}
+static void test_propagate_mirror(void)
+{
+ IOThread *iothread = iothread_new();
+ AioContext *ctx = iothread_get_aio_context(iothread);
+ AioContext *main_ctx = qemu_get_aio_context();
+ BlockDriverState *src, *target;
+ BlockBackend *blk;
+ Job *job;
+ Error *local_err = NULL;
+
+ /* Create src and target*/
+ src = bdrv_new_open_driver(&bdrv_test, "src", BDRV_O_RDWR, &error_abort);
+ target = bdrv_new_open_driver(&bdrv_test, "target", BDRV_O_RDWR,
+ &error_abort);
+
+ /* Start a mirror job */
+ mirror_start("job0", src, target, NULL, JOB_DEFAULT, 0, 0, 0,
+ MIRROR_SYNC_MODE_NONE, MIRROR_OPEN_BACKING_CHAIN,
+ BLOCKDEV_ON_ERROR_REPORT, BLOCKDEV_ON_ERROR_REPORT,
+ false, "filter_node", MIRROR_COPY_MODE_BACKGROUND,
+ &error_abort);
+ job = job_get("job0");
+
+ /* Change the AioContext of src */
+ bdrv_try_set_aio_context(src, ctx, &error_abort);
+ g_assert(bdrv_get_aio_context(src) == ctx);
+ g_assert(bdrv_get_aio_context(target) == ctx);
+ g_assert(job->aio_context == ctx);
+
+ /* Change the AioContext of target */
+ aio_context_acquire(ctx);
+ bdrv_try_set_aio_context(target, main_ctx, &error_abort);
+ aio_context_release(ctx);
+ g_assert(bdrv_get_aio_context(src) == main_ctx);
+ g_assert(bdrv_get_aio_context(target) == main_ctx);
+
+ /* With a BlockBackend on src, changing target must fail */
+ blk = blk_new(0, BLK_PERM_ALL);
+ blk_insert_bs(blk, src, &error_abort);
+
+ bdrv_try_set_aio_context(target, ctx, &local_err);
+ g_assert(local_err);
+ error_free(local_err);
+
+ g_assert(blk_get_aio_context(blk) == main_ctx);
+ g_assert(bdrv_get_aio_context(src) == main_ctx);
+ g_assert(bdrv_get_aio_context(target) == main_ctx);
+
+ /* ...unless we explicitly allow it */
+ aio_context_acquire(ctx);
+ blk_set_allow_aio_context_change(blk, true);
+ bdrv_try_set_aio_context(target, ctx, &error_abort);
+ aio_context_release(ctx);
+
+ g_assert(blk_get_aio_context(blk) == ctx);
+ g_assert(bdrv_get_aio_context(src) == ctx);
+ g_assert(bdrv_get_aio_context(target) == ctx);
+
+ job_cancel_sync_all();
+
+ aio_context_acquire(ctx);
+ blk_set_aio_context(blk, main_ctx);
+ bdrv_try_set_aio_context(target, main_ctx, &error_abort);
+ aio_context_release(ctx);
+
+ blk_unref(blk);
+ bdrv_unref(src);
+ bdrv_unref(target);
+}
+
int main(int argc, char **argv)
{
int i;
@@ -605,6 +675,7 @@ int main(int argc, char **argv)
g_test_add_func("/attach/blockjob", test_attach_blockjob);
g_test_add_func("/propagate/basic", test_propagate_basic);
g_test_add_func("/propagate/diamond", test_propagate_diamond);
+ g_test_add_func("/propagate/mirror", test_propagate_mirror);
return g_test_run();
}
--
2.20.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH 00/10] block: AioContext management, part 1
2019-05-06 17:17 [Qemu-devel] [PATCH 00/10] block: AioContext management, part 1 Kevin Wolf
` (9 preceding siblings ...)
2019-05-06 17:18 ` [Qemu-devel] [PATCH 10/10] test-block-iothread: Test AioContext propagation for block jobs Kevin Wolf
@ 2019-05-20 11:08 ` Kevin Wolf
10 siblings, 0 replies; 12+ messages in thread
From: Kevin Wolf @ 2019-05-20 11:08 UTC (permalink / raw)
To: qemu-block; +Cc: qemu-devel, mreitz
Am 06.05.2019 um 19:17 hat Kevin Wolf geschrieben:
> Recently, a few bugs were reported that resulted from an inconsistent
> state regarding AioContexts. Block nodes can end up in different
> contexts than their users expect - the AioContext of a node can even
> change under the feet of a device with no way for the device to forbid
> this. We recently added a few basic checks to scsi-disk and virtio-blk,
> but they are by far not enough.
>
> This is the first part of my work to actually properly manage
> AioContexts in the block layer rather than just doing some ad-hoc calls
> to bdrv_set_aio_context() and hoping that everything will work out.
>
> The goal of this first part is that bdrv_set_aio_context() propagates
> the AioContext change not only to the children of the node like we
> already do, but also to any other affected nodes, such as additional
> parents or nodes connected to the requested one only through a block job
> that operates on both nodes.
>
> Keep in mind that a second part will follow and that this is visible in
> some functions that may not seem that useful in this series. In
> particular, bdrv_try_set_aio_context() isn't used much outside of test
> cases in this series. This will change in the second part.
Applied to the block branch.
Kevin
^ permalink raw reply [flat|nested] 12+ messages in thread