Message ID | 1297185328-23210-1-git-send-email-kwolf@redhat.com |
---|---|
State | New |
Headers | show |
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?
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
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)
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(-)