[Qemu-devel] [PATCH v5 0/5] qcow2: async handling of fragmented io

Vladimir Sementsov-Ogievskiy posted 5 patches 4 years, 7 months ago
Test docker-clang@ubuntu failed
Test FreeBSD failed
Test checkpatch failed
Test docker-quick@centos7 failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190916175324.18478-1-vsementsov@virtuozzo.com
Maintainers: Kevin Wolf <kwolf@redhat.com>, Max Reitz <mreitz@redhat.com>
block/qcow2.h                      |   3 +
include/block/aio_task.h           |  54 ++++
block/aio_task.c                   | 124 ++++++++
block/qcow2.c                      | 466 +++++++++++++++++++----------
block/Makefile.objs                |   2 +
block/trace-events                 |   1 +
tests/qemu-iotests/026             |   6 +-
tests/qemu-iotests/026.out         |  80 ++---
tests/qemu-iotests/026.out.nocache |  80 ++---
tests/qemu-iotests/common.rc       |  17 ++
10 files changed, 549 insertions(+), 284 deletions(-)
create mode 100644 include/block/aio_task.h
create mode 100644 block/aio_task.c
[Qemu-devel] [PATCH v5 0/5] qcow2: async handling of fragmented io
Posted by Vladimir Sementsov-Ogievskiy 4 years, 7 months ago
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.

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.

Based-on: https://github.com/XanClic/qemu.git block

About testing:

I have four 4G qcow2 images (with default 64k block size) on my ssd disk:
t-seq.qcow2 - sequentially written qcow2 image
t-reverse.qcow2 - filled by writing 64k portions from end to the start
t-rand.qcow2 - filled by writing 64k portions (aligned) in random order
t-part-rand.qcow2 - filled by shuffling order of 64k writes in 1m clusters
(see source code of image generation in the end for details)

and I've done several runs like the following (sequential io by 1mb chunks):

    out=/tmp/block; echo > $out; cat /tmp/files | while read file; do for wr in {"","-w"}; do echo "$file" $wr; ./qemu-img bench -c 4096 -d 1 -f qcow2 -n -s 1m -t none $wr "$file" | grep 'Run completed in' | awk '{print $4}' >> $out; done; done


short info about parameters:
  -w - do writes (otherwise do reads)
  -c - count of blocks
  -s - block size
  -t none - disable cache
  -n - native aio
  -d 1 - don't use parallel requests provided by qemu-img bench itself

results:

    +---------------------------+---------+---------+
    | file                      | master  | async   |
    +---------------------------+---------+---------+
    | /ssd/t-part-rand.qcow2    | 14.671  | 9.193   |
    +---------------------------+---------+---------+
    | /ssd/t-part-rand.qcow2 -w | 11.434  | 8.621   |
    +---------------------------+---------+---------+
    | /ssd/t-rand.qcow2         | 20.421  | 10.05   |
    +---------------------------+---------+---------+
    | /ssd/t-rand.qcow2 -w      | 11.097  | 8.915   |
    +---------------------------+---------+---------+
    | /ssd/t-reverse.qcow2      | 17.515  | 9.407   |
    +---------------------------+---------+---------+
    | /ssd/t-reverse.qcow2 -w   | 11.255  | 8.649   |
    +---------------------------+---------+---------+
    | /ssd/t-seq.qcow2          | 9.081   | 9.072   |
    +---------------------------+---------+---------+
    | /ssd/t-seq.qcow2 -w       | 8.761   | 8.747   |
    +---------------------------+---------+---------+
    | /tmp/t-part-rand.qcow2    | 41.179  | 41.37   |
    +---------------------------+---------+---------+
    | /tmp/t-part-rand.qcow2 -w | 54.097  | 55.323  |
    +---------------------------+---------+---------+
    | /tmp/t-rand.qcow2         | 711.899 | 514.339 |
    +---------------------------+---------+---------+
    | /tmp/t-rand.qcow2 -w      | 546.259 | 642.114 |
    +---------------------------+---------+---------+
    | /tmp/t-reverse.qcow2      | 86.065  | 96.522  |
    +---------------------------+---------+---------+
    | /tmp/t-reverse.qcow2 -w   | 46.557  | 48.499  |
    +---------------------------+---------+---------+
    | /tmp/t-seq.qcow2          | 33.804  | 33.862  |
    +---------------------------+---------+---------+
    | /tmp/t-seq.qcow2 -w       | 34.299  | 34.233  |
    +---------------------------+---------+---------+


Performance gain is obvious, especially for read and especially for ssd.
For hdd there is a degradation for reverse case, but this is the most
impossible case and seems not critical.

How images are generated:

    ==== gen-writes ======
    #!/usr/bin/env python
    import random
    import sys

    size = 4 * 1024 * 1024 * 1024
    block = 64 * 1024
    block2 = 1024 * 1024

    arg = sys.argv[1]

    if arg in ('rand', 'reverse', 'seq'):
        writes = list(range(0, size, block))

    if arg == 'rand':
        random.shuffle(writes)
    elif arg == 'reverse':
        writes.reverse()
    elif arg == 'part-rand':
        writes = []
        for off in range(0, size, block2):
            wr = list(range(off, off + block2, block))
            random.shuffle(wr)
            writes.extend(wr)
    elif arg != 'seq':
        sys.exit(1)

    for w in writes:
        print 'write -P 0xff {} {}'.format(w, block)

    print 'q'
    ==========================

    ===== gen-test-images.sh =====
    #!/bin/bash

    IMG_PATH=/ssd

    for name in seq reverse rand part-rand; do
        IMG=$IMG_PATH/t-$name.qcow2
        echo createing $IMG ...
        rm -f $IMG
        qemu-img create -f qcow2 $IMG 4G
        gen-writes $name | qemu-io $IMG
    done
    ==============================

Vladimir Sementsov-Ogievskiy (5):
  qemu-iotests: ignore leaks on failure paths in 026
  block: introduce aio task pool
  block/qcow2: refactor qcow2_co_preadv_part
  block/qcow2: refactor qcow2_co_pwritev_part
  block/qcow2: introduce parallel subrequest handling in read and write

 block/qcow2.h                      |   3 +
 include/block/aio_task.h           |  54 ++++
 block/aio_task.c                   | 124 ++++++++
 block/qcow2.c                      | 466 +++++++++++++++++++----------
 block/Makefile.objs                |   2 +
 block/trace-events                 |   1 +
 tests/qemu-iotests/026             |   6 +-
 tests/qemu-iotests/026.out         |  80 ++---
 tests/qemu-iotests/026.out.nocache |  80 ++---
 tests/qemu-iotests/common.rc       |  17 ++
 10 files changed, 549 insertions(+), 284 deletions(-)
 create mode 100644 include/block/aio_task.h
 create mode 100644 block/aio_task.c

-- 
2.21.0


Re: [Qemu-devel] [PATCH v5 0/5] qcow2: async handling of fragmented io
Posted by Vladimir Sementsov-Ogievskiy 4 years, 7 months ago
16.09.2019 20: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.
> 
> 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.
> 
> Based-on: https://github.com/XanClic/qemu.git block

Now based on master.



-- 
Best regards,
Vladimir
Re: [PATCH v5 0/5] qcow2: async handling of fragmented io
Posted by Max Reitz 4 years, 7 months ago
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

> 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.)

Max

Re: [PATCH v5 0/5] qcow2: async handling of fragmented io
Posted by Vladimir Sementsov-Ogievskiy 4 years, 7 months ago
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.


-- 
Best regards,
Vladimir
Re: [PATCH v5 0/5] qcow2: async handling of fragmented io
Posted by Max Reitz 4 years, 7 months ago
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. :-)

Max

Re: [PATCH v5 0/5] qcow2: async handling of fragmented io
Posted by Vladimir Sementsov-Ogievskiy 4 years, 7 months ago
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.


-- 
Best regards,
Vladimir
Re: [PATCH v5 0/5] qcow2: async handling of fragmented io
Posted by Max Reitz 4 years, 7 months ago
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 :-/)

Max

Re: [PATCH v5 0/5] qcow2: async handling of fragmented io
Posted by Vladimir Sementsov-Ogievskiy 4 years, 7 months ago
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)

---

Usage:

eat-diff-numbers is just sed script, used by both following scripts, to eat git numbers and hashes.

---

check-rebase - searches patches by name in original branch and compares

