[Qemu-devel] [PATCH] QCOW2: fix bug - report read success on failure

[Qemu-devel] [PATCH] QCOW2: fix bug - report read success on failure

[Chunqiang Tang]

This patch fixes bugs in QCOW2's error handling paths of read operations.
When an I/O operation fails, the QCOW2 driver mistakenly reports it as success
to the uper layer.

This bug was found by Fast Virtual Disk (FVD)'s fully automated testing tool,
when it injected failures. Specifically, the following test triggered the bug.

/bin/rm -rf /var/ramdisk/truth.raw /var/ramdisk/test.qcow2 /var/ramdisk/zero-500M.raw
dd if=/dev/zero of=/var/ramdisk/truth.raw count=0 bs=1 seek=1112250368
dd if=/dev/zero of=/var/ramdisk/zero-500M.raw count=0 bs=1 seek=575525376
./qemu-img create -f qcow2 -ocluster_size=65536,backing_fmt=blksim -b /var/ramdisk/zero-500M.raw /var/ramdisk/test.qcow2 1112250368
./qemu-io --auto --seed=184915369 --truth=/var/ramdisk/truth.raw --format=qcow2 --test=blksim:/var/ramdisk/test.qcow2 --verify_write=true --compare_before=false --compare_after=true --round=100000 --parallel=100 --io_size=1048576 --fail_prob=0.1 --cancel_prob=0 --instant_qemubh=true

Signed-off-by: Chunqiang Tang <ctang@us.ibm.com>
---
 block/qcow2.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 8c906d1..6f6d56f 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -479,8 +479,10 @@ static void qcow2_aio_read_cb(void *opaque, int ret)
                 BLKDBG_EVENT(bs->file, BLKDBG_READ_BACKING_AIO);
                 acb->hd_aiocb = bdrv_aio_readv(bs->backing_hd, acb->sector_num,
                                     &acb->hd_qiov, n1, qcow2_aio_read_cb, acb);
-                if (acb->hd_aiocb == NULL)
+                if (acb->hd_aiocb == NULL) {
+                    ret = -EIO;
                     goto done;
+                }
             } else {
                 ret = qcow2_schedule_bh(qcow2_aio_read_bh, acb);
                 if (ret < 0)
@@ -495,8 +497,10 @@ static void qcow2_aio_read_cb(void *opaque, int ret)
         }
     } else if (acb->cluster_offset & QCOW_OFLAG_COMPRESSED) {
         /* add AIO support for compressed blocks ? */
-        if (qcow2_decompress_cluster(bs, acb->cluster_offset) < 0)
+        if (qcow2_decompress_cluster(bs, acb->cluster_offset) < 0) {
+            ret = -EIO;
             goto done;
+        }
 
         qemu_iovec_from_buffer(&acb->hd_qiov,
             s->cluster_cache + index_in_cluster * 512,
-- 
1.7.0.4

[Qemu-devel] Re: [PATCH] QCOW2: fix bug - report read success on failure

[Kevin Wolf]

Am 03.02.2011 17:53, schrieb Chunqiang Tang:
> This patch fixes bugs in QCOW2's error handling paths of read operations.
> When an I/O operation fails, the QCOW2 driver mistakenly reports it as success
> to the uper layer.
> 
> This bug was found by Fast Virtual Disk (FVD)'s fully automated testing tool,
> when it injected failures. Specifically, the following test triggered the bug.
> 
> /bin/rm -rf /var/ramdisk/truth.raw /var/ramdisk/test.qcow2 /var/ramdisk/zero-500M.raw
> dd if=/dev/zero of=/var/ramdisk/truth.raw count=0 bs=1 seek=1112250368
> dd if=/dev/zero of=/var/ramdisk/zero-500M.raw count=0 bs=1 seek=575525376
> ./qemu-img create -f qcow2 -ocluster_size=65536,backing_fmt=blksim -b /var/ramdisk/zero-500M.raw /var/ramdisk/test.qcow2 1112250368
> ./qemu-io --auto --seed=184915369 --truth=/var/ramdisk/truth.raw --format=qcow2 --test=blksim:/var/ramdisk/test.qcow2 --verify_write=true --compare_before=false --compare_after=true --round=100000 --parallel=100 --io_size=1048576 --fail_prob=0.1 --cancel_prob=0 --instant_qemubh=true
> 
> Signed-off-by: Chunqiang Tang <ctang@us.ibm.com>
> ---
>  block/qcow2.c |    8 ++++++--
>  1 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 8c906d1..6f6d56f 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -479,8 +479,10 @@ static void qcow2_aio_read_cb(void *opaque, int ret)
>                  BLKDBG_EVENT(bs->file, BLKDBG_READ_BACKING_AIO);
>                  acb->hd_aiocb = bdrv_aio_readv(bs->backing_hd, acb->sector_num,
>                                      &acb->hd_qiov, n1, qcow2_aio_read_cb, acb);
> -                if (acb->hd_aiocb == NULL)
> +                if (acb->hd_aiocb == NULL) {
> +                    ret = -EIO;
>                      goto done;
> +                }
>              } else {
>                  ret = qcow2_schedule_bh(qcow2_aio_read_bh, acb);
>                  if (ret < 0)

Oops, thanks for catching this. I thought this was fixed long ago, but
apparently it wasn't.

> @@ -495,8 +497,10 @@ static void qcow2_aio_read_cb(void *opaque, int ret)
>          }
>      } else if (acb->cluster_offset & QCOW_OFLAG_COMPRESSED) {
>          /* add AIO support for compressed blocks ? */
> -        if (qcow2_decompress_cluster(bs, acb->cluster_offset) < 0)
> +        if (qcow2_decompress_cluster(bs, acb->cluster_offset) < 0) {
> +            ret = -EIO;
>              goto done;
> +        }

I think here we should change qcow2_decompessed_cluster() to return the
real error code instead of -1, so that we can pass it on instead of
turning everything into -EIO.

Kevin

[Qemu-devel] Re: [PATCH] QCOW2: fix bug - report read success on failure

[Chunqiang Tang]

> Oops, thanks for catching this. I thought this was fixed long ago, but
> apparently it wasn't.

Not me, the testing tool caught it without my supervision. :-)

> > @@ -495,8 +497,10 @@ static void qcow2_aio_read_cb(void *opaque, int 
ret)
> >          }
> >      } else if (acb->cluster_offset & QCOW_OFLAG_COMPRESSED) {
> >          /* add AIO support for compressed blocks ? */
> > -        if (qcow2_decompress_cluster(bs, acb->cluster_offset) < 0)
> > +        if (qcow2_decompress_cluster(bs, acb->cluster_offset) < 0) {
> > +            ret = -EIO;
> >              goto done;
> > +        }
> 
> I think here we should change qcow2_decompessed_cluster() to return the
> real error code instead of -1, so that we can pass it on instead of
> turning everything into -EIO.

I agree. Could you please prepare the official patch? I just throw in 
ideas I got from my testing tool.