[Qemu-devel] [PATCH] avoid core reading with bdrv_read (qemu-io)
This patch apply to kevin coroutine-block branch and avoid code. It fix "qcow: Use coroutines" patch. Test case: $ ./qemu-img create -f qcow aaa.img 1G Formatting 'aaa.img', fmt=qcow size=1073741824 encryption=off $ ./qemu-io aaa.img qemu-io> read 1024 1024 Segmentation fault Signed-off-by: Frediano Ziglio --- block/qcow.c |5 - 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/block/qcow.c b/block/qcow.c index 6f7973c..1386e92 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -573,7 +573,8 @@ static int qcow_aio_read_cb(void *opaque) if (acb->nb_sectors == 0) { /* request completed */ -qemu_iovec_from_buffer(acb->qiov, acb->orig_buf, acb->qiov->size); +if (acb->orig_buf) +qemu_iovec_from_buffer(acb->qiov, acb->orig_buf, acb->qiov->size); return 0; } @@ -648,6 +649,7 @@ static int qcow_co_readv(BlockDriverState *bs, int64_t sector_num, if (acb->qiov->niov > 1) { qemu_vfree(acb->orig_buf); +acb->orig_buf = NULL; } qemu_aio_release(acb); @@ -729,6 +731,7 @@ static int qcow_co_writev(BlockDriverState *bs, int64_t sector_num, if (acb->qiov->niov > 1) { qemu_vfree(acb->orig_buf); +acb->orig_buf = NULL; } qemu_aio_release(acb); -- 1.7.1
Re: [Qemu-devel] [PATCH] avoid core reading with bdrv_read (qemu-io)
Am 19.07.2011 09:33, schrieb Frediano Ziglio: > This patch apply to kevin coroutine-block branch and avoid code. It > fix "qcow: Use coroutines" patch. Test case: > > $ ./qemu-img create -f qcow aaa.img 1G > Formatting 'aaa.img', fmt=qcow size=1073741824 encryption=off > $ ./qemu-io aaa.img > qemu-io> read 1024 1024 > Segmentation fault > > Signed-off-by: Frediano Ziglio Thanks for the report. I'll update the patch, but in a slightly different way that matches the old code better: diff --git a/block/qcow.c b/block/qcow.c index 6f7973c..6447c2a 100644 --- a/block/qcow.c +++ b/block/qcow.c @@ -573,7 +573,6 @@ static int qcow_aio_read_cb(void *opaque) if (acb->nb_sectors == 0) { /* request completed */ -qemu_iovec_from_buffer(acb->qiov, acb->orig_buf, acb->qiov->size); return 0; } @@ -647,6 +646,7 @@ static int qcow_co_readv(BlockDriverState *bs, int64_t sector_num, qemu_co_mutex_unlock(&s->lock); if (acb->qiov->niov > 1) { +qemu_iovec_from_buffer(acb->qiov, acb->orig_buf, acb->qiov->size); qemu_vfree(acb->orig_buf); } qemu_aio_release(acb);
Re: [Qemu-devel] [PATCH] avoid core reading with bdrv_read (qemu-io)
2011/7/19 Kevin Wolf : > Am 19.07.2011 09:33, schrieb Frediano Ziglio: >> This patch apply to kevin coroutine-block branch and avoid code. It >> fix "qcow: Use coroutines" patch. Test case: >> >> $ ./qemu-img create -f qcow aaa.img 1G >> Formatting 'aaa.img', fmt=qcow size=1073741824 encryption=off >> $ ./qemu-io aaa.img >> qemu-io> read 1024 1024 >> Segmentation fault >> >> Signed-off-by: Frediano Ziglio > > Thanks for the report. I'll update the patch, but in a slightly > different way that matches the old code better: > > diff --git a/block/qcow.c b/block/qcow.c > index 6f7973c..6447c2a 100644 > --- a/block/qcow.c > +++ b/block/qcow.c > @@ -573,7 +573,6 @@ static int qcow_aio_read_cb(void *opaque) > > if (acb->nb_sectors == 0) { > /* request completed */ > - qemu_iovec_from_buffer(acb->qiov, acb->orig_buf, acb->qiov->size); > return 0; > } > > @@ -647,6 +646,7 @@ static int qcow_co_readv(BlockDriverState *bs, > int64_t sector_num, > qemu_co_mutex_unlock(&s->lock); > > if (acb->qiov->niov > 1) { > + qemu_iovec_from_buffer(acb->qiov, acb->orig_buf, acb->qiov->size); > qemu_vfree(acb->orig_buf); > } > qemu_aio_release(acb); > Yes, my patch also removed some dandling pointer which I don't like but are not a problem with current code. In case of ret < 0 (error) your code could copy data that probably are not initialized. I don't know if data is used in case of failure but in case memory is shared with guest (I don't know, perhaps using virtio) this lead to security issues. Also some memory debugger like valgrind could not like that copy. Frediano