Re: [Qemu-block] [PATCH 04/13] block: Switch bdrv_write_zeroes() to byte interface

[Kevin Wolf]

Am 25.05.2016 um 00:25 hat Eric Blake geschrieben:
> Rename to bdrv_pwrite_zeroes() to let the compiler ensure we
> cater to the updated semantics.  Do the same for
> bdrv_aio_write_zeroes() and bdrv_co_write_zeroes().  For now,
> we still require sector alignment in the callers, via assertions.
> 
> Signed-off-by: Eric Blake <address@hidden>

> --- a/block/io.c
> +++ b/block/io.c
> @@ -603,18 +603,21 @@ int bdrv_write(BlockDriverState *bs, int64_t sector_num,
>      return bdrv_rw_co(bs, sector_num, (uint8_t *)buf, nb_sectors, true, 0);
>  }
> 
> -int bdrv_write_zeroes(BlockDriverState *bs, int64_t sector_num,
> -                      int nb_sectors, BdrvRequestFlags flags)
> +int bdrv_pwrite_zeroes(BlockDriverState *bs, int64_t offset,
> +                       int count, BdrvRequestFlags flags)
>  {
> -    return bdrv_rw_co(bs, sector_num, NULL, nb_sectors, true,
> +    assert(offset % BDRV_SECTOR_SIZE == 0);
> +    assert(count % BDRV_SECTOR_SIZE == 0);
> +    return bdrv_rw_co(bs, offset >> BDRV_SECTOR_BITS, NULL,
> +                      count >> BDRV_SECTOR_BITS, true,
>                        BDRV_REQ_ZERO_WRITE | flags);
>  }

Should we go directly to bdrv_prwv_co() here so that we don't need to
assert BDRV_SECTOR_SIZE alignment in a byte-based function?

> -BlockAIOCB *bdrv_aio_write_zeroes(BlockDriverState *bs,
> -        int64_t sector_num, int nb_sectors, BdrvRequestFlags flags,
> +BlockAIOCB *bdrv_aio_pwrite_zeroes(BlockDriverState *bs,
> +        int64_t offset, int count, BdrvRequestFlags flags,
>          BlockCompletionFunc *cb, void *opaque)
>  {
> -    trace_bdrv_aio_write_zeroes(bs, sector_num, nb_sectors, flags, opaque);
> +    trace_bdrv_aio_pwrite_zeroes(bs, offset, count, flags, opaque);
> +    assert(offset % BDRV_SECTOR_SIZE == 0);
> +    assert(count % BDRV_SECTOR_SIZE == 0);
> 
> -    return bdrv_co_aio_rw_vector(bs, sector_num, NULL, nb_sectors,
> +    return bdrv_co_aio_rw_vector(bs, offset >> BDRV_SECTOR_BITS, NULL,
> +                                 count >> BDRV_SECTOR_BITS,
>                                   BDRV_REQ_ZERO_WRITE | flags,
>                                   cb, opaque, true);
>  }

Here the same would be nice, but we don't have a byte-based AIO
interface yet, so I'd agree with leaving the assertion here.

Kevin