Re: [Qemu-devel] [PATCH 5/8] block/raw-posix: refactor handle_aiocb_write_zeroes a bit

[Fam Zheng <famz@redhat.com>]

On Tue, 12/30 12:20, Denis V. Lunev wrote:
> move code dealing with a block device to a separate function. This will
> allow to implement additional processing for an ordinary files.

s/an//

> 
> Pls note, that xfs_code has been moved before checking for
> s->has_write_zeroes as xfs_write_zeroes does not touch this flag inside.
> This makes code a bit more consistent.
> 
> Signed-off-by: Denis V. Lunev <den@openvz.org>
> CC: Kevin Wolf <kwolf@redhat.com>
> CC: Stefan Hajnoczi <stefanha@redhat.com>
> CC: Peter Lieven <pl@kamp.de>
> ---
>  block/raw-posix.c | 60 +++++++++++++++++++++++++++++++++++--------------------
>  1 file changed, 38 insertions(+), 22 deletions(-)
> 
> diff --git a/block/raw-posix.c b/block/raw-posix.c
> index 25a6947..7866d31 100644
> --- a/block/raw-posix.c
> +++ b/block/raw-posix.c
> @@ -915,46 +915,62 @@ static int do_fallocate(int fd, int mode, off_t offset, off_t len)
>  }
>  #endif
>  
> -static ssize_t handle_aiocb_write_zeroes(RawPosixAIOData *aiocb)
> +static ssize_t handle_aiocb_write_zeroes_block(RawPosixAIOData *aiocb)
>  {
>      int ret = -EOPNOTSUPP;
>      BDRVRawState *s = aiocb->bs->opaque;
>  
> -    if (s->has_write_zeroes == 0) {
> +    if (!s->has_write_zeroes) {
>          return -ENOTSUP;
>      }
>  
> -    if (aiocb->aio_type & QEMU_AIO_BLKDEV) {
>  #ifdef BLKZEROOUT
> -        do {
> -            uint64_t range[2] = { aiocb->aio_offset, aiocb->aio_nbytes };
> -            if (ioctl(aiocb->aio_fildes, BLKZEROOUT, range) == 0) {
> -                return 0;
> -            }
> -        } while (errno == EINTR);
> -
> -        ret = -errno;
> -#endif
> -    } else {
> -#ifdef CONFIG_XFS
> -        if (s->is_xfs) {
> -            return xfs_write_zeroes(s, aiocb->aio_offset, aiocb->aio_nbytes);
> +    do {
> +        uint64_t range[2] = { aiocb->aio_offset, aiocb->aio_nbytes };
> +        if (ioctl(aiocb->aio_fildes, BLKZEROOUT, range) == 0) {
> +            return 0;
>          }
> -#endif
> +    } while (errno == EINTR);
>  
> -#ifdef CONFIG_FALLOCATE_ZERO_RANGE
> -        ret = do_fallocate(s->fd, FALLOC_FL_ZERO_RANGE,
> -                           aiocb->aio_offset, aiocb->aio_nbytes);
> +    ret = translate_err(-errno);
>  #endif
> -    }
>  
> -    ret = translate_err(ret);
>      if (ret == -ENOTSUP) {
>          s->has_write_zeroes = false;
>      }
>      return ret;
>  }
>  
> +static ssize_t handle_aiocb_write_zeroes(RawPosixAIOData *aiocb)
> +{
> +    BDRVRawState *s;
> +
> +    if (aiocb->aio_type & QEMU_AIO_BLKDEV) {
> +        return handle_aiocb_write_zeroes_block(aiocb);
> +    }
> +
> +#ifdef CONFIG_XFS
> +    if (s->is_xfs) {

s is not initialized here.

Also, could you please do these refactoring before adding new code in the
series? That way the new code needn't to be moved, which makes the patches
easier to review.

Fam

> +        return xfs_write_zeroes(s, aiocb->aio_offset, aiocb->aio_nbytes);
> +    }
> +#endif
> +
> +    s = aiocb->bs->opaque;
> +
> +#ifdef CONFIG_FALLOCATE_ZERO_RANGE
> +    if (s->has_write_zeroes) {
> +        int ret = do_fallocate(s->fd, FALLOC_FL_ZERO_RANGE,
> +                               aiocb->aio_offset, aiocb->aio_nbytes);
> +        if (ret == 0 && ret != -ENOTSUP) {
> +            return ret;
> +        }
> +    }
> +#endif
> +
> +    s->has_write_zeroes = false;
> +    return -ENOTSUP;
> +}
> +
>  static ssize_t handle_aiocb_discard(RawPosixAIOData *aiocb)
>  {
>      int ret = -EOPNOTSUPP;
> -- 
> 1.9.1
> 
>