0727d3
From be54c6206b0f0a19e0ffe6a058f4f97277027a17 Mon Sep 17 00:00:00 2001
0727d3
From: Hanna Reitz <hreitz@redhat.com>
0727d3
Date: Tue, 5 Apr 2022 15:46:50 +0200
0727d3
Subject: [PATCH 3/6] qcow2: Improve refcount structure rebuilding
0727d3
0727d3
RH-Author: Hanna Reitz <hreitz@redhat.com>
0727d3
RH-MergeRequest: 171: qcow2: Improve refcount structure rebuilding
0727d3
RH-Commit: [1/4] 0bb78f7735a0730204670ae5ec2e040ad1d23942
0727d3
RH-Bugzilla: 1519071
0727d3
RH-Acked-by: Miroslav Rezanina <mrezanin@redhat.com>
0727d3
RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
0727d3
RH-Acked-by: Eric Blake <eblake@redhat.com>
0727d3
0727d3
When rebuilding the refcount structures (when qemu-img check -r found
0727d3
errors with refcount = 0, but reference count > 0), the new refcount
0727d3
table defaults to being put at the image file end[1].  There is no good
0727d3
reason for that except that it means we will not have to rewrite any
0727d3
refblocks we already wrote to disk.
0727d3
0727d3
Changing the code to rewrite those refblocks is not too difficult,
0727d3
though, so let us do that.  That is beneficial for images on block
0727d3
devices, where we cannot really write beyond the end of the image file.
0727d3
0727d3
Use this opportunity to add extensive comments to the code, and refactor
0727d3
it a bit, getting rid of the backwards-jumping goto.
0727d3
0727d3
[1] Unless there is something allocated in the area pointed to by the
0727d3
    last refblock, so we have to write that refblock.  In that case, we
0727d3
    try to put the reftable in there.
0727d3
0727d3
Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1519071
0727d3
Closes: https://gitlab.com/qemu-project/qemu/-/issues/941
0727d3
Reviewed-by: Eric Blake <eblake@redhat.com>
0727d3
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
0727d3
Message-Id: <20220405134652.19278-2-hreitz@redhat.com>
0727d3
(cherry picked from commit a8c07ec287554dcefd33733f0e5888a281ddc95e)
0727d3
Signed-off-by: Hanna Reitz <hreitz@redhat.com>
0727d3
---
0727d3
 block/qcow2-refcount.c | 332 +++++++++++++++++++++++++++++------------
0727d3
 1 file changed, 235 insertions(+), 97 deletions(-)
0727d3
0727d3
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
0727d3
index 4614572252..555d8ba5ac 100644
0727d3
--- a/block/qcow2-refcount.c
0727d3
+++ b/block/qcow2-refcount.c
0727d3
@@ -2435,111 +2435,140 @@ static int64_t alloc_clusters_imrt(BlockDriverState *bs,
0727d3
 }
0727d3
 
0727d3
 /*
0727d3
- * Creates a new refcount structure based solely on the in-memory information
0727d3
- * given through *refcount_table. All necessary allocations will be reflected
0727d3
- * in that array.
0727d3
+ * Helper function for rebuild_refcount_structure().
0727d3
  *
0727d3
- * On success, the old refcount structure is leaked (it will be covered by the
0727d3
- * new refcount structure).
0727d3
+ * Scan the range of clusters [first_cluster, end_cluster) for allocated
0727d3
+ * clusters and write all corresponding refblocks to disk.  The refblock
0727d3
+ * and allocation data is taken from the in-memory refcount table
0727d3
+ * *refcount_table[] (of size *nb_clusters), which is basically one big
0727d3
+ * (unlimited size) refblock for the whole image.
0727d3
+ *
0727d3
+ * For these refblocks, clusters are allocated using said in-memory
0727d3
+ * refcount table.  Care is taken that these allocations are reflected
0727d3
+ * in the refblocks written to disk.
0727d3
+ *
0727d3
+ * The refblocks' offsets are written into a reftable, which is
0727d3
+ * *on_disk_reftable_ptr[] (of size *on_disk_reftable_entries_ptr).  If
0727d3
+ * that reftable is of insufficient size, it will be resized to fit.
0727d3
+ * This reftable is not written to disk.
0727d3
+ *
0727d3
+ * (If *on_disk_reftable_ptr is not NULL, the entries within are assumed
0727d3
+ * to point to existing valid refblocks that do not need to be allocated
0727d3
+ * again.)
0727d3
+ *
0727d3
+ * Return whether the on-disk reftable array was resized (true/false),
0727d3
+ * or -errno on error.
0727d3
  */
