Re: [Qemu-devel] [PATCH v3 07/17] block/parallels: use QEMU_IOVEC_INIT_BUF

[Eric Blake]

On 2/7/19 4:24 AM, Vladimir Sementsov-Ogievskiy wrote:
> Use new QEMU_IOVEC_INIT_BUF() instead of
> qemu_iovec_init_external( ... , 1), which simplifies the code.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> ---
>  block/parallels.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/block/parallels.c b/block/parallels.c
> index cc9445879d..6d24ee6d8e 100644
> --- a/block/parallels.c
> +++ b/block/parallels.c
> @@ -220,23 +220,20 @@ static int64_t allocate_clusters(BlockDriverState *bs, 
> int64_t sector_num,
>      if (bs->backing) {
>          int64_t nb_cow_sectors = to_allocate * s->tracks;
>          int64_t nb_cow_bytes = nb_cow_sectors << BDRV_SECTOR_BITS;
> -        QEMUIOVector qiov;
> -        struct iovec iov = {
> -            .iov_len = nb_cow_bytes,
> -            .iov_base = qemu_blockalign(bs, nb_cow_bytes)
> -        };
> -        qemu_iovec_init_external(&qiov, &iov, 1);
> +        QEMUIOVector qiov =
> +            QEMU_IOVEC_INIT_BUF(qiov, qemu_blockalign(bs, nb_cow_bytes),
> +                                nb_cow_bytes);

I might have done:

char *buf = qemu_blockalign(bs, nb_cow_bytes);
QEMUIOVector qiov = QEME_IOVEC_INIT_BUF(qiov, buf, nb_cow_bytes);

>  
>          ret = bdrv_co_preadv(bs->backing, idx * s->tracks * BDRV_SECTOR_SIZE,
>                               nb_cow_bytes, &qiov, 0);
>          if (ret < 0) {
> -            qemu_vfree(iov.iov_base);
> +            qemu_vfree(qemu_iovec_get_buf(&qiov));

and then qemu_vfree(buf);

Depending on how much else in the series needs qemu_iovec_get_buf(),
that function may not actually be needed in patch 1.

But since my proposed changes are aesthetic rather than semantic, and
yours works as written, I'm also okay giving:

Reviewed-by: Eric Blake <address@hidden>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature