[Qemu-devel] [STABLE PULL 0/6] Block patches for 0.12.x

[Qemu-devel] [STABLE PULL 0/6] Block patches for 0.12.x

[Kevin Wolf]

The following changes since commit 0c459361a1117a6c434c7b2b008a4c6c035eb4bf:
  Rabin Vincent (1):
        arm_timer: fix oneshot mode

are available in the git repository at:

  git://repo.or.cz/qemu/kevin.git for-stable-0.12

Kevin Wolf (6):
      ide: Fix ide_dma_cancel
      qcow2: Clear L2 table cache after write error
      qcow2: Fix error handling in l2_allocate
      block: Fix multiwrite with overlapping requests
      qcow2: Fix corruption after refblock allocation
      qcow2: Fix corruption after error in update_refcount

 block.c                |    2 +-
 block/qcow2-cluster.c  |   26 +++++++++++++++-----------
 block/qcow2-refcount.c |   15 +++++++++++++--
 hw/ide/core.c          |    8 ++++----
 4 files changed, 33 insertions(+), 18 deletions(-)

[Qemu-devel] [STABLE PATCH 1/6] ide: Fix ide_dma_cancel

[Kevin Wolf]

When cancelling a request, bdrv_aio_cancel may decide that it waits for
completion of a request rather than for cancellation. IDE therefore can't
abandon its DMA status before calling bdrv_aio_cancel; otherwise the callback
of a completed request would use invalid data.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
(cherry picked from commit 38d8dfa193e9a45f0f08b06aab2ba2a94f40a041)
---
 hw/ide/core.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 64aebc2..f9bb338 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -2827,10 +2827,6 @@ static void ide_dma_restart(IDEState *s, int is_read)
 void ide_dma_cancel(BMDMAState *bm)
 {
     if (bm->status & BM_STATUS_DMAING) {
-        bm->status &= ~BM_STATUS_DMAING;
-        /* cancel DMA request */
-        bm->unit = -1;
-        bm->dma_cb = NULL;
         if (bm->aiocb) {
 #ifdef DEBUG_AIO
             printf("aio_cancel\n");
@@ -2838,6 +2834,10 @@ void ide_dma_cancel(BMDMAState *bm)
             bdrv_aio_cancel(bm->aiocb);
             bm->aiocb = NULL;
         }
+        bm->status &= ~BM_STATUS_DMAING;
+        /* cancel DMA request */
+        bm->unit = -1;
+        bm->dma_cb = NULL;
     }
 }
 
-- 
1.6.6.1

[Qemu-devel] [STABLE PATCH 2/6] qcow2: Clear L2 table cache after write error

[Kevin Wolf]

If the L2 table was already updated in cache, but writing it to disk has
failed, we must not continue using the changed version in the cache to stay
consistent with what's on the disk.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
(cherry picked from commit 1b7c801b40ce90795397bb566d019c9b76ef9c13)

Conflicts:

	block/qcow2-cluster.c

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2-cluster.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index c7057b1..0dc4f1d 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -672,8 +672,9 @@ int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m)
                     (i << s->cluster_bits)) | QCOW_OFLAG_COPIED);
      }
 
-    if (write_l2_entries(s, l2_table, l2_offset, l2_index, m->nb_clusters) < 0) {
-        ret = -1;
+    ret = write_l2_entries(s, l2_table, l2_offset, l2_index, m->nb_clusters);
+    if (ret < 0) {
+        qcow2_l2_cache_reset(bs);
         goto err;
     }
 
-- 
1.6.6.1

[Qemu-devel] [STABLE PATCH 3/6] qcow2: Fix error handling in l2_allocate

[Kevin Wolf]

l2_allocate has some intermediate states in which the image is inconsistent.
Change the order to write to the L1 table only after the new L2 table has
successfully been initialized.

Also reset the L2 cache in failure case, it's very likely wrong.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
(cherry picked from commit 175e11526e2613b3dc031c23fec3107aa4a80307)

Conflicts:

	block/qcow2-cluster.c

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2-cluster.c |   21 ++++++++++++---------
 1 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 0dc4f1d..b7a5b35 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -231,13 +231,6 @@ static uint64_t *l2_allocate(BlockDriverState *bs, int l1_index)
         return NULL;
     }
 
-    /* update the L1 entry */
-
-    s->l1_table[l1_index] = l2_offset | QCOW_OFLAG_COPIED;
-    if (write_l1_entry(s, l1_index) < 0) {
-        return NULL;
-    }
-
     /* allocate a new entry in the l2 cache */
 
     min_index = l2_cache_new_entry(bs);
@@ -251,13 +244,19 @@ static uint64_t *l2_allocate(BlockDriverState *bs, int l1_index)
         if (bdrv_pread(s->hd, old_l2_offset,
                        l2_table, s->l2_size * sizeof(uint64_t)) !=
             s->l2_size * sizeof(uint64_t))
-            return NULL;
+            goto fail;
     }
     /* write the l2 table to the file */
     if (bdrv_pwrite(s->hd, l2_offset,
                     l2_table, s->l2_size * sizeof(uint64_t)) !=
         s->l2_size * sizeof(uint64_t))
-        return NULL;
+        goto fail;
+
+    /* update the L1 entry */
+    s->l1_table[l1_index] = l2_offset | QCOW_OFLAG_COPIED;
+    if (write_l1_entry(s, l1_index) < 0) {
+        goto fail;
+    }
 
     /* update the l2 cache entry */
 
@@ -265,6 +264,10 @@ static uint64_t *l2_allocate(BlockDriverState *bs, int l1_index)
     s->l2_cache_counts[min_index] = 1;
 
     return l2_table;
