[Qemu-devel] [PATCH] qcow2: Rewrite qcow2_alloc_bytes()

[Max Reitz]

qcow2_alloc_bytes() is a function with insufficient error handling and
an unnecessary goto. This patch rewrites it.

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

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 9afdb40..cafca0a 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -759,46 +759,51 @@ int qcow2_alloc_clusters_at(BlockDriverState *bs, 
uint64_t offset,
 int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size)
 {
     BDRVQcowState *s = bs->opaque;
-    int64_t offset, cluster_offset;
-    int free_in_cluster;
+    int64_t offset, new_cluster = 0, cluster_end;
+    size_t free_in_cluster;
 
     BLKDBG_EVENT(bs->file, BLKDBG_CLUSTER_ALLOC_BYTES);
     assert(size > 0 && size <= s->cluster_size);
-    if (s->free_byte_offset == 0) {
-        offset = qcow2_alloc_clusters(bs, s->cluster_size);
-        if (offset < 0) {
-            return offset;
+
+    if (s->free_byte_offset) {
+        int refcount = qcow2_get_refcount(bs,
+            s->free_byte_offset >> s->cluster_bits);
+        if (refcount < 0) {
+            return refcount;
+        }
+
+        if (refcount == 0xffff) {
+            s->free_byte_offset = 0;
         }
-        s->free_byte_offset = offset;
     }
- redo:
+
     free_in_cluster = s->cluster_size -
         offset_into_cluster(s, s->free_byte_offset);
-    if (size <= free_in_cluster) {
-        /* enough space in current cluster */
-        offset = s->free_byte_offset;
-        s->free_byte_offset += size;
-        free_in_cluster -= size;
-        if (free_in_cluster == 0)
-            s->free_byte_offset = 0;
-        if (offset_into_cluster(s, offset) != 0)
-            qcow2_update_cluster_refcount(bs, offset >> s->cluster_bits, 1,
-                                          QCOW2_DISCARD_NEVER);
-    } else {
-        offset = qcow2_alloc_clusters(bs, s->cluster_size);
-        if (offset < 0) {
-            return offset;
+
+    if (!s->free_byte_offset || free_in_cluster < size) {
+        new_cluster = qcow2_alloc_clusters(bs, s->cluster_size);
+        if (new_cluster < 0) {
+            return new_cluster;
         }
-        cluster_offset = start_of_cluster(s, s->free_byte_offset);
-        if ((cluster_offset + s->cluster_size) == offset) {
-            /* we are lucky: contiguous data */
-            offset = s->free_byte_offset;
-            qcow2_update_cluster_refcount(bs, offset >> s->cluster_bits, 1,
-                                          QCOW2_DISCARD_NEVER);
-            s->free_byte_offset += size;
-        } else {
-            s->free_byte_offset = offset;
-            goto redo;
+
+        cluster_end = start_of_cluster(s, s->free_byte_offset) +
+            s->cluster_size;
+
+        if (!s->free_cluster_index || cluster_end != new_cluster) {
+            s->free_byte_offset = new_cluster;
+        }
+    }
+
+    if (offset_into_cluster(s, s->free_byte_offset)) {
+        int ret = qcow2_update_cluster_refcount(bs,
+                s->free_byte_offset >> s->cluster_bits, 1,
+                QCOW2_DISCARD_NEVER);
+        if (ret < 0) {
+            if (new_cluster > 0) {
+                qcow2_free_clusters(bs, new_cluster, s->cluster_size,
+                                    QCOW2_DISCARD_OTHER);
+            }
+            return ret;
         }
     }
 
@@ -807,6 +812,14 @@ int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size)
      * be flushed before the caller's L2 table updates.
      */
     qcow2_cache_set_dependency(bs, s->l2_table_cache, s->refcount_block_cache);
+
+    offset = s->free_byte_offset;
+
+    s->free_byte_offset += size;
+    if (!offset_into_cluster(s, s->free_byte_offset)) {
+        s->free_byte_offset = 0;
+    }
+
     return offset;
 }
 
-- 
2.1.0