Re: [PATCH v11 13/13] block: apply COR-filter to block-stream jobs

[Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>]

On 16.10.2020 18:45, Vladimir Sementsov-Ogievskiy wrote:
> 15.10.2020 20:16, Andrey Shinkevich wrote:
>> On 14.10.2020 19:24, Max Reitz wrote:
>>> On 12.10.20 19:43, Andrey Shinkevich wrote:
>>
>> [...]
>>
>>>> ---
>>>>   block/stream.c             | 93 
>>>> +++++++++++++++++++++++++++++-----------------
>>>>   tests/qemu-iotests/030     | 51 +++----------------------
>>>>   tests/qemu-iotests/030.out |  4 +-
>>>>   tests/qemu-iotests/141.out |  2 +-
>>>>   tests/qemu-iotests/245     | 19 +++++++---
>>>>   5 files changed, 81 insertions(+), 88 deletions(-)
>>>
>>> Looks like stream_run() could be a bit streamlined now (the allocation
>>> checking should be unnecessary, unconditionally calling
>>> stream_populate() should be sufficient), but not necessary now.
>>>
>>
>> That is what I had kept in my mind when I tackled this patch. But 
>> there is an underwater reef to streamline. Namely, how the 
>> block-stream job gets known about a long unallocated tail to exit the 
>> loop earlier in the stream_run(). Shall we return the '-EOF' or 
>> another error code from the cor_co_preadv_part() to be handled by the 
>> stream_run()? Any other suggestions, if any, will be appreciated.
> 
> Just calling read CHUNK by CHUNK may be less efficient than 
> is_allocated()-driven loop: you may end up with splitting regions 
> unaligned to CHUNK-granularity, which would not be splitted with 
> is_allocated()-driven loop. Current loop allows chunks unaligned to CHUNK.

The cor_co_preadv_part() will check for the end of a file in the next 
version. So, the unalignment is not going to be the issue.

Andrey

> 
> So, I think, it's better to keep is_allocated() logic as is for now.
> 
> 
>