qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: anthony@codemonkey.ws
Cc: kwolf@redhat.com, qemu-devel@nongnu.org
Subject: [Qemu-devel] [PATCH 2/5] qcow2: Limit COW to where it's needed
Date: Mon,  7 May 2012 19:55:51 +0200	[thread overview]
Message-ID: <1336413354-5244-3-git-send-email-kwolf@redhat.com> (raw)
In-Reply-To: <1336413354-5244-1-git-send-email-kwolf@redhat.com>

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 <mtosatti@redhat.com>
Tested-by: Marcelo Tosatti <mtosatti@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2-cluster.c |   14 +++++++++-----
 1 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 353889d..10c22fe 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -883,15 +883,19 @@ again:
         assert(keep_clusters <= nb_clusters);
         nb_clusters -= keep_clusters;
     } else {
+        keep_clusters = 0;
+        cluster_offset = 0;
+    }
+
+    if (nb_clusters > 0) {
         /* For the moment, overwrite compressed clusters one by one */
-        if (cluster_offset & QCOW_OFLAG_COMPRESSED) {
+        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);
+            nb_clusters = count_cow_clusters(s, nb_clusters, l2_table,
+                                             l2_index + keep_clusters);
         }
-
-        keep_clusters = 0;
-        cluster_offset = 0;
     }
 
     cluster_offset &= L2E_OFFSET_MASK;
-- 
1.7.6.5

  parent reply	other threads:[~2012-05-07 17:56 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-07 17:55 [Qemu-devel] [PULL 0/5] Block patches for 1.1 Kevin Wolf
2012-05-07 17:55 ` [Qemu-devel] [PATCH 1/5] sheepdog: switch to writethrough mode if cluster doesn't support flush Kevin Wolf
2012-05-07 17:55 ` Kevin Wolf [this message]
2012-05-07 17:55 ` [Qemu-devel] [PATCH 3/5] block: make bdrv_create adopt coroutine Kevin Wolf
2012-05-07 17:55 ` [Qemu-devel] [PATCH 4/5] qcow2: lock on prealloc Kevin Wolf
2012-05-07 17:55 ` [Qemu-devel] [PATCH 5/5] fdc: simplify media change handling Kevin Wolf
2012-05-08 16:11 ` [Qemu-devel] [PULL 0/5] Block patches for 1.1 Anthony Liguori

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1336413354-5244-3-git-send-email-kwolf@redhat.com \
    --to=kwolf@redhat.com \
    --cc=anthony@codemonkey.ws \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).