diff mbox series

[v3,4/5] block: allow commit to unmap zero blocks

Message ID 20240901142405.3183874-5-libvirt-e6954efa@volkihar.be (mailing list archive)
State New, archived
Headers show
Series block: allow commit to unmap zero blocks | expand

Commit Message

Vincent Vanlaer Sept. 1, 2024, 2:24 p.m. UTC
Non-active block commits do not discard blocks only containing zeros,
causing images to lose sparseness after the commit. This commit fixes
that by writing zero blocks using blk_co_pwrite_zeroes rather than
writing them out as any other arbitrary data.

Signed-off-by: Vincent Vanlaer <libvirt-e6954efa@volkihar.be>
---
 block/commit.c | 34 +++++++++++++++++++++++++---------
 1 file changed, 25 insertions(+), 9 deletions(-)

Comments

Vladimir Sementsov-Ogievskiy Oct. 2, 2024, 9:35 a.m. UTC | #1
On 01.09.24 17:24, Vincent Vanlaer wrote:
> Non-active block commits do not discard blocks only containing zeros,
> causing images to lose sparseness after the commit. This commit fixes
> that by writing zero blocks using blk_co_pwrite_zeroes rather than
> writing them out as any other arbitrary data.
> 
> Signed-off-by: Vincent Vanlaer<libvirt-e6954efa@volkihar.be>

Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
diff mbox series

Patch

diff --git a/block/commit.c b/block/commit.c
index 288e413be3..2594917a74 100644
--- a/block/commit.c
+++ b/block/commit.c
@@ -146,19 +146,35 @@  static int commit_iteration(CommitBlockJob *s, int64_t offset, int64_t *n, void
     trace_commit_one_iteration(s, offset, *n, ret);
 
     if (ret & BDRV_BLOCK_ALLOCATED) {
-        assert(*n < SIZE_MAX);
+        if (ret & BDRV_BLOCK_ZERO) {
+            /* If the top (sub)clusters are smaller than the base
+             * (sub)clusters, this will not unmap unless the underlying device
+             * does some tracking of these requests. Ideally, we would find
+             * the maximal extent of the zero clusters. */
+            ret = blk_co_pwrite_zeroes(s->base, offset, *n,
+                                       BDRV_REQ_MAY_UNMAP);
+            if (ret < 0) {
+                error_in_source = false;
+                goto handle_error;
+            }
+        } else {
+            assert(*n < SIZE_MAX);
 
-        ret = blk_co_pread(s->top, offset, *n, buf, 0);
-        if (ret < 0) {
-            goto handle_error;
-        }
+            ret = blk_co_pread(s->top, offset, *n, buf, 0);
+            if (ret < 0) {
+                goto handle_error;
+            }
 
-        ret = blk_co_pwrite(s->base, offset, *n, buf, 0);
-        if (ret < 0) {
-            error_in_source = false;
-            goto handle_error;
+            ret = blk_co_pwrite(s->base, offset, *n, buf, 0);
+            if (ret < 0) {
+                error_in_source = false;
+                goto handle_error;
+            }
         }
 
+        /* Whether zeroes actually end up on disk depends on the details of
+         * the underlying driver. Therefore, this might rate limit more than
+         * is necessary. */
         block_job_ratelimit_processed_bytes(&s->common, *n);
     }