Re: [Qemu-block] [PATCH v10 6/9] file-posix: support BDRV_REQ_ALLOCATE

[Anton Nefedov]

On 5/12/2018 4:25 PM, Vladimir Sementsov-Ogievskiy wrote:
> 03.12.2018 13:14, Anton Nefedov wrote:
>> Current write_zeroes implementation is good enough to satisfy this flag too
>>
>> Signed-off-by: Anton Nefedov <address@hidden>
>> ---
>>    block/file-posix.c | 8 +++++++-
>>    1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/block/file-posix.c b/block/file-posix.c
>> index 07bbdab953..b0b7ab0159 100644
>> --- a/block/file-posix.c
>> +++ b/block/file-posix.c
>> @@ -603,6 +603,7 @@ static int raw_open_common(BlockDriverState *bs, QDict 
>> *options,
>>            } else {
>>                s->discard_zeroes = true;
>>                s->has_fallocate = true;
>> +            bs->supported_zero_flags = BDRV_REQ_ALLOCATE;
>>            }
>>        } else {
>>            if (!(S_ISCHR(st.st_mode) || S_ISBLK(st.st_mode))) {
>> @@ -646,10 +647,11 @@ static int raw_open_common(BlockDriverState *bs, QDict 
>> *options,
>>    #ifdef CONFIG_XFS
>>        if (platform_test_xfs_fd(s->fd)) {
>>            s->is_xfs = true;
>> +        bs->supported_zero_flags = BDRV_REQ_ALLOCATE;
> 
> why we should handle xfs separately? is there a case with xfs, not handled by 
> previous generic if ()?
> 

The driver supports ALLOCATE either when it's XFS, or when fallocate is
available. Currently there is no test for fallocate, it's just implied
it's supported until ENOTSUP returned.
I think it's safer (for possible future changes) to set it twice even
though you're right and first condition currently covers the XFS 
condition too.

>>        }
>>    #endif
>>    
>> -    bs->supported_zero_flags = BDRV_REQ_MAY_UNMAP;
>> +    bs->supported_zero_flags |= BDRV_REQ_MAY_UNMAP;
>>        ret = 0;
>>    fail:
>>        if (filename && (bdrv_flags & BDRV_O_TEMPORARY)) {
>> @@ -1520,6 +1522,10 @@ static ssize_t 
>> handle_aiocb_write_zeroes(RawPosixAIOData *aiocb)
>>            }
>>            s->has_fallocate = false;
>>        }
>> +
>> +    if (!s->has_fallocate) {
>> +        aiocb->bs->supported_zero_flags &= ~BDRV_REQ_ALLOCATE;
>> +    }
>>    #endif
>>    
>>        return -ENOTSUP;
>>
> 
>