Re: [PATCH v5 0/5] qcow2: async handling of fragmented io

[Max Reitz]

On 20.09.19 15:26, Vladimir Sementsov-Ogievskiy wrote:
> 20.09.2019 16:10, Max Reitz wrote:
>> On 20.09.19 14:53, Vladimir Sementsov-Ogievskiy wrote:
>>> 20.09.2019 15:40, Max Reitz wrote:
>>>> On 20.09.19 13:53, Vladimir Sementsov-Ogievskiy wrote:
>>>>> 20.09.2019 14:10, Max Reitz wrote:
>>>>>> On 16.09.19 19:53, Vladimir Sementsov-Ogievskiy wrote:
>>>>>>> Hi all!
>>>>>>>
>>>>>>> Here is an asynchronous scheme for handling fragmented qcow2
>>>>>>> reads and writes. Both qcow2 read and write functions loops through
>>>>>>> sequential portions of data. The series aim it to parallelize these
>>>>>>> loops iterations.
>>>>>>> It improves performance for fragmented qcow2 images, I've tested it
>>>>>>> as described below.
>>>>>>
>>>>>> Thanks again, applied to my block branch:
>>>>>>
>>>>>> https://git.xanclic.moe/XanClic/qemu/commits/branch/block
>>>>>
>>>>> Thanks a lot!
>>>>>
>>>>>>
>>>>>>> v5: fix 026 and rebase on Max's block branch [perf results not updated]:
>>>>>>>
>>>>>>> 01: new, prepare 026 to not fail
>>>>>>> 03: - drop read_encrypted blkdbg event [Kevin]
>>>>>>>        - assert((x & (BDRV_SECTOR_SIZE - 1)) == 0) -> 
>>>>>>> assert(QEMU_IS_ALIGNED(x, BDRV_SECTOR_SIZE)) [rebase]
>>>>>>>        - full host offset in argument of qcow2_co_decrypt [rebase]
>>>>>>> 04: - substitute remaining qcow2_co_do_pwritev by qcow2_co_pwritev_task 
>>>>>>> in comment [Max]
>>>>>>>        - full host offset in argument of qcow2_co_encrypt [rebase]
>>>>>>> 05: - Now patch don't affect 026 iotest, so its output is not changed
>>>>>>>
>>>>>>> Rebase changes seems trivial, so, I've kept r-b marks.
>>>>>>
>>>>>> (For the record, I didn’t consider them trivial, or I’d’ve applied
>>>>>> Maxim’s series on top of yours.  I consider a conflict to be trivially
>>>>>> resolvable only if there is only one way of doing it; but when I
>>>>>> resolved the conflicts myself, I resolved the one in patch 3 differently
>>>>>> from you – I added an offset_in_cluster variable to
>>>>>> qcow2_co_preadv_encrypted().  Sure, it’s still simple and the difference
>>>>>> is minor, but that was exactly where I thought that I can’t consider
>>>>>> this trivial.)
>>>>>>
>>>>>
>>>>> Hmm. May be it's trivial enough to keep r-b (as my change is trivial 
>>>>> itself), but not
>>>>> trivial enough to change alien patch on queuing? If you disagree, I'll be 
>>>>> more
>>>>> careful on keeping r-b in changed patches, sorry.
>>>>
>>>> It doesn’t matter much to me, I diff all patches anyway. :-)
>>>>
>>>
>>> then a bit offtopic:
>>>
>>> Which tools are you use?
>>>
>>> I've some scripts to compare different versions of one serie (or to check, 
>>> what
>>> was changed in patches during some porting process..).. The core thing is 
>>> to filter
>>> some not interesting numbers and hashes, which makes diffs dirty, and then 
>>> call vimdiff.
>>> But maybe I've reinvented the wheel.
>>
>> Just kompare as a graphical diff tool; I just scroll past the hash diffs.
>>
>> But now that you gave me the idea, maybe I should write a script to
>> filter them...  (So, no, I don’t know of a tool that would do that
>> already :-/)
>>
> 
> 
> Then you may find my scripts somehow useful, at least as a hint (I'm afraid 
> code is not beautiful at all)

Thanks! :-)

Max

signature.asc
Description: OpenPGP digital signature