Re: [Qemu-devel] [PATCH 06/27] block/parallels: provide _co_readv routine for parallels format driver

[Stefan Hajnoczi]

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 <address@hidden>
> Reviewed-by: Roman Kagan <address@hidden>
> CC: Kevin Wolf <address@hidden>
> CC: Stefan Hajnoczi <address@hidden>
> ---
>  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;
>  }

pgpKekKLSwKt6.pgp
Description: PGP signature