From 0591d5a32ee441eba8ba327e9dc1c73ba7b61303 Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Thu, 1 Oct 2026 12:20:54 -0300 Subject: [PATCH 1/3] qemu: carry upstream's discard accounting for virtio-blk QEMU 11.1.1's virtio-blk never starts accounting for a DISCARD, so query-blockstats reports 0 unmap operations however much a guest trims: the page-cache lab trimmed 1.1 GiB and saw 0 (run 36815822140), and could only see the trim in the overlay's allocation. Upstream fixed it in 331ce936c133 (master, 2026-09-11); no release has it yet, v11.1.2 included. This is that commit unchanged, to delete in the bump that brings it. A new QEMU binary, so a new machine fingerprint. The page-cache probe now fails when the guest's fstrim is not counted. Co-Authored-By: Claude Opus 5.5 (1M context) --- boot/pagecache_test.go | 7 ++ ...irtio-blk-account-discard-operations.patch | 118 ++++++++++++++++++ 2 files changed, 125 insertions(+) create mode 100644 qemu/patches/0002-hw-virtio-blk-account-discard-operations.patch diff --git a/boot/pagecache_test.go b/boot/pagecache_test.go index b7198c0..e7089fb 100644 --- a/boot/pagecache_test.go +++ b/boot/pagecache_test.go @@ -663,10 +663,17 @@ func discardNote(t *testing.T, out, socket, overlay string) string { t.Fatalf("reading query-blockstats: %v", err) } var unmap string + var unmapOps int64 for _, s := range stats { + unmapOps += s.Stats.UnmapOperations unmap += fmt.Sprintf(" %d ops %d MB (%d failed, %d invalid), beside %d writes %d MB", s.Stats.UnmapOperations, s.Stats.UnmapBytes>>20, s.Stats.FailedUnmap, s.Stats.InvalidUnmap, s.Stats.WrOperations, s.Stats.WrBytes>>20) } + // The guest has just trimmed more than a gigabyte. Upstream virtio-blk counted none of it + // until qemu/patches/0002; zero here is a QEMU built without that patch. + if unmapOps == 0 { + t.Errorf("QEMU counted no discard after the guest's fstrim:%s", unmap) + } // -U: the image is open in the running QEMU, and this only reads its tables. raw, err := exec.Command(filepath.Join(out, "bin", "qemu-img"), "map", "-U", "--output=json", overlay).Output() if err != nil { diff --git a/qemu/patches/0002-hw-virtio-blk-account-discard-operations.patch b/qemu/patches/0002-hw-virtio-blk-account-discard-operations.patch new file mode 100644 index 0000000..2d5d063 --- /dev/null +++ b/qemu/patches/0002-hw-virtio-blk-account-discard-operations.patch @@ -0,0 +1,118 @@ +From 331ce936c13359474cadcbbd548e018197fde426 Mon Sep 17 00:00:00 2001 +From: Hanna Czenczek +Date: Fri, 24 Jul 2026 17:23:10 +0200 +Subject: [PATCH] hw/virtio-blk: Account discard operations + +The acct_failed argument to virtio_blk_handle_rw_error() tells whether +there is accounting for this operation or not. The only operation for +which there is none is discarding, which seems to be because at the time +of the introducing commit 37b06f8d46f ("virtio-blk: add DISCARD and +WRITE_ZEROES features"), BlockAcctType did not yet have a +BLOCK_ACCT_UNMAP variant. + +It does have that now, though, so we may as well track those discard +operations with it, and can thus remove the acct_failed parameter. + +Signed-off-by: Hanna Czenczek +Reviewed-by: Michael S. Tsirkin +Reviewed-by: Stefan Hajnoczi +Signed-off-by: Michael S. Tsirkin +Message-ID: <20260724152315.234183-2-hreitz@redhat.com> +--- +Backported unchanged from QEMU master, commit 331ce936c133 (merged 2026-09-11); no tag +had it on 2026-10-01, v11.1.2 included. Without it query-blockstats reports 0 unmap +operations for a virtio-blk disk however much a guest trims, so a trim can only be seen +in the overlay's allocation (qemu-img map), as the page-cache lab had to (spin-machine +boot/pagecache_test.go, run 36815822140: 1.1 GiB trimmed, unmap_operations 0). Delete +this file in the bump to a release that contains the commit: it will then not apply. + + hw/block/virtio-blk.c | 26 +++++++++++--------------- + 1 file changed, 11 insertions(+), 15 deletions(-) + +diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c +index cb6a276a82..4110380392 100644 +--- a/hw/block/virtio-blk.c ++++ b/hw/block/virtio-blk.c +@@ -69,7 +69,7 @@ void virtio_blk_req_complete(VirtIOBlockReq *req, unsigned char status) + } + + static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error, +- bool is_read, bool acct_failed) ++ bool is_read) + { + VirtIOBlock *s = req->dev; + BlockErrorAction action = blk_get_error_action(s->blk, is_read, error); +@@ -85,9 +85,7 @@ static int virtio_blk_handle_rw_error(VirtIOBlockReq *req, int error, + } + } else if (action == BLOCK_ERROR_ACTION_REPORT) { + virtio_blk_req_complete(req, VIRTIO_BLK_S_IOERR); +- if (acct_failed) { +- block_acct_failed(blk_get_stats(s->blk), &req->acct); +- } ++ block_acct_failed(blk_get_stats(s->blk), &req->acct); + g_free(req); + } + +@@ -124,7 +122,7 @@ static void virtio_blk_rw_complete(void *opaque, int ret) + * the memory until the request is completed (which will + * happen on the other side of the migration). + */ +- if (virtio_blk_handle_rw_error(req, -ret, is_read, true)) { ++ if (virtio_blk_handle_rw_error(req, -ret, is_read)) { + continue; + } + } +@@ -140,7 +138,7 @@ static void virtio_blk_flush_complete(void *opaque, int ret) + VirtIOBlockReq *req = opaque; + VirtIOBlock *s = req->dev; + +- if (ret && virtio_blk_handle_rw_error(req, -ret, 0, true)) { ++ if (ret && virtio_blk_handle_rw_error(req, -ret, 0)) { + return; + } + +@@ -153,17 +151,13 @@ static void virtio_blk_discard_write_zeroes_complete(void *opaque, int ret) + { + VirtIOBlockReq *req = opaque; + VirtIOBlock *s = req->dev; +- bool is_write_zeroes = (virtio_ldl_p(VIRTIO_DEVICE(s), &req->out.type) & +- ~VIRTIO_BLK_T_BARRIER) == VIRTIO_BLK_T_WRITE_ZEROES; + +- if (ret && virtio_blk_handle_rw_error(req, -ret, false, is_write_zeroes)) { ++ if (ret && virtio_blk_handle_rw_error(req, -ret, false)) { + return; + } + + virtio_blk_req_complete(req, VIRTIO_BLK_S_OK); +- if (is_write_zeroes) { +- block_acct_done(blk_get_stats(s->blk), &req->acct); +- } ++ block_acct_done(blk_get_stats(s->blk), &req->acct); + g_free(req); + } + +@@ -443,6 +437,9 @@ static uint8_t virtio_blk_handle_discard_write_zeroes(VirtIOBlockReq *req, + goto err; + } + ++ block_acct_start(blk_get_stats(s->blk), &req->acct, bytes, ++ BLOCK_ACCT_UNMAP); ++ + blk_aio_pdiscard(s->blk, sector << BDRV_SECTOR_BITS, bytes, + virtio_blk_discard_write_zeroes_complete, req); + } +@@ -450,9 +447,8 @@ static uint8_t virtio_blk_handle_discard_write_zeroes(VirtIOBlockReq *req, + return VIRTIO_BLK_S_OK; + + err: +- if (is_write_zeroes) { +- block_acct_invalid(blk_get_stats(s->blk), BLOCK_ACCT_WRITE); +- } ++ block_acct_invalid(blk_get_stats(s->blk), ++ is_write_zeroes ? BLOCK_ACCT_WRITE : BLOCK_ACCT_UNMAP); + return err_status; + } + +-- +GitLab + From c56853e4f9413c30a9fccb489efce23d6333e053 Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Thu, 1 Oct 2026 12:22:15 -0300 Subject: [PATCH 2/3] boot: only the probe that trims asks QEMU to have counted discards Co-Authored-By: Claude Opus 5.5 (1M context) --- boot/pagecache_test.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/boot/pagecache_test.go b/boot/pagecache_test.go index e7089fb..4376644 100644 --- a/boot/pagecache_test.go +++ b/boot/pagecache_test.go @@ -188,6 +188,9 @@ type cacheProbe struct { steps []string script string variants []cacheVariant + // trims is a script that ends in fstrim after removing what it wrote, so QEMU must have + // counted discards. + trims bool } // cacheVariant is one way of starting the machine a probe runs in. @@ -233,6 +236,7 @@ t=$(ms); dd if=/dev/zero of=$f bs=1M count=%[1]d conv=fsync status=none || { ech // host's page cache or, with the overlay O_DIRECT, the device. var cachesProbe = cacheProbe{ title: "written, read and removed", + trims: true, steps: []string{ "idle", "file written", @@ -618,7 +622,7 @@ func cacheBoot(t *testing.T, out string, v cacheVariant, fileMB int, probe cache } // A scope's QEMU is root's, and so is its socket. if scope == "" { - run.notes = append(run.notes, discardNote(t, out, qmpSock, overlay)) + run.notes = append(run.notes, discardNote(t, out, qmpSock, overlay, probe.trims)) } if cgroup != "" { raw, err := os.ReadFile(cgroup + "/memory.events") @@ -632,8 +636,8 @@ func cacheBoot(t *testing.T, out string, v cacheVariant, fileMB int, probe cache // discardNote is where the guest's discards went: how many QEMU's block layer took from the // guest (query-blockstats' unmap counters), and what the overlay's clusters are now by qemu-img -// map - data, zeroes, or nothing allocated. The guest trimmed its free space just before. -func discardNote(t *testing.T, out, socket, overlay string) string { +// map - data, zeroes, or nothing allocated. trimmed is a guest that ran fstrim just before. +func discardNote(t *testing.T, out, socket, overlay string, trimmed bool) string { t.Helper() conn, err := net.Dial("unix", socket) if err != nil { @@ -669,9 +673,9 @@ func discardNote(t *testing.T, out, socket, overlay string) string { unmap += fmt.Sprintf(" %d ops %d MB (%d failed, %d invalid), beside %d writes %d MB", s.Stats.UnmapOperations, s.Stats.UnmapBytes>>20, s.Stats.FailedUnmap, s.Stats.InvalidUnmap, s.Stats.WrOperations, s.Stats.WrBytes>>20) } - // The guest has just trimmed more than a gigabyte. Upstream virtio-blk counted none of it - // until qemu/patches/0002; zero here is a QEMU built without that patch. - if unmapOps == 0 { + // The caches probe trims more than a gigabyte. Upstream virtio-blk counted none of it until + // qemu/patches/0002; zero there is a QEMU built without that patch. + if trimmed && unmapOps == 0 { t.Errorf("QEMU counted no discard after the guest's fstrim:%s", unmap) } // -U: the image is open in the running QEMU, and this only reads its tables. From 9360df1f7142d4724e584b0b3db0bfe6f5968f7f Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Thu, 1 Oct 2026 12:22:55 -0300 Subject: [PATCH 3/3] qemu: drop the mbox line that pinned upstream's commit outside versions.yaml Co-Authored-By: Claude Opus 5.5 (1M context) --- qemu/patches/0002-hw-virtio-blk-account-discard-operations.patch | 1 - 1 file changed, 1 deletion(-) diff --git a/qemu/patches/0002-hw-virtio-blk-account-discard-operations.patch b/qemu/patches/0002-hw-virtio-blk-account-discard-operations.patch index 2d5d063..1262e6a 100644 --- a/qemu/patches/0002-hw-virtio-blk-account-discard-operations.patch +++ b/qemu/patches/0002-hw-virtio-blk-account-discard-operations.patch @@ -1,4 +1,3 @@ -From 331ce936c13359474cadcbbd548e018197fde426 Mon Sep 17 00:00:00 2001 From: Hanna Czenczek Date: Fri, 24 Jul 2026 17:23:10 +0200 Subject: [PATCH] hw/virtio-blk: Account discard operations