Re: [Qemu-devel] [PATCH 5/7] block/raw-posix: call plain fallocate in handle_aiocb_write_zeroes

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

On 30/01/15 17:58, Max Reitz wrote:
> On 2015-01-30 at 03:42, Denis V. Lunev wrote:
>> There is a possibility that we are extending our image and thus writing
>> zeroes beyond the end of the file. In this case we do not need to care
>> about the hole to make sure that there is no data in the file under
>> this offset (pre-condition to fallocate(0) to work). We could simply 
>> call
>> fallocate(0).
>>
>> This improves the performance of writing zeroes even on really old
>> platforms which do not have even FALLOC_FL_PUNCH_HOLE.
>>
>> Before the patch do_fallocate was used when either
>> CONFIG_FALLOCATE_PUNCH_HOLE or CONFIG_FALLOCATE_ZERO_RANGE are defined.
>> Now the story is different. CONFIG_FALLOCATE is defined when Linux
>> fallocate is defined, posix_fallocate is completely different story
>> (CONFIG_POSIX_FALLOCATE). CONFIG_FALLOCATE is mandatory prerequite
>> for both CONFIG_FALLOCATE_PUNCH_HOLE and CONFIG_FALLOCATE_ZERO_RANGE
>> thus we are on the safe side.
>>
>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>> CC: Max Reitz <mreitz@redhat.com>
>> CC: Kevin Wolf <kwolf@redhat.com>
>> CC: Stefan Hajnoczi <stefanha@redhat.com>
>> CC: Peter Lieven <pl@kamp.de>
>> CC: Fam Zheng <famz@redhat.com>
>> ---
>>   block/raw-posix.c | 14 +++++++++++++-
>>   1 file changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/block/raw-posix.c b/block/raw-posix.c
>> index 5a777e7..1c88ad8 100644
>> --- a/block/raw-posix.c
>> +++ b/block/raw-posix.c
>> @@ -147,6 +147,7 @@ typedef struct BDRVRawState {
>>       bool has_discard:1;
>>       bool has_write_zeroes:1;
>>       bool discard_zeroes:1;
>> +    bool has_fallocate;
>>       bool needs_alignment;
>>   } BDRVRawState;
>>   @@ -452,6 +453,7 @@ static int raw_open_common(BlockDriverState 
>> *bs, QDict *options,
>>       }
>>       if (S_ISREG(st.st_mode)) {
>>           s->discard_zeroes = true;
>> +        s->has_fallocate = true;
>
> This could be moved upwards where has_discard and has_write_zeroes are 
> initialized; but it won't matter in practice, I hope. Thus:
>
> Reviewed-by: Max Reitz <mreitz@redhat.com>

This does matter as has_discard and has_write_zeroes are bit fields
thus I can not insert something useful into the middle of those
fields.