* [PATCH 01/13] block/file-posix: drop the XFS workaround for post-EOF fallocate
2026-09-30 16:49 [PATCH 00/13] block: fix data loss in the preallocate filter Denis V. Lunev
@ 2026-09-30 16:49 ` Denis V. Lunev
2026-10-02 16:16 ` Andrey Drobyshev
2026-09-30 16:49 ` [PATCH 02/13] block/preallocate: keep zero_start above written data Denis V. Lunev
` (11 subsequent siblings)
12 siblings, 1 reply; 26+ messages in thread
From: Denis V. Lunev @ 2026-09-30 16:49 UTC (permalink / raw)
To: qemu-devel
Cc: den, qemu-block, qemu-stable, Kevin Wolf, Hanna Reitz,
Vladimir Sementsov-Ogievskiy, Andrey Drobyshev
From: Denis V. Lunev <den@openvz.org>
292d06b925b extended the tracked request of a post-EOF write zeroes to
infinity and made it serialising, to work around an XFS bug where AIO
writes past EOF were discarded by a concurrently extending fallocate().
The kernel bug was fixed by 249bd9087a52 ("xfs: properly serialise
fallocate against AIO+DIO"), which moves inode_dio_wait() up under
IOLOCK_EXCL so fallocate cannot run alongside in-flight AIO. That
landed in v5.5, three days before the workaround was committed here,
and predates every stable series still maintained.
The workaround runs on every filesystem and on every write zeroes past
EOF, which for the preallocate filter is every preallocation. It widens
the request to the end of the address space and makes it serialising,
so it excludes every other request above its offset, and it does that
after bdrv_co_write_req_prepare() has honoured BDRV_REQ_NO_WAIT, so the
flag no longer holds for the rest of the request.
Drop it. The only series that never took the kernel fix is 4.19.y,
which is end of life.
Signed-off-by: Denis V. Lunev <den@openvz.org>
Cc: qemu-stable@nongnu.org
CC: Kevin Wolf <kwolf@redhat.com>
CC: Hanna Reitz <hreitz@redhat.com>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
block/file-posix.c | 34 ----------------------------------
1 file changed, 34 deletions(-)
diff --git a/block/file-posix.c b/block/file-posix.c
index 9aad156aad..529e302041 100644
--- a/block/file-posix.c
+++ b/block/file-posix.c
@@ -3658,40 +3658,6 @@ raw_do_pwrite_zeroes(BlockDriverState *bs, int64_t offset, int64_t bytes,
RawPosixAIOData acb;
ThreadPoolFunc *handler;
-#ifdef CONFIG_FALLOCATE
- if (offset + bytes > bs->total_sectors * BDRV_SECTOR_SIZE) {
- BdrvTrackedRequest *req;
-
- /*
- * This is a workaround for a bug in the Linux XFS driver,
- * where writes submitted through the AIO interface will be
- * discarded if they happen beyond a concurrently running
- * fallocate() that increases the file length (i.e., both the
- * write and the fallocate() happen beyond the EOF).
- *
- * To work around it, we extend the tracked request for this
- * zero write until INT64_MAX (effectively infinity), and mark
- * it as serializing.
- *
- * We have to enable this workaround for all filesystems and
- * AIO modes (not just XFS with aio=native), because for
- * remote filesystems we do not know the host configuration.
- */
-
- req = bdrv_co_get_self_request(bs);
- assert(req);
- assert(req->type == BDRV_TRACKED_WRITE);
- assert(req->offset <= offset);
- assert(req->offset + req->bytes >= offset + bytes);
-
- req->bytes = BDRV_MAX_LENGTH - req->offset;
-
- bdrv_check_request(req->offset, req->bytes, &error_abort);
-
- bdrv_make_request_serialising(req, bs->bl.request_alignment);
- }
-#endif
-
acb = (RawPosixAIOData) {
.bs = bs,
.aio_fildes = s->fd,
--
2.53.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 01/13] block/file-posix: drop the XFS workaround for post-EOF fallocate
2026-09-30 16:49 ` [PATCH 01/13] block/file-posix: drop the XFS workaround for post-EOF fallocate Denis V. Lunev
@ 2026-10-02 16:16 ` Andrey Drobyshev
0 siblings, 0 replies; 26+ messages in thread
From: Andrey Drobyshev @ 2026-10-02 16:16 UTC (permalink / raw)
To: Denis V. Lunev
Cc: qemu-devel, qemu-block, qemu-stable, Kevin Wolf, Hanna Reitz,
Vladimir Sementsov-Ogievskiy, Andrey Drobyshev
> 292d06b925b extended the tracked request of a post-EOF write zeroes to
> infinity and made it serialising, to work around an XFS bug where AIO
> writes past EOF were discarded by a concurrently extending fallocate().
>
> The kernel bug was fixed by 249bd9087a52 ("xfs: properly serialise
> fallocate against AIO+DIO"), which moves inode_dio_wait() up under
> IOLOCK_EXCL so fallocate cannot run alongside in-flight AIO. That
> landed in v5.5, three days before the workaround was committed here,
> and predates every stable series still maintained.
>
> [...]
Reviewed-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
One nit: looks like after this patch no more callers exist for
bdrv_co_get_self_request(). Maybe we can remove it from this or
subsequent patch?
Andrey
--
Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 02/13] block/preallocate: keep zero_start above written data
2026-09-30 16:49 [PATCH 00/13] block: fix data loss in the preallocate filter Denis V. Lunev
2026-09-30 16:49 ` [PATCH 01/13] block/file-posix: drop the XFS workaround for post-EOF fallocate Denis V. Lunev
@ 2026-09-30 16:49 ` Denis V. Lunev
2026-10-02 16:16 ` Andrey Drobyshev
2026-09-30 16:49 ` [PATCH 03/13] block/preallocate: collect the state assignments in one place Denis V. Lunev
` (10 subsequent siblings)
12 siblings, 1 reply; 26+ messages in thread
From: Denis V. Lunev @ 2026-09-30 16:49 UTC (permalink / raw)
To: qemu-devel
Cc: den, qemu-block, qemu-stable, Kevin Wolf, Hanna Reitz,
Vladimir Sementsov-Ogievskiy, Andrey Drobyshev
From: Denis V. Lunev <den@openvz.org>
The filter records where the trailing preallocated zeroes begin, and
skips a write zeroes request that starts at or above that point,
because the range is zero already. A plain write below data_end returns
before that bookkeeping, so a write into the zero region leaves the
mark behind it.
The guest then loses an acknowledged write zeroes over the range: it is
skipped as already zero, and the plain write's data stays. Reachable
from qcow2 when a cluster is allocated again after a discard and the
allocating write zeroes straddles data_end.
Move the mark past any write that reaches into the region it covers and
leaves data there. want_merge_zero is what the function already uses to
tell those apart when the write extends the file: a request that writes
zeroes leaves the region zero and the mark where it is.
Signed-off-by: Denis V. Lunev <den@openvz.org>
Cc: qemu-stable@nongnu.org
CC: Kevin Wolf <kwolf@redhat.com>
CC: Hanna Reitz <hreitz@redhat.com>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
block/preallocate.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/block/preallocate.c b/block/preallocate.c
index d215bc5d6d..99bd6b1ae7 100644
--- a/block/preallocate.c
+++ b/block/preallocate.c
@@ -347,6 +347,10 @@ handle_write(BlockDriverState *bs, int64_t offset, int64_t bytes,
}
}
+ if (!want_merge_zero && s->zero_start >= 0 && end > s->zero_start) {
+ s->zero_start = end;
+ }
+
if (end <= s->data_end) {
return false;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 02/13] block/preallocate: keep zero_start above written data
2026-09-30 16:49 ` [PATCH 02/13] block/preallocate: keep zero_start above written data Denis V. Lunev
@ 2026-10-02 16:16 ` Andrey Drobyshev
0 siblings, 0 replies; 26+ messages in thread
From: Andrey Drobyshev @ 2026-10-02 16:16 UTC (permalink / raw)
To: Denis V. Lunev
Cc: qemu-devel, qemu-block, qemu-stable, Kevin Wolf, Hanna Reitz,
Vladimir Sementsov-Ogievskiy, Andrey Drobyshev
> The filter records where the trailing preallocated zeroes begin, and
> skips a write zeroes request that starts at or above that point,
Just "starts"? Maybe "skips a write zeroes request which lies within
the preallocated zeroed area"?
> because the range is zero already. A plain write below data_end returns
> before that bookkeeping, so a write into the zero region leaves the
> mark behind it.
IMHO this is hard to comprehend. Which mark? The 'zero_start' mark,
I suppose? And behind the written data?
So, If I follow correctly, by this point we have 'zero_start' pointing
not at the beginning of zeroes as it should, but at actual data - that
is the bug.
> The guest then loses an acknowledged write zeroes over the range: it is
> skipped as already zero, and the plain write's data stays. Reachable
> from qcow2 when a cluster is allocated again after a discard and the
> allocating write zeroes straddles data_end.
I guess this is covered in one of subsequent tests? Maybe either name
the test/commit, or put the test triggering this bug directly after this
commit.
> Move the mark past any write that reaches into the region it covers and
> leaves data there. want_merge_zero is what the function already uses to
> tell those apart when the write extends the file: a request that writes
> zeroes leaves the region zero and the mark where it is.
>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> Cc: qemu-stable@nongnu.org
> CC: Kevin Wolf <kwolf@redhat.com>
> CC: Hanna Reitz <hreitz@redhat.com>
> CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
>
> diff --git a/block/preallocate.c b/block/preallocate.c
> index d215bc5d6d0..99bd6b1ae76 100644
> --- a/block/preallocate.c
> +++ b/block/preallocate.c
> @@ -347,6 +347,10 @@ handle_write(BlockDriverState *bs, int64_t offset, int64_t bytes,
> }
> }
>
> + if (!want_merge_zero && s->zero_start >= 0 && end > s->zero_start) {
> + s->zero_start = end;
> + }
Maybe (0 <= s->zero_start && s->zero_start < end). Cosmetic, for
readability only.
The actual code and the logic LGTM. I'd only like to make this more
readable and easier to comprehend.
Thanks,
Andrey
--
Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 03/13] block/preallocate: collect the state assignments in one place
2026-09-30 16:49 [PATCH 00/13] block: fix data loss in the preallocate filter Denis V. Lunev
2026-09-30 16:49 ` [PATCH 01/13] block/file-posix: drop the XFS workaround for post-EOF fallocate Denis V. Lunev
2026-09-30 16:49 ` [PATCH 02/13] block/preallocate: keep zero_start above written data Denis V. Lunev
@ 2026-09-30 16:49 ` Denis V. Lunev
2026-10-02 16:16 ` Andrey Drobyshev
2026-09-30 16:49 ` [PATCH 04/13] block/preallocate: serialise the filter state Denis V. Lunev
` (9 subsequent siblings)
12 siblings, 1 reply; 26+ messages in thread
From: Denis V. Lunev @ 2026-09-30 16:49 UTC (permalink / raw)
To: qemu-devel
Cc: den, qemu-block, qemu-stable, Kevin Wolf, Hanna Reitz,
Vladimir Sementsov-Ogievskiy, Andrey Drobyshev
From: Denis V. Lunev <den@openvz.org>
The filter invalidates or re-establishes data_end, zero_start and
file_end together in six places, each spelling out the same chained
assignment. Put them in one function, so that a change to what the
state consists of is one edit rather than six.
No functional change.
Signed-off-by: Denis V. Lunev <den@openvz.org>
Cc: qemu-stable@nongnu.org
CC: Kevin Wolf <kwolf@redhat.com>
CC: Hanna Reitz <hreitz@redhat.com>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
block/preallocate.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/block/preallocate.c b/block/preallocate.c
index 99bd6b1ae7..8cd98684f4 100644
--- a/block/preallocate.c
+++ b/block/preallocate.c
@@ -104,6 +104,11 @@ static QemuOptsList runtime_opts = {
},
};
+static void preallocate_set_state(BDRVPreallocateState *s, int64_t v)
+{
+ s->file_end = s->zero_start = s->data_end = v;
+}
+
static bool preallocate_absorb_opts(PreallocateOpts *dest, QDict *options,
BlockDriverState *child_bs, Error **errp)
{
@@ -149,7 +154,7 @@ static int preallocate_open(BlockDriverState *bs, QDict *options, int flags,
* s->data_end and friends should be initialized on permission update.
* For this to work, mark them invalid.
*/
- s->file_end = s->zero_start = s->data_end = -EINVAL;
+ preallocate_set_state(s, -EINVAL);
s->drop_resize_bh = qemu_bh_new(preallocate_drop_resize_bh, bs);
ret = bdrv_open_file_child(NULL, options, "file", bs, errp);
@@ -480,12 +485,12 @@ preallocate_co_truncate(BlockDriverState *bs, int64_t offset,
ret = bdrv_co_truncate(bs->file, offset, exact, prealloc, flags, errp);
if (ret < 0) {
- s->file_end = s->zero_start = s->data_end = ret;
+ preallocate_set_state(s, ret);
return ret;
}
if (has_prealloc_perms(bs)) {
- s->file_end = s->zero_start = s->data_end = offset;
+ preallocate_set_state(s, offset);
}
return 0;
}
@@ -508,7 +513,7 @@ preallocate_co_getlength(BlockDriverState *bs)
ret = bdrv_co_getlength(bs->file->bs);
if (has_prealloc_perms(bs)) {
- s->file_end = s->zero_start = s->data_end = ret;
+ preallocate_set_state(s, ret);
}
return ret;
@@ -539,7 +544,7 @@ preallocate_drop_resize(BlockDriverState *bs, Error **errp)
* change the child, so mark all states invalid. We'll regain control if a
* parent requests write access again.
*/
- s->data_end = s->file_end = s->zero_start = -EINVAL;
+ preallocate_set_state(s, -EINVAL);
bdrv_child_refresh_perms(bs, bs->file, NULL);
@@ -566,8 +571,8 @@ preallocate_set_perm(BlockDriverState *bs, uint64_t perm, uint64_t shared)
if (can_write_resize(perm)) {
qemu_bh_cancel(s->drop_resize_bh);
if (s->data_end < 0) {
- s->data_end = s->file_end = s->zero_start =
- bs->file->bs->total_sectors * BDRV_SECTOR_SIZE;
+ preallocate_set_state(s,
+ bs->file->bs->total_sectors * BDRV_SECTOR_SIZE);
}
} else {
qemu_bh_schedule(s->drop_resize_bh);
--
2.53.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 03/13] block/preallocate: collect the state assignments in one place
2026-09-30 16:49 ` [PATCH 03/13] block/preallocate: collect the state assignments in one place Denis V. Lunev
@ 2026-10-02 16:16 ` Andrey Drobyshev
0 siblings, 0 replies; 26+ messages in thread
From: Andrey Drobyshev @ 2026-10-02 16:16 UTC (permalink / raw)
To: Denis V. Lunev
Cc: qemu-devel, qemu-block, qemu-stable, Kevin Wolf, Hanna Reitz,
Vladimir Sementsov-Ogievskiy, Andrey Drobyshev
The patch itself is trivial and LGTM.
Reviewed-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
However see below.
> The filter invalidates or re-establishes data_end, zero_start and
> file_end together in six places, each spelling out the same chained
> assignment. Put them in one function, so that a change to what the
> state consists of is one edit rather than six.
>
> No functional change.
>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> Cc: qemu-stable@nongnu.org
> CC: Kevin Wolf <kwolf@redhat.com>
> CC: Hanna Reitz <hreitz@redhat.com>
> CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
>
> diff --git a/block/preallocate.c b/block/preallocate.c
> index 99bd6b1ae76..8cd98684f49 100644
> --- a/block/preallocate.c
> +++ b/block/preallocate.c
> @@ -104,6 +104,11 @@ static QemuOptsList runtime_opts = {
> },
> };
>
> +static void preallocate_set_state(BDRVPreallocateState *s, int64_t v)
> +{
> + s->file_end = s->zero_start = s->data_end = v;
> +}
> +
> static bool preallocate_absorb_opts(PreallocateOpts *dest, QDict *options,
> BlockDriverState *child_bs, Error **errp)
> {
> @@ -149,7 +154,7 @@ static int preallocate_open(BlockDriverState *bs, QDict *options, int flags,
> * s->data_end and friends should be initialized on permission update.
> * For this to work, mark them invalid.
> */
> - s->file_end = s->zero_start = s->data_end = -EINVAL;
> + preallocate_set_state(s, -EINVAL);
> s->drop_resize_bh = qemu_bh_new(preallocate_drop_resize_bh, bs);
>
> ret = bdrv_open_file_child(NULL, options, "file", bs, errp);
> @@ -480,12 +485,12 @@ preallocate_co_truncate(BlockDriverState *bs, int64_t offset,
>
> ret = bdrv_co_truncate(bs->file, offset, exact, prealloc, flags, errp);
> if (ret < 0) {
> - s->file_end = s->zero_start = s->data_end = ret;
> + preallocate_set_state(s, ret);
This is preexisting and we aren't changing the logic here, but when
reading this code I got questions:
In most cases we determine filter's validity by checking the condition
data_end >= 0.
1. Why we set this field to -EINVAL specifically? Why not just -1?
2. Why we set it to 'ret' here? bdrv_co_truncate() as well as
bdrv_co_getlength() below might return other errors, e.g. -ENOMEDIUM,
-EACCES. Is there a specific intention on using other errno values for
this state?
3. If there is such intention - does it have smth to do with the check
in preallocate_child_perm() - the only place where we compare
s->data_end != -EINVAL?
IMHO this looks error-prone. If there's no solid logic behind those
values and it's just a legacy code - how about we use exclusively -1 to
indicate filter's invalidity? And then:
s->data_end >= 0 - state is valid;
s->data_end < 0 - state is invalid/uninitialized.
Thanks,
Andrey
--
Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 04/13] block/preallocate: serialise the filter state
2026-09-30 16:49 [PATCH 00/13] block: fix data loss in the preallocate filter Denis V. Lunev
` (2 preceding siblings ...)
2026-09-30 16:49 ` [PATCH 03/13] block/preallocate: collect the state assignments in one place Denis V. Lunev
@ 2026-09-30 16:49 ` Denis V. Lunev
2026-10-02 16:16 ` Andrey Drobyshev
2026-09-30 16:49 ` [PATCH 05/13] block/preallocate: hold back writes into a preallocation in flight Denis V. Lunev
` (8 subsequent siblings)
12 siblings, 1 reply; 26+ messages in thread
From: Denis V. Lunev @ 2026-09-30 16:49 UTC (permalink / raw)
To: qemu-devel
Cc: den, qemu-block, qemu-stable, Kevin Wolf, Hanna Reitz,
Vladimir Sementsov-Ogievskiy, Andrey Drobyshev
From: Denis V. Lunev <den@openvz.org>
The filter keeps the file length it has seen, the length it has
reserved and where the trailing zeroes start in plain fields, and
updates them with a read, a compare and a store. That was indivisible
while one node was driven by one AioContext. It is not any more:
virtio-blk with iothread-vq-mapping drives one node from several.
Two writers extending the file then both pass the compare, and the
smaller end lands last:
writer A, [1G, 1G+64k) writer B, [1G+64k, 1G+128k)
-------------------------------- --------------------------------
reads data_end 1G, 1G+64k > 1G
reads data_end 1G, 1G+128k > 1G
stores data_end = 1G+128k
stores data_end = 1G+64k
Both writes are acknowledged, and data_end now sits below B's data.
Nothing raises it again except a later write ending beyond it, and
closing the image truncates the file to it, so B's data is cut off the
end. The last pair of extending writes before a shutdown is exactly
where this bites.
Take a mutex across handle_write() and preallocate_co_getlength(). The
remaining readers, apart from preallocate_co_truncate(), reach the
state from global state code with the node drained, where no request
is in flight.
Holding it cannot deadlock. A writer waiting for it has not reached
bs->file yet, so it owns nothing below the filter, and the only calls
made under it go to the child. preallocate_co_truncate() is left out
for that reason: growing the file waits for the requests in flight on
the child, and once the next patch issues the preallocation without
the mutex, one of those can be a preallocation that needs it back.
Signed-off-by: Denis V. Lunev <den@openvz.org>
Cc: qemu-stable@nongnu.org
CC: Kevin Wolf <kwolf@redhat.com>
CC: Hanna Reitz <hreitz@redhat.com>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
block/preallocate.c | 27 ++++++++++++++++++++++++---
1 file changed, 24 insertions(+), 3 deletions(-)
diff --git a/block/preallocate.c b/block/preallocate.c
index 8cd98684f4..442cdb0176 100644
--- a/block/preallocate.c
+++ b/block/preallocate.c
@@ -76,6 +76,8 @@ typedef struct BDRVPreallocateState {
* BLK_PERM_WRITE permissions on file child.
*/
+ CoMutex lock;
+
/* Gives up the resize permission on children when parents don't need it */
QEMUBH *drop_resize_bh;
} BDRVPreallocateState;
@@ -155,6 +157,7 @@ static int preallocate_open(BlockDriverState *bs, QDict *options, int flags,
* For this to work, mark them invalid.
*/
preallocate_set_state(s, -EINVAL);
+ qemu_co_mutex_init(&s->lock);
s->drop_resize_bh = qemu_bh_new(preallocate_drop_resize_bh, bs);
ret = bdrv_open_file_child(NULL, options, "file", bs, errp);
@@ -324,8 +327,8 @@ static bool GRAPH_RDLOCK has_prealloc_perms(BlockDriverState *bs)
* one bdrv_co_pwrite_zeroes() call.
*/
static bool coroutine_fn GRAPH_RDLOCK
-handle_write(BlockDriverState *bs, int64_t offset, int64_t bytes,
- bool want_merge_zero)
+handle_write_locked(BlockDriverState *bs, int64_t offset, int64_t bytes,
+ bool want_merge_zero)
{
BDRVPreallocateState *s = bs->opaque;
int64_t end = offset + bytes;
@@ -404,6 +407,20 @@ handle_write(BlockDriverState *bs, int64_t offset, int64_t bytes,
return want_merge_zero;
}
+static bool coroutine_fn GRAPH_RDLOCK
+handle_write(BlockDriverState *bs, int64_t offset, int64_t bytes,
+ bool want_merge_zero)
+{
+ BDRVPreallocateState *s = bs->opaque;
+ bool ret;
+
+ qemu_co_mutex_lock(&s->lock);
+ ret = handle_write_locked(bs, offset, bytes, want_merge_zero);
+ qemu_co_mutex_unlock(&s->lock);
+
+ return ret;
+}
+
static int coroutine_fn GRAPH_RDLOCK
preallocate_co_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
int64_t bytes, BdrvRequestFlags flags)
@@ -506,8 +523,11 @@ preallocate_co_getlength(BlockDriverState *bs)
int64_t ret;
BDRVPreallocateState *s = bs->opaque;
+ qemu_co_mutex_lock(&s->lock);
if (s->data_end >= 0) {
- return s->data_end;
+ ret = s->data_end;
+ qemu_co_mutex_unlock(&s->lock);
+ return ret;
}
ret = bdrv_co_getlength(bs->file->bs);
@@ -515,6 +535,7 @@ preallocate_co_getlength(BlockDriverState *bs)
if (has_prealloc_perms(bs)) {
preallocate_set_state(s, ret);
}
+ qemu_co_mutex_unlock(&s->lock);
return ret;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 04/13] block/preallocate: serialise the filter state
2026-09-30 16:49 ` [PATCH 04/13] block/preallocate: serialise the filter state Denis V. Lunev
@ 2026-10-02 16:16 ` Andrey Drobyshev
0 siblings, 0 replies; 26+ messages in thread
From: Andrey Drobyshev @ 2026-10-02 16:16 UTC (permalink / raw)
To: Denis V. Lunev
Cc: qemu-devel, qemu-block, qemu-stable, Kevin Wolf, Hanna Reitz,
Vladimir Sementsov-Ogievskiy, Andrey Drobyshev
> The filter keeps the file length it has seen, the length it has
> reserved and where the trailing zeroes start in plain fields, and
> updates them with a read, a compare and a store. That was indivisible
> while one node was driven by one AioContext. It is not any more:
> virtio-blk with iothread-vq-mapping drives one node from several.
>
> Two writers extending the file then both pass the compare, and the
> smaller end lands last:
>
> writer A, [1G, 1G+64k) writer B, [1G+64k, 1G+128k)
> -------------------------------- --------------------------------
> reads data_end 1G, 1G+64k > 1G
> reads data_end 1G, 1G+128k > 1G
> stores data_end = 1G+128k
> stores data_end = 1G+64k
>
> Both writes are acknowledged, and data_end now sits below B's data.
> Nothing raises it again except a later write ending beyond it, and
> closing the image truncates the file to it, so B's data is cut off the
> end. The last pair of extending writes before a shutdown is exactly
> where this bites.
>
> Take a mutex across handle_write() and preallocate_co_getlength(). The
> remaining readers, apart from preallocate_co_truncate(), reach the
> state from global state code with the node drained, where no request
> is in flight.
The patch logic LGTM. However if this claim is true, why don't we go
through each and every other reader of s->data_end and add
GLOBAL_STATE_CODE() in their preamble, to make sure this claim holds?
> Holding it cannot deadlock. A writer waiting for it has not reached
> bs->file yet, so it owns nothing below the filter, and the only calls
> made under it go to the child. preallocate_co_truncate() is left out
> for that reason: growing the file waits for the requests in flight on
> the child, and once the next patch issues the preallocation without
> the mutex, one of those can be a preallocation that needs it back.
>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> Cc: qemu-stable@nongnu.org
> CC: Kevin Wolf <kwolf@redhat.com>
> CC: Hanna Reitz <hreitz@redhat.com>
> CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
>
> diff --git a/block/preallocate.c b/block/preallocate.c
> index 8cd98684f49..442cdb01768 100644
> --- a/block/preallocate.c
> +++ b/block/preallocate.c
> @@ -76,6 +76,8 @@ typedef struct BDRVPreallocateState {
> * BLK_PERM_WRITE permissions on file child.
> */
>
> + CoMutex lock;
Also why don't we add some comment above the 'lock' field explaining
that we serialize updates of s->data_end (or state in general) and it's
relevant with iothread-vq-mapping and multiple AioContext's.
Thanks,
Andrey
--
Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 05/13] block/preallocate: hold back writes into a preallocation in flight
2026-09-30 16:49 [PATCH 00/13] block: fix data loss in the preallocate filter Denis V. Lunev
` (3 preceding siblings ...)
2026-09-30 16:49 ` [PATCH 04/13] block/preallocate: serialise the filter state Denis V. Lunev
@ 2026-09-30 16:49 ` Denis V. Lunev
2026-10-02 16:16 ` Andrey Drobyshev
2026-09-30 16:49 ` [PATCH 06/13] iotests/298: add testcase for async writes with preallocation filter Denis V. Lunev
` (7 subsequent siblings)
12 siblings, 1 reply; 26+ messages in thread
From: Denis V. Lunev @ 2026-09-30 16:49 UTC (permalink / raw)
To: qemu-devel
Cc: den, qemu-block, qemu-stable, Kevin Wolf, Hanna Reitz,
Vladimir Sementsov-Ogievskiy, Andrey Drobyshev
From: Denis V. Lunev <den@openvz.org>
The previous patch holds the mutex across all of handle_write(),
including the preallocating write zeroes. That is correct and far too
coarse: the fallocate takes as long as it takes, and every writer that
extends the file waits for it.
Reserve the range instead of holding the lock over it. file_end moves
to the end of the preallocated range before the request is issued, so
a writer arriving while it is in flight works from a length that
already covers the range being zeroed.
That leaves it going straight into the range, and BDRV_REQ_SERIALISING
cannot stop it. Serialisation orders a request against the ones in
bs->tracked_requests, and the zeroing request is not there yet: the
filter runs above bdrv_co_pwritev_part(), which is what tracks it.
Whatever the filter lets through in that window is unordered by
construction, so the filter has to hold it back itself.
safe_end trails file_end, standing at the lowest start of a
preallocation in flight and reaching file_end once none is left. A
write above it waits on a queue and re-reads the state when the
preallocations drain. A write zeroes merged into a preallocation
starts it below file_end, so safe_end has to move down as well as up.
Below safe_end nothing can be zeroed any more, so those writes do not
queue, and below zero_start there is nothing to record either, so
those take no mutex at all. The stores are qatomic_set() because that
path reads them without it.
Signed-off-by: Denis V. Lunev <den@openvz.org>
Cc: qemu-stable@nongnu.org
CC: Kevin Wolf <kwolf@redhat.com>
CC: Hanna Reitz <hreitz@redhat.com>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
block/preallocate.c | 95 ++++++++++++++++++++++++++++++++++++---------
1 file changed, 76 insertions(+), 19 deletions(-)
diff --git a/block/preallocate.c b/block/preallocate.c
index 442cdb0176..128f1da014 100644
--- a/block/preallocate.c
+++ b/block/preallocate.c
@@ -32,6 +32,7 @@
#include "qemu/units.h"
#include "block/block-io.h"
#include "block/block_int.h"
+#include "qemu/atomic.h"
typedef struct PreallocateOpts {
@@ -76,6 +77,21 @@ typedef struct BDRVPreallocateState {
* BLK_PERM_WRITE permissions on file child.
*/
+ /*
+ * @file_end is reserved as soon as a preallocation is decided on, so
+ * that the next one starts above it. @safe_end is the lowest start of
+ * a preallocation in flight, or @file_end when there is none: nothing
+ * below it can still be zeroed, so a write ending below it does not
+ * queue.
+ */
+ int64_t safe_end;
+ int prealloc_in_flight;
+ CoQueue prealloc_waiters;
+
+ /*
+ * Nothing done while this is held may wait for a request that has already
+ * passed the filter: that request can need it back.
+ */
CoMutex lock;
/* Gives up the resize permission on children when parents don't need it */
@@ -106,9 +122,17 @@ static QemuOptsList runtime_opts = {
},
};
+static void preallocate_set_file_end(BDRVPreallocateState *s, int64_t v)
+{
+ s->file_end = v;
+ qatomic_set(&s->safe_end, v);
+}
+
static void preallocate_set_state(BDRVPreallocateState *s, int64_t v)
{
- s->file_end = s->zero_start = s->data_end = v;
+ preallocate_set_file_end(s, v);
+ qatomic_set(&s->data_end, v);
+ qatomic_set(&s->zero_start, v);
}
static bool preallocate_absorb_opts(PreallocateOpts *dest, QDict *options,
@@ -158,6 +182,7 @@ static int preallocate_open(BlockDriverState *bs, QDict *options, int flags,
*/
preallocate_set_state(s, -EINVAL);
qemu_co_mutex_init(&s->lock);
+ qemu_co_queue_init(&s->prealloc_waiters);
s->drop_resize_bh = qemu_bh_new(preallocate_drop_resize_bh, bs);
ret = bdrv_open_file_child(NULL, options, "file", bs, errp);
@@ -188,7 +213,7 @@ preallocate_truncate_to_real_size(BlockDriverState *bs, Error **errp)
int ret;
if (s->file_end < 0) {
- s->file_end = bdrv_getlength(bs->file->bs);
+ preallocate_set_file_end(s, bdrv_getlength(bs->file->bs));
if (s->file_end < 0) {
error_setg_errno(errp, -s->file_end, "Failed to get file length");
return s->file_end;
@@ -200,10 +225,10 @@ preallocate_truncate_to_real_size(BlockDriverState *bs, Error **errp)
NULL);
if (ret < 0) {
error_setg_errno(errp, -ret, "Failed to drop preallocation");
- s->file_end = ret;
+ preallocate_set_file_end(s, ret);
return ret;
}
- s->file_end = s->data_end;
+ preallocate_set_file_end(s, s->data_end);
}
return 0;
@@ -345,18 +370,21 @@ handle_write_locked(BlockDriverState *bs, int64_t offset, int64_t bytes,
}
if (s->data_end < 0) {
- s->data_end = bdrv_co_getlength(bs->file->bs);
- if (s->data_end < 0) {
+ int64_t len = bdrv_co_getlength(bs->file->bs);
+
+ if (len < 0) {
+ s->data_end = len;
return false;
}
+ qatomic_set(&s->data_end, len);
if (s->file_end < 0) {
- s->file_end = s->data_end;
+ preallocate_set_file_end(s, len);
}
}
if (!want_merge_zero && s->zero_start >= 0 && end > s->zero_start) {
- s->zero_start = end;
+ qatomic_set(&s->zero_start, end);
}
if (end <= s->data_end) {
@@ -365,13 +393,13 @@ handle_write_locked(BlockDriverState *bs, int64_t offset, int64_t bytes,
/* We have valid s->data_end, and request writes beyond it. */
- s->data_end = end;
+ qatomic_set(&s->data_end, end);
if (s->zero_start < 0 || !want_merge_zero) {
- s->zero_start = end;
+ qatomic_set(&s->zero_start, end);
}
if (s->file_end < 0) {
- s->file_end = bdrv_co_getlength(bs->file->bs);
+ preallocate_set_file_end(s, bdrv_co_getlength(bs->file->bs));
if (s->file_end < 0) {
return false;
}
@@ -395,16 +423,27 @@ handle_write_locked(BlockDriverState *bs, int64_t offset, int64_t bytes,
want_merge_zero = want_merge_zero && (prealloc_start <= offset);
+ /* Reserve the range; safe_end keeps writers out of it until it is zero. */
+ s->file_end = prealloc_end;
+ if (s->prealloc_in_flight++ == 0 || prealloc_start < s->safe_end) {
+ qatomic_set(&s->safe_end, prealloc_start);
+ }
+ qemu_co_mutex_unlock(&s->lock);
+
ret = bdrv_co_pwrite_zeroes(
bs->file, prealloc_start, prealloc_end - prealloc_start,
BDRV_REQ_NO_FALLBACK | BDRV_REQ_SERIALISING | BDRV_REQ_NO_WAIT);
+
+ qemu_co_mutex_lock(&s->lock);
if (ret < 0) {
s->file_end = ret;
- return false;
}
+ if (--s->prealloc_in_flight == 0) {
+ qatomic_set(&s->safe_end, s->file_end);
+ }
+ qemu_co_queue_restart_all(&s->prealloc_waiters);
- s->file_end = prealloc_end;
- return want_merge_zero;
+ return ret < 0 ? false : want_merge_zero;
}
static bool coroutine_fn GRAPH_RDLOCK
@@ -412,9 +451,22 @@ handle_write(BlockDriverState *bs, int64_t offset, int64_t bytes,
bool want_merge_zero)
{
BDRVPreallocateState *s = bs->opaque;
+ int64_t zero_start = qatomic_read(&s->zero_start);
+ int64_t end = offset + bytes;
bool ret;
+ /*
+ * Below zero_start there is nothing to record: data_end covers it, and
+ * only a write zeroes that already covers it can preallocate over it.
+ */
+ if (zero_start >= 0 && end <= zero_start) {
+ return false;
+ }
+
qemu_co_mutex_lock(&s->lock);
+ while (s->prealloc_in_flight && (s->safe_end < 0 || end > s->safe_end)) {
+ qemu_co_queue_wait(&s->prealloc_waiters, &s->lock);
+ }
ret = handle_write_locked(bs, offset, bytes, want_merge_zero);
qemu_co_mutex_unlock(&s->lock);
@@ -456,7 +508,7 @@ preallocate_co_truncate(BlockDriverState *bs, int64_t offset,
if (s->data_end >= 0 && offset > s->data_end) {
if (s->file_end < 0) {
- s->file_end = bdrv_co_getlength(bs->file->bs);
+ preallocate_set_file_end(s, bdrv_co_getlength(bs->file->bs));
if (s->file_end < 0) {
error_setg(errp, "failed to get file length");
return s->file_end;
@@ -471,7 +523,7 @@ preallocate_co_truncate(BlockDriverState *bs, int64_t offset,
* Otherwise just proceed to preallocate missing part.
*/
if (offset <= s->file_end) {
- s->data_end = offset;
+ qatomic_set(&s->data_end, offset);
return 0;
}
} else {
@@ -488,16 +540,16 @@ preallocate_co_truncate(BlockDriverState *bs, int64_t offset,
ret = bdrv_co_truncate(bs->file, s->data_end, true,
PREALLOC_MODE_OFF, 0, errp);
if (ret < 0) {
- s->file_end = ret;
+ preallocate_set_file_end(s, ret);
error_prepend(errp, "preallocate-filter: failed to drop "
"write-zero preallocation: ");
return ret;
}
- s->file_end = s->data_end;
+ preallocate_set_file_end(s, s->data_end);
}
}
- s->data_end = offset;
+ qatomic_set(&s->data_end, offset);
}
ret = bdrv_co_truncate(bs->file, offset, exact, prealloc, flags, errp);
@@ -522,6 +574,11 @@ preallocate_co_getlength(BlockDriverState *bs)
{
int64_t ret;
BDRVPreallocateState *s = bs->opaque;
+ int64_t data_end = qatomic_read(&s->data_end);
+
+ if (data_end >= 0) {
+ return data_end;
+ }
qemu_co_mutex_lock(&s->lock);
if (s->data_end >= 0) {
--
2.53.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 05/13] block/preallocate: hold back writes into a preallocation in flight
2026-09-30 16:49 ` [PATCH 05/13] block/preallocate: hold back writes into a preallocation in flight Denis V. Lunev
@ 2026-10-02 16:16 ` Andrey Drobyshev
0 siblings, 0 replies; 26+ messages in thread
From: Andrey Drobyshev @ 2026-10-02 16:16 UTC (permalink / raw)
To: Denis V. Lunev
Cc: qemu-devel, qemu-block, qemu-stable, Kevin Wolf, Hanna Reitz,
Vladimir Sementsov-Ogievskiy, Andrey Drobyshev
> The previous patch holds the mutex across all of handle_write(),
> including the preallocating write zeroes. That is correct and far too
> coarse: the fallocate takes as long as it takes, and every writer that
> extends the file waits for it.
>
> Reserve the range instead of holding the lock over it. file_end moves
> to the end of the preallocated range before the request is issued, so
> a writer arriving while it is in flight works from a length that
> already covers the range being zeroed.
>
> That leaves it going straight into the range, and BDRV_REQ_SERIALISING
> cannot stop it. Serialisation orders a request against the ones in
> bs->tracked_requests, and the zeroing request is not there yet: the
> filter runs above bdrv_co_pwritev_part(), which is what tracks it.
> Whatever the filter lets through in that window is unordered by
> construction, so the filter has to hold it back itself.
>
> safe_end trails file_end, standing at the lowest start of a
> preallocation in flight and reaching file_end once none is left. A
> write above it waits on a queue and re-reads the state when the
> preallocations drain. A write zeroes merged into a preallocation
> starts it below file_end, so safe_end has to move down as well as up.
>
> Below safe_end nothing can be zeroed any more, so those writes do not
> queue, and below zero_start there is nothing to record either, so
> those take no mutex at all. The stores are qatomic_set() because that
> path reads them without it.
>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> Cc: qemu-stable@nongnu.org
> CC: Kevin Wolf <kwolf@redhat.com>
> CC: Hanna Reitz <hreitz@redhat.com>
> CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
>
> diff --git a/block/preallocate.c b/block/preallocate.c
> index 442cdb01768..128f1da0145 100644
> --- a/block/preallocate.c
> +++ b/block/preallocate.c
> @@ -32,6 +32,7 @@
> #include "qemu/units.h"
> #include "block/block-io.h"
> #include "block/block_int.h"
> +#include "qemu/atomic.h"
>
>
> typedef struct PreallocateOpts {
> @@ -76,6 +77,21 @@ typedef struct BDRVPreallocateState {
> * BLK_PERM_WRITE permissions on file child.
> */
>
> + /*
> + * @file_end is reserved as soon as a preallocation is decided on, so
> + * that the next one starts above it. @safe_end is the lowest start of
> + * a preallocation in flight, or @file_end when there is none: nothing
> + * below it can still be zeroed, so a write ending below it does not
> + * queue.
> + */
> + int64_t safe_end;
> + int prealloc_in_flight;
> + CoQueue prealloc_waiters;
> +
> + /*
> + * Nothing done while this is held may wait for a request that has already
> + * passed the filter: that request can need it back.
> + */
> CoMutex lock;
>
> /* Gives up the resize permission on children when parents don't need it */
> @@ -106,9 +122,17 @@ static QemuOptsList runtime_opts = {
> },
> };
>
> +static void preallocate_set_file_end(BDRVPreallocateState *s, int64_t v)
> +{
> + s->file_end = v;
> + qatomic_set(&s->safe_end, v);
> +}
> +
> static void preallocate_set_state(BDRVPreallocateState *s, int64_t v)
> {
> - s->file_end = s->zero_start = s->data_end = v;
> + preallocate_set_file_end(s, v);
> + qatomic_set(&s->data_end, v);
> + qatomic_set(&s->zero_start, v);
> }
>
> static bool preallocate_absorb_opts(PreallocateOpts *dest, QDict *options,
> @@ -158,6 +182,7 @@ static int preallocate_open(BlockDriverState *bs, QDict *options, int flags,
> */
> preallocate_set_state(s, -EINVAL);
> qemu_co_mutex_init(&s->lock);
> + qemu_co_queue_init(&s->prealloc_waiters);
> s->drop_resize_bh = qemu_bh_new(preallocate_drop_resize_bh, bs);
>
> ret = bdrv_open_file_child(NULL, options, "file", bs, errp);
> @@ -188,7 +213,7 @@ preallocate_truncate_to_real_size(BlockDriverState *bs, Error **errp)
> int ret;
>
> if (s->file_end < 0) {
> - s->file_end = bdrv_getlength(bs->file->bs);
> + preallocate_set_file_end(s, bdrv_getlength(bs->file->bs));
> if (s->file_end < 0) {
> error_setg_errno(errp, -s->file_end, "Failed to get file length");
> return s->file_end;
> @@ -200,10 +225,10 @@ preallocate_truncate_to_real_size(BlockDriverState *bs, Error **errp)
> NULL);
> if (ret < 0) {
> error_setg_errno(errp, -ret, "Failed to drop preallocation");
> - s->file_end = ret;
> + preallocate_set_file_end(s, ret);
> return ret;
> }
> - s->file_end = s->data_end;
> + preallocate_set_file_end(s, s->data_end);
> }
>
> return 0;
> @@ -345,18 +370,21 @@ handle_write_locked(BlockDriverState *bs, int64_t offset, int64_t bytes,
> }
>
> if (s->data_end < 0) {
> - s->data_end = bdrv_co_getlength(bs->file->bs);
> - if (s->data_end < 0) {
> + int64_t len = bdrv_co_getlength(bs->file->bs);
> +
> + if (len < 0) {
> + s->data_end = len;
> return false;
> }
> + qatomic_set(&s->data_end, len);
>
> if (s->file_end < 0) {
> - s->file_end = s->data_end;
> + preallocate_set_file_end(s, len);
> }
> }
>
> if (!want_merge_zero && s->zero_start >= 0 && end > s->zero_start) {
> - s->zero_start = end;
> + qatomic_set(&s->zero_start, end);
> }
>
> if (end <= s->data_end) {
> @@ -365,13 +393,13 @@ handle_write_locked(BlockDriverState *bs, int64_t offset, int64_t bytes,
>
> /* We have valid s->data_end, and request writes beyond it. */
>
> - s->data_end = end;
> + qatomic_set(&s->data_end, end);
> if (s->zero_start < 0 || !want_merge_zero) {
> - s->zero_start = end;
> + qatomic_set(&s->zero_start, end);
> }
>
> if (s->file_end < 0) {
> - s->file_end = bdrv_co_getlength(bs->file->bs);
> + preallocate_set_file_end(s, bdrv_co_getlength(bs->file->bs));
> if (s->file_end < 0) {
> return false;
> }
> @@ -395,16 +423,27 @@ handle_write_locked(BlockDriverState *bs, int64_t offset, int64_t bytes,
>
> want_merge_zero = want_merge_zero && (prealloc_start <= offset);
>
> + /* Reserve the range; safe_end keeps writers out of it until it is zero. */
> + s->file_end = prealloc_end;
> + if (s->prealloc_in_flight++ == 0 || prealloc_start < s->safe_end) {
> + qatomic_set(&s->safe_end, prealloc_start);
> + }
> + qemu_co_mutex_unlock(&s->lock);
> +
> ret = bdrv_co_pwrite_zeroes(
> bs->file, prealloc_start, prealloc_end - prealloc_start,
> BDRV_REQ_NO_FALLBACK | BDRV_REQ_SERIALISING | BDRV_REQ_NO_WAIT);
> +
> + qemu_co_mutex_lock(&s->lock);
> if (ret < 0) {
> s->file_end = ret;
> - return false;
> }
> + if (--s->prealloc_in_flight == 0) {
> + qatomic_set(&s->safe_end, s->file_end);
> + }
> + qemu_co_queue_restart_all(&s->prealloc_waiters);
>
> - s->file_end = prealloc_end;
> - return want_merge_zero;
> + return ret < 0 ? false : want_merge_zero;
> }
>
> static bool coroutine_fn GRAPH_RDLOCK
> @@ -412,9 +451,22 @@ handle_write(BlockDriverState *bs, int64_t offset, int64_t bytes,
> bool want_merge_zero)
> {
> BDRVPreallocateState *s = bs->opaque;
> + int64_t zero_start = qatomic_read(&s->zero_start);
> + int64_t end = offset + bytes;
> bool ret;
>
> + /*
> + * Below zero_start there is nothing to record: data_end covers it, and
> + * only a write zeroes that already covers it can preallocate over it.
> + */
> + if (zero_start >= 0 && end <= zero_start) {
> + return false;
> + }
> +
> qemu_co_mutex_lock(&s->lock);
> + while (s->prealloc_in_flight && (s->safe_end < 0 || end > s->safe_end)) {
> + qemu_co_queue_wait(&s->prealloc_waiters, &s->lock);
> + }
I feel this patch logic might be over-engineered. Let's see when we
ever get past the above waiting condition.
1. To start a zeroing a writer needs 'end > file_end' (otherwise "no
preallocation needed").
2. 'safe_end' is always either a start of the running zeroing
preallocation, or just 'file_end'. Thus condition 'safe_end <= file_end'
is always true.
Now the above wait loop. A writer can get past it in 2 cases:
* nothing is in flight -> it might start a preallocation zeroing, and
it's the only one;
* something is in flight but 'end <= safe_end'. But then based on 2)
'end <= file_end'. Then then based on 1) this writer cannot start
zeroing.
Please correct me if I'm wrong somewhere up until this point.
Now, if the above facts hold, it means that while 1st zeroing runs
nobody can ever start a 2nd. Thus 'prealloc_in_flight' only ever goes
between 0 and 1.
And if so, nothing prevents us from safely getting rid of the queue, the
in_flight counter and 'safe_end'. We can just do:
static bool handle_write(...)
{
...
QEMU_LOCK_GUARD(&s->lock);
return handle_write_locked(...);
}
What do you think?
Andrey
--
Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 06/13] iotests/298: add testcase for async writes with preallocation filter
2026-09-30 16:49 [PATCH 00/13] block: fix data loss in the preallocate filter Denis V. Lunev
` (4 preceding siblings ...)
2026-09-30 16:49 ` [PATCH 05/13] block/preallocate: hold back writes into a preallocation in flight Denis V. Lunev
@ 2026-09-30 16:49 ` Denis V. Lunev
2026-09-30 16:49 ` [PATCH 07/13] tests/unit: prefix the iothread test helper Denis V. Lunev
` (6 subsequent siblings)
12 siblings, 0 replies; 26+ messages in thread
From: Denis V. Lunev @ 2026-09-30 16:49 UTC (permalink / raw)
To: qemu-devel
Cc: den, qemu-block, Andrey Drobyshev, Kevin Wolf, Hanna Reitz,
Vladimir Sementsov-Ogievskiy
From: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
The testcase creates a 64G image with 1M clusters, generates a list of
1M aligned offsets and feeds aio_write commands with those offsets to
qemu-io run with '--aio native --nocache'. Then we check the data
written at each of the offsets. Before this series this could result
in a race within the preallocation filter which would zeroize some
clusters after actually writing data to them.
Note: the test does not fail every time as there's a race involved, but
the failures are pretty consistent so it should be good enough for
detecting the problem.
Signed-off-by: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Hanna Reitz <hreitz@redhat.com>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
---
tests/qemu-iotests/298 | 49 ++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/298.out | 4 ++--
2 files changed, 51 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/298 b/tests/qemu-iotests/298
index f2b94077f4..23e7789d09 100755
--- a/tests/qemu-iotests/298
+++ b/tests/qemu-iotests/298
@@ -20,8 +20,10 @@
import os
import iotests
+import random
MiB = 1024 * 1024
+GiB = MiB * 1024
disk = os.path.join(iotests.test_dir, 'disk')
overlay = os.path.join(iotests.test_dir, 'overlay')
refdisk = os.path.join(iotests.test_dir, 'refdisk')
@@ -176,6 +178,53 @@ class TestTruncate(iotests.QMPTestCase):
self.do_test('off', '150M')
+class TestPreallocAsyncWrites(iotests.QMPTestCase):
+ def setUp(self):
+ # Make sure we get reproducible write patterns on each run
+ random.seed(42)
+ iotests.qemu_img_create('-f', iotests.imgfmt, disk, '-o',
+ f'cluster_size={MiB},lazy_refcounts=on',
+ str(64 * GiB))
+
+ def tearDown(self):
+ os.remove(disk)
+
+ def test_prealloc_async_writes(self):
+ def gen_write_pattern():
+ n = 0
+ while True:
+ yield '-P 0xaa' if n else '-z'
+ n = 1 - n
+
+ def gen_read_pattern():
+ n = 0
+ while True:
+ yield '-P 0xaa' if n else '-P 0x00'
+ n = 1 - n
+
+ requests = 2048 # Number of write/read requests to feed to qemu-io
+ total_clusters = 64 * 1024 # 64G / 1M
+
+ wpgen = gen_write_pattern()
+ rpgen = gen_read_pattern()
+
+ offsets = random.sample(range(0, total_clusters), requests)
+ aio_write_cmds = [f'aio_write {next(wpgen)} {off}M 1M' for off in offsets]
+ read_cmds = [f'read {next(rpgen)} {off}M 1M' for off in offsets]
+
+ proc = iotests.QemuIoInteractive('--aio', 'native', '--nocache',
+ '--image-opts', drive_opts)
+ for cmd in aio_write_cmds:
+ proc.cmd(cmd)
+ proc.close()
+
+ proc = iotests.QemuIoInteractive('-f', iotests.imgfmt, disk)
+ for cmd in read_cmds:
+ out = proc.cmd(cmd)
+ self.assertFalse('Pattern verification failed' in str(out))
+ proc.close()
+
+
if __name__ == '__main__':
iotests.main(supported_fmts=['qcow2'], required_fmts=['preallocate'],
require_hmp=True)
diff --git a/tests/qemu-iotests/298.out b/tests/qemu-iotests/298.out
index fa16b5ccef..6323079e08 100644
--- a/tests/qemu-iotests/298.out
+++ b/tests/qemu-iotests/298.out
@@ -1,5 +1,5 @@
-.............
+..............
----------------------------------------------------------------------
-Ran 13 tests
+Ran 14 tests
OK
--
2.53.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH 07/13] tests/unit: prefix the iothread test helper
2026-09-30 16:49 [PATCH 00/13] block: fix data loss in the preallocate filter Denis V. Lunev
` (5 preceding siblings ...)
2026-09-30 16:49 ` [PATCH 06/13] iotests/298: add testcase for async writes with preallocation filter Denis V. Lunev
@ 2026-09-30 16:49 ` Denis V. Lunev
2026-10-05 20:06 ` Eric Blake
2026-09-30 16:49 ` [PATCH 08/13] qemu-io: accept --object iothread Denis V. Lunev
` (5 subsequent siblings)
12 siblings, 1 reply; 26+ messages in thread
From: Denis V. Lunev @ 2026-09-30 16:49 UTC (permalink / raw)
To: qemu-devel
Cc: den, qemu-block, Kevin Wolf, Hanna Reitz,
Vladimir Sementsov-Ogievskiy, Andrey Drobyshev
From: Denis V. Lunev <den@openvz.org>
tests/unit/iothread.c defines a struct IOThread and an
iothread_get_aio_context() under the same names as the IOThread QOM
object in iothread.c, so nothing that links the real object can use
the helper. Prefix the helper and its type with test_ and Test.
This is preparation for building the QOM object into the block layer.
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Hanna Reitz <hreitz@redhat.com>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
tests/unit/iothread.c | 28 +++++++++++++--------------
tests/unit/iothread.h | 8 ++++----
tests/unit/test-aio-multithread.c | 8 ++++----
tests/unit/test-bdrv-drain.c | 32 +++++++++++++++----------------
tests/unit/test-block-iothread.c | 28 +++++++++++++--------------
5 files changed, 52 insertions(+), 52 deletions(-)
diff --git a/tests/unit/iothread.c b/tests/unit/iothread.c
index a363bf8f70..ff05ae8b11 100644
--- a/tests/unit/iothread.c
+++ b/tests/unit/iothread.c
@@ -19,7 +19,7 @@
#include "qemu/rcu.h"
#include "iothread.h"
-struct IOThread {
+struct TestIOThread {
AioContext *ctx;
GMainContext *worker_context;
GMainLoop *main_loop;
@@ -30,20 +30,20 @@ struct IOThread {
bool stopping;
};
-static void iothread_init_gcontext(IOThread *iothread)
+static void test_iothread_init_gcontext(TestIOThread *iothread)
{
GSource *source;
iothread->worker_context = g_main_context_new();
- source = aio_get_g_source(iothread_get_aio_context(iothread));
+ source = aio_get_g_source(test_iothread_get_aio_context(iothread));
g_source_attach(source, iothread->worker_context);
g_source_unref(source);
iothread->main_loop = g_main_loop_new(iothread->worker_context, TRUE);
}
-static void *iothread_run(void *opaque)
+static void *test_iothread_run(void *opaque)
{
- IOThread *iothread = opaque;
+ TestIOThread *iothread = opaque;
rcu_register_thread();
@@ -56,7 +56,7 @@ static void *iothread_run(void *opaque)
* of glib the g_source_ref()/unref() functions are not threadsafe
* on sources without a context.
*/
- iothread_init_gcontext(iothread);
+ test_iothread_init_gcontext(iothread);
/*
* g_main_context_push_thread_default() must be called before anything
@@ -76,16 +76,16 @@ static void *iothread_run(void *opaque)
return NULL;
}
-static void iothread_stop_bh(void *opaque)
+static void test_iothread_stop_bh(void *opaque)
{
- IOThread *iothread = opaque;
+ TestIOThread *iothread = opaque;
iothread->stopping = true;
}
-void iothread_join(IOThread *iothread)
+void test_iothread_join(TestIOThread *iothread)
{
- aio_bh_schedule_oneshot(iothread->ctx, iothread_stop_bh, iothread);
+ aio_bh_schedule_oneshot(iothread->ctx, test_iothread_stop_bh, iothread);
qemu_thread_join(&iothread->thread);
g_main_context_unref(iothread->worker_context);
g_main_loop_unref(iothread->main_loop);
@@ -95,13 +95,13 @@ void iothread_join(IOThread *iothread)
g_free(iothread);
}
-IOThread *iothread_new(void)
+TestIOThread *test_iothread_new(void)
{
- IOThread *iothread = g_new0(IOThread, 1);
+ TestIOThread *iothread = g_new0(TestIOThread, 1);
qemu_mutex_init(&iothread->init_done_lock);
qemu_cond_init(&iothread->init_done_cond);
- qemu_thread_create(&iothread->thread, NULL, iothread_run,
+ qemu_thread_create(&iothread->thread, NULL, test_iothread_run,
iothread, QEMU_THREAD_JOINABLE);
/* Wait for initialization to complete */
@@ -114,7 +114,7 @@ IOThread *iothread_new(void)
return iothread;
}
-AioContext *iothread_get_aio_context(IOThread *iothread)
+AioContext *test_iothread_get_aio_context(TestIOThread *iothread)
{
return iothread->ctx;
}
diff --git a/tests/unit/iothread.h b/tests/unit/iothread.h
index eb4d0c77f8..8475170eaa 100644
--- a/tests/unit/iothread.h
+++ b/tests/unit/iothread.h
@@ -16,10 +16,10 @@
#include "qemu/aio.h"
#include "qemu/thread.h"
-typedef struct IOThread IOThread;
+typedef struct TestIOThread TestIOThread;
-IOThread *iothread_new(void);
-void iothread_join(IOThread *iothread);
-AioContext *iothread_get_aio_context(IOThread *iothread);
+TestIOThread *test_iothread_new(void);
+void test_iothread_join(TestIOThread *iothread);
+AioContext *test_iothread_get_aio_context(TestIOThread *iothread);
#endif
diff --git a/tests/unit/test-aio-multithread.c b/tests/unit/test-aio-multithread.c
index 9179cdc6a3..d07376455b 100644
--- a/tests/unit/test-aio-multithread.c
+++ b/tests/unit/test-aio-multithread.c
@@ -21,7 +21,7 @@
#define NUM_CONTEXTS 5
-static IOThread *threads[NUM_CONTEXTS];
+static TestIOThread *threads[NUM_CONTEXTS];
static AioContext *ctx[NUM_CONTEXTS];
static __thread int id = -1;
@@ -68,8 +68,8 @@ static void create_aio_contexts(void)
int i;
for (i = 0; i < NUM_CONTEXTS; i++) {
- threads[i] = iothread_new();
- ctx[i] = iothread_get_aio_context(threads[i]);
+ threads[i] = test_iothread_new();
+ ctx[i] = test_iothread_get_aio_context(threads[i]);
}
qemu_event_init(&done_event, false);
@@ -88,7 +88,7 @@ static void join_aio_contexts(void)
aio_context_ref(ctx[i]);
}
for (i = 0; i < NUM_CONTEXTS; i++) {
- iothread_join(threads[i]);
+ test_iothread_join(threads[i]);
}
for (i = 0; i < NUM_CONTEXTS; i++) {
aio_context_unref(ctx[i]);
diff --git a/tests/unit/test-bdrv-drain.c b/tests/unit/test-bdrv-drain.c
index 43b0ba8648..a852874f74 100644
--- a/tests/unit/test-bdrv-drain.c
+++ b/tests/unit/test-bdrv-drain.c
@@ -535,10 +535,10 @@ static void test_iothread_common(enum drain_type drain_type, int drain_thread)
int aio_ret;
struct test_iothread_data data;
- IOThread *a = iothread_new();
- IOThread *b = iothread_new();
- AioContext *ctx_a = iothread_get_aio_context(a);
- AioContext *ctx_b = iothread_get_aio_context(b);
+ TestIOThread *a = test_iothread_new();
+ TestIOThread *b = test_iothread_new();
+ AioContext *ctx_a = test_iothread_get_aio_context(a);
+ AioContext *ctx_b = test_iothread_get_aio_context(b);
QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, NULL, 0);
@@ -614,8 +614,8 @@ static void test_iothread_common(enum drain_type drain_type, int drain_thread)
blk_unref(blk);
out:
- iothread_join(a);
- iothread_join(b);
+ test_iothread_join(a);
+ test_iothread_join(b);
}
static void test_iothread_drain_all(void)
@@ -727,7 +727,7 @@ static void test_blockjob_common_drain_node(enum drain_type drain_type,
BlockDriverState *src, *src_backing, *src_overlay, *target, *drain_bs;
BlockJob *job;
TestBlockJob *tjob;
- IOThread *iothread = NULL;
+ TestIOThread *iothread = NULL;
int ret = -1;
src = bdrv_new_open_driver(&bdrv_test, "source", BDRV_O_RDWR,
@@ -761,8 +761,8 @@ static void test_blockjob_common_drain_node(enum drain_type drain_type,
if (use_iothread) {
AioContext *ctx;
- iothread = iothread_new();
- ctx = iothread_get_aio_context(iothread);
+ iothread = test_iothread_new();
+ ctx = test_iothread_get_aio_context(iothread);
blk_set_aio_context(blk_src, ctx, &error_abort);
}
@@ -893,7 +893,7 @@ static void test_blockjob_common_drain_node(enum drain_type drain_type,
bdrv_unref(target);
if (iothread) {
- iothread_join(iothread);
+ test_iothread_join(iothread);
}
}
@@ -1396,10 +1396,10 @@ static void test_append_to_drained(void)
static void test_set_aio_context(void)
{
BlockDriverState *bs;
- IOThread *a = iothread_new();
- IOThread *b = iothread_new();
- AioContext *ctx_a = iothread_get_aio_context(a);
- AioContext *ctx_b = iothread_get_aio_context(b);
+ TestIOThread *a = test_iothread_new();
+ TestIOThread *b = test_iothread_new();
+ AioContext *ctx_a = test_iothread_get_aio_context(a);
+ AioContext *ctx_b = test_iothread_get_aio_context(b);
bs = bdrv_new_open_driver(&bdrv_test, "test-node", BDRV_O_RDWR,
&error_abort);
@@ -1410,8 +1410,8 @@ static void test_set_aio_context(void)
bdrv_try_change_aio_context(bs, qemu_get_aio_context(), NULL, &error_abort);
bdrv_unref(bs);
- iothread_join(a);
- iothread_join(b);
+ test_iothread_join(a);
+ test_iothread_join(b);
}
diff --git a/tests/unit/test-block-iothread.c b/tests/unit/test-block-iothread.c
index 5273ff235a..5a150adfe8 100644
--- a/tests/unit/test-block-iothread.c
+++ b/tests/unit/test-block-iothread.c
@@ -465,8 +465,8 @@ const SyncOpTest sync_op_tests[] = {
static void test_sync_op(const void *opaque)
{
const SyncOpTest *t = opaque;
- IOThread *iothread = iothread_new();
- AioContext *ctx = iothread_get_aio_context(iothread);
+ TestIOThread *iothread = test_iothread_new();
+ AioContext *ctx = test_iothread_get_aio_context(iothread);
BlockBackend *blk;
BlockDriverState *bs;
BdrvChild *c;
@@ -548,8 +548,8 @@ BlockJobDriver test_job_driver = {
static void test_attach_blockjob(void)
{
- IOThread *iothread = iothread_new();
- AioContext *ctx = iothread_get_aio_context(iothread);
+ TestIOThread *iothread = test_iothread_new();
+ AioContext *ctx = test_iothread_get_aio_context(iothread);
BlockBackend *blk;
BlockDriverState *bs;
TestBlockJob *tjob;
@@ -611,8 +611,8 @@ static void test_attach_blockjob(void)
*/
static void test_propagate_basic(void)
{
- IOThread *iothread = iothread_new();
- AioContext *ctx = iothread_get_aio_context(iothread);
+ TestIOThread *iothread = test_iothread_new();
+ AioContext *ctx = test_iothread_get_aio_context(iothread);
AioContext *main_ctx;
BlockBackend *blk;
BlockDriverState *bs_a, *bs_b, *bs_verify;
@@ -675,8 +675,8 @@ static void test_propagate_basic(void)
*/
static void test_propagate_diamond(void)
{
- IOThread *iothread = iothread_new();
- AioContext *ctx = iothread_get_aio_context(iothread);
+ TestIOThread *iothread = test_iothread_new();
+ AioContext *ctx = test_iothread_get_aio_context(iothread);
AioContext *main_ctx;
BlockBackend *blk;
BlockDriverState *bs_a, *bs_b, *bs_c, *bs_verify;
@@ -740,8 +740,8 @@ static void test_propagate_diamond(void)
static void test_propagate_mirror(void)
{
- IOThread *iothread = iothread_new();
- AioContext *ctx = iothread_get_aio_context(iothread);
+ TestIOThread *iothread = test_iothread_new();
+ AioContext *ctx = test_iothread_get_aio_context(iothread);
AioContext *main_ctx = qemu_get_aio_context();
BlockDriverState *src, *target, *filter;
BlockBackend *blk;
@@ -811,8 +811,8 @@ static void test_propagate_mirror(void)
static void test_attach_second_node(void)
{
- IOThread *iothread = iothread_new();
- AioContext *ctx = iothread_get_aio_context(iothread);
+ TestIOThread *iothread = test_iothread_new();
+ AioContext *ctx = test_iothread_get_aio_context(iothread);
AioContext *main_ctx = qemu_get_aio_context();
BlockBackend *blk;
BlockDriverState *bs, *filter;
@@ -844,8 +844,8 @@ static void test_attach_second_node(void)
static void test_attach_preserve_blk_ctx(void)
{
- IOThread *iothread = iothread_new();
- AioContext *ctx = iothread_get_aio_context(iothread);
+ TestIOThread *iothread = test_iothread_new();
+ AioContext *ctx = test_iothread_get_aio_context(iothread);
BlockBackend *blk;
BlockDriverState *bs;
--
2.53.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 07/13] tests/unit: prefix the iothread test helper
2026-09-30 16:49 ` [PATCH 07/13] tests/unit: prefix the iothread test helper Denis V. Lunev
@ 2026-10-05 20:06 ` Eric Blake
0 siblings, 0 replies; 26+ messages in thread
From: Eric Blake @ 2026-10-05 20:06 UTC (permalink / raw)
To: Denis V. Lunev
Cc: qemu-devel, qemu-block, Kevin Wolf, Hanna Reitz,
Vladimir Sementsov-Ogievskiy, Andrey Drobyshev
On Wed, Sep 30, 2026 at 06:49:53PM +0200, Denis V. Lunev wrote:
> From: Denis V. Lunev <den@openvz.org>
>
> tests/unit/iothread.c defines a struct IOThread and an
> iothread_get_aio_context() under the same names as the IOThread QOM
> object in iothread.c, so nothing that links the real object can use
> the helper. Prefix the helper and its type with test_ and Test.
>
> This is preparation for building the QOM object into the block layer.
>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Kevin Wolf <kwolf@redhat.com>
> CC: Hanna Reitz <hreitz@redhat.com>
> CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
> ---
> tests/unit/iothread.c | 28 +++++++++++++--------------
> tests/unit/iothread.h | 8 ++++----
> tests/unit/test-aio-multithread.c | 8 ++++----
> tests/unit/test-bdrv-drain.c | 32 +++++++++++++++----------------
> tests/unit/test-block-iothread.c | 28 +++++++++++++--------------
> 5 files changed, 52 insertions(+), 52 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 08/13] qemu-io: accept --object iothread
2026-09-30 16:49 [PATCH 00/13] block: fix data loss in the preallocate filter Denis V. Lunev
` (6 preceding siblings ...)
2026-09-30 16:49 ` [PATCH 07/13] tests/unit: prefix the iothread test helper Denis V. Lunev
@ 2026-09-30 16:49 ` Denis V. Lunev
2026-10-05 20:11 ` Eric Blake
2026-09-30 16:49 ` [PATCH 09/13] qemu-io: add -t to run an aio request in a named iothread Denis V. Lunev
` (4 subsequent siblings)
12 siblings, 1 reply; 26+ messages in thread
From: Denis V. Lunev @ 2026-09-30 16:49 UTC (permalink / raw)
To: qemu-devel
Cc: den, qemu-block, Kevin Wolf, Hanna Reitz,
Vladimir Sementsov-Ogievskiy, Andrey Drobyshev
From: Denis V. Lunev <den@openvz.org>
qemu-io refused "--object iothread,id=t0" with "invalid object type:
iothread", because iothread.c was built into blockdev_ss, which qemu-io
does not link. Move it to block_ss: it wraps an AioContext and has
nothing to do with blockdev, and linking blockdev instead would drag in
os-posix.c, whose qemu_system_killed() every program linking it has to
define.
Collect the --object arguments during option parsing and process them
once qemu_init_main_loop() has run; nothing before that point uses an
object. An iothread created earlier registers the timer lists of its
AioContext with clocks that qemu_init_clocks() then resets, unlinking
them silently: no failure, just timers that never fire.
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Hanna Reitz <hreitz@redhat.com>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
meson.build | 2 +-
qemu-io.c | 9 ++++++++-
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/meson.build b/meson.build
index cfac634cf1..4c3f6518fd 100644
--- a/meson.build
+++ b/meson.build
@@ -3816,6 +3816,7 @@ if have_block
block_ss.add(files(
'block.c',
'blockjob.c',
+ 'iothread.c',
'job.c',
'qemu-io-cmds.c',
))
@@ -3830,7 +3831,6 @@ if have_block
blockdev_ss.add(files(
'blockdev.c',
'blockdev-nbd.c',
- 'iothread.c',
'job-qmp.c',
))
diff --git a/qemu-io.c b/qemu-io.c
index 598d5b1c9c..2b684ccf55 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -532,6 +532,7 @@ int main(int argc, char **argv)
QDict *opts = NULL;
const char *format = NULL;
bool force_share = false;
+ g_autoptr(GPtrArray) objects = g_ptr_array_new();
#ifdef CONFIG_POSIX
signal(SIGPIPE, SIG_IGN);
@@ -607,7 +608,7 @@ int main(int argc, char **argv)
force_share = true;
break;
case OPTION_OBJECT:
- user_creatable_process_cmdline(optarg);
+ g_ptr_array_add(objects, optarg);
break;
case OPTION_IMAGE_OPTS:
imageOpts = true;
@@ -630,6 +631,11 @@ int main(int argc, char **argv)
qemu_init_main_loop(&error_fatal);
+ /* An iothread object cannot be created before the main loop exists. */
+ for (guint i = 0; i < objects->len; i++) {
+ user_creatable_process_cmdline(g_ptr_array_index(objects, i));
+ }
+
if (!trace_init_backends()) {
exit(1);
}
@@ -686,6 +692,7 @@ int main(int argc, char **argv)
bdrv_drain_all();
blk_unref(qemuio_blk);
+ user_creatable_cleanup();
g_free(readline_state);
if (ret < 0) {
--
2.53.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 08/13] qemu-io: accept --object iothread
2026-09-30 16:49 ` [PATCH 08/13] qemu-io: accept --object iothread Denis V. Lunev
@ 2026-10-05 20:11 ` Eric Blake
0 siblings, 0 replies; 26+ messages in thread
From: Eric Blake @ 2026-10-05 20:11 UTC (permalink / raw)
To: Denis V. Lunev
Cc: qemu-devel, qemu-block, Kevin Wolf, Hanna Reitz,
Vladimir Sementsov-Ogievskiy, Andrey Drobyshev
On Wed, Sep 30, 2026 at 06:49:54PM +0200, Denis V. Lunev wrote:
> From: Denis V. Lunev <den@openvz.org>
>
> qemu-io refused "--object iothread,id=t0" with "invalid object type:
> iothread", because iothread.c was built into blockdev_ss, which qemu-io
> does not link. Move it to block_ss: it wraps an AioContext and has
> nothing to do with blockdev, and linking blockdev instead would drag in
> os-posix.c, whose qemu_system_killed() every program linking it has to
> define.
>
> Collect the --object arguments during option parsing and process them
> once qemu_init_main_loop() has run; nothing before that point uses an
> object. An iothread created earlier registers the timer lists of its
> AioContext with clocks that qemu_init_clocks() then resets, unlinking
> them silently: no failure, just timers that never fire.
>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Kevin Wolf <kwolf@redhat.com>
> CC: Hanna Reitz <hreitz@redhat.com>
> CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
> ---
> meson.build | 2 +-
> qemu-io.c | 9 ++++++++-
> 2 files changed, 9 insertions(+), 2 deletions(-)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 09/13] qemu-io: add -t to run an aio request in a named iothread
2026-09-30 16:49 [PATCH 00/13] block: fix data loss in the preallocate filter Denis V. Lunev
` (7 preceding siblings ...)
2026-09-30 16:49 ` [PATCH 08/13] qemu-io: accept --object iothread Denis V. Lunev
@ 2026-09-30 16:49 ` Denis V. Lunev
2026-10-05 20:28 ` Eric Blake
2026-09-30 16:49 ` [PATCH 10/13] qemu-io: add -g to write past the end of the image Denis V. Lunev
` (3 subsequent siblings)
12 siblings, 1 reply; 26+ messages in thread
From: Denis V. Lunev @ 2026-09-30 16:49 UTC (permalink / raw)
To: qemu-devel
Cc: den, qemu-block, Kevin Wolf, Hanna Reitz,
Vladimir Sementsov-Ogievskiy, Andrey Drobyshev
From: Denis V. Lunev <den@openvz.org>
A multiqueue virtio-blk drives one node from several AioContexts at
once, and nothing in the test tools could reproduce that shape: every
qemu-io request ran in the main loop. Filters and drivers that cache
state across a yield have therefore never been exercised the way
production runs them.
blk_aio_preadv() and friends enter the request coroutine in the
caller's context, so submitting from a bottom half scheduled on an
iothread is all it takes to put the request on that thread. The node
itself stays where it is, which is what virtio-blk does with the
queues it maps to iothreads other than the first.
aio_read and aio_write promise their caller that the request has been
submitted by the time they return, so wait for the bottom half to run
before returning. That also makes a test deterministic: entering a
coroutine from its own context runs it until it yields, so by the time
the command returns the request has reached, or blocked at, whatever
the test set up for it.
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Hanna Reitz <hreitz@redhat.com>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
qemu-io-cmds.c | 111 ++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 101 insertions(+), 10 deletions(-)
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index aa795fd87a..ce5bb53f4e 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -22,6 +22,8 @@
#include "qemu/timer.h"
#include "qemu/cutils.h"
#include "qemu/memalign.h"
+#include "system/iothread.h"
+#include "qemu/aio-wait.h"
#define CMD_NOFILE_OK 0x01
@@ -1367,8 +1369,84 @@ struct aio_ctx {
int pattern;
BdrvRequestFlags flags;
struct timespec t1;
+ AioContext *aio_context;
};
+enum aio_submit_type {
+ AIO_SUBMIT_READ,
+ AIO_SUBMIT_WRITE,
+ AIO_SUBMIT_WRITE_ZEROES,
+};
+
+struct aio_submit {
+ struct aio_ctx *ctx;
+ enum aio_submit_type type;
+ int64_t bytes;
+ BlockCompletionFunc *cb;
+ bool submitted;
+};
+
+static AioContext *aio_context_by_iothread_id(const char *id)
+{
+ IOThread *iothread = iothread_by_id(id);
+
+ if (!iothread) {
+ printf("iothread \"%s\" not found\n", id);
+ return NULL;
+ }
+
+ return iothread_get_aio_context(iothread);
+}
+
+static void aio_submit_bh(void *opaque)
+{
+ struct aio_submit *s = opaque;
+ struct aio_ctx *ctx = s->ctx;
+
+ switch (s->type) {
+ case AIO_SUBMIT_READ:
+ blk_aio_preadv(ctx->blk, ctx->offset, &ctx->qiov, ctx->flags,
+ s->cb, ctx);
+ break;
+ case AIO_SUBMIT_WRITE:
+ blk_aio_pwritev(ctx->blk, ctx->offset, &ctx->qiov, ctx->flags,
+ s->cb, ctx);
+ break;
+ case AIO_SUBMIT_WRITE_ZEROES:
+ blk_aio_pwrite_zeroes(ctx->blk, ctx->offset, s->bytes, ctx->flags,
+ s->cb, ctx);
+ break;
+ default:
+ g_assert_not_reached();
+ }
+
+ qatomic_set(&s->submitted, true);
+ aio_wait_kick();
+}
+
+/*
+ * blk_aio_*() runs the request in the caller's AioContext, so submitting from
+ * a bottom half is what puts it on another thread.
+ */
+static void aio_submit(struct aio_ctx *ctx, enum aio_submit_type type,
+ int64_t bytes, BlockCompletionFunc *cb)
+{
+ struct aio_submit s = {
+ .ctx = ctx,
+ .type = type,
+ .bytes = bytes,
+ .cb = cb,
+ };
+
+ if (!ctx->aio_context) {
+ aio_submit_bh(&s);
+ return;
+ }
+
+ aio_bh_schedule_oneshot(ctx->aio_context, aio_submit_bh, &s);
+ AIO_WAIT_WHILE_UNLOCKED(NULL, !qatomic_read(&s.submitted));
+}
+
static void aio_write_done(void *opaque, int ret)
{
struct aio_ctx *ctx = opaque;
@@ -1468,6 +1546,7 @@ static void aio_read_help(void)
" -P, -- use a pattern to verify read data\n"
" -q, -- quiet mode, do not show I/O statistics\n"
" -r, -- register I/O buffer\n"
+" -t, -- run the request in the named iothread\n"
" -v, -- dump buffer to standard output\n"
"\n");
}
@@ -1479,7 +1558,7 @@ static const cmdinfo_t aio_read_cmd = {
.cfunc = aio_read_f,
.argmin = 2,
.argmax = -1,
- .args = "[-Ciqrv] [-P pattern] off len [len..]",
+ .args = "[-Ciqrv] [-P pattern] [-t iothread] off len [len..]",
.oneline = "asynchronously reads a number of bytes",
.help = aio_read_help,
};
@@ -1490,7 +1569,7 @@ static int aio_read_f(BlockBackend *blk, int argc, char **argv, Error **errp)
struct aio_ctx *ctx = g_new0(struct aio_ctx, 1);
ctx->blk = blk;
- while ((c = getopt(argc, argv, "CiP:qrv")) != -1) {
+ while ((c = getopt(argc, argv, "CiP:qrt:v")) != -1) {
switch (c) {
case 'C':
ctx->Cflag = true;
@@ -1514,6 +1593,13 @@ static int aio_read_f(BlockBackend *blk, int argc, char **argv, Error **errp)
case 'r':
ctx->flags |= BDRV_REQ_REGISTERED_BUF;
break;
+ case 't':
+ ctx->aio_context = aio_context_by_iothread_id(optarg);
+ if (!ctx->aio_context) {
+ g_free(ctx);
+ return -EINVAL;
+ }
+ break;
case 'v':
ctx->vflag = true;
break;
@@ -1551,8 +1637,7 @@ static int aio_read_f(BlockBackend *blk, int argc, char **argv, Error **errp)
clock_gettime(CLOCK_MONOTONIC, &ctx->t1);
block_acct_start(blk_get_stats(blk), &ctx->acct, ctx->qiov.size,
BLOCK_ACCT_READ);
- blk_aio_preadv(blk, ctx->offset, &ctx->qiov, ctx->flags, aio_read_done,
- ctx);
+ aio_submit(ctx, AIO_SUBMIT_READ, 0, aio_read_done);
return 0;
}
@@ -1579,6 +1664,7 @@ static void aio_write_help(void)
" -P, -- use different pattern to fill file\n"
" -q, -- quiet mode, do not show I/O statistics\n"
" -r, -- register I/O buffer\n"
+" -t, -- run the request in the named iothread\n"
" -u, -- with -z, allow unmapping\n"
" -z, -- write zeroes using blk_aio_pwrite_zeroes\n"
"\n");
@@ -1592,7 +1678,7 @@ static const cmdinfo_t aio_write_cmd = {
.perm = BLK_PERM_WRITE,
.argmin = 2,
.argmax = -1,
- .args = "[-Cfiqruz] [-P pattern] off len [len..]",
+ .args = "[-Cfiqruz] [-P pattern] [-t iothread] off len [len..]",
.oneline = "asynchronously writes a number of bytes",
.help = aio_write_help,
};
@@ -1604,7 +1690,7 @@ static int aio_write_f(BlockBackend *blk, int argc, char **argv, Error **errp)
struct aio_ctx *ctx = g_new0(struct aio_ctx, 1);
ctx->blk = blk;
- while ((c = getopt(argc, argv, "CfiP:qruz")) != -1) {
+ while ((c = getopt(argc, argv, "CfiP:qrt:uz")) != -1) {
switch (c) {
case 'C':
ctx->Cflag = true;
@@ -1618,6 +1704,13 @@ static int aio_write_f(BlockBackend *blk, int argc, char **argv, Error **errp)
case 'r':
ctx->flags |= BDRV_REQ_REGISTERED_BUF;
break;
+ case 't':
+ ctx->aio_context = aio_context_by_iothread_id(optarg);
+ if (!ctx->aio_context) {
+ g_free(ctx);
+ return -EINVAL;
+ }
+ break;
case 'u':
ctx->flags |= BDRV_REQ_MAY_UNMAP;
break;
@@ -1692,8 +1785,7 @@ static int aio_write_f(BlockBackend *blk, int argc, char **argv, Error **errp)
}
ctx->qiov.size = count;
- blk_aio_pwrite_zeroes(blk, ctx->offset, count, ctx->flags,
- aio_write_done, ctx);
+ aio_submit(ctx, AIO_SUBMIT_WRITE_ZEROES, count, aio_write_done);
} else {
nr_iov = argc - optind;
ctx->buf = create_iovec(blk, &ctx->qiov, &argv[optind], nr_iov,
@@ -1709,8 +1801,7 @@ static int aio_write_f(BlockBackend *blk, int argc, char **argv, Error **errp)
block_acct_start(blk_get_stats(blk), &ctx->acct, ctx->qiov.size,
BLOCK_ACCT_WRITE);
- blk_aio_pwritev(blk, ctx->offset, &ctx->qiov, ctx->flags,
- aio_write_done, ctx);
+ aio_submit(ctx, AIO_SUBMIT_WRITE, 0, aio_write_done);
}
return 0;
--
2.53.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 09/13] qemu-io: add -t to run an aio request in a named iothread
2026-09-30 16:49 ` [PATCH 09/13] qemu-io: add -t to run an aio request in a named iothread Denis V. Lunev
@ 2026-10-05 20:28 ` Eric Blake
0 siblings, 0 replies; 26+ messages in thread
From: Eric Blake @ 2026-10-05 20:28 UTC (permalink / raw)
To: Denis V. Lunev
Cc: qemu-devel, qemu-block, Kevin Wolf, Hanna Reitz,
Vladimir Sementsov-Ogievskiy, Andrey Drobyshev
On Wed, Sep 30, 2026 at 06:49:55PM +0200, Denis V. Lunev wrote:
> From: Denis V. Lunev <den@openvz.org>
>
> A multiqueue virtio-blk drives one node from several AioContexts at
> once, and nothing in the test tools could reproduce that shape: every
> qemu-io request ran in the main loop. Filters and drivers that cache
> state across a yield have therefore never been exercised the way
> production runs them.
Useful addition!
>
> blk_aio_preadv() and friends enter the request coroutine in the
> caller's context, so submitting from a bottom half scheduled on an
> iothread is all it takes to put the request on that thread. The node
> itself stays where it is, which is what virtio-blk does with the
> queues it maps to iothreads other than the first.
>
> aio_read and aio_write promise their caller that the request has been
> submitted by the time they return, so wait for the bottom half to run
> before returning. That also makes a test deterministic: entering a
> coroutine from its own context runs it until it yields, so by the time
> the command returns the request has reached, or blocked at, whatever
> the test set up for it.
>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Kevin Wolf <kwolf@redhat.com>
> CC: Hanna Reitz <hreitz@redhat.com>
> CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
> ---
> qemu-io-cmds.c | 111 ++++++++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 101 insertions(+), 10 deletions(-)
>
> diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
> index aa795fd87a..ce5bb53f4e 100644
> --- a/qemu-io-cmds.c
> +++ b/qemu-io-cmds.c
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 10/13] qemu-io: add -g to write past the end of the image
2026-09-30 16:49 [PATCH 00/13] block: fix data loss in the preallocate filter Denis V. Lunev
` (8 preceding siblings ...)
2026-09-30 16:49 ` [PATCH 09/13] qemu-io: add -t to run an aio request in a named iothread Denis V. Lunev
@ 2026-09-30 16:49 ` Denis V. Lunev
2026-10-05 20:36 ` Eric Blake
2026-09-30 16:49 ` [PATCH 11/13] block/blkdebug: make a breakpoint work across AioContexts Denis V. Lunev
` (2 subsequent siblings)
12 siblings, 1 reply; 26+ messages in thread
From: Denis V. Lunev @ 2026-09-30 16:49 UTC (permalink / raw)
To: qemu-devel
Cc: den, qemu-block, Kevin Wolf, Hanna Reitz,
Vladimir Sementsov-Ogievskiy, Andrey Drobyshev
From: Denis V. Lunev <den@openvz.org>
The preallocate filter only does anything for a write that lands beyond
the end of its file, and that is what a format driver above it produces.
qemu-io could not produce it at all: the backend rejects such a request
with EIO, and the write path asserts without the resize permission, so
the filter could only be reached by stacking a real format on top and
letting it choose the offsets.
Take the resize permission and allow writes beyond the end when -g is
given, which leaves the tool free to address the filter directly.
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Hanna Reitz <hreitz@redhat.com>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
qemu-io.c | 32 +++++++++++++++++++++++++++++---
1 file changed, 29 insertions(+), 3 deletions(-)
diff --git a/qemu-io.c b/qemu-io.c
index 2b684ccf55..87254b32ff 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -45,6 +45,7 @@ static bool quit_qemu_io;
static int ncmdline;
static char **cmdline;
static bool imageOpts;
+static bool write_beyond_eof;
static ReadLineState *readline_state;
@@ -114,6 +115,21 @@ static int openfile(char *name, int flags, bool writethrough, bool force_share,
blk_set_enable_write_cache(qemuio_blk, !writethrough);
+ if (write_beyond_eof) {
+ uint64_t perm, shared;
+
+ blk_set_allow_write_beyond_eof(qemuio_blk, true);
+
+ blk_get_perm(qemuio_blk, &perm, &shared);
+ if (blk_set_perm(qemuio_blk, perm | BLK_PERM_RESIZE, shared,
+ &local_err) < 0) {
+ error_reportf_err(local_err, "can't take the resize permission: ");
+ blk_unref(qemuio_blk);
+ qemuio_blk = NULL;
+ return 1;
+ }
+ }
+
return 0;
}
@@ -129,6 +145,7 @@ static void open_help(void)
" Opens a file for subsequent use by all of the other qemu-io commands.\n"
" -r, -- open file read-only\n"
" -s, -- use snapshot file\n"
+" -g, -- let writes extend the image past its end\n"
" -C, -- use copy-on-read\n"
" -n, -- disable host cache, short for -t none\n"
" -U, -- force shared permissions\n"
@@ -149,7 +166,7 @@ static const cmdinfo_t open_cmd = {
.argmin = 1,
.argmax = -1,
.flags = CMD_NOFILE_OK,
- .args = "[-rsCnkU] [-t cache] [-d discard] [-o options] [path]",
+ .args = "[-grsCnkU] [-t cache] [-d discard] [-o options] [path]",
.oneline = "open the file specified by path",
.help = open_help,
};
@@ -175,7 +192,7 @@ static int open_f(BlockBackend *blk, int argc, char **argv, Error **errp)
QDict *opts;
bool force_share = false;
- while ((c = getopt(argc, argv, "snCro:ki:t:d:U")) != -1) {
+ while ((c = getopt(argc, argv, "gsnCro:ki:t:d:U")) != -1) {
switch (c) {
case 's':
flags |= BDRV_O_SNAPSHOT;
@@ -228,6 +245,9 @@ static int open_f(BlockBackend *blk, int argc, char **argv, Error **errp)
case 'U':
force_share = true;
break;
+ case 'g':
+ write_beyond_eof = true;
+ break;
default:
qemu_opts_reset(&empty_opts);
qemuio_command_usage(&open_cmd);
@@ -310,6 +330,8 @@ static void usage(const char *name)
" specify tracing options\n"
" see qemu-img(1) man page for full description\n"
" -U, --force-share force shared permissions\n"
+" -g, --grow let writes extend the image past its end,\n"
+" as a format driver does to its file\n"
" -h, --help display this help and exit\n"
" -V, --version output version information and exit\n"
"\n"
@@ -503,7 +525,7 @@ static QemuOptsList file_opts = {
int main(int argc, char **argv)
{
int readonly = 0;
- const char *sopt = "hVc:d:f:rsnCmki:t:T:U";
+ const char *sopt = "hVc:d:f:grsnCmki:t:T:U";
const struct option lopt[] = {
{ "help", no_argument, NULL, 'h' },
{ "version", no_argument, NULL, 'V' },
@@ -522,6 +544,7 @@ int main(int argc, char **argv)
{ "object", required_argument, NULL, OPTION_OBJECT },
{ "image-opts", no_argument, NULL, OPTION_IMAGE_OPTS },
{ "force-share", no_argument, 0, 'U'},
+ { "grow", no_argument, NULL, 'g' },
{ NULL, 0, NULL, 0 }
};
int c;
@@ -607,6 +630,9 @@ int main(int argc, char **argv)
case 'U':
force_share = true;
break;
+ case 'g':
+ write_beyond_eof = true;
+ break;
case OPTION_OBJECT:
g_ptr_array_add(objects, optarg);
break;
--
2.53.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 10/13] qemu-io: add -g to write past the end of the image
2026-09-30 16:49 ` [PATCH 10/13] qemu-io: add -g to write past the end of the image Denis V. Lunev
@ 2026-10-05 20:36 ` Eric Blake
0 siblings, 0 replies; 26+ messages in thread
From: Eric Blake @ 2026-10-05 20:36 UTC (permalink / raw)
To: Denis V. Lunev
Cc: qemu-devel, qemu-block, Kevin Wolf, Hanna Reitz,
Vladimir Sementsov-Ogievskiy, Andrey Drobyshev
On Wed, Sep 30, 2026 at 06:49:56PM +0200, Denis V. Lunev wrote:
> From: Denis V. Lunev <den@openvz.org>
>
> The preallocate filter only does anything for a write that lands beyond
> the end of its file, and that is what a format driver above it produces.
> qemu-io could not produce it at all: the backend rejects such a request
> with EIO, and the write path asserts without the resize permission, so
> the filter could only be reached by stacking a real format on top and
> letting it choose the offsets.
>
> Take the resize permission and allow writes beyond the end when -g is
> given, which leaves the tool free to address the filter directly.
>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Kevin Wolf <kwolf@redhat.com>
> CC: Hanna Reitz <hreitz@redhat.com>
> CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
> ---
> qemu-io.c | 32 +++++++++++++++++++++++++++++---
> 1 file changed, 29 insertions(+), 3 deletions(-)
>
> diff --git a/qemu-io.c b/qemu-io.c
> index 2b684ccf55..87254b32ff 100644
> --- a/qemu-io.c
> +++ b/qemu-io.c
> @@ -45,6 +45,7 @@ static bool quit_qemu_io;
> static int ncmdline;
> @@ -149,7 +166,7 @@ static const cmdinfo_t open_cmd = {
> .argmin = 1,
> .argmax = -1,
> .flags = CMD_NOFILE_OK,
> - .args = "[-rsCnkU] [-t cache] [-d discard] [-o options] [path]",
> + .args = "[-grsCnkU] [-t cache] [-d discard] [-o options] [path]",
No real pattern to how we sort the letters in this help synopsis, so
your choice of sticking -g first is no worse.
> @@ -175,7 +192,7 @@ static int open_f(BlockBackend *blk, int argc, char **argv, Error **errp)
> QDict *opts;
> bool force_share = false;
>
> - while ((c = getopt(argc, argv, "snCro:ki:t:d:U")) != -1) {
> + while ((c = getopt(argc, argv, "gsnCro:ki:t:d:U")) != -1) {
> switch (c) {
> case 's':
> flags |= BDRV_O_SNAPSHOT;
> @@ -228,6 +245,9 @@ static int open_f(BlockBackend *blk, int argc, char **argv, Error **errp)
> case 'U':
> force_share = true;
> break;
> + case 'g':
> + write_beyond_eof = true;
> + break;
> default:
With a maintainer's hat on, I do find it easier to patch a getopt()
call where the short-option string was in alphabetical order (mixed
case okay) and then the case statements are in the same order. Even if
the option string is NOT in alphabetical order, keeping the case
statements in the same order makes some sense (ie. since you put 'g'
before 's' in the optstring, it should be before 's' in the case
statements). However, while the maintainance of sticking 'g' at the
end rather than in any particular order is harder, the logic is not
wrong; and shuffling cases is minor enough that whether or not you do
that on a respin, you can add:
Reviewed-by: Eric Blake <eblake@redhat.com>
> @@ -522,6 +544,7 @@ int main(int argc, char **argv)
> { "object", required_argument, NULL, OPTION_OBJECT },
> { "image-opts", no_argument, NULL, OPTION_IMAGE_OPTS },
> { "force-share", no_argument, 0, 'U'},
> + { "grow", no_argument, NULL, 'g' },
Pre-existing, but since you're touching this, it might be nice to use
NULL in place of 0 on the force-share line.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 11/13] block/blkdebug: make a breakpoint work across AioContexts
2026-09-30 16:49 [PATCH 00/13] block: fix data loss in the preallocate filter Denis V. Lunev
` (9 preceding siblings ...)
2026-09-30 16:49 ` [PATCH 10/13] qemu-io: add -g to write past the end of the image Denis V. Lunev
@ 2026-09-30 16:49 ` Denis V. Lunev
2026-10-05 20:42 ` Eric Blake
2026-09-30 16:49 ` [PATCH 12/13] block: add blkdebug events for a write before it is tracked Denis V. Lunev
2026-09-30 16:49 ` [PATCH 13/13] iotests/298: cover the preallocation races Denis V. Lunev
12 siblings, 1 reply; 26+ messages in thread
From: Denis V. Lunev @ 2026-09-30 16:49 UTC (permalink / raw)
To: qemu-devel
Cc: den, qemu-block, Kevin Wolf, Hanna Reitz,
Vladimir Sementsov-Ogievskiy, Andrey Drobyshev
From: Denis V. Lunev <den@openvz.org>
A request that stops at a breakpoint belongs to whichever AioContext
issued it, and the monitor or qemu-io that resumes it runs somewhere
else. Two things go wrong once those are not the same thread.
resume_req_by_tag() woke the request with qemu_coroutine_enter(), which
enters the coroutine on the caller's thread. That moves the request off
its own thread for the rest of its life, and it aborts outright when it
wins the race against the yield, because the request is published to
the list before the coroutine gets there.
wait_break polled the block backend's AioContext, where the event will
never arrive, so it slept through the suspension it was waiting for.
Wake the request in the context it came from and wait for it the way
the rest of the block layer waits across contexts. Where the two
contexts are the same the coroutine is still entered directly, so
single-threaded behaviour, and the output tests depending on it, do not
change.
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Hanna Reitz <hreitz@redhat.com>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
block/blkdebug.c | 3 ++-
qemu-io-cmds.c | 5 ++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/block/blkdebug.c b/block/blkdebug.c
index 78cc03746f..1522015b34 100644
--- a/block/blkdebug.c
+++ b/block/blkdebug.c
@@ -813,6 +813,7 @@ static void suspend_request(BlockDriverState *bs, BlkdebugRule *rule)
remove_rule(rule);
QLIST_INSERT_HEAD(&s->suspended_reqs, r, next);
+ aio_wait_kick();
if (!qtest_enabled()) {
printf("blkdebug: Suspended request '%s'\n", r->tag);
@@ -925,7 +926,7 @@ retry:
g_free(r);
qemu_mutex_unlock(&s->lock);
- qemu_coroutine_enter(co);
+ aio_co_wake(co);
qemu_mutex_lock(&s->lock);
if (all) {
diff --git a/qemu-io-cmds.c b/qemu-io-cmds.c
index ce5bb53f4e..eed39ab93c 100644
--- a/qemu-io-cmds.c
+++ b/qemu-io-cmds.c
@@ -2791,9 +2791,8 @@ static const cmdinfo_t resume_cmd = {
static int wait_break_f(BlockBackend *blk, int argc, char **argv, Error **errp)
{
- while (!bdrv_debug_is_suspended(blk_bs(blk), argv[1])) {
- aio_poll(blk_get_aio_context(blk), true);
- }
+ AIO_WAIT_WHILE_UNLOCKED(NULL,
+ !bdrv_debug_is_suspended(blk_bs(blk), argv[1]));
return 0;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 11/13] block/blkdebug: make a breakpoint work across AioContexts
2026-09-30 16:49 ` [PATCH 11/13] block/blkdebug: make a breakpoint work across AioContexts Denis V. Lunev
@ 2026-10-05 20:42 ` Eric Blake
0 siblings, 0 replies; 26+ messages in thread
From: Eric Blake @ 2026-10-05 20:42 UTC (permalink / raw)
To: Denis V. Lunev
Cc: qemu-devel, qemu-block, Kevin Wolf, Hanna Reitz,
Vladimir Sementsov-Ogievskiy, Andrey Drobyshev
On Wed, Sep 30, 2026 at 06:49:57PM +0200, Denis V. Lunev wrote:
> From: Denis V. Lunev <den@openvz.org>
>
> A request that stops at a breakpoint belongs to whichever AioContext
> issued it, and the monitor or qemu-io that resumes it runs somewhere
> else. Two things go wrong once those are not the same thread.
>
> resume_req_by_tag() woke the request with qemu_coroutine_enter(), which
> enters the coroutine on the caller's thread. That moves the request off
> its own thread for the rest of its life, and it aborts outright when it
> wins the race against the yield, because the request is published to
> the list before the coroutine gets there.
>
> wait_break polled the block backend's AioContext, where the event will
> never arrive, so it slept through the suspension it was waiting for.
>
> Wake the request in the context it came from and wait for it the way
> the rest of the block layer waits across contexts. Where the two
> contexts are the same the coroutine is still entered directly, so
> single-threaded behaviour, and the output tests depending on it, do not
> change.
>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Kevin Wolf <kwolf@redhat.com>
> CC: Hanna Reitz <hreitz@redhat.com>
> CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
> ---
> block/blkdebug.c | 3 ++-
> qemu-io-cmds.c | 5 ++---
> 2 files changed, 4 insertions(+), 4 deletions(-)
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 12/13] block: add blkdebug events for a write before it is tracked
2026-09-30 16:49 [PATCH 00/13] block: fix data loss in the preallocate filter Denis V. Lunev
` (10 preceding siblings ...)
2026-09-30 16:49 ` [PATCH 11/13] block/blkdebug: make a breakpoint work across AioContexts Denis V. Lunev
@ 2026-09-30 16:49 ` Denis V. Lunev
2026-10-05 20:45 ` Eric Blake
2026-09-30 16:49 ` [PATCH 13/13] iotests/298: cover the preallocation races Denis V. Lunev
12 siblings, 1 reply; 26+ messages in thread
From: Denis V. Lunev @ 2026-09-30 16:49 UTC (permalink / raw)
To: qemu-devel
Cc: den, qemu-block, Kevin Wolf, Hanna Reitz,
Vladimir Sementsov-Ogievskiy, Andrey Drobyshev, Eric Blake,
Markus Armbruster
From: Denis V. Lunev <den@openvz.org>
A breakpoint on pwritev or pwritev_zero stops a request that has
already joined the list of tracked requests, so it is ordered against
everything that arrives later. There was no way to hold a request in
the window before that, which is where a filter has published its
state but the block layer does not yet know the request exists.
Fire pwritev_untracked and pwritev_zero_untracked from
bdrv_co_pwritev_part() once the request counts as in flight and before
tracked_request_begin(). They come in a pair like the events they
mirror, so that a test can tell a preallocation from the writes racing
with it: blkdebug matches on the event, not on the request.
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Hanna Reitz <hreitz@redhat.com>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
CC: Eric Blake <eblake@redhat.com>
CC: Markus Armbruster <armbru@redhat.com>
---
block/io.c | 3 +++
qapi/block-core.json | 10 +++++++++-
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/block/io.c b/block/io.c
index a916b236c3..a32b567264 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2302,6 +2302,9 @@ int coroutine_fn bdrv_co_pwritev_part(BdrvChild *child,
}
bdrv_inc_in_flight(bs);
+ bdrv_co_debug_event(bs, (flags & BDRV_REQ_ZERO_WRITE) ?
+ BLKDBG_PWRITEV_ZERO_UNTRACKED :
+ BLKDBG_PWRITEV_UNTRACKED);
tracked_request_begin(&req, bs, offset, bytes, BDRV_TRACKED_WRITE);
if (flags & BDRV_REQ_ZERO_WRITE) {
diff --git a/qapi/block-core.json b/qapi/block-core.json
index aed2888147..340ee8c9e2 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -3897,6 +3897,13 @@
#
# @none: triggers once at creation of the blkdebug node (since 4.1)
#
+# @pwritev_untracked: a write, before it joins the list of tracked
+# requests and so before it is ordered against any other request
+# (since 11.2)
+#
+# @pwritev_zero_untracked: the same for a write zeroes request
+# (since 11.2)
+#
# Since: 2.9
##
{ 'enum': 'BlkdebugEvent', 'prefix': 'BLKDBG',
@@ -3915,7 +3922,8 @@
'pwritev_rmw_tail', 'pwritev_rmw_after_tail', 'pwritev',
'pwritev_zero', 'pwritev_done', 'empty_image_prepare',
'l1_shrink_write_table', 'l1_shrink_free_l2_clusters',
- 'cor_write', 'cluster_alloc_space', 'none'] }
+ 'cor_write', 'cluster_alloc_space', 'none',
+ 'pwritev_untracked', 'pwritev_zero_untracked'] }
##
# @BlkdebugIOType:
--
2.53.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 12/13] block: add blkdebug events for a write before it is tracked
2026-09-30 16:49 ` [PATCH 12/13] block: add blkdebug events for a write before it is tracked Denis V. Lunev
@ 2026-10-05 20:45 ` Eric Blake
0 siblings, 0 replies; 26+ messages in thread
From: Eric Blake @ 2026-10-05 20:45 UTC (permalink / raw)
To: Denis V. Lunev
Cc: qemu-devel, qemu-block, Kevin Wolf, Hanna Reitz,
Vladimir Sementsov-Ogievskiy, Andrey Drobyshev, Markus Armbruster
On Wed, Sep 30, 2026 at 06:49:58PM +0200, Denis V. Lunev wrote:
> From: Denis V. Lunev <den@openvz.org>
>
> A breakpoint on pwritev or pwritev_zero stops a request that has
> already joined the list of tracked requests, so it is ordered against
> everything that arrives later. There was no way to hold a request in
> the window before that, which is where a filter has published its
> state but the block layer does not yet know the request exists.
>
> Fire pwritev_untracked and pwritev_zero_untracked from
> bdrv_co_pwritev_part() once the request counts as in flight and before
> tracked_request_begin(). They come in a pair like the events they
> mirror, so that a test can tell a preallocation from the writes racing
> with it: blkdebug matches on the event, not on the request.
>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Kevin Wolf <kwolf@redhat.com>
> CC: Hanna Reitz <hreitz@redhat.com>
> CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
> CC: Eric Blake <eblake@redhat.com>
> CC: Markus Armbruster <armbru@redhat.com>
> ---
> block/io.c | 3 +++
> qapi/block-core.json | 10 +++++++++-
> 2 files changed, 12 insertions(+), 1 deletion(-)
>
> +++ b/qapi/block-core.json
> @@ -3897,6 +3897,13 @@
> #
> # @none: triggers once at creation of the blkdebug node (since 4.1)
> #
> +# @pwritev_untracked: a write, before it joins the list of tracked
> +# requests and so before it is ordered against any other request
> +# (since 11.2)
> +#
> +# @pwritev_zero_untracked: the same for a write zeroes request
> +# (since 11.2)
I don't know if Markus has any guidance to whether the text for one
parameter can reliably refer to an earlier one. But I'm assuming that
the generated docs list things sequentially by declaration order
rather than reordering to alphabetical or randomized hash layout
ordering, so the shorter second text is probably fine.
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
^ permalink raw reply [flat|nested] 26+ messages in thread
* [PATCH 13/13] iotests/298: cover the preallocation races
2026-09-30 16:49 [PATCH 00/13] block: fix data loss in the preallocate filter Denis V. Lunev
` (11 preceding siblings ...)
2026-09-30 16:49 ` [PATCH 12/13] block: add blkdebug events for a write before it is tracked Denis V. Lunev
@ 2026-09-30 16:49 ` Denis V. Lunev
2026-10-05 21:06 ` Eric Blake
12 siblings, 1 reply; 26+ messages in thread
From: Denis V. Lunev @ 2026-09-30 16:49 UTC (permalink / raw)
To: qemu-devel
Cc: den, qemu-block, Kevin Wolf, Hanna Reitz,
Vladimir Sementsov-Ogievskiy, Andrey Drobyshev
From: Denis V. Lunev <den@openvz.org>
Three cases the filter has to get right, all deterministic: a write the
preallocation cannot reach goes through without waiting for it, a write
into the range it covers does not, and a write zeroes over data written
inside that range is not skipped as already zero.
The second one cannot wait for the write to finish, because that is
what it is checking does not happen, and the data survives either way:
a preallocation that finds the write still in flight gives up. It asks
whether the write got past the filter at all instead, by leaving a
breakpoint below it.
Signed-off-by: Denis V. Lunev <den@openvz.org>
CC: Kevin Wolf <kwolf@redhat.com>
CC: Hanna Reitz <hreitz@redhat.com>
CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
---
tests/qemu-iotests/298 | 64 ++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/298.out | 4 +--
2 files changed, 66 insertions(+), 2 deletions(-)
diff --git a/tests/qemu-iotests/298 b/tests/qemu-iotests/298
index 23e7789d09..866e00f8e0 100755
--- a/tests/qemu-iotests/298
+++ b/tests/qemu-iotests/298
@@ -27,6 +27,7 @@ GiB = MiB * 1024
disk = os.path.join(iotests.test_dir, 'disk')
overlay = os.path.join(iotests.test_dir, 'overlay')
refdisk = os.path.join(iotests.test_dir, 'refdisk')
+racedisk = os.path.join(iotests.test_dir, 'racedisk')
drive_opts = f'node-name=disk,driver={iotests.imgfmt},' \
f'file.node-name=filter,file.driver=preallocate,' \
f'file.file.node-name=file,file.file.filename={disk}'
@@ -225,6 +226,69 @@ class TestPreallocAsyncWrites(iotests.QMPTestCase):
proc.close()
+class TestPreallocateRaces(iotests.QMPTestCase):
+ # blkdebug under the filter holds the preallocation before it is tracked.
+ opts = f'driver=preallocate,prealloc-align=65536,prealloc-size=262144,' \
+ f'file.driver=blkdebug,file.image.driver=file,' \
+ f'file.image.filename={racedisk}'
+
+ def setUp(self):
+ self.proc = None
+ with open(racedisk, 'wb') as f:
+ f.truncate(MiB)
+
+ def tearDown(self):
+ if self.proc:
+ self.proc.close()
+ os.remove(racedisk)
+
+ def qemu_io(self, *args):
+ self.proc = iotests.QemuIoInteractive('-g', *args, '--image-opts',
+ self.opts)
+ return self.proc
+
+ def test_write_below_preallocation(self):
+ proc = self.qemu_io('--object', 'iothread,id=t0')
+ proc.cmd('break pwritev_zero_untracked prealloc')
+ proc.cmd('aio_write -q -t t0 -P 0xaa 1M 64k')
+ proc.cmd('wait_break prealloc')
+
+ # Returns while the preallocation is parked only if it never waited.
+ out = proc.cmd('write -P 0xcc 4k 4k')
+
+ proc.cmd('resume prealloc')
+ proc.cmd('aio_flush')
+ self.assertIn('wrote 4096/4096', str(out))
+
+ def test_write_into_preallocation(self):
+ proc = self.qemu_io('--object', 'iothread,id=t0',
+ '--object', 'iothread,id=t1')
+ proc.cmd('break pwritev_zero_untracked prealloc')
+ proc.cmd('aio_write -q -t t0 -P 0xaa 1M 64k')
+ proc.cmd('wait_break prealloc')
+ proc.cmd('break pwritev late')
+ proc.cmd('aio_write -q -t t1 -P 0xbb 1152k 64k')
+
+ # A held-back write never finishes; ask whether it got past the filter.
+ late = proc.cmd('resume late')
+
+ proc.cmd('remove_break late')
+ proc.cmd('resume prealloc')
+ proc.cmd('aio_flush')
+ data = proc.cmd('read -P 0xbb 1152k 64k')
+
+ self.assertIn('Could not resume request', str(late))
+ self.assertNotIn('Pattern verification failed', str(data))
+
+ def test_write_zeroes_over_written_data(self):
+ proc = self.qemu_io()
+ proc.cmd('write -q -z 1M 128k')
+ proc.cmd('write -q -P 0xaa 1M 64k')
+ proc.cmd('write -q -z 1M 256k')
+ out = proc.cmd('read -P 0 1M 64k')
+ self.assertNotIn('Pattern verification failed', str(out))
+
+
if __name__ == '__main__':
iotests.main(supported_fmts=['qcow2'], required_fmts=['preallocate'],
require_hmp=True)
diff --git a/tests/qemu-iotests/298.out b/tests/qemu-iotests/298.out
index 6323079e08..52d796e60b 100644
--- a/tests/qemu-iotests/298.out
+++ b/tests/qemu-iotests/298.out
@@ -1,5 +1,5 @@
-..............
+.................
----------------------------------------------------------------------
-Ran 14 tests
+Ran 17 tests
OK
--
2.53.0
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [PATCH 13/13] iotests/298: cover the preallocation races
2026-09-30 16:49 ` [PATCH 13/13] iotests/298: cover the preallocation races Denis V. Lunev
@ 2026-10-05 21:06 ` Eric Blake
0 siblings, 0 replies; 26+ messages in thread
From: Eric Blake @ 2026-10-05 21:06 UTC (permalink / raw)
To: Denis V. Lunev
Cc: qemu-devel, qemu-block, Kevin Wolf, Hanna Reitz,
Vladimir Sementsov-Ogievskiy, Andrey Drobyshev
On Wed, Sep 30, 2026 at 06:49:59PM +0200, Denis V. Lunev wrote:
> From: Denis V. Lunev <den@openvz.org>
>
> Three cases the filter has to get right, all deterministic: a write the
> preallocation cannot reach goes through without waiting for it, a write
> into the range it covers does not, and a write zeroes over data written
> inside that range is not skipped as already zero.
>
> The second one cannot wait for the write to finish, because that is
> what it is checking does not happen, and the data survives either way:
> a preallocation that finds the write still in flight gives up. It asks
> whether the write got past the filter at all instead, by leaving a
> breakpoint below it.
>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Kevin Wolf <kwolf@redhat.com>
> CC: Hanna Reitz <hreitz@redhat.com>
> CC: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
> CC: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>
> ---
> tests/qemu-iotests/298 | 64 ++++++++++++++++++++++++++++++++++++++
> tests/qemu-iotests/298.out | 4 +--
> 2 files changed, 66 insertions(+), 2 deletions(-)
>
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
^ permalink raw reply [flat|nested] 26+ messages in thread