[Qemu-devel] [PATCH] Fix occassional crashes during qcow2 recompression

[Qemu-devel] [PATCH] Fix occassional crashes during qcow2 recompression

[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".

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

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

-- 
2.1.4

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

[Jindřich Makovička]

Fixes a crash during image compression

Signed-off-by: Jindřich Makovička <makovick@gmail.com>
---
 block/qcow2-refcount.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 0632fc3..f8d18e7 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -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;
     }
-- 
2.1.4

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

[Jindřich Makovička]

Fixes a crash during image compression

Signed-off-by: Jindřich Makovička <makovick@gmail.com>
---
 block/qcow2-refcount.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 0632fc3..b0ee42d 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -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;
     }
-- 
2.1.4

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

[Cole Robinson]

On 06/24/2015 01:05 AM, Jindřich Makovička wrote:
> Fixes a crash during image compression
> 
> Signed-off-by: Jindřich Makovička <makovick@gmail.com>
> ---
>  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

> diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
> index 0632fc3..b0ee42d 100644
> --- a/block/qcow2-refcount.c
> +++ b/block/qcow2-refcount.c
> @@ -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;
>      }
> 

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

[Richard W.M. Jones]

On Wed, Jun 24, 2015 at 10:15:21AM -0400, Cole Robinson wrote:
> On 06/24/2015 01:05 AM, Jindřich Makovička wrote:
> > Fixes a crash during image compression
> > 
> > Signed-off-by: Jindřich Makovička <makovick@gmail.com>
> > ---
> >  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

Confirmed.  You can add

  Tested-by: Richard W.M. Jones <rjones@redhat.com>

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.

-- 
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/

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

[Max Reitz]

On 24.06.2015 07:05, Jindřich Makovička wrote:
> Fixes a crash during image compression
>
> Signed-off-by: Jindřich Makovička <makovick@gmail.com>
> ---
>   block/qcow2-refcount.c | 22 ++++++++++++----------
>   1 file changed, 12 insertions(+), 10 deletions(-)

Reviewed-by: Max Reitz <mreitz@redhat.com>

Re: [Qemu-devel] [PATCH] Fix occassional crashes during qcow2 recompression

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 635 bytes --]

On Tue, Jun 23, 2015 at 11:34:48PM +0200, Jindřich Makovička wrote:
> 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(-)
> 
> -- 
> 2.1.4
> 
> 

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

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]