Skip to contentSkip to conversation

Fix occassional crashes during qcow2 recompression

PATCH Applied Jindřich Makovička 5 participants 7 messages Jun 23, 2015 – Jun 25, 2015 Block layer

lore
Share & tools
lore

Jindřich Makovička

author
#0 lore raw

PATCH 1/1 Fix occassional crashes during qcow2 recompression

Hi,

the following patch adds missing EAGAIN handling in qcow2 code.

When the conditions for throwing EAGAIN from the refcount allocator are met, qcow-img image recompression fails with "error while writing sector".

Jindřich Makovička (1):
qcow2: Handle EAGAIN returned from update_refcount

 block/qcow2-refcount.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)
Signature · 1 line
2.1.4
author
#1 lore raw

PATCH 1/1 qcow2: Handle EAGAIN returned from update_refcount

Fixes a crash during image compression

Signed-off-by Jindřich Makovička <[email protected]>
---
 block/qcow2-refcount.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)
block/qcow2-refcount.c +13 -10
index 0632fc3..f8d18e7 100644
@@ -940,19 +940,22 @@ int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size)
     }
 
     free_in_cluster = s->cluster_size - offset_into_cluster(s, offset);
-    if (!offset || free_in_cluster < size) {
-        int64_t new_cluster = alloc_clusters_noref(bs, s->cluster_size);
-        if (new_cluster < 0) {
-            return new_cluster;
-        }
+    do {
+        if (!offset || free_in_cluster < size) {
+            int64_t new_cluster = alloc_clusters_noref(bs, s->cluster_size);
+            if (new_cluster < 0) {
+                fprintf(stderr, "error getting new cluster\n");
+                return new_cluster;
+            }
 
-        if (!offset || ROUND_UP(offset, s->cluster_size) != new_cluster) {
-            offset = new_cluster;
+            if (!offset || ROUND_UP(offset, s->cluster_size) != new_cluster) {
+                offset = new_cluster;
+            }
         }
-    }
 
-    assert(offset);
-    ret = update_refcount(bs, offset, size, 1, false, QCOW2_DISCARD_NEVER);
+        assert(offset);
+        ret = update_refcount(bs, offset, size, 1, false, QCOW2_DISCARD_NEVER);
+    } while (ret == -EAGAIN);
     if (ret < 0) {
         return ret;
     }
Signature · 1 line
2.1.4
author
#2 lore raw

PATCH 1/1 qcow2: Handle EAGAIN returned from update_refcount

Fixes a crash during image compression

Signed-off-by Jindřich Makovička <[email protected]>
---
 block/qcow2-refcount.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)
block/qcow2-refcount.c +12 -10
index 0632fc3..b0ee42d 100644
@@ -940,19 +940,21 @@ int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size)
     }
 
     free_in_cluster = s->cluster_size - offset_into_cluster(s, offset);
-    if (!offset || free_in_cluster < size) {
-        int64_t new_cluster = alloc_clusters_noref(bs, s->cluster_size);
-        if (new_cluster < 0) {
-            return new_cluster;
-        }
+    do {
+        if (!offset || free_in_cluster < size) {
+            int64_t new_cluster = alloc_clusters_noref(bs, s->cluster_size);
+            if (new_cluster < 0) {
+                return new_cluster;
+            }
 
-        if (!offset || ROUND_UP(offset, s->cluster_size) != new_cluster) {
-            offset = new_cluster;
+            if (!offset || ROUND_UP(offset, s->cluster_size) != new_cluster) {
+                offset = new_cluster;
+            }
         }
-    }
 
-    assert(offset);
-    ret = update_refcount(bs, offset, size, 1, false, QCOW2_DISCARD_NEVER);
+        assert(offset);
+        ret = update_refcount(bs, offset, size, 1, false, QCOW2_DISCARD_NEVER);
+    } while (ret == -EAGAIN);
     if (ret < 0) {
         return ret;
     }
Signature · 1 line
2.1.4
#3 lore raw

Re: [Qemu-devel] [PATCH] qcow2: Handle EAGAIN returned from update_refcount

Quoted 7 lines · Jindřich Makovička

Fixes a crash during image compression

Signed-off-by Jindřich Makovička <[email protected]>
---
 block/qcow2-refcount.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

Rich Jones already confirmed that this patch fixes a bug he can reliably reproduce:

https://bugzilla.redhat.com/show_bug.cgi?id=1214855

- Cole

Quoted 37 lines
block/qcow2-refcount.c +12 -10
index 0632fc3..b0ee42d 100644
@@ -940,19 +940,21 @@ int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size)
     }
 
     free_in_cluster = s->cluster_size - offset_into_cluster(s, offset);
-    if (!offset || free_in_cluster < size) {
-        int64_t new_cluster = alloc_clusters_noref(bs, s->cluster_size);
-        if (new_cluster < 0) {
-            return new_cluster;
-        }
+    do {
+        if (!offset || free_in_cluster < size) {
+            int64_t new_cluster = alloc_clusters_noref(bs, s->cluster_size);
+            if (new_cluster < 0) {
+                return new_cluster;
+            }
 
-        if (!offset || ROUND_UP(offset, s->cluster_size) != new_cluster) {
-            offset = new_cluster;
+            if (!offset || ROUND_UP(offset, s->cluster_size) != new_cluster) {
+                offset = new_cluster;
+            }
         }
-    }
 
-    assert(offset);
-    ret = update_refcount(bs, offset, size, 1, false, QCOW2_DISCARD_NEVER);
+        assert(offset);
+        ret = update_refcount(bs, offset, size, 1, false, QCOW2_DISCARD_NEVER);
+    } while (ret == -EAGAIN);
     if (ret < 0) {
         return ret;
     }
Tested-by
#4 lore raw

Re: [Qemu-devel] [PATCH] qcow2: Handle EAGAIN returned from update_refcount

Quoted 9 lines · Cole Robinson
Quoted 7 lines · Jindřich Makovička

Fixes a crash during image compression

Signed-off-by Jindřich Makovička <[email protected]>
---
 block/qcow2-refcount.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)
Quoted 4 lines

Rich Jones already confirmed that this patch fixes a bug he can reliably reproduce:

https://bugzilla.redhat.com/show_bug.cgi?id=1214855

Confirmed. You can add

Tested-by Richard W.M. Jones <[email protected]>

to this patch.

Also note that if you view the patch without whitespace changes (eg. git diff -b) then it's actually quite simple.

Rich.

Signature · 5 lines
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/
Applied
#6 lore raw
In reply to Jindřich Makovička
Quoted 8 lines · Jindřich Makovička

Hi,

the following patch adds missing EAGAIN handling in qcow2 code.

When the conditions for throwing EAGAIN from the refcount allocator are met, qcow-img image recompression fails with "error while writing sector".

Quoted 10 lines

Jindřich Makovička (1):
qcow2: Handle EAGAIN returned from update_refcount

 block/qcow2-refcount.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)
Signature · 1 line
2.1.4

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan