From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id 006781FF09B for ; Mon, 09 Nov 2026 17:02:28 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id D6B1F2158E; Mon, 09 Nov 2026 17:02:26 +0100 (CET) From: Thomas Lamprecht To: pve-devel@lists.proxmox.com Subject: [PATCH] PVE backup: vma reader: merge contiguous writes on restore Date: Mon, 9 Nov 2026 17:02:05 +0100 Message-ID: <20261109160210.3380582-1-t.lamprecht@proxmox.com> X-Mailer: git-send-email 2.47.3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1794240140945 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.276 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: JWYDZU7PFZW4DNZWYBOY7V5347CKEHUK X-Message-ID-Hash: JWYDZU7PFZW4DNZWYBOY7V5347CKEHUK X-MailFrom: t.lamprecht@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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 --- 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