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

[Vladimir Sementsov-Ogievskiy]

05.12.2018 17:11, Anton Nefedov wrote:
> 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.

Aha, ok.



> 
>>>         }
>>>     #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;
>>> +    }

hm, if CONFIG_FALLOCATE is disabled, flag will remain in supported_zero_flags

>>>     #endif
>>>     
>>>         return -ENOTSUP;
>>>
>>
>>


-- 
Best regards,
Vladimir