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

["Denis V. Lunev" <den@openvz.org>]

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 <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
ok