[pve-devel] [PATCH qemu] add patch fixing io_uring short read slow path

[pve-devel] [PATCH qemu] add patch fixing io_uring short read slow path

[Fabian Ebner]

Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
---
 ...20-io_uring-fix-short-read-slow-path.patch | 47 +++++++++++++++++++
 debian/patches/series                         |  1 +
 2 files changed, 48 insertions(+)
 create mode 100644 debian/patches/extra/0020-io_uring-fix-short-read-slow-path.patch

diff --git a/debian/patches/extra/0020-io_uring-fix-short-read-slow-path.patch b/debian/patches/extra/0020-io_uring-fix-short-read-slow-path.patch
new file mode 100644
index 0000000..6078d00
--- /dev/null
+++ b/debian/patches/extra/0020-io_uring-fix-short-read-slow-path.patch
@@ -0,0 +1,47 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Dominique Martinet <dominique.martinet@atmark-techno.com>
+Date: Thu, 30 Jun 2022 10:01:37 +0900
+Subject: [PATCH] io_uring: fix short read slow path
+
+sqeq.off here is the offset to read within the disk image, so obviously
+not 'nread' (the amount we just read), but as the author meant to write
+its current value incremented by the amount we just read.
+
+Normally recent versions of linux will not issue short reads,
+but it can happen so we should fix this.
+
+This lead to weird image corruptions when short read happened
+
+Fixes: 6663a0a33764 ("block/io_uring: implements interfaces for io_uring")
+Link: https://lkml.kernel.org/r/YrrFGO4A1jS0GI0G@atmark-techno.com
+Reviewed-by: Hanna Reitz <hreitz@redhat.com>
+Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
+Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com>
+(taken from https://lists.nongnu.org/archive/html/qemu-devel/2022-06/msg05619.html)
+Signed-off-by: Fabian Ebner <f.ebner@proxmox.com>
+---
+ block/io_uring.c | 4 ++--
+ 1 file changed, 2 insertions(+), 2 deletions(-)
+
+diff --git a/block/io_uring.c b/block/io_uring.c
+index 782afdb433..e451e55de0 100644
+--- a/block/io_uring.c
++++ b/block/io_uring.c
+@@ -89,7 +89,7 @@ static void luring_resubmit_short_read(LuringState *s, LuringAIOCB *luringcb,
+     trace_luring_resubmit_short_read(s, luringcb, nread);
+ 
+     /* Update read position */
+-    luringcb->total_read = nread;
++    luringcb->total_read += nread;
+     remaining = luringcb->qiov->size - luringcb->total_read;
+ 
+     /* Shorten qiov */
+@@ -103,7 +103,7 @@ static void luring_resubmit_short_read(LuringState *s, LuringAIOCB *luringcb,
+                       remaining);
+ 
+     /* Update sqe */
+-    luringcb->sqeq.off = nread;
++    luringcb->sqeq.off += nread;
+     luringcb->sqeq.addr = (__u64)(uintptr_t)luringcb->resubmit_qiov.iov;
+     luringcb->sqeq.len = luringcb->resubmit_qiov.niov;
+ 
diff --git a/debian/patches/series b/debian/patches/series
index 3850a52..146b27e 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -17,6 +17,7 @@ extra/0016-vdpa-Fix-bad-index-calculus-at-vhost_vdpa_get_vring_.patch
 extra/0017-vdpa-Fix-index-calculus-at-vhost_vdpa_svqs_start.patch
 extra/0018-hw-virtio-Replace-g_memdup-by-g_memdup2.patch
 extra/0019-vhost-Fix-element-in-vhost_svq_add-failure.patch
+extra/0020-io_uring-fix-short-read-slow-path.patch
 bitmap-mirror/0001-drive-mirror-add-support-for-sync-bitmap-mode-never.patch
 bitmap-mirror/0002-drive-mirror-add-support-for-conditional-and-always-.patch
 bitmap-mirror/0003-mirror-add-check-for-bitmap-mode-without-bitmap.patch
-- 
2.30.2

Re: [pve-devel] [PATCH qemu] add patch fixing io_uring short read slow path

[Thomas Lamprecht]

How likely is this to trigger? Did you came across such events or is this a backport
for the sake of being cautious?

Mostly asking because 7.0 is already on staging, so it'd be good to know if we should
add another 6.0 release between (a bit of an hassle but possible due it not having yet
seen any public repo), or if there isn't as much time pressure and we can just roll it
out with the slower rollout of QEMU 7.0.

Re: [pve-devel] [PATCH qemu] add patch fixing io_uring short read slow path

[Fabian Ebner]

Am 01.07.22 um 13:53 schrieb Thomas Lamprecht:
> How likely is this to trigger? Did you came across such events or is this a backport
> for the sake of being cautious?
> 
> Mostly asking because 7.0 is already on staging, so it'd be good to know if we should
> add another 6.0 release between (a bit of an hassle but possible due it not having yet
> seen any public repo), or if there isn't as much time pressure and we can just roll it
> out with the slower rollout of QEMU 7.0.

I didn't run into this recently. I do remember that I wondered about a
luring_resubmit_short_read trace for one VM while looking at the
LVM+io_uring issues a year ago, but IIRC that one behaved a bit
differently - got a weird GRUB error rather than the hang reported in
the forum, which I was able to reproduce with another VM then. But maybe
this was actually the root cause for these issues? Would have to find a
reproducer again to see if the patch fixes it.

v1 of the patch mentions:
> Normally recent versions of linux will not issue short reads,
> but apparently btrfs with O_DIRECT (cache=none) does.

but might not be the only situation. I tried a bit with a VM with an
image on BTRFS, but didn't manage to have the call show up in a trace.

So I can't really tell how likely. It can happen with BTRFS, it could
happen with LVM in the past (we don't allow io_uring+LVM currently). I'd
say since there is a big unknown, it might be worth a backport.