+
+fail:
+    qcow2_l2_cache_reset(bs);
+    return NULL;
 }
 
 static int count_contiguous_clusters(uint64_t nb_clusters, int cluster_size,
-- 
1.6.6.1

[Qemu-devel] [STABLE PATCH 4/6] block: Fix multiwrite with overlapping requests

[Kevin Wolf]

With overlapping requests, the total number of sectors is smaller than the sum
of the nb_sectors of both requests.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
(cherry picked from commit cbf1dff2f1033cadcb15c0ffc9c0a3d039d8ed42)
---
 block.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/block.c b/block.c
index 955eeaa..298414c 100644
--- a/block.c
+++ b/block.c
@@ -1711,7 +1711,7 @@ static int multiwrite_merge(BlockDriverState *bs, BlockRequest *reqs,
             // Add the second request
             qemu_iovec_concat(qiov, reqs[i].qiov, reqs[i].qiov->size);
 
-            reqs[outidx].nb_sectors += reqs[i].nb_sectors;
+            reqs[outidx].nb_sectors = qiov->size >> 9;
             reqs[outidx].qiov = qiov;
 
             mcb->callbacks[i].free_qiov = reqs[outidx].qiov;
-- 
1.6.6.1

[Qemu-devel] [STABLE PATCH 5/6] qcow2: Fix corruption after refblock allocation

[Kevin Wolf]

Refblock allocation code needs to take into consideration that update_refcount
will load a different refcount block into the cache, so it must initialize the
cache for a new refcount block only afterwards. Not doing this means that not
only the refcount in the wrong block is updated, but also that the caller will
work on the wrong block.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
(cherry picked from commit 25408c09502be036e5575754fe54019ed4ed5dfa)
---
 block/qcow2-refcount.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 5ebbcb6..fa78e46 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -215,8 +215,6 @@ static int64_t alloc_refcount_block(BlockDriverState *bs, int64_t cluster_index)
 
     /* Allocate the refcount block itself and mark it as used */
     uint64_t new_block = alloc_clusters_noref(bs, s->cluster_size);
-    memset(s->refcount_block_cache, 0, s->cluster_size);
-    s->refcount_block_cache_offset = new_block;
 
 #ifdef DEBUG_ALLOC2
     fprintf(stderr, "qcow2: Allocate refcount block %d for %" PRIx64
@@ -225,6 +223,10 @@ static int64_t alloc_refcount_block(BlockDriverState *bs, int64_t cluster_index)
 #endif
 
     if (in_same_refcount_block(s, new_block, cluster_index << s->cluster_bits)) {
+        /* Zero the new refcount block before updating it */
+        memset(s->refcount_block_cache, 0, s->cluster_size);
+        s->refcount_block_cache_offset = new_block;
+
         /* The block describes itself, need to update the cache */
         int block_index = (new_block >> s->cluster_bits) &
             ((1 << (s->cluster_bits - REFCOUNT_SHIFT)) - 1);
@@ -236,6 +238,11 @@ static int64_t alloc_refcount_block(BlockDriverState *bs, int64_t cluster_index)
         if (ret < 0) {
             goto fail_block;
         }
+
+        /* Initialize the new refcount block only after updating its refcount,
+         * update_refcount uses the refcount cache itself */
+        memset(s->refcount_block_cache, 0, s->cluster_size);
+        s->refcount_block_cache_offset = new_block;
     }
 
     /* Now the new refcount block needs to be written to disk */
-- 
1.6.6.1

[Qemu-devel] [STABLE PATCH 6/6] qcow2: Fix corruption after error in update_refcount

[Kevin Wolf]

After it is done with updating refcounts in the cache, update_refcount writes
all changed entries to disk. If a refcount block allocation fails, however,
there was no change yet and therefore first_index = last_index = -1. Don't
treat -1 as a normal sector index (resulting in a 512 byte write!) but return
without updating anything in this case.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
(cherry picked from commit 86fa8da83771238de55dc44819a1a27bafef5353)
---
 block/qcow2-refcount.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index fa78e46..465d5d3 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -402,6 +402,10 @@ static int write_refcount_block_entries(BDRVQcowState *s,
         return 0;
     }
 
+    if (first_index < 0) {
+        return 0;
+    }
+
     first_index &= ~(REFCOUNTS_PER_SECTOR - 1);
     last_index = (last_index + REFCOUNTS_PER_SECTOR)
         & ~(REFCOUNTS_PER_SECTOR - 1);
-- 
1.6.6.1

Re: [Qemu-devel] [STABLE PULL 0/6] Block patches for 0.12.x

[Aurelien Jarno]

On Fri, May 28, 2010 at 08:01:26PM +0200, Kevin Wolf wrote:
> The following changes since commit 0c459361a1117a6c434c7b2b008a4c6c035eb4bf:
>   Rabin Vincent (1):
>         arm_timer: fix oneshot mode
> 
> are available in the git repository at:
> 
>   git://repo.or.cz/qemu/kevin.git for-stable-0.12
> 
> Kevin Wolf (6):
>       ide: Fix ide_dma_cancel
>       qcow2: Clear L2 table cache after write error
>       qcow2: Fix error handling in l2_allocate
>       block: Fix multiwrite with overlapping requests
>       qcow2: Fix corruption after refblock allocation
>       qcow2: Fix corruption after error in update_refcount
> 
>  block.c                |    2 +-
>  block/qcow2-cluster.c  |   26 +++++++++++++++-----------
>  block/qcow2-refcount.c |   15 +++++++++++++--
>  hw/ide/core.c          |    8 ++++----
>  4 files changed, 33 insertions(+), 18 deletions(-)
> 
> 

Thanks, all applied.

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net