git check-rebase <original branch> <commits to check>

for example (check several backported commits):

git check-rebase master HEAD^^^^..HEAD

-----------

check-rebase2 - don't search by name, but just compare two sequences of patches of same length.

git check-rebase2 <original top commit>  <new top commit>  <how many commits>

for example (check one series version vs another)

git check-rebase2 salvage-v1 salvage-v2 7


-- 
Best regards,
Vladimir
#!/usr/bin/sh

sed -e 's/^index .*/index <some index>/' -e 's/^@@ .* @@/@@ <some lines> @@/' -e 's/^commit .*/commit <some commit>/' -e 's/^Date:.*00/Date: <some date>/'
#!/usr/bin/bash

function dshow {
    git show $1 | eat-diff-numbers
}

function comp {
    ap="/tmp/[a $3] $(git log --format=%f -n 1 $1).patch"
    bp="/tmp/[b $3] $(git log --format=%f -n 1 $2).patch"
    dshow $1 > "$ap"
    dshow $2 > "$bp"
    if ! diff "$ap" "$bp" > /dev/null ; then
        co=$(git log --format=%s -n 1 $2)
        echo -n $co - differs | tee -a loglog
        echo "$ap - $1"
        echo "$bp - $2"
        echo "type exit to continue"
        #git diff --no-index --color-words --word-diff-regex=. /tmp/{a,b}.patch
        if vimdiff  < /dev/tty -c 0 "$ap" "$bp" ; then
            echo " - ok" | tee -a loglog
        else
            echo " - bad" | tee -a loglog
        fi
        return 1
    fi
}

a=$1
b=$2
n=$3

for ((i=1; i<=n; i++)); do
    rr=$((n - i))
    co=$(git log --format=%s -n 1 $b~$rr)
    if comp $a~$rr $b~$rr $i; then
        echo $co - ok | tee -a loglog
    fi
done
#!/usr/bin/bash

function dshow {
    git show $1 | eat-diff-numbers
}

function comp {
    local b="/tmp/[b-$3]-$(git log --format=%f -n 1 $2).patch"
    dshow $2 > $b
    dshow $(git-find-subj $1 $2 || echo "NOT FOUND") > /tmp/a.patch
    if ! diff /tmp/a.patch $b > /dev/null ; then
        co=$(git log --format=%s -n 1 $c)
        echo -n $co - differs | tee -a loglog
        echo "/tmp/a.patch - found in $1: "
        echo "$b - $c from current branch"
        echo "type exit to continue"
        #git diff --no-index --color-words --word-diff-regex=. /tmp/{a,b}.patch
        if vimdiff  < /dev/tty -c 0 /tmp/a.patch $b ; then
            echo " - ok" | tee -a loglog
        else
            echo " - bad" | tee -a loglog
        fi
        return 1
    fi
}

rm loglog logcur
a=0
git log $2 --format="%h" --reverse | while read c; do
    ((a++))
    co=$(git log --format=%s -n 1 $c)
    git log -n 1 $c > logcur
    if comp $1 $c $a; then
        echo $co - ok | tee -a loglog
    fi
done
Re: [PATCH v5 0/5] qcow2: async handling of fragmented io
Posted by Max Reitz 4 years, 7 months ago
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

Re: [PATCH v5 0/5] qcow2: async handling of fragmented io
Posted by Eric Blake 4 years, 7 months ago
On 9/20/19 7:53 AM, Vladimir Sementsov-Ogievskiy wrote:

>>
>> 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.

I use git-backport-diff to get a gui comparison between two patch
series; https://github.com/codyprime/git-scripts.git

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org

Re: [PATCH v5 0/5] qcow2: async handling of fragmented io
Posted by Vladimir Sementsov-Ogievskiy 4 years, 7 months ago
20.09.2019 17:17, Eric Blake wrote:
> On 9/20/19 7:53 AM, Vladimir Sementsov-Ogievskiy wrote:
> 
>>>
>>> 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.
> 
> I use git-backport-diff to get a gui comparison between two patch
> series; https://github.com/codyprime/git-scripts.git
> 

Oh, that's cool, thanks!

Hmm, but it definitely lacks my eat-git-numbers script..

-- 
Best regards,
Vladimir