diff mbox

[for-2.0] qcow2: link all L2 meta updates in preallocate()

Message ID 1396343577-10563-1-git-send-email-stefanha@redhat.com
State New
Headers show

Commit Message

Stefan Hajnoczi April 1, 2014, 9:12 a.m. UTC
preallocate() only links the first QCowL2Meta's data clusters into the
L2 table and ignores any chained QCowL2Metas in the linked list.

Chains of QCowL2Meta structs are built up when contiguous clusters span
L2 tables.  Each QCowL2Meta describes one L2 table update.  This is a
rare case in preallocate() but can happen.

This patch fixes preallocate() by iterating over the whole list of
QCowL2Metas.  Compare with the qcow2_co_writev() function's
implementation, which is similar but also also handles request
dependencies.  preallocate() only performs one allocation at a time so
there can be no dependencies.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/qcow2.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Stefan Hajnoczi April 1, 2014, 1:48 p.m. UTC | #1
On Tue, Apr 01, 2014 at 11:12:57AM +0200, Stefan Hajnoczi wrote:
> preallocate() only links the first QCowL2Meta's data clusters into the
> L2 table and ignores any chained QCowL2Metas in the linked list.
> 
> Chains of QCowL2Meta structs are built up when contiguous clusters span
> L2 tables.  Each QCowL2Meta describes one L2 table update.  This is a
> rare case in preallocate() but can happen.
> 
> This patch fixes preallocate() by iterating over the whole list of
> QCowL2Metas.  Compare with the qcow2_co_writev() function's
> implementation, which is similar but also also handles request
> dependencies.  preallocate() only performs one allocation at a time so
> there can be no dependencies.
> 
> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  block/qcow2.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)

Discussed with Kevin and applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan
diff mbox

Patch

diff --git a/block/qcow2.c b/block/qcow2.c
index b9dc960..8994e03 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1432,7 +1432,9 @@  static int preallocate(BlockDriverState *bs)
             return ret;
         }
 
-        if (meta != NULL) {
+        while (meta) {
+            QCowL2Meta *next = meta->next;
+
             ret = qcow2_alloc_cluster_link_l2(bs, meta);
             if (ret < 0) {
                 qcow2_free_any_clusters(bs, meta->alloc_offset,
@@ -1443,6 +1445,9 @@  static int preallocate(BlockDriverState *bs)
             /* There are no dependent requests, but we need to remove our
              * request from the list of in-flight requests */
             QLIST_REMOVE(meta, next_in_flight);
+
+            g_free(meta);
+            meta = next;
         }
 
         /* TODO Preallocate data if requested */