| Message ID | 1426069701-1405-7-git-send-email-den@openvz.org |
|---|---|
| State | New |
| Headers | show |
On Wed, Mar 11, 2015 at 01:28:00PM +0300, Denis V. Lunev wrote: > Main approach is taken from qcow2_co_readv. > > The patch drops coroutine lock for the duration of IO operation and > peforms normal scatter-gather IO using standard QEMU backend. > > Signed-off-by: Denis V. Lunev <den@openvz.org> > Reviewed-by: Roman Kagan <rkagan@parallels.com> > CC: Kevin Wolf <kwolf@redhat.com> > CC: Stefan Hajnoczi <stefanha@redhat.com> > --- > block/parallels.c | 46 +++++++++++++++++++++++++++------------------- > 1 file changed, 27 insertions(+), 19 deletions(-) > > diff --git a/block/parallels.c b/block/parallels.c > index b469984..64b169b 100644 > --- a/block/parallels.c > +++ b/block/parallels.c > @@ -186,37 +186,45 @@ static int64_t coroutine_fn parallels_co_get_block_status(BlockDriverState *bs, > BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID; > } > > -static int parallels_read(BlockDriverState *bs, int64_t sector_num, > - uint8_t *buf, int nb_sectors) > +static coroutine_fn int parallels_co_readv(BlockDriverState *bs, > + int64_t sector_num, int nb_sectors, QEMUIOVector *qiov) > { > BDRVParallelsState *s = bs->opaque; > + uint64_t bytes_done = 0; > + QEMUIOVector hd_qiov; > + int ret = 0; > > + qemu_iovec_init(&hd_qiov, qiov->niov); > + > + qemu_co_mutex_lock(&s->lock); > while (nb_sectors > 0) { > int64_t position = seek_to_sector(s, sector_num); > int n = cluster_remainder(s, sector_num, nb_sectors); > - if (position >= 0) { > - int ret = bdrv_read(bs->file, position, buf, n); > + int nbytes = n << BDRV_SECTOR_BITS; > + > + if (position < 0) { > + qemu_iovec_memset(qiov, bytes_done, 0, nbytes); > + } else { > + qemu_iovec_reset(&hd_qiov); > + qemu_iovec_concat(&hd_qiov, qiov, bytes_done, nbytes); > + > + qemu_co_mutex_unlock(&s->lock); > + ret = bdrv_co_readv(bs->file, position, n, &hd_qiov); > + qemu_co_mutex_lock(&s->lock); > + > if (ret < 0) { > - return ret; > + goto fail; The lock is never unlocked if the goto is taken. > } > - } else { > - memset(buf, 0, n << BDRV_SECTOR_BITS); > } > + > nb_sectors -= n; > sector_num += n; > - buf += n << BDRV_SECTOR_BITS; > + bytes_done += nbytes; > } > - return 0; > -} > - > -static coroutine_fn int parallels_co_read(BlockDriverState *bs, int64_t sector_num, > - uint8_t *buf, int nb_sectors) > -{ > - int ret; > - BDRVParallelsState *s = bs->opaque; > - qemu_co_mutex_lock(&s->lock); > - ret = parallels_read(bs, sector_num, buf, nb_sectors); > qemu_co_mutex_unlock(&s->lock); > + > +fail: > + qemu_iovec_destroy(&hd_qiov); > return ret; > }
On 22/04/15 15:41, Stefan Hajnoczi wrote: > On Wed, Mar 11, 2015 at 01:28:00PM +0300, Denis V. Lunev wrote: >> Main approach is taken from qcow2_co_readv. >> >> The patch drops coroutine lock for the duration of IO operation and >> peforms normal scatter-gather IO using standard QEMU backend. >> >> Signed-off-by: Denis V. Lunev <den@openvz.org> >> Reviewed-by: Roman Kagan <rkagan@parallels.com> >> CC: Kevin Wolf <kwolf@redhat.com> >> CC: Stefan Hajnoczi <stefanha@redhat.com> >> --- >> block/parallels.c | 46 +++++++++++++++++++++++++++------------------- >> 1 file changed, 27 insertions(+), 19 deletions(-) >> >> diff --git a/block/parallels.c b/block/parallels.c >> index b469984..64b169b 100644 >> --- a/block/parallels.c >> +++ b/block/parallels.c >> @@ -186,37 +186,45 @@ static int64_t coroutine_fn parallels_co_get_block_status(BlockDriverState *bs, >> BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID; >> } >> >> -static int parallels_read(BlockDriverState *bs, int64_t sector_num, >> - uint8_t *buf, int nb_sectors) >> +static coroutine_fn int parallels_co_readv(BlockDriverState *bs, >> + int64_t sector_num, int nb_sectors, QEMUIOVector *qiov) >> { >> BDRVParallelsState *s = bs->opaque; >> + uint64_t bytes_done = 0; >> + QEMUIOVector hd_qiov; >> + int ret = 0; >> >> + qemu_iovec_init(&hd_qiov, qiov->niov); >> + >> + qemu_co_mutex_lock(&s->lock); >> while (nb_sectors > 0) { >> int64_t position = seek_to_sector(s, sector_num); >> int n = cluster_remainder(s, sector_num, nb_sectors); >> - if (position >= 0) { >> - int ret = bdrv_read(bs->file, position, buf, n); >> + int nbytes = n << BDRV_SECTOR_BITS; >> + >> + if (position < 0) { >> + qemu_iovec_memset(qiov, bytes_done, 0, nbytes); >> + } else { >> + qemu_iovec_reset(&hd_qiov); >> + qemu_iovec_concat(&hd_qiov, qiov, bytes_done, nbytes); >> + >> + qemu_co_mutex_unlock(&s->lock); >> + ret = bdrv_co_readv(bs->file, position, n, &hd_qiov); >> + qemu_co_mutex_lock(&s->lock); >> + >> if (ret < 0) { >> - return ret; >> + goto fail; > The lock is never unlocked if the goto is taken. nice catch! We have missed that... >> } >> - } else { >> - memset(buf, 0, n << BDRV_SECTOR_BITS); >> } >> + >> nb_sectors -= n; >> sector_num += n; >> - buf += n << BDRV_SECTOR_BITS; >> + bytes_done += nbytes; >> } >> - return 0; >> -} >> - >> -static coroutine_fn int parallels_co_read(BlockDriverState *bs, int64_t sector_num, >> - uint8_t *buf, int nb_sectors) >> -{ >> - int ret; >> - BDRVParallelsState *s = bs->opaque; >> - qemu_co_mutex_lock(&s->lock); >> - ret = parallels_read(bs, sector_num, buf, nb_sectors); >> qemu_co_mutex_unlock(&s->lock); >> + >> +fail: >> + qemu_iovec_destroy(&hd_qiov); >> return ret; >> }
diff --git a/block/parallels.c b/block/parallels.c index b469984..64b169b 100644 --- a/block/parallels.c +++ b/block/parallels.c @@ -186,37 +186,45 @@ static int64_t coroutine_fn parallels_co_get_block_status(BlockDriverState *bs, BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID; } -static int parallels_read(BlockDriverState *bs, int64_t sector_num, - uint8_t *buf, int nb_sectors) +static coroutine_fn int parallels_co_readv(BlockDriverState *bs, + int64_t sector_num, int nb_sectors, QEMUIOVector *qiov) { BDRVParallelsState *s = bs->opaque; + uint64_t bytes_done = 0; + QEMUIOVector hd_qiov; + int ret = 0; + qemu_iovec_init(&hd_qiov, qiov->niov); + + qemu_co_mutex_lock(&s->lock); while (nb_sectors > 0) { int64_t position = seek_to_sector(s, sector_num); int n = cluster_remainder(s, sector_num, nb_sectors); - if (position >= 0) { - int ret = bdrv_read(bs->file, position, buf, n); + int nbytes = n << BDRV_SECTOR_BITS; + + if (position < 0) { + qemu_iovec_memset(qiov, bytes_done, 0, nbytes); + } else { + qemu_iovec_reset(&hd_qiov); + qemu_iovec_concat(&hd_qiov, qiov, bytes_done, nbytes); + + qemu_co_mutex_unlock(&s->lock); + ret = bdrv_co_readv(bs->file, position, n, &hd_qiov); + qemu_co_mutex_lock(&s->lock); + if (ret < 0) { - return ret; + goto fail; } - } else { - memset(buf, 0, n << BDRV_SECTOR_BITS); } + nb_sectors -= n; sector_num += n; - buf += n << BDRV_SECTOR_BITS; + bytes_done += nbytes; } - return 0; -} - -static coroutine_fn int parallels_co_read(BlockDriverState *bs, int64_t sector_num, - uint8_t *buf, int nb_sectors) -{ - int ret; - BDRVParallelsState *s = bs->opaque; - qemu_co_mutex_lock(&s->lock); - ret = parallels_read(bs, sector_num, buf, nb_sectors); qemu_co_mutex_unlock(&s->lock); + +fail: + qemu_iovec_destroy(&hd_qiov); return ret; } @@ -231,9 +239,9 @@ static BlockDriver bdrv_parallels = { .instance_size = sizeof(BDRVParallelsState), .bdrv_probe = parallels_probe, .bdrv_open = parallels_open, - .bdrv_read = parallels_co_read, .bdrv_close = parallels_close, .bdrv_co_get_block_status = parallels_co_get_block_status, + .bdrv_co_readv = parallels_co_readv, }; static void bdrv_parallels_init(void)