0727d3
-static int rebuild_refcount_structure(BlockDriverState *bs,
0727d3
-                                      BdrvCheckResult *res,
0727d3
-                                      void **refcount_table,
0727d3
-                                      int64_t *nb_clusters)
0727d3
+static int rebuild_refcounts_write_refblocks(
0727d3
+        BlockDriverState *bs, void **refcount_table, int64_t *nb_clusters,
0727d3
+        int64_t first_cluster, int64_t end_cluster,
0727d3
+        uint64_t **on_disk_reftable_ptr, uint32_t *on_disk_reftable_entries_ptr
0727d3
+    )
0727d3
 {
0727d3
     BDRVQcow2State *s = bs->opaque;
0727d3
-    int64_t first_free_cluster = 0, reftable_offset = -1, cluster = 0;
0727d3
+    int64_t cluster;
0727d3
     int64_t refblock_offset, refblock_start, refblock_index;
0727d3
-    uint32_t reftable_size = 0;
0727d3
-    uint64_t *on_disk_reftable = NULL;
0727d3
+    int64_t first_free_cluster = 0;
0727d3
+    uint64_t *on_disk_reftable = *on_disk_reftable_ptr;
0727d3
+    uint32_t on_disk_reftable_entries = *on_disk_reftable_entries_ptr;
0727d3
     void *on_disk_refblock;
0727d3
-    int ret = 0;
0727d3
-    struct {
0727d3
-        uint64_t reftable_offset;
0727d3
-        uint32_t reftable_clusters;
0727d3
-    } QEMU_PACKED reftable_offset_and_clusters;
0727d3
-
0727d3
-    qcow2_cache_empty(bs, s->refcount_block_cache);
0727d3
+    bool reftable_grown = false;
0727d3
+    int ret;
0727d3
 
0727d3
-write_refblocks:
0727d3
-    for (; cluster < *nb_clusters; cluster++) {
0727d3
+    for (cluster = first_cluster; cluster < end_cluster; cluster++) {
0727d3
+        /* Check all clusters to find refblocks that contain non-zero entries */
0727d3
         if (!s->get_refcount(*refcount_table, cluster)) {
0727d3
             continue;
0727d3
         }
0727d3
 
0727d3
+        /*
0727d3
+         * This cluster is allocated, so we need to create a refblock
0727d3
+         * for it.  The data we will write to disk is just the
0727d3
+         * respective slice from *refcount_table, so it will contain
0727d3
+         * accurate refcounts for all clusters belonging to this
0727d3
+         * refblock.  After we have written it, we will therefore skip
0727d3
+         * all remaining clusters in this refblock.
0727d3
+         */
0727d3
+
0727d3
         refblock_index = cluster >> s->refcount_block_bits;
0727d3
         refblock_start = refblock_index << s->refcount_block_bits;
0727d3
 
0727d3
-        /* Don't allocate a cluster in a refblock already written to disk */
0727d3
-        if (first_free_cluster < refblock_start) {
0727d3
-            first_free_cluster = refblock_start;
0727d3
-        }
0727d3
-        refblock_offset = alloc_clusters_imrt(bs, 1, refcount_table,
0727d3
-                                              nb_clusters, &first_free_cluster);
0727d3
-        if (refblock_offset < 0) {
0727d3
-            fprintf(stderr, "ERROR allocating refblock: %s\n",
0727d3
-                    strerror(-refblock_offset));
0727d3
-            res->check_errors++;
0727d3
-            ret = refblock_offset;
0727d3
-            goto fail;
0727d3
-        }
0727d3
+        if (on_disk_reftable_entries > refblock_index &&
0727d3
+            on_disk_reftable[refblock_index])
0727d3
+        {
0727d3
+            /*
0727d3
+             * We can get here after a `goto write_refblocks`: We have a
0727d3
+             * reftable from a previous run, and the refblock is already
0727d3
+             * allocated.  No need to allocate it again.
0727d3
+             */
0727d3
+            refblock_offset = on_disk_reftable[refblock_index];
0727d3
+        } else {
0727d3
+            int64_t refblock_cluster_index;
0727d3
 
0727d3
-        if (reftable_size <= refblock_index) {
0727d3
-            uint32_t old_reftable_size = reftable_size;
0727d3
-            uint64_t *new_on_disk_reftable;
0727d3
+            /* Don't allocate a cluster in a refblock already written to disk */
0727d3
+            if (first_free_cluster < refblock_start) {
0727d3
+                first_free_cluster = refblock_start;
0727d3
+            }
0727d3
+            refblock_offset = alloc_clusters_imrt(bs, 1, refcount_table,
0727d3
+                                                  nb_clusters,
0727d3
+                                                  &first_free_cluster);
0727d3
+            if (refblock_offset < 0) {
0727d3
+                fprintf(stderr, "ERROR allocating refblock: %s\n",
0727d3
+                        strerror(-refblock_offset));
0727d3
+                return refblock_offset;
0727d3
+            }
0727d3
 
0727d3
-            reftable_size = ROUND_UP((refblock_index + 1) * REFTABLE_ENTRY_SIZE,
0727d3
-                                     s->cluster_size) / REFTABLE_ENTRY_SIZE;
0727d3
-            new_on_disk_reftable = g_try_realloc(on_disk_reftable,
0727d3
-                                                 reftable_size *
0727d3
-                                                 REFTABLE_ENTRY_SIZE);
0727d3
-            if (!new_on_disk_reftable) {
0727d3
-                res->check_errors++;
0727d3
-                ret = -ENOMEM;
0727d3
-                goto fail;
0727d3
+            refblock_cluster_index = refblock_offset / s->cluster_size;
0727d3
+            if (refblock_cluster_index >= end_cluster) {
0727d3
+                /*
0727d3
+                 * We must write the refblock that holds this refblock's
0727d3
+                 * refcount
0727d3
+                 */
0727d3
+                end_cluster = refblock_cluster_index + 1;
0727d3
             }
0727d3
-            on_disk_reftable = new_on_disk_reftable;
0727d3
 
0727d3
-            memset(on_disk_reftable + old_reftable_size, 0,
0727d3
-                   (reftable_size - old_reftable_size) * REFTABLE_ENTRY_SIZE);
0727d3
+            if (on_disk_reftable_entries <= refblock_index) {
0727d3
+                on_disk_reftable_entries =
0727d3
+                    ROUND_UP((refblock_index + 1) * REFTABLE_ENTRY_SIZE,
0727d3
+                             s->cluster_size) / REFTABLE_ENTRY_SIZE;
0727d3
+                on_disk_reftable =
0727d3
+                    g_try_realloc(on_disk_reftable,
0727d3
+                                  on_disk_reftable_entries *
0727d3
+                                  REFTABLE_ENTRY_SIZE);
0727d3
+                if (!on_disk_reftable) {
0727d3
+                    return -ENOMEM;
0727d3
+                }
0727d3
 
0727d3
-            /* The offset we have for the reftable is now no longer valid;
0727d3
-             * this will leak that range, but we can easily fix that by running
0727d3
-             * a leak-fixing check after this rebuild operation */
0727d3
-            reftable_offset = -1;
0727d3
-        } else {
0727d3
-            assert(on_disk_reftable);
0727d3
-        }
0727d3
-        on_disk_reftable[refblock_index] = refblock_offset;
0727d3
+                memset(on_disk_reftable + *on_disk_reftable_entries_ptr, 0,
0727d3
+                       (on_disk_reftable_entries -
0727d3
+                        *on_disk_reftable_entries_ptr) *
0727d3
+                       REFTABLE_ENTRY_SIZE);
0727d3
 
0727d3
-        /* If this is apparently the last refblock (for now), try to squeeze the
0727d3
-         * reftable in */
0727d3
-        if (refblock_index == (*nb_clusters - 1) >> s->refcount_block_bits &&
0727d3
-            reftable_offset < 0)
0727d3
-        {
0727d3
-            uint64_t reftable_clusters = size_to_clusters(s, reftable_size *
0727d3
-                                                          REFTABLE_ENTRY_SIZE);
0727d3
-            reftable_offset = alloc_clusters_imrt(bs, reftable_clusters,
0727d3
-                                                  refcount_table, nb_clusters,
0727d3
-                                                  &first_free_cluster);
0727d3
-            if (reftable_offset < 0) {
0727d3
-                fprintf(stderr, "ERROR allocating reftable: %s\n",
0727d3
-                        strerror(-reftable_offset));
0727d3
-                res->check_errors++;
0727d3
-                ret = reftable_offset;
0727d3
-                goto fail;
0727d3
+                *on_disk_reftable_ptr = on_disk_reftable;
0727d3
+                *on_disk_reftable_entries_ptr = on_disk_reftable_entries;
0727d3
+
0727d3
+                reftable_grown = true;
0727d3
+            } else {
0727d3
+                assert(on_disk_reftable);
0727d3
             }
0727d3
+            on_disk_reftable[refblock_index] = refblock_offset;
0727d3
         }
0727d3
 
0727d3
+        /* Refblock is allocated, write it to disk */
0727d3
+
0727d3
         ret = qcow2_pre_write_overlap_check(bs, 0, refblock_offset,
0727d3
                                             s->cluster_size, false);
0727d3
         if (ret < 0) {
0727d3
             fprintf(stderr, "ERROR writing refblock: %s\n", strerror(-ret));
0727d3
-            goto fail;
0727d3
+            return ret;
0727d3
         }
0727d3
 
0727d3
-        /* The size of *refcount_table is always cluster-aligned, therefore the
0727d3
-         * write operation will not overflow */
0727d3
+        /*
0727d3
+         * The refblock is simply a slice of *refcount_table.
0727d3
+         * Note that the size of *refcount_table is always aligned to
0727d3
+         * whole clusters, so the write operation will not result in
0727d3
+         * out-of-bounds accesses.
0727d3
+         */
0727d3
         on_disk_refblock = (void *)((char *) *refcount_table +
0727d3
                                     refblock_index * s->cluster_size);
0727d3
 
0727d3
@@ -2547,23 +2576,99 @@ write_refblocks:
0727d3
                           s->cluster_size);
0727d3
         if (ret < 0) {
0727d3
             fprintf(stderr, "ERROR writing refblock: %s\n", strerror(-ret));
0727d3
-            goto fail;
0727d3
+            return ret;
0727d3
         }
0727d3
 
0727d3
-        /* Go to the end of this refblock */
0727d3
+        /* This refblock is done, skip to its end */
0727d3
         cluster = refblock_start + s->refcount_block_size - 1;
0727d3
     }
0727d3
 
0727d3
-    if (reftable_offset < 0) {
0727d3
-        uint64_t post_refblock_start, reftable_clusters;
0727d3
+    return reftable_grown;
0727d3
+}
0727d3
+
0727d3
+/*
0727d3
+ * Creates a new refcount structure based solely on the in-memory information
0727d3
+ * given through *refcount_table (this in-memory information is basically just
0727d3
+ * the concatenation of all refblocks).  All necessary allocations will be
0727d3
+ * reflected in that array.
0727d3
+ *
0727d3
+ * On success, the old refcount structure is leaked (it will be covered by the
0727d3
+ * new refcount structure).
0727d3
+ */
0727d3
+static int rebuild_refcount_structure(BlockDriverState *bs,
0727d3
+                                      BdrvCheckResult *res,
0727d3
+                                      void **refcount_table,
0727d3
+                                      int64_t *nb_clusters)
0727d3
+{
0727d3
+    BDRVQcow2State *s = bs->opaque;
0727d3
+    int64_t reftable_offset = -1;
0727d3
+    int64_t reftable_length = 0;
0727d3
+    int64_t reftable_clusters;
0727d3
+    int64_t refblock_index;
0727d3
+    uint32_t on_disk_reftable_entries = 0;
0727d3
+    uint64_t *on_disk_reftable = NULL;
0727d3
+    int ret = 0;
0727d3
+    int reftable_size_changed = 0;
0727d3
+    struct {
0727d3
+        uint64_t reftable_offset;
0727d3
+        uint32_t reftable_clusters;
0727d3
+    } QEMU_PACKED reftable_offset_and_clusters;
0727d3
+
0727d3
+    qcow2_cache_empty(bs, s->refcount_block_cache);
0727d3
+
0727d3
+    /*
0727d3
+     * For each refblock containing entries, we try to allocate a
0727d3
+     * cluster (in the in-memory refcount table) and write its offset
0727d3
+     * into on_disk_reftable[].  We then write the whole refblock to
0727d3
+     * disk (as a slice of the in-memory refcount table).
0727d3
+     * This is done by rebuild_refcounts_write_refblocks().
0727d3
+     *
0727d3
+     * Once we have scanned all clusters, we try to find space for the
0727d3
+     * reftable.  This will dirty the in-memory refcount table (i.e.
0727d3
+     * make it differ from the refblocks we have already written), so we
0727d3
+     * need to run rebuild_refcounts_write_refblocks() again for the
0727d3
+     * range of clusters where the reftable has been allocated.
0727d3
+     *
0727d3
+     * This second run might make the reftable grow again, in which case
0727d3
+     * we will need to allocate another space for it, which is why we
0727d3
+     * repeat all this until the reftable stops growing.
0727d3
+     *
0727d3
+     * (This loop will terminate, because with every cluster the
0727d3
+     * reftable grows, it can accomodate a multitude of more refcounts,
0727d3
+     * so that at some point this must be able to cover the reftable
0727d3
+     * and all refblocks describing it.)
0727d3
+     *
0727d3
+     * We then convert the reftable to big-endian and write it to disk.
0727d3
+     *
0727d3
+     * Note that we never free any reftable allocations.  Doing so would
0727d3
+     * needlessly complicate the algorithm: The eventual second check
0727d3
+     * run we do will clean up all leaks we have caused.
0727d3
+     */
0727d3
+
0727d3
+    reftable_size_changed =
0727d3
+        rebuild_refcounts_write_refblocks(bs, refcount_table, nb_clusters,
0727d3
+                                          0, *nb_clusters,
0727d3
+                                          &on_disk_reftable,
0727d3
+                                          &on_disk_reftable_entries);
0727d3
+    if (reftable_size_changed < 0) {
0727d3
+        res->check_errors++;
0727d3
+        ret = reftable_size_changed;
0727d3
+        goto fail;
0727d3
+    }
0727d3
+
0727d3
+    /*
0727d3
+     * There was no reftable before, so rebuild_refcounts_write_refblocks()
0727d3
+     * must have increased its size (from 0 to something).
0727d3
+     */
0727d3
+    assert(reftable_size_changed);
0727d3
+
0727d3
+    do {
0727d3
+        int64_t reftable_start_cluster, reftable_end_cluster;
0727d3
+        int64_t first_free_cluster = 0;
0727d3
+
0727d3
+        reftable_length = on_disk_reftable_entries * REFTABLE_ENTRY_SIZE;
0727d3
+        reftable_clusters = size_to_clusters(s, reftable_length);
0727d3
 
0727d3
-        post_refblock_start = ROUND_UP(*nb_clusters, s->refcount_block_size);
0727d3
-        reftable_clusters =
0727d3
-            size_to_clusters(s, reftable_size * REFTABLE_ENTRY_SIZE);
0727d3
-        /* Not pretty but simple */
0727d3
-        if (first_free_cluster < post_refblock_start) {
0727d3
-            first_free_cluster = post_refblock_start;
0727d3
-        }
0727d3
         reftable_offset = alloc_clusters_imrt(bs, reftable_clusters,
0727d3
                                               refcount_table, nb_clusters,
0727d3
                                               &first_free_cluster);
0727d3
@@ -2575,24 +2680,55 @@ write_refblocks:
0727d3
             goto fail;
0727d3
         }
0727d3
 
0727d3
-        goto write_refblocks;
0727d3
-    }
0727d3
+        /*
0727d3
+         * We need to update the affected refblocks, so re-run the
0727d3
+         * write_refblocks loop for the reftable's range of clusters.
0727d3
+         */
0727d3
+        assert(offset_into_cluster(s, reftable_offset) == 0);
0727d3
+        reftable_start_cluster = reftable_offset / s->cluster_size;
0727d3
+        reftable_end_cluster = reftable_start_cluster + reftable_clusters;
0727d3
+        reftable_size_changed =
0727d3
+            rebuild_refcounts_write_refblocks(bs, refcount_table, nb_clusters,
0727d3
+                                              reftable_start_cluster,
0727d3
+                                              reftable_end_cluster,
0727d3
+                                              &on_disk_reftable,
0727d3
+                                              &on_disk_reftable_entries);
0727d3
+        if (reftable_size_changed < 0) {
0727d3
+            res->check_errors++;
0727d3
+            ret = reftable_size_changed;
0727d3
+            goto fail;
0727d3
+        }
0727d3
+
0727d3
+        /*
0727d3
+         * If the reftable size has changed, we will need to find a new
0727d3
+         * allocation, repeating the loop.
0727d3
+         */
0727d3
+    } while (reftable_size_changed);
0727d3
 
0727d3
-    for (refblock_index = 0; refblock_index < reftable_size; refblock_index++) {
0727d3
+    /* The above loop must have run at least once */
0727d3
+    assert(reftable_offset >= 0);
0727d3
+
0727d3
+    /*
0727d3
+     * All allocations are done, all refblocks are written, convert the
0727d3
+     * reftable to big-endian and write it to disk.
0727d3
+     */
0727d3
+
0727d3
+    for (refblock_index = 0; refblock_index < on_disk_reftable_entries;
0727d3
+         refblock_index++)
0727d3
+    {
0727d3
         cpu_to_be64s(&on_disk_reftable[refblock_index]);
0727d3
     }
0727d3
 
0727d3
-    ret = qcow2_pre_write_overlap_check(bs, 0, reftable_offset,
0727d3
-                                        reftable_size * REFTABLE_ENTRY_SIZE,
0727d3
+    ret = qcow2_pre_write_overlap_check(bs, 0, reftable_offset, reftable_length,
0727d3
                                         false);
0727d3
     if (ret < 0) {
0727d3
         fprintf(stderr, "ERROR writing reftable: %s\n", strerror(-ret));
0727d3
         goto fail;
0727d3
     }
0727d3
 
0727d3
-    assert(reftable_size < INT_MAX / REFTABLE_ENTRY_SIZE);
0727d3
+    assert(reftable_length < INT_MAX);
0727d3
     ret = bdrv_pwrite(bs->file, reftable_offset, on_disk_reftable,
0727d3
-                      reftable_size * REFTABLE_ENTRY_SIZE);
0727d3
+                      reftable_length);
0727d3
     if (ret < 0) {
0727d3
         fprintf(stderr, "ERROR writing reftable: %s\n", strerror(-ret));
0727d3
         goto fail;
0727d3
@@ -2601,7 +2737,7 @@ write_refblocks:
0727d3
     /* Enter new reftable into the image header */
0727d3
     reftable_offset_and_clusters.reftable_offset = cpu_to_be64(reftable_offset);
0727d3
     reftable_offset_and_clusters.reftable_clusters =
0727d3
-        cpu_to_be32(size_to_clusters(s, reftable_size * REFTABLE_ENTRY_SIZE));
0727d3
+        cpu_to_be32(reftable_clusters);
0727d3
     ret = bdrv_pwrite_sync(bs->file,
0727d3
                            offsetof(QCowHeader, refcount_table_offset),
0727d3
                            &reftable_offset_and_clusters,
0727d3
@@ -2611,12 +2747,14 @@ write_refblocks:
0727d3
         goto fail;
0727d3
     }
0727d3
 
0727d3
-    for (refblock_index = 0; refblock_index < reftable_size; refblock_index++) {
0727d3
+    for (refblock_index = 0; refblock_index < on_disk_reftable_entries;
0727d3
+         refblock_index++)
0727d3
+    {
0727d3
         be64_to_cpus(&on_disk_reftable[refblock_index]);
0727d3
     }
0727d3
     s->refcount_table = on_disk_reftable;
0727d3
     s->refcount_table_offset = reftable_offset;
0727d3
-    s->refcount_table_size = reftable_size;
0727d3
+    s->refcount_table_size = on_disk_reftable_entries;
0727d3
     update_max_refcount_table_index(s);
0727d3
 
0727d3
     return 0;
0727d3
-- 
0727d3
2.27.0
0727d3