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

[Denis V. Lunev]

On 05/01/15 09:57, Fam Zheng wrote:
> 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 <address@hidden>
> > CC: Kevin Wolf <address@hidden>
> > CC: Stefan Hajnoczi <address@hidden>
> > CC: Peter Lieven <address@hidden>
> > ---
> >   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
ok