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.

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



--
Best regards,
Vladimir

Reply via email to