[PATCH] PVE backup: vma reader: merge contiguous writes on restore

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

Restoring wrote every 64 KiB cluster, or every 4 KiB block of a
cluster with zero blocks, as a separate synchronous request. With the
page cache this mostly costs extra system calls, as writeback merges the
data again there. Without page cache – like cache=none that qemu-server
uses for restores to ZFS pools – each request goes to the storage on its
own and has to complete before vma continues reading the archive.

To improve this, queue writes that continue the previous one on the same
target and issue them as a single request once the target changes, a gap
occurs, or the extent is done, as the queued data points into the extent
buffer. The vma writer puts up to 58 of the 64 KiB clusters into an
extent, so a request can now cover about 3.6 MiB. Writes to each target
stay synchronous and in order, and a successful restore produces the
same data.

One small drawback: QEMU admits a throttled request as a whole and
delays the next one, so a bandwidth limit now takes effect in steps of
one such request instead of one cluster. Over a longer restore the
average rate stays the same, but it can be "burstier" if the limit is
less than ~30 Mbps (one 3.6 MiB batched write submitted). Might be
improvable, but not sure if its really worth the complexity.

A simple benchmark of restoring a 3 GiB image from a zstd-compressed
archive like qemu-server does (inside a VM with ext4 on a virtual disk)
took on average 45s instead of 66s for a sparse target and 38s instead
of 83s when also writing zeroes, both with cache=none; so a rather
noticable improvement. With the page cache, the before/after times
stayed within the measurement noise.

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
---

Patch on top of the current pve downstream series. This fell out of
reviewing the impact that Elias noticed in Markus' "{pbs-,vma-}restore:
disable caching for LVM-thin" series [0], where vma restore performed
worse after over PBS restore.

[0]: https://lore.proxmox.com/pve-devel/20260908123727.285848-1-m.frank@proxmox.com/

 vma-reader.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 70 insertions(+), 7 deletions(-)

diff --git a/vma-reader.c b/vma-reader.c
index 2bce1b9d70..4209d6dc45 100644
--- a/vma-reader.c
+++ b/vma-reader.c
@@ -49,6 +49,10 @@ struct VmaReader {
     int64_t zero_cluster_data;
     int64_t partial_zero_cluster_data;
     int clusters_read_per;
+    /* queued writes, covering one contiguous range of a single target */
+    BlockBackend *batch_target;
+    int64_t batch_offset;
+    QEMUIOVector batch_qiov;
 };
 
 static guint
@@ -210,6 +214,8 @@ void vma_reader_destroy(VmaReader *vmar)
         g_free(vmar->head_data);
     }
 
+    qemu_iovec_destroy(&vmar->batch_qiov);
+
     g_free(vmar);
 
 };
@@ -372,6 +378,7 @@ VmaReader *vma_reader_create(const char *filename, Error **errp)
     assert(errp);
 
     VmaReader *vmar = g_new0(VmaReader, 1);
+    qemu_iovec_init(&vmar->batch_qiov, 1);
 
     if (strcmp(filename, "-") == 0) {
         vmar->fd = dup(0);
@@ -523,6 +530,64 @@ static size_t full_write(int fd, void *buf, size_t len)
     return total;
 }
 
+static int restore_flush_writes(VmaReader *vmar, Error **errp)
+{
+    QEMUIOVector *qiov = &vmar->batch_qiov;
+    BlockBackend *target = vmar->batch_target;
+
+    if (!qiov->size) {
+        return 0;
+    }
+
+    int res = blk_pwritev(target, vmar->batch_offset, qiov->size, qiov, 0);
+    qemu_iovec_reset(qiov);
+    if (res < 0) {
+        bdrv_graph_rdlock_main_loop();
+        error_setg(errp, "blk_pwrite to %s failed (%d)",
+                   bdrv_get_device_name(blk_bs(target)), res);
+        bdrv_graph_rdunlock_main_loop();
+        return -1;
+    }
+
+    return 0;
+}
+
+/*
+ * Writing every cluster, or every 4 KiB block of a partial cluster, on its
+ * own is slow without the host page cache, which otherwise merges them. So
+ * extend the queued write as long as the data continues it on the same target.
+ */
+static int restore_queue_write(VmaReader *vmar, BlockBackend *target,
+                               unsigned char *buf, int64_t offset,
+                               size_t bytes, Error **errp)
+{
+    QEMUIOVector *qiov = &vmar->batch_qiov;
+
+    if (qiov->size && (target != vmar->batch_target ||
+                       offset != vmar->batch_offset + (int64_t)qiov->size ||
+                       qiov->niov >= blk_get_max_iov(target))) {
+        if (restore_flush_writes(vmar, errp) < 0) {
+            return -1;
+        }
+    }
+
+    if (!qiov->size) {
+        vmar->batch_target = target;
+        vmar->batch_offset = offset;
+    }
+
+    struct iovec *last = qiov->niov ? &qiov->iov[qiov->niov - 1] : NULL;
+    if (last && (unsigned char *)last->iov_base + last->iov_len == buf) {
+        /* adjacent in memory, keep the vector short */
+        last->iov_len += bytes;
+        qiov->size += bytes;
+    } else {
+        qemu_iovec_add(qiov, buf, bytes);
+    }
+
+    return 0;
+}
+
 static int restore_write_data(VmaReader *vmar, guint8 dev_id,
                               BlockBackend *target, int vmstate_fd,
                               unsigned char *buf, int64_t sector_num,
@@ -540,12 +605,9 @@ static int restore_write_data(VmaReader *vmar, guint8 dev_id,
             }
         }
     } else {
-        int res = blk_pwrite(target, sector_num * BDRV_SECTOR_SIZE, nb_sectors * BDRV_SECTOR_SIZE, buf, 0);
-        if (res < 0) {
-            bdrv_graph_rdlock_main_loop();
-            error_setg(errp, "blk_pwrite to %s failed (%d)",
-                       bdrv_get_device_name(blk_bs(target)), res);
-            bdrv_graph_rdunlock_main_loop();
+        if (restore_queue_write(vmar, target, buf,
+                                sector_num * BDRV_SECTOR_SIZE,
+                                nb_sectors * BDRV_SECTOR_SIZE, errp) < 0) {
             return -1;
         }
     }
@@ -720,7 +782,8 @@ static int restore_extent(VmaReader *vmar, unsigned char *buf,
         return -1;
     }
 
-    return 0;
+    /* queued writes point into the extent buffer, which gets reused */
+    return restore_flush_writes(vmar, errp);
 }
 
 static int vma_reader_restore_full(VmaReader *vmar, int vmstate_fd,
-- 
2.47.3