qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v7 10/13] qcow2: Rebuild refcount structure duri


From: Max Reitz
Subject: Re: [Qemu-devel] [PATCH v7 10/13] qcow2: Rebuild refcount structure during check
Date: Wed, 22 Oct 2014 11:15:48 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0

On 2014-10-22 at 11:14, Kevin Wolf wrote:
Am 22.10.2014 um 10:08 hat Max Reitz geschrieben:
The previous commit introduced the "rebuild" variable to qcow2's
implementation of the image consistency check. Now make use of this by
adding a function which creates a completely new refcount structure
based solely on the in-memory information gathered before.

The old refcount structure will be leaked, however. This leak will be
dealt with in a follow-up commit.

Signed-off-by: Max Reitz <address@hidden>
---
  block/qcow2-refcount.c | 310 ++++++++++++++++++++++++++++++++++++++++++++++++-
  1 file changed, 304 insertions(+), 6 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 101d7bb..8ca2324 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1640,6 +1640,284 @@ static void compare_refcounts(BlockDriverState *bs, 
BdrvCheckResult *res,
  }
/*
+ * Allocates clusters using an in-memory refcount table (IMRT) in contrast to
+ * the on-disk refcount structures.
+ *
+ * On input, *first_free_cluster tells where to start looking, and need not
+ * actually be a free cluster; the returned offset will not be before that
+ * cluster.  On output, *first_free_cluster points to the first gap found, even
+ * if that gap was too small to be used as the returned offset.
+ *
+ * Note that *first_free_cluster is a cluster index whereas the return value is
+ * an offset.
+ */
+static int64_t alloc_clusters_imrt(BlockDriverState *bs,
+                                   int cluster_count,
+                                   uint16_t **refcount_table,
+                                   int64_t *imrt_nb_clusters,
+                                   int64_t *first_free_cluster)
+{
+    BDRVQcowState *s = bs->opaque;
+    int64_t cluster = *first_free_cluster, i;
+    bool first_gap = true;
+    int contiguous_free_clusters;
+
+    /* Starting at *first_free_cluster, find a range of at least cluster_count
+     * continuously free clusters */
+    for (contiguous_free_clusters = 0;
+         cluster < *imrt_nb_clusters &&
+         contiguous_free_clusters < cluster_count;
+         cluster++)
+    {
+        if (!(*refcount_table)[cluster]) {
+            contiguous_free_clusters++;
+            if (first_gap) {
+                /* If this is the first free cluster found, update
+                 * *first_free_cluster accordingly */
+                *first_free_cluster = cluster;
+                first_gap = false;
+            }
+        } else if (contiguous_free_clusters) {
+            contiguous_free_clusters = 0;
+        }
+    }
+
+    /* If contiguous_free_clusters is greater than zero, it contains the number
+     * of continuously free clusters until the current cluster; the first free
+     * cluster in the current "gap" is therefore
+     * cluster - contiguous_free_clusters */
+
+    /* If no such range could be found, grow the in-memory refcount table
+     * accordingly to append free clusters at the end of the image */
+    if (contiguous_free_clusters < cluster_count) {
+        int64_t old_imrt_nb_clusters = *imrt_nb_clusters;
+        uint16_t *new_refcount_table;
+
+        /* contiguous_free_clusters clusters are already empty at the image 
end;
+         * we need cluster_count clusters; therefore, we have to allocate
+         * cluster_count - contiguous_free_clusters new clusters at the end of
+         * the image (which is the current value of cluster; note that cluster
+         * may exceed old_imrt_nb_clusters if *first_free_cluster pointed 
beyond
+         * the image end) */
+        *imrt_nb_clusters = cluster + cluster_count - contiguous_free_clusters;
+        new_refcount_table = g_try_realloc(*refcount_table,
+                                           *imrt_nb_clusters *
+                                           sizeof(**refcount_table));
+        if (!new_refcount_table) {
+            return -ENOMEM;
Now the old refcount table is preserved, but nb_clusters has already
been updated. Smells like the next thing will be a buffer overflow,
doesn't it?

If the caller continues after ENOMEM, well, yes. I'd call it his own fault, but I'll fix it of course.

Max

+        }
+        *refcount_table = new_refcount_table;
+
+        memset(*refcount_table + old_imrt_nb_clusters, 0,
+               (*imrt_nb_clusters - old_imrt_nb_clusters) *
+               sizeof(**refcount_table));
+    }
Kevin




reply via email to

[Prev in Thread] Current Thread [Next in Thread]