[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PULL 45/50] block: Make bdrv_refresh_limits() non-recursive
From: |
Kevin Wolf |
Subject: |
[PULL 45/50] block: Make bdrv_refresh_limits() non-recursive |
Date: |
Fri, 4 Mar 2022 17:47:06 +0100 |
From: Hanna Reitz <hreitz@redhat.com>
bdrv_refresh_limits() recurses down to the node's children. That does
not seem necessary: When we refresh limits on some node, and then
recurse down and were to change one of its children's BlockLimits, then
that would mean we noticed the changed limits by pure chance. The fact
that we refresh the parent's limits has nothing to do with it, so the
reason for the change probably happened before this point in time, and
we should have refreshed the limits then.
Consequently, we should actually propagate block limits changes upwards,
not downwards. That is a separate and pre-existing issue, though, and
so will not be addressed in this patch.
The problem with recursing is that bdrv_refresh_limits() is not atomic.
It begins with zeroing BDS.bl, and only then sets proper, valid limits.
If we do not drain all nodes whose limits are refreshed, then concurrent
I/O requests can encounter invalid request_alignment values and crash
qemu. Therefore, a recursing bdrv_refresh_limits() requires the whole
subtree to be drained, which is currently not ensured by most callers.
A non-recursive bdrv_refresh_limits() only requires the node in question
to not receive I/O requests, and this is done by most callers in some
way or another:
- bdrv_open_driver() deals with a new node with no parents yet
- bdrv_set_file_or_backing_noperm() acts on a drained node
- bdrv_reopen_commit() acts only on drained nodes
- bdrv_append() should in theory require the node to be drained; in
practice most callers just lock the AioContext, which should at least
be enough to prevent concurrent I/O requests from accessing invalid
limits
So we can resolve the bug by making bdrv_refresh_limits() non-recursive.
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1879437
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20220216105355.30729-2-hreitz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/io.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/block/io.c b/block/io.c
index 4b1d97c7da..efc011ce65 100644
--- a/block/io.c
+++ b/block/io.c
@@ -193,10 +193,6 @@ void bdrv_refresh_limits(BlockDriverState *bs, Transaction
*tran, Error **errp)
QLIST_FOREACH(c, &bs->children, next) {
if (c->role & (BDRV_CHILD_DATA | BDRV_CHILD_FILTERED | BDRV_CHILD_COW))
{
- bdrv_refresh_limits(c->bs, tran, errp);
- if (*errp) {
- return;
- }
bdrv_merge_limits(&bs->bl, &c->bs->bl);
have_limits = true;
}
--
2.35.1
- [PULL 30/50] block.c: add assertions to static functions, (continued)
- [PULL 30/50] block.c: add assertions to static functions, Kevin Wolf, 2022/03/04
- [PULL 36/50] block/copy-before-write.h: global state API + assertions, Kevin Wolf, 2022/03/04
- [PULL 37/50] block/coroutines: I/O and "I/O or GS" API, Kevin Wolf, 2022/03/04
- [PULL 38/50] block_int-common.h: split function pointers in BlockDriver, Kevin Wolf, 2022/03/04
- [PULL 39/50] block_int-common.h: assertions in the callers of BlockDriver function pointers, Kevin Wolf, 2022/03/04
- [PULL 41/50] block_int-common.h: assertions in the callers of BdrvChildClass function pointers, Kevin Wolf, 2022/03/04
- [PULL 42/50] block-backend-common.h: split function pointers in BlockDevOps, Kevin Wolf, 2022/03/04
- [PULL 44/50] job.h: assertions in the callers of JobDriver function pointers, Kevin Wolf, 2022/03/04
- [PULL 40/50] block_int-common.h: split function pointers in BdrvChildClass, Kevin Wolf, 2022/03/04
- [PULL 43/50] job.h: split function pointers in JobDriver, Kevin Wolf, 2022/03/04
- [PULL 45/50] block: Make bdrv_refresh_limits() non-recursive,
Kevin Wolf <=
- [PULL 47/50] iotests/graph-changes-while-io: New test, Kevin Wolf, 2022/03/04
- [PULL 46/50] iotests: Allow using QMP with the QSD, Kevin Wolf, 2022/03/04
- [PULL 50/50] block/amend: Keep strong reference to BDS, Kevin Wolf, 2022/03/04
- [PULL 49/50] block/amend: Always call .bdrv_amend_clean(), Kevin Wolf, 2022/03/04
- [PULL 48/50] tests/qemu-iotests: Rework the checks and spots using GNU sed, Kevin Wolf, 2022/03/04
- Re: [PULL 00/50] Block layer patches, Peter Maydell, 2022/03/05