qemu: carry upstream's discard accounting for virtio-blk

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 {
@@ -663,10 +667,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 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.
 	raw, err := exec.Command(filepath.Join(out, "bin", "qemu-img"), "map", "-U", "--output=json", overlay).Output()
 	if err != nil {

qemu/patches/0002-hw-virtio-blk-account-discard-operations.patch

@@ -0,0 +1,117 @@
+From: Hanna Czenczek <hreitz@redhat.com>
+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 <hreitz@redhat.com>
+Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
+Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
+Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
+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
+