From 54e6814360ab2110ed3ed07b2b9a3f9907e1202a Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Thu, 26 Apr 2012 19:41:22 +0200 Subject: [PATCH] qcow2: Limit COW to where it's needed This fixes a regression introduced in commit 250196f1. The bug leads to data corruption, found during an Autotest run with a Fedora 8 guest. Consider a write request whose first part is covered by an already allocated cluster, but additional clusters need to be newly allocated. When counting the number of clusters to allocate, the qcow2 code would decide to do COW for all remaining clusters of the write request, even if some of them are already allocated. If during this COW operation another write request is issued that touches the same cluster, it will still refer to the old cluster. When the COW completes, the first request will update the L2 table and the second write request will be lost. Note that the requests need not overlap, it's enough for them to touch the same cluster. This patch ensures that only clusters that really require COW are considered for allocation. In this case any other request writing to the same cluster will be an allocating write and gets serialised. Reported-by: Marcelo Tosatti Tested-by: Marcelo Tosatti Signed-off-by: Kevin Wolf --- block/qcow2-cluster.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index 353889d41b..10c22fe12b 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -883,17 +883,21 @@ again: assert(keep_clusters <= nb_clusters); nb_clusters -= keep_clusters; } else { - /* For the moment, overwrite compressed clusters one by one */ - if (cluster_offset & QCOW_OFLAG_COMPRESSED) { - nb_clusters = 1; - } else { - nb_clusters = count_cow_clusters(s, nb_clusters, l2_table, l2_index); - } - keep_clusters = 0; cluster_offset = 0; } + if (nb_clusters > 0) { + /* For the moment, overwrite compressed clusters one by one */ + uint64_t entry = be64_to_cpu(l2_table[l2_index + keep_clusters]); + if (entry & QCOW_OFLAG_COMPRESSED) { + nb_clusters = 1; + } else { + nb_clusters = count_cow_clusters(s, nb_clusters, l2_table, + l2_index + keep_clusters); + } + } + cluster_offset &= L2E_OFFSET_MASK; /*