* [PULL 1/3] block/fuse: Let PUNCH_HOLE write zeroes
2023-03-10 17:55 [PULL 0/3] Block layer patches Kevin Wolf
@ 2023-03-10 17:55 ` Kevin Wolf
2023-03-10 17:55 ` [PULL 2/3] iotests/308: Add test for 'write -zu' Kevin Wolf
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Kevin Wolf @ 2023-03-10 17:55 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
From: Hanna Czenczek <hreitz@redhat.com>
fallocate(2) says about PUNCH_HOLE: "After a successful call, subsequent
reads from this range will return zeros." As it is, PUNCH_HOLE is
implemented as a call to blk_pdiscard(), which does not guarantee this.
We must call blk_pwrite_zeroes() instead. The difference to ZERO_RANGE
is that we pass the `BDRV_REQ_MAY_UNMAP | BDRV_REQ_NO_FALLBACK` flags to
the call -- the storage is supposed to be unmapped, and a slow fallback
by actually writing zeroes as data is not allowed.
Closes: https://gitlab.com/qemu-project/qemu/-/issues/1507
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230227104725.33511-2-hreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/export/fuse.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/block/export/fuse.c b/block/export/fuse.c
index e5fc4af165..06fa41079e 100644
--- a/block/export/fuse.c
+++ b/block/export/fuse.c
@@ -673,7 +673,16 @@ static void fuse_fallocate(fuse_req_t req, fuse_ino_t inode, int mode,
do {
int size = MIN(length, BDRV_REQUEST_MAX_BYTES);
- ret = blk_pdiscard(exp->common.blk, offset, size);
+ ret = blk_pwrite_zeroes(exp->common.blk, offset, size,
+ BDRV_REQ_MAY_UNMAP | BDRV_REQ_NO_FALLBACK);
+ if (ret == -ENOTSUP) {
+ /*
+ * fallocate() specifies to return EOPNOTSUPP for unsupported
+ * operations
+ */
+ ret = -EOPNOTSUPP;
+ }
+
offset += size;
length -= size;
} while (ret == 0 && length > 0);
--
2.39.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PULL 2/3] iotests/308: Add test for 'write -zu'
2023-03-10 17:55 [PULL 0/3] Block layer patches Kevin Wolf
2023-03-10 17:55 ` [PULL 1/3] block/fuse: Let PUNCH_HOLE write zeroes Kevin Wolf
@ 2023-03-10 17:55 ` Kevin Wolf
2023-03-10 17:55 ` [PULL 3/3] qed: remove spurious BDRV_POLL_WHILE() Kevin Wolf
2023-03-12 17:41 ` [PULL 0/3] Block layer patches Peter Maydell
3 siblings, 0 replies; 5+ messages in thread
From: Kevin Wolf @ 2023-03-10 17:55 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
From: Hanna Czenczek <hreitz@redhat.com>
Try writing zeroes to a FUSE export while allowing the area to be
unmapped; block/file-posix.c generally implements writing zeroes with
BDRV_REQ_MAY_UNMAP ('write -zu') by calling fallocate(PUNCH_HOLE). This
used to lead to a blk_pdiscard() in the FUSE export, which may or may
not lead to the area being zeroed. HEAD^ fixed this to use
blk_pwrite_zeroes() instead (again with BDRV_REQ_MAY_UNMAP), so verify
that running `qemu-io 'write -zu'` on a FUSE exports always results in
zeroes being written.
Signed-off-by: Hanna Czenczek <hreitz@redhat.com>
Message-Id: <20230227104725.33511-3-hreitz@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
tests/qemu-iotests/308 | 43 ++++++++++++++++++++++++++++++++++++++
tests/qemu-iotests/308.out | 35 +++++++++++++++++++++++++++++++
2 files changed, 78 insertions(+)
diff --git a/tests/qemu-iotests/308 b/tests/qemu-iotests/308
index 09275e9a10..de12b2b1b9 100755
--- a/tests/qemu-iotests/308
+++ b/tests/qemu-iotests/308
@@ -370,6 +370,49 @@ echo
echo '=== Compare copy with original ==='
$QEMU_IMG compare -f raw -F $IMGFMT "$COPIED_IMG" "$TEST_IMG"
+_cleanup_test_img
+
+echo
+echo '=== Writing zeroes while unmapping ==='
+# Regression test for https://gitlab.com/qemu-project/qemu/-/issues/1507
+_make_test_img 64M
+$QEMU_IO -c 'write -s /dev/urandom 0 64M' "$TEST_IMG" | _filter_qemu_io
+
+_launch_qemu
+_send_qemu_cmd $QEMU_HANDLE \
+ "{'execute': 'qmp_capabilities'}" \
+ 'return'
+
+_send_qemu_cmd $QEMU_HANDLE \
+ "{'execute': 'blockdev-add',
+ 'arguments': {
+ 'driver': '$IMGFMT',
+ 'node-name': 'node-format',
+ 'file': {
+ 'driver': 'file',
+ 'filename': '$TEST_IMG'
+ }
+ } }" \
+ 'return'
+
+fuse_export_add 'export' "'mountpoint': '$EXT_MP', 'writable': true"
+
+# Try writing zeroes by unmapping
+$QEMU_IO -f raw -c 'write -zu 0 64M' "$EXT_MP" | _filter_qemu_io
+
+# Check the result
+$QEMU_IO -f raw -c 'read -P 0 0 64M' "$EXT_MP" | _filter_qemu_io
+
+_send_qemu_cmd $QEMU_HANDLE \
+ "{'execute': 'quit'}" \
+ 'return'
+
+wait=yes _cleanup_qemu
+
+# Check the original image
+$QEMU_IO -c 'read -P 0 0 64M' "$TEST_IMG" | _filter_qemu_io
+
+_cleanup_test_img
# success, all done
echo "*** done"
diff --git a/tests/qemu-iotests/308.out b/tests/qemu-iotests/308.out
index e4467a10cf..d5767133b1 100644
--- a/tests/qemu-iotests/308.out
+++ b/tests/qemu-iotests/308.out
@@ -171,4 +171,39 @@ OK: Post-truncate image size is as expected
=== Compare copy with original ===
Images are identical.
+
+=== Writing zeroes while unmapping ===
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+wrote 67108864/67108864 bytes at offset 0
+64 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+{'execute': 'qmp_capabilities'}
+{"return": {}}
+{'execute': 'blockdev-add',
+ 'arguments': {
+ 'driver': 'IMGFMT',
+ 'node-name': 'node-format',
+ 'file': {
+ 'driver': 'file',
+ 'filename': 'TEST_DIR/t.IMGFMT'
+ }
+ } }
+{"return": {}}
+{'execute': 'block-export-add',
+ 'arguments': {
+ 'type': 'fuse',
+ 'id': 'export',
+ 'node-name': 'node-format',
+ 'mountpoint': 'TEST_DIR/t.IMGFMT.fuse', 'writable': true
+ } }
+{"return": {}}
+wrote 67108864/67108864 bytes at offset 0
+64 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 67108864/67108864 bytes at offset 0
+64 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+{'execute': 'quit'}
+{"return": {}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false, "reason": "host-qmp-quit"}}
+{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "BLOCK_EXPORT_DELETED", "data": {"id": "export"}}
+read 67108864/67108864 bytes at offset 0
+64 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
*** done
--
2.39.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PULL 3/3] qed: remove spurious BDRV_POLL_WHILE()
2023-03-10 17:55 [PULL 0/3] Block layer patches Kevin Wolf
2023-03-10 17:55 ` [PULL 1/3] block/fuse: Let PUNCH_HOLE write zeroes Kevin Wolf
2023-03-10 17:55 ` [PULL 2/3] iotests/308: Add test for 'write -zu' Kevin Wolf
@ 2023-03-10 17:55 ` Kevin Wolf
2023-03-12 17:41 ` [PULL 0/3] Block layer patches Peter Maydell
3 siblings, 0 replies; 5+ messages in thread
From: Kevin Wolf @ 2023-03-10 17:55 UTC (permalink / raw)
To: qemu-block; +Cc: kwolf, peter.maydell, qemu-devel
From: Stefan Hajnoczi <stefanha@redhat.com>
This looks like a copy-paste or merge error. BDRV_POLL_WHILE() is
already called above. It's not needed in the qemu_in_coroutine() case.
Fixes: 9fb4dfc570ce ("qed: make bdrv_qed_do_open a coroutine_fn")
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20230309163134.398707-1-stefanha@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
block/qed.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/block/qed.c b/block/qed.c
index ed94bb61ca..0705a7b4e2 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -594,7 +594,6 @@ static int bdrv_qed_open(BlockDriverState *bs, QDict *options, int flags,
qemu_coroutine_enter(qemu_coroutine_create(bdrv_qed_open_entry, &qoc));
BDRV_POLL_WHILE(bs, qoc.ret == -EINPROGRESS);
}
- BDRV_POLL_WHILE(bs, qoc.ret == -EINPROGRESS);
return qoc.ret;
}
--
2.39.2
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PULL 0/3] Block layer patches
2023-03-10 17:55 [PULL 0/3] Block layer patches Kevin Wolf
` (2 preceding siblings ...)
2023-03-10 17:55 ` [PULL 3/3] qed: remove spurious BDRV_POLL_WHILE() Kevin Wolf
@ 2023-03-12 17:41 ` Peter Maydell
3 siblings, 0 replies; 5+ messages in thread
From: Peter Maydell @ 2023-03-12 17:41 UTC (permalink / raw)
To: Kevin Wolf; +Cc: qemu-block, qemu-devel
On Fri, 10 Mar 2023 at 17:55, Kevin Wolf <kwolf@redhat.com> wrote:
>
> The following changes since commit ee59483267de29056b5b2ee2421ef3844e5c9932:
>
> Merge tag 'qemu-openbios-20230307' of https://github.com/mcayland/qemu into staging (2023-03-09 16:55:03 +0000)
>
> are available in the Git repository at:
>
> https://repo.or.cz/qemu/kevin.git tags/for-upstream
>
> for you to fetch changes up to ecf8191314798391b1df80bcb829c0ead4f8acc9:
>
> qed: remove spurious BDRV_POLL_WHILE() (2023-03-10 15:14:46 +0100)
>
> ----------------------------------------------------------------
> Block layer patches
>
> - fuse: Fix fallocate(PUNCH_HOLE) to zero out the range
> - qed: remove spurious BDRV_POLL_WHILE()
>
> ----------------------------------------------------------------
Applied, thanks.
Please update the changelog at https://wiki.qemu.org/ChangeLog/8.0
for any user-visible changes.
-- PMM
^ permalink raw reply [flat|nested] 5+ messages in thread