[Qemu-devel] [PATCH] qcow2: Fix error handling for immediate backing file read failure

[Qemu-devel] [PATCH] qcow2: Fix error handling for immediate backing file read failure

[Kevin Wolf]

Requests could return success even though they failed when bdrv_aio_readv
returned NULL for a backing file read.

Reported-by: Chunqiang Tang <ctang@us.ibm.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 28338bf..647c2a4 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)
-- 
1.7.2.3

Re: [Qemu-devel] [PATCH] qcow2: Fix error handling for immediate backing file read failure

[Markus Armbruster]

Kevin Wolf <kwolf@redhat.com> writes:

> Requests could return success even though they failed when bdrv_aio_readv
> returned NULL for a backing file read.
>
> Reported-by: Chunqiang Tang <ctang@us.ibm.com>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/qcow2.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/block/qcow2.c b/block/qcow2.c
> index 28338bf..647c2a4 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)

Good catch.

Nearby

        if (qcow2_decompress_cluster(bs, acb->cluster_offset) < 0)
            goto done;

looks suspicious to my ignorant eyes.  Is it okay?

Re: [Qemu-devel] [PATCH] qcow2: Fix error handling for immediate backing file read failure

[Kevin Wolf]

Am 08.02.2011 18:43, schrieb Markus Armbruster:
> Kevin Wolf <kwolf@redhat.com> writes:
> 
>> Requests could return success even though they failed when bdrv_aio_readv
>> returned NULL for a backing file read.
>>
>> Reported-by: Chunqiang Tang <ctang@us.ibm.com>
>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>> ---
>>  block/qcow2.c |    4 +++-
>>  1 files changed, 3 insertions(+), 1 deletions(-)
>>
>> diff --git a/block/qcow2.c b/block/qcow2.c
>> index 28338bf..647c2a4 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)
> 
> Good catch.
> 
> Nearby
> 
>         if (qcow2_decompress_cluster(bs, acb->cluster_offset) < 0)
>             goto done;
> 
> looks suspicious to my ignorant eyes.  Is it okay?

No, it's wrong as well, and it's the second part of Chunquiang Tang's
bug report. I started a patch for it yesterday, but didn't have time to
test it because I didn't expect my train to wait. ;-) I'll send it today.

Kevin