Re: [Qemu-devel] [PATCH] qcow2: Fix refcount block allocation during qcow2_allocate_cluster_at()
Am 23.04.2012 20:30, schrieb Marcelo Tosatti: On Mon, Apr 23, 2012 at 02:33:49PM +0200, Kevin Wolf wrote: Am 23.04.2012 01:35, schrieb Marcelo Tosatti: On Sun, Apr 22, 2012 at 08:18:49PM -0300, Marcelo Tosatti wrote: On Fri, Apr 20, 2012 at 03:56:01PM +0200, Kevin Wolf wrote: Refcount block allocation and refcount table growth rely on s-free_cluster_index pointing to somewhere after the current allocation. Change qcow2_allocate_cluster_at() to fulfill this assumption. Without this change it could happen that a newly allocated refcount block and the allocated data block point to the same area in the image file, causing data corruption in the long run. This fixes a bug that became first visible after commit 250196f1. Signed-off-by: Kevin Wolf kw...@redhat.com Kevin, This patch fixes explicit filesystem errors (qemu-img check also OK), but autotest is still failing, see attached screenshot. It is not reproducible without f081987ad20a8c8dc391deded55161ea8d38be5f Sorry, i meant commit 250196f19c6e7df12965d74a5073e10aba06c802 Author: Kevin Wolf kw...@redhat.com Date: Fri Mar 2 14:10:54 2012 +0100 qcow2: Reduce number of I/O requests The screenshot doesn't really give a lot of information, but let's assume that _something_ must have been corrupted... Can you try finding the corrupted file (e.g. using rpm -V) and see in which way it differs from the real one? Kevin Unfortunately no, /boot/vmlinuz is gone on the last install (screenshot). It should be easy to reproduce, install Fedora.8.64, or i can upload an image later tonight if that is helpful. Nope, can't reproduce. Fedora 8 installs and boots just fine (8 GB disk, default qcow2 cluster size, just clicked through the installer with default options). Kevin
Re: [Qemu-devel] [PATCH] qcow2: Fix refcount block allocation during qcow2_allocate_cluster_at()
Am 23.04.2012 01:35, schrieb Marcelo Tosatti: On Sun, Apr 22, 2012 at 08:18:49PM -0300, Marcelo Tosatti wrote: On Fri, Apr 20, 2012 at 03:56:01PM +0200, Kevin Wolf wrote: Refcount block allocation and refcount table growth rely on s-free_cluster_index pointing to somewhere after the current allocation. Change qcow2_allocate_cluster_at() to fulfill this assumption. Without this change it could happen that a newly allocated refcount block and the allocated data block point to the same area in the image file, causing data corruption in the long run. This fixes a bug that became first visible after commit 250196f1. Signed-off-by: Kevin Wolf kw...@redhat.com Kevin, This patch fixes explicit filesystem errors (qemu-img check also OK), but autotest is still failing, see attached screenshot. It is not reproducible without f081987ad20a8c8dc391deded55161ea8d38be5f Sorry, i meant commit 250196f19c6e7df12965d74a5073e10aba06c802 Author: Kevin Wolf kw...@redhat.com Date: Fri Mar 2 14:10:54 2012 +0100 qcow2: Reduce number of I/O requests The screenshot doesn't really give a lot of information, but let's assume that _something_ must have been corrupted... Can you try finding the corrupted file (e.g. using rpm -V) and see in which way it differs from the real one? Kevin
Re: [Qemu-devel] [PATCH] qcow2: Fix refcount block allocation during qcow2_allocate_cluster_at()
On Fri, Apr 20, 2012 at 03:56:01PM +0200, Kevin Wolf wrote: Refcount block allocation and refcount table growth rely on s-free_cluster_index pointing to somewhere after the current allocation. Change qcow2_allocate_cluster_at() to fulfill this assumption. Without this change it could happen that a newly allocated refcount block and the allocated data block point to the same area in the image file, causing data corruption in the long run. This fixes a bug that became first visible after commit 250196f1. Signed-off-by: Kevin Wolf kw...@redhat.com Kevin, This patch fixes explicit filesystem errors (qemu-img check also OK), but autotest is still failing, see attached screenshot. It is not reproducible without f081987ad20a8c8dc391deded55161ea8d38be5f attachment: f8-64-failure.png
Re: [Qemu-devel] [PATCH] qcow2: Fix refcount block allocation during qcow2_allocate_cluster_at()
On Sun, Apr 22, 2012 at 08:18:49PM -0300, Marcelo Tosatti wrote: On Fri, Apr 20, 2012 at 03:56:01PM +0200, Kevin Wolf wrote: Refcount block allocation and refcount table growth rely on s-free_cluster_index pointing to somewhere after the current allocation. Change qcow2_allocate_cluster_at() to fulfill this assumption. Without this change it could happen that a newly allocated refcount block and the allocated data block point to the same area in the image file, causing data corruption in the long run. This fixes a bug that became first visible after commit 250196f1. Signed-off-by: Kevin Wolf kw...@redhat.com Kevin, This patch fixes explicit filesystem errors (qemu-img check also OK), but autotest is still failing, see attached screenshot. It is not reproducible without f081987ad20a8c8dc391deded55161ea8d38be5f Sorry, i meant commit 250196f19c6e7df12965d74a5073e10aba06c802 Author: Kevin Wolf kw...@redhat.com Date: Fri Mar 2 14:10:54 2012 +0100 qcow2: Reduce number of I/O requests
[Qemu-devel] [PATCH] qcow2: Fix refcount block allocation during qcow2_allocate_cluster_at()
Refcount block allocation and refcount table growth rely on
s-free_cluster_index pointing to somewhere after the current
allocation. Change qcow2_allocate_cluster_at() to fulfill this
assumption.
Without this change it could happen that a newly allocated refcount
block and the allocated data block point to the same area in the image
file, causing data corruption in the long run.
This fixes a bug that became first visible after commit 250196f1.
Signed-off-by: Kevin Wolf kw...@redhat.com
---
block/qcow2-refcount.c |6 ++
1 files changed, 6 insertions(+), 0 deletions(-)
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 565bd54..6c38337 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -587,6 +587,7 @@ int qcow2_alloc_clusters_at(BlockDriverState *bs, uint64_t
offset,
{
BDRVQcowState *s = bs-opaque;
uint64_t cluster_index;
+uint64_t old_free_cluster_index;
int i, refcount, ret;
/* Check how many clusters there are free */
@@ -602,11 +603,16 @@ int qcow2_alloc_clusters_at(BlockDriverState *bs,
uint64_t offset,
}
/* And then allocate them */
+old_free_cluster_index = s-free_cluster_index;
+s-free_cluster_index = cluster_index + i;
+
ret = update_refcount(bs, offset, i s-cluster_bits, 1);
if (ret 0) {
return ret;
}
+s-free_cluster_index = old_free_cluster_index;
+
return i;
}
--
1.7.6.5
