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

[Max Reitz <mreitz@redhat.com>]

On 2015-01-30 at 10:41, Denis V. Lunev wrote:
> 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.

Right, but I did not mean the placement inside of the structure but the 
placement of the initialization statement (s->has_fallocate = true) in 
raw_open_common().

Max