qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v5 0/5] qcow2: async handling of fragmented io
@ 2019-09-16 17:53 Vladimir Sementsov-Ogievskiy
  2019-09-16 17:53 ` [Qemu-devel] [PATCH v5 1/5] qemu-iotests: ignore leaks on failure paths in 026 Vladimir Sementsov-Ogievskiy
                   ` (6 more replies)
  0 siblings, 7 replies; 16+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-09-16 17:53 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, den, vsementsov, qemu-devel, mreitz

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



^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v5 1/5] qemu-iotests: ignore leaks on failure paths in 026
  2019-09-16 17:53 [Qemu-devel] [PATCH v5 0/5] qcow2: async handling of fragmented io Vladimir Sementsov-Ogievskiy
@ 2019-09-16 17:53 ` Vladimir Sementsov-Ogievskiy
  2019-09-16 17:53 ` [Qemu-devel] [PATCH v5 2/5] block: introduce aio task pool Vladimir Sementsov-Ogievskiy
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-09-16 17:53 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, den, vsementsov, qemu-devel, mreitz

Upcoming asynchronous handling of sub-parts of qcow2 requests will
change number of leaked clusters and even make it racy. As a
preparation, ignore leaks on failure parts in 026.

It's not trivial to just grep or substitute qemu-img output for such
thing. Instead do better: 3 is a error code of qemu-img check, if only
leaks are found. Catch this case and print success output.

Suggested-by: Anton Nefedov <anton.nefedov@virtuozzo.com>
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 tests/qemu-iotests/026             |  6 +--
 tests/qemu-iotests/026.out         | 80 ++++++++----------------------
 tests/qemu-iotests/026.out.nocache | 80 ++++++++----------------------
 tests/qemu-iotests/common.rc       | 17 +++++++
 4 files changed, 60 insertions(+), 123 deletions(-)

diff --git a/tests/qemu-iotests/026 b/tests/qemu-iotests/026
index ffb18ab6b5..3430029ed6 100755
--- a/tests/qemu-iotests/026
+++ b/tests/qemu-iotests/026
@@ -107,7 +107,7 @@ if [ "$event" == "l2_load" ]; then
     $QEMU_IO -c "read $vmstate 0 128k " "$BLKDBG_TEST_IMG" | _filter_qemu_io
 fi
 
-_check_test_img 2>&1 | grep -v "refcount=1 reference=0"
+_check_test_img_ignore_leaks 2>&1 | grep -v "refcount=1 reference=0"
 
 done
 done
@@ -152,7 +152,7 @@ echo
 echo "Event: $event; errno: $errno; imm: $imm; once: $once; write $vmstate"
 $QEMU_IO -c "write $vmstate 0 64M" "$BLKDBG_TEST_IMG" | _filter_qemu_io
 
-_check_test_img 2>&1 | grep -v "refcount=1 reference=0"
+_check_test_img_ignore_leaks 2>&1 | grep -v "refcount=1 reference=0"
 
 done
 done
@@ -191,7 +191,7 @@ echo
 echo "Event: $event; errno: $errno; imm: $imm; once: $once"
 $QEMU_IO -c "write -b 0 64k" "$BLKDBG_TEST_IMG" | _filter_qemu_io
 
-_check_test_img 2>&1 | grep -v "refcount=1 reference=0"
+_check_test_img_ignore_leaks 2>&1 | grep -v "refcount=1 reference=0"
 
 done
 done
diff --git a/tests/qemu-iotests/026.out b/tests/qemu-iotests/026.out
index fb89b8480c..ff0817b6f2 100644
--- a/tests/qemu-iotests/026.out
+++ b/tests/qemu-iotests/026.out
@@ -17,18 +17,14 @@ Event: l1_update; errno: 5; imm: off; once: off; write
 qemu-io: Failed to flush the L2 table cache: Input/output error
 qemu-io: Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
-
-1 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 
 Event: l1_update; errno: 5; imm: off; once: off; write -b
 qemu-io: Failed to flush the L2 table cache: Input/output error
 qemu-io: Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
-
-1 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 
 Event: l1_update; errno: 28; imm: off; once: on; write
@@ -45,18 +41,14 @@ Event: l1_update; errno: 28; imm: off; once: off; write
 qemu-io: Failed to flush the L2 table cache: No space left on device
 qemu-io: Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
-
-1 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 
 Event: l1_update; errno: 28; imm: off; once: off; write -b
 qemu-io: Failed to flush the L2 table cache: No space left on device
 qemu-io: Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
-
-1 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 
 Event: l2_load; errno: 5; imm: off; once: on; write
@@ -137,18 +129,14 @@ Event: l2_update; errno: 5; imm: off; once: off; write
 qemu-io: Failed to flush the L2 table cache: Input/output error
 qemu-io: Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
-
-127 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 
 Event: l2_update; errno: 5; imm: off; once: off; write -b
 qemu-io: Failed to flush the L2 table cache: Input/output error
 qemu-io: Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
-
-127 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 
 Event: l2_update; errno: 28; imm: off; once: on; write
@@ -165,18 +153,14 @@ Event: l2_update; errno: 28; imm: off; once: off; write
 qemu-io: Failed to flush the L2 table cache: No space left on device
 qemu-io: Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
-
-127 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 
 Event: l2_update; errno: 28; imm: off; once: off; write -b
 qemu-io: Failed to flush the L2 table cache: No space left on device
 qemu-io: Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
-
-127 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 
 Event: l2_alloc_write; errno: 5; imm: off; once: on; write
@@ -200,9 +184,7 @@ Event: l2_alloc_write; errno: 5; imm: off; once: off; write -b
 qemu-io: Failed to flush the L2 table cache: Input/output error
 qemu-io: Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
-
-1 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 
 Event: l2_alloc_write; errno: 28; imm: off; once: on; write
@@ -226,9 +208,7 @@ Event: l2_alloc_write; errno: 28; imm: off; once: off; write -b
 qemu-io: Failed to flush the L2 table cache: No space left on device
 qemu-io: Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
-
-1 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 
 Event: write_aio; errno: 5; imm: off; once: on; write
@@ -480,18 +460,14 @@ Event: refblock_alloc_hookup; errno: 28; imm: off; once: off; write
 qemu-io: Failed to flush the L2 table cache: No space left on device
 qemu-io: Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
-
-55 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 
 Event: refblock_alloc_hookup; errno: 28; imm: off; once: off; write -b
 qemu-io: Failed to flush the L2 table cache: No space left on device
 qemu-io: Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
-
-251 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 
 Event: refblock_alloc_write; errno: 28; imm: off; once: on; write
@@ -532,18 +508,14 @@ Event: refblock_alloc_write_blocks; errno: 28; imm: off; once: off; write
 qemu-io: Failed to flush the L2 table cache: No space left on device
 qemu-io: Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
-
-10 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 
 Event: refblock_alloc_write_blocks; errno: 28; imm: off; once: off; write -b
 qemu-io: Failed to flush the L2 table cache: No space left on device
 qemu-io: Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
-
-23 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 
 Event: refblock_alloc_write_table; errno: 28; imm: off; once: on; write
@@ -560,18 +532,14 @@ Event: refblock_alloc_write_table; errno: 28; imm: off; once: off; write
 qemu-io: Failed to flush the L2 table cache: No space left on device
 qemu-io: Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
-
-10 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 
 Event: refblock_alloc_write_table; errno: 28; imm: off; once: off; write -b
 qemu-io: Failed to flush the L2 table cache: No space left on device
 qemu-io: Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
-
-23 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 
 Event: refblock_alloc_switch_table; errno: 28; imm: off; once: on; write
@@ -588,18 +556,14 @@ Event: refblock_alloc_switch_table; errno: 28; imm: off; once: off; write
 qemu-io: Failed to flush the L2 table cache: No space left on device
 qemu-io: Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
-
-10 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 
 Event: refblock_alloc_switch_table; errno: 28; imm: off; once: off; write -b
 qemu-io: Failed to flush the L2 table cache: No space left on device
 qemu-io: Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
-
-23 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 
 === L1 growth tests ===
 
@@ -658,9 +622,7 @@ Event: l1_grow_activate_table; errno: 5; imm: off; once: off
 qemu-io: Failed to flush the L2 table cache: Input/output error
 qemu-io: Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
-
-96 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 
 Event: l1_grow_activate_table; errno: 28; imm: off; once: on
@@ -672,9 +634,7 @@ Event: l1_grow_activate_table; errno: 28; imm: off; once: off
 qemu-io: Failed to flush the L2 table cache: No space left on device
 qemu-io: Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
-
-96 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 
 === Avoid cluster leaks after temporary failure ===
 
diff --git a/tests/qemu-iotests/026.out.nocache b/tests/qemu-iotests/026.out.nocache
index 6dda95dfb4..495d013007 100644
--- a/tests/qemu-iotests/026.out.nocache
+++ b/tests/qemu-iotests/026.out.nocache
@@ -17,18 +17,14 @@ Event: l1_update; errno: 5; imm: off; once: off; write
 qemu-io: Failed to flush the L2 table cache: Input/output error
 qemu-io: Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
-
-1 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l1_update; errno: 5; imm: off; once: off; write -b
 qemu-io: Failed to flush the L2 table cache: Input/output error
 qemu-io: Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
-
-1 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l1_update; errno: 28; imm: off; once: on; write 
@@ -45,18 +41,14 @@ Event: l1_update; errno: 28; imm: off; once: off; write
 qemu-io: Failed to flush the L2 table cache: No space left on device
 qemu-io: Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
-
-1 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l1_update; errno: 28; imm: off; once: off; write -b
 qemu-io: Failed to flush the L2 table cache: No space left on device
 qemu-io: Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
-
-1 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l2_load; errno: 5; imm: off; once: on; write 
@@ -140,9 +132,7 @@ qemu-io: Failed to flush the L2 table cache: Input/output error
 qemu-io: Failed to flush the refcount block cache: Input/output error
 wrote 131072/131072 bytes at offset 0
 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-
-127 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l2_update; errno: 5; imm: off; once: off; write -b
@@ -150,9 +140,7 @@ qemu-io: Failed to flush the L2 table cache: Input/output error
 qemu-io: Failed to flush the refcount block cache: Input/output error
 wrote 131072/131072 bytes at offset 0
 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-
-127 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l2_update; errno: 28; imm: off; once: on; write 
@@ -172,9 +160,7 @@ qemu-io: Failed to flush the L2 table cache: No space left on device
 qemu-io: Failed to flush the refcount block cache: No space left on device
 wrote 131072/131072 bytes at offset 0
 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-
-127 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l2_update; errno: 28; imm: off; once: off; write -b
@@ -182,9 +168,7 @@ qemu-io: Failed to flush the L2 table cache: No space left on device
 qemu-io: Failed to flush the refcount block cache: No space left on device
 wrote 131072/131072 bytes at offset 0
 128 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
-
-127 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l2_alloc_write; errno: 5; imm: off; once: on; write 
@@ -208,9 +192,7 @@ Event: l2_alloc_write; errno: 5; imm: off; once: off; write -b
 qemu-io: Failed to flush the L2 table cache: Input/output error
 qemu-io: Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
-
-1 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l2_alloc_write; errno: 28; imm: off; once: on; write 
@@ -234,9 +216,7 @@ Event: l2_alloc_write; errno: 28; imm: off; once: off; write -b
 qemu-io: Failed to flush the L2 table cache: No space left on device
 qemu-io: Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
-
-1 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: write_aio; errno: 5; imm: off; once: on; write 
@@ -488,18 +468,14 @@ Event: refblock_alloc_hookup; errno: 28; imm: off; once: off; write
 qemu-io: Failed to flush the L2 table cache: No space left on device
 qemu-io: Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
-
-55 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_alloc_hookup; errno: 28; imm: off; once: off; write -b
 qemu-io: Failed to flush the L2 table cache: No space left on device
 qemu-io: Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
-
-251 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_alloc_write; errno: 28; imm: off; once: on; write 
@@ -540,18 +516,14 @@ Event: refblock_alloc_write_blocks; errno: 28; imm: off; once: off; write
 qemu-io: Failed to flush the L2 table cache: No space left on device
 qemu-io: Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
-
-10 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_alloc_write_blocks; errno: 28; imm: off; once: off; write -b
 qemu-io: Failed to flush the L2 table cache: No space left on device
 qemu-io: Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
-
-23 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_alloc_write_table; errno: 28; imm: off; once: on; write 
@@ -568,18 +540,14 @@ Event: refblock_alloc_write_table; errno: 28; imm: off; once: off; write
 qemu-io: Failed to flush the L2 table cache: No space left on device
 qemu-io: Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
-
-10 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_alloc_write_table; errno: 28; imm: off; once: off; write -b
 qemu-io: Failed to flush the L2 table cache: No space left on device
 qemu-io: Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
-
-23 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_alloc_switch_table; errno: 28; imm: off; once: on; write 
@@ -596,18 +564,14 @@ Event: refblock_alloc_switch_table; errno: 28; imm: off; once: off; write
 qemu-io: Failed to flush the L2 table cache: No space left on device
 qemu-io: Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
-
-10 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: refblock_alloc_switch_table; errno: 28; imm: off; once: off; write -b
 qemu-io: Failed to flush the L2 table cache: No space left on device
 qemu-io: Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
-
-23 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 
 === L1 growth tests ===
 
@@ -666,9 +630,7 @@ Event: l1_grow_activate_table; errno: 5; imm: off; once: off
 qemu-io: Failed to flush the L2 table cache: Input/output error
 qemu-io: Failed to flush the refcount block cache: Input/output error
 write failed: Input/output error
-
-96 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
 Event: l1_grow_activate_table; errno: 28; imm: off; once: on
@@ -680,9 +642,7 @@ Event: l1_grow_activate_table; errno: 28; imm: off; once: off
 qemu-io: Failed to flush the L2 table cache: No space left on device
 qemu-io: Failed to flush the refcount block cache: No space left on device
 write failed: No space left on device
-
-96 leaked clusters were found on the image.
-This means waste of disk space, but no harm to data.
+No errors were found on the image.
 
 === Avoid cluster leaks after temporary failure ===
 
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index e45cdfa66b..12b4751848 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -405,6 +405,23 @@ _check_test_img()
             $QEMU_IMG check "$@" -f $IMGFMT "$TEST_IMG" 2>&1
         fi
     ) | _filter_testdir | _filter_qemu_img_check
+
+    # return real qemu_img check status, to analyze in
+    # _check_test_img_ignore_leaks
+    return ${PIPESTATUS[0]}
+}
+
+_check_test_img_ignore_leaks()
+{
+    out=$(_check_test_img "$@")
+    status=$?
+    if [ $status = 3 ]; then
+        # This must correspond to success output in dump_human_image_check()
+        echo "No errors were found on the image."
+        return 0
+    fi
+    echo "$out"
+    return $status
 }
 
 _img_info()
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v5 2/5] block: introduce aio task pool
  2019-09-16 17:53 [Qemu-devel] [PATCH v5 0/5] qcow2: async handling of fragmented io Vladimir Sementsov-Ogievskiy
  2019-09-16 17:53 ` [Qemu-devel] [PATCH v5 1/5] qemu-iotests: ignore leaks on failure paths in 026 Vladimir Sementsov-Ogievskiy
@ 2019-09-16 17:53 ` Vladimir Sementsov-Ogievskiy
  2019-09-16 17:53 ` [Qemu-devel] [PATCH v5 3/5] block/qcow2: refactor qcow2_co_preadv_part Vladimir Sementsov-Ogievskiy
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-09-16 17:53 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, den, vsementsov, qemu-devel, mreitz

Common interface for aio task loops. To be used for improving
performance of synchronous io loops in qcow2, block-stream,
copy-on-read, and may be other places.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 include/block/aio_task.h |  54 +++++++++++++++++
 block/aio_task.c         | 124 +++++++++++++++++++++++++++++++++++++++
 block/Makefile.objs      |   2 +
 3 files changed, 180 insertions(+)
 create mode 100644 include/block/aio_task.h
 create mode 100644 block/aio_task.c

diff --git a/include/block/aio_task.h b/include/block/aio_task.h
new file mode 100644
index 0000000000..50bc1e1817
--- /dev/null
+++ b/include/block/aio_task.h
@@ -0,0 +1,54 @@
+/*
+ * Aio tasks loops
+ *
+ * Copyright (c) 2019 Virtuozzo International GmbH.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef BLOCK_AIO_TASK_H
+#define BLOCK_AIO_TASK_H
+
+#include "qemu/coroutine.h"
+
+typedef struct AioTaskPool AioTaskPool;
+typedef struct AioTask AioTask;
+typedef int coroutine_fn (*AioTaskFunc)(AioTask *task);
+struct AioTask {
+    AioTaskPool *pool;
+    AioTaskFunc func;
+    int ret;
+};
+
+AioTaskPool *coroutine_fn aio_task_pool_new(int max_busy_tasks);
+void aio_task_pool_free(AioTaskPool *);
+
+/* error code of failed task or 0 if all is OK */
+int aio_task_pool_status(AioTaskPool *pool);
+
+bool aio_task_pool_empty(AioTaskPool *pool);
+
+/* User provides filled @task, however task->pool will be set automatically */
+void coroutine_fn aio_task_pool_start_task(AioTaskPool *pool, AioTask *task);
+
+void coroutine_fn aio_task_pool_wait_slot(AioTaskPool *pool);
+void coroutine_fn aio_task_pool_wait_one(AioTaskPool *pool);
+void coroutine_fn aio_task_pool_wait_all(AioTaskPool *pool);
+
+#endif /* BLOCK_AIO_TASK_H */
diff --git a/block/aio_task.c b/block/aio_task.c
new file mode 100644
index 0000000000..88989fa248
--- /dev/null
+++ b/block/aio_task.c
@@ -0,0 +1,124 @@
+/*
+ * Aio tasks loops
+ *
+ * Copyright (c) 2019 Virtuozzo International GmbH.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "block/aio.h"
+#include "block/aio_task.h"
+
+struct AioTaskPool {
+    Coroutine *main_co;
+    int status;
+    int max_busy_tasks;
+    int busy_tasks;
+    bool waiting;
+};
+
+static void coroutine_fn aio_task_co(void *opaque)
+{
+    AioTask *task = opaque;
+    AioTaskPool *pool = task->pool;
+
+    assert(pool->busy_tasks < pool->max_busy_tasks);
+    pool->busy_tasks++;
+
+    task->ret = task->func(task);
+
+    pool->busy_tasks--;
+
+    if (task->ret < 0 && pool->status == 0) {
+        pool->status = task->ret;
+    }
+
+    g_free(task);
+
+    if (pool->waiting) {
+        pool->waiting = false;
+        aio_co_wake(pool->main_co);
+    }
+}
+
+void coroutine_fn aio_task_pool_wait_one(AioTaskPool *pool)
+{
+    assert(pool->busy_tasks > 0);
+    assert(qemu_coroutine_self() == pool->main_co);
+
+    pool->waiting = true;
+    qemu_coroutine_yield();
+
+    assert(!pool->waiting);
+    assert(pool->busy_tasks < pool->max_busy_tasks);
+}
+
+void coroutine_fn aio_task_pool_wait_slot(AioTaskPool *pool)
+{
+    if (pool->busy_tasks < pool->max_busy_tasks) {
+        return;
+    }
+
+    aio_task_pool_wait_one(pool);
+}
+
+void coroutine_fn aio_task_pool_wait_all(AioTaskPool *pool)
+{
+    while (pool->busy_tasks > 0) {
+        aio_task_pool_wait_one(pool);
+    }
+}
+
+void coroutine_fn aio_task_pool_start_task(AioTaskPool *pool, AioTask *task)
+{
+    aio_task_pool_wait_slot(pool);
+
+    task->pool = pool;
+    qemu_coroutine_enter(qemu_coroutine_create(aio_task_co, task));
+}
+
+AioTaskPool *coroutine_fn aio_task_pool_new(int max_busy_tasks)
+{
+    AioTaskPool *pool = g_new0(AioTaskPool, 1);
+
+    pool->main_co = qemu_coroutine_self();
+    pool->max_busy_tasks = max_busy_tasks;
+
+    return pool;
+}
+
+void aio_task_pool_free(AioTaskPool *pool)
+{
+    g_free(pool);
+}
+
+int aio_task_pool_status(AioTaskPool *pool)
+{
+    if (!pool) {
+        return 0; /* Sugar for lazy allocation of aio pool */
+    }
+
+    return pool->status;
+}
+
+bool aio_task_pool_empty(AioTaskPool *pool)
+{
+    return pool->busy_tasks == 0;
+}
diff --git a/block/Makefile.objs b/block/Makefile.objs
index 35f3bca4d9..c2eb8c8769 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -40,6 +40,8 @@ block-obj-y += throttle.o copy-on-read.o
 
 block-obj-y += crypto.o
 
+block-obj-y += aio_task.o
+
 common-obj-y += stream.o
 
 nfs.o-libs         := $(LIBNFS_LIBS)
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v5 3/5] block/qcow2: refactor qcow2_co_preadv_part
  2019-09-16 17:53 [Qemu-devel] [PATCH v5 0/5] qcow2: async handling of fragmented io Vladimir Sementsov-Ogievskiy
  2019-09-16 17:53 ` [Qemu-devel] [PATCH v5 1/5] qemu-iotests: ignore leaks on failure paths in 026 Vladimir Sementsov-Ogievskiy
  2019-09-16 17:53 ` [Qemu-devel] [PATCH v5 2/5] block: introduce aio task pool Vladimir Sementsov-Ogievskiy
@ 2019-09-16 17:53 ` Vladimir Sementsov-Ogievskiy
  2019-09-16 17:53 ` [Qemu-devel] [PATCH v5 4/5] block/qcow2: refactor qcow2_co_pwritev_part Vladimir Sementsov-Ogievskiy
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-09-16 17:53 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, den, vsementsov, qemu-devel, mreitz

Further patch will run partial requests of iterations of
qcow2_co_preadv in parallel for performance reasons. To prepare for
this, separate part which may be parallelized into separate function
(qcow2_co_preadv_task).

While being here, also separate encrypted clusters reading to own
function, like it is done for compressed reading.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2.c | 209 +++++++++++++++++++++++++++-----------------------
 1 file changed, 113 insertions(+), 96 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 4d16393e61..6feb169f7c 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1972,17 +1972,117 @@ out:
     return ret;
 }
 
+static coroutine_fn int
+qcow2_co_preadv_encrypted(BlockDriverState *bs,
+                           uint64_t file_cluster_offset,
+                           uint64_t offset,
+                           uint64_t bytes,
+                           QEMUIOVector *qiov,
+                           uint64_t qiov_offset)
+{
+    int ret;
+    BDRVQcow2State *s = bs->opaque;
+    uint8_t *buf;
+
+    assert(bs->encrypted && s->crypto);
+    assert(bytes <= QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size);
+
+    /*
+     * For encrypted images, read everything into a temporary
+     * contiguous buffer on which the AES functions can work.
+     * Also, decryption in a separate buffer is better as it
+     * prevents the guest from learning information about the
+     * encrypted nature of the virtual disk.
+     */
+
+    buf = qemu_try_blockalign(s->data_file->bs, bytes);
+    if (buf == NULL) {
+        return -ENOMEM;
+    }
+
+    BLKDBG_EVENT(bs->file, BLKDBG_READ_AIO);
+    ret = bdrv_co_pread(s->data_file,
+                        file_cluster_offset + offset_into_cluster(s, offset),
+                        bytes, buf, 0);
+    if (ret < 0) {
+        goto fail;
+    }
+
+    assert(QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE));
+    assert(QEMU_IS_ALIGNED(bytes, BDRV_SECTOR_SIZE));
+    if (qcow2_co_decrypt(bs,
+                         file_cluster_offset + offset_into_cluster(s, offset),
+                         offset, buf, bytes) < 0)
+    {
+        ret = -EIO;
+        goto fail;
+    }
+    qemu_iovec_from_buf(qiov, qiov_offset, buf, bytes);
+
+fail:
+    qemu_vfree(buf);
+
+    return ret;
+}
+
+static coroutine_fn int qcow2_co_preadv_task(BlockDriverState *bs,
+                                             QCow2ClusterType cluster_type,
+                                             uint64_t file_cluster_offset,
+                                             uint64_t offset, uint64_t bytes,
+                                             QEMUIOVector *qiov,
+                                             size_t qiov_offset)
+{
+    BDRVQcow2State *s = bs->opaque;
+    int offset_in_cluster = offset_into_cluster(s, offset);
+
+    switch (cluster_type) {
+    case QCOW2_CLUSTER_ZERO_PLAIN:
+    case QCOW2_CLUSTER_ZERO_ALLOC:
+        /* Both zero types are handled in qcow2_co_preadv_part */
+        g_assert_not_reached();
+
+    case QCOW2_CLUSTER_UNALLOCATED:
+        assert(bs->backing); /* otherwise handled in qcow2_co_preadv_part */
+
+        BLKDBG_EVENT(bs->file, BLKDBG_READ_BACKING_AIO);
+        return bdrv_co_preadv_part(bs->backing, offset, bytes,
+                                   qiov, qiov_offset, 0);
+
+    case QCOW2_CLUSTER_COMPRESSED:
+        return qcow2_co_preadv_compressed(bs, file_cluster_offset,
+                                          offset, bytes, qiov, qiov_offset);
+
+    case QCOW2_CLUSTER_NORMAL:
+        if ((file_cluster_offset & 511) != 0) {
+            return -EIO;
+        }
+
+        if (bs->encrypted) {
+            return qcow2_co_preadv_encrypted(bs, file_cluster_offset,
+                                             offset, bytes, qiov, qiov_offset);
+        }
+
+        BLKDBG_EVENT(bs->file, BLKDBG_READ_AIO);
+        return bdrv_co_preadv_part(s->data_file,
+                                   file_cluster_offset + offset_in_cluster,
+                                   bytes, qiov, qiov_offset, 0);
+
+    default:
+        g_assert_not_reached();
+    }
+
+    g_assert_not_reached();
+}
+
 static coroutine_fn int qcow2_co_preadv_part(BlockDriverState *bs,
                                              uint64_t offset, uint64_t bytes,
                                              QEMUIOVector *qiov,
                                              size_t qiov_offset, int flags)
 {
     BDRVQcow2State *s = bs->opaque;
-    int offset_in_cluster;
     int ret;
     unsigned int cur_bytes; /* number of bytes in current iteration */
     uint64_t cluster_offset = 0;
-    uint8_t *cluster_data = NULL;
 
     while (bytes != 0) {
 
@@ -1997,112 +2097,29 @@ static coroutine_fn int qcow2_co_preadv_part(BlockDriverState *bs,
         ret = qcow2_get_cluster_offset(bs, offset, &cur_bytes, &cluster_offset);
         qemu_co_mutex_unlock(&s->lock);
         if (ret < 0) {
-            goto fail;
+            return ret;
         }
 
-        offset_in_cluster = offset_into_cluster(s, offset);
-
-        switch (ret) {
-        case QCOW2_CLUSTER_UNALLOCATED:
-
-            if (bs->backing) {
-                BLKDBG_EVENT(bs->file, BLKDBG_READ_BACKING_AIO);
-                ret = bdrv_co_preadv_part(bs->backing, offset, cur_bytes,
-                                          qiov, qiov_offset, 0);
-                if (ret < 0) {
-                    goto fail;
-                }
-            } else {
-                /* Note: in this case, no need to wait */
-                qemu_iovec_memset(qiov, qiov_offset, 0, cur_bytes);
-            }
-            break;
-
-        case QCOW2_CLUSTER_ZERO_PLAIN:
-        case QCOW2_CLUSTER_ZERO_ALLOC:
+        if (ret == QCOW2_CLUSTER_ZERO_PLAIN ||
+            ret == QCOW2_CLUSTER_ZERO_ALLOC ||
+            (ret == QCOW2_CLUSTER_UNALLOCATED && !bs->backing))
+        {
             qemu_iovec_memset(qiov, qiov_offset, 0, cur_bytes);
-            break;
-
-        case QCOW2_CLUSTER_COMPRESSED:
-            ret = qcow2_co_preadv_compressed(bs, cluster_offset,
-                                             offset, cur_bytes,
-                                             qiov, qiov_offset);
+        } else {
+            ret = qcow2_co_preadv_task(bs, ret,
+                                       cluster_offset, offset, cur_bytes,
+                                       qiov, qiov_offset);
             if (ret < 0) {
-                goto fail;
-            }
-
-            break;
-
-        case QCOW2_CLUSTER_NORMAL:
-            if ((cluster_offset & 511) != 0) {
-                ret = -EIO;
-                goto fail;
-            }
-
-            if (bs->encrypted) {
-                assert(s->crypto);
-
-                /*
-                 * For encrypted images, read everything into a temporary
-                 * contiguous buffer on which the AES functions can work.
-                 */
-                if (!cluster_data) {
-                    cluster_data =
-                        qemu_try_blockalign(s->data_file->bs,
-                                            QCOW_MAX_CRYPT_CLUSTERS
-                                            * s->cluster_size);
-                    if (cluster_data == NULL) {
-                        ret = -ENOMEM;
-                        goto fail;
-                    }
-                }
-
-                assert(cur_bytes <= QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size);
-
-                BLKDBG_EVENT(bs->file, BLKDBG_READ_AIO);
-                ret = bdrv_co_pread(s->data_file,
-                                    cluster_offset + offset_in_cluster,
-                                    cur_bytes, cluster_data, 0);
-                if (ret < 0) {
-                    goto fail;
-                }
-
-                assert(QEMU_IS_ALIGNED(offset, BDRV_SECTOR_SIZE));
-                assert(QEMU_IS_ALIGNED(cur_bytes, BDRV_SECTOR_SIZE));
-                if (qcow2_co_decrypt(bs, cluster_offset + offset_in_cluster,
-                                     offset,
-                                     cluster_data, cur_bytes) < 0) {
-                    ret = -EIO;
-                    goto fail;
-                }
-                qemu_iovec_from_buf(qiov, qiov_offset, cluster_data, cur_bytes);
-            } else {
-                BLKDBG_EVENT(bs->file, BLKDBG_READ_AIO);
-                ret = bdrv_co_preadv_part(s->data_file,
-                                          cluster_offset + offset_in_cluster,
-                                          cur_bytes, qiov, qiov_offset, 0);
-                if (ret < 0) {
-                    goto fail;
-                }
+                return ret;
             }
-            break;
-
-        default:
-            g_assert_not_reached();
-            ret = -EIO;
-            goto fail;
         }
 
         bytes -= cur_bytes;
         offset += cur_bytes;
         qiov_offset += cur_bytes;
     }
-    ret = 0;
-
-fail:
-    qemu_vfree(cluster_data);
 
-    return ret;
+    return 0;
 }
 
 /* Check if it's possible to merge a write request with the writing of
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v5 4/5] block/qcow2: refactor qcow2_co_pwritev_part
  2019-09-16 17:53 [Qemu-devel] [PATCH v5 0/5] qcow2: async handling of fragmented io Vladimir Sementsov-Ogievskiy
                   ` (2 preceding siblings ...)
  2019-09-16 17:53 ` [Qemu-devel] [PATCH v5 3/5] block/qcow2: refactor qcow2_co_preadv_part Vladimir Sementsov-Ogievskiy
@ 2019-09-16 17:53 ` Vladimir Sementsov-Ogievskiy
  2019-09-16 17:53 ` [Qemu-devel] [PATCH v5 5/5] block/qcow2: introduce parallel subrequest handling in read and write Vladimir Sementsov-Ogievskiy
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-09-16 17:53 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, den, vsementsov, qemu-devel, mreitz

Similarly to previous commit, prepare for parallelizing write-loop
iterations.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
---
 block/qcow2.c | 154 +++++++++++++++++++++++++++++---------------------
 1 file changed, 90 insertions(+), 64 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 6feb169f7c..164b22e4a2 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2242,6 +2242,88 @@ static int handle_alloc_space(BlockDriverState *bs, QCowL2Meta *l2meta)
     return 0;
 }
 
+/*
+ * qcow2_co_pwritev_task
+ * Called with s->lock unlocked
+ * l2meta  - if not NULL, qcow2_co_pwritev_task() will consume it. Caller must
+ *           not use it somehow after qcow2_co_pwritev_task() call
+ */
+static coroutine_fn int qcow2_co_pwritev_task(BlockDriverState *bs,
+                                              uint64_t file_cluster_offset,
+                                              uint64_t offset, uint64_t bytes,
+                                              QEMUIOVector *qiov,
+                                              uint64_t qiov_offset,
+                                              QCowL2Meta *l2meta)
+{
+    int ret;
+    BDRVQcow2State *s = bs->opaque;
+    void *crypt_buf = NULL;
+    int offset_in_cluster = offset_into_cluster(s, offset);
+    QEMUIOVector encrypted_qiov;
+
+    if (bs->encrypted) {
+        assert(s->crypto);
+        assert(bytes <= QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size);
+        crypt_buf = qemu_try_blockalign(bs->file->bs, bytes);
+        if (crypt_buf == NULL) {
+            ret = -ENOMEM;
+            goto out_unlocked;
+        }
+        qemu_iovec_to_buf(qiov, qiov_offset, crypt_buf, bytes);
+
+        if (qcow2_co_encrypt(bs, file_cluster_offset + offset_in_cluster,
+                             offset, crypt_buf, bytes) < 0)
+        {
+            ret = -EIO;
+            goto out_unlocked;
+        }
+
+        qemu_iovec_init_buf(&encrypted_qiov, crypt_buf, bytes);
+        qiov = &encrypted_qiov;
+        qiov_offset = 0;
+    }
+
+    /* Try to efficiently initialize the physical space with zeroes */
+    ret = handle_alloc_space(bs, l2meta);
+    if (ret < 0) {
+        goto out_unlocked;
+    }
+
+    /*
+     * If we need to do COW, check if it's possible to merge the
+     * writing of the guest data together with that of the COW regions.
+     * If it's not possible (or not necessary) then write the
+     * guest data now.
+     */
+    if (!merge_cow(offset, bytes, qiov, qiov_offset, l2meta)) {
+        BLKDBG_EVENT(bs->file, BLKDBG_WRITE_AIO);
+        trace_qcow2_writev_data(qemu_coroutine_self(),
+                                file_cluster_offset + offset_in_cluster);
+        ret = bdrv_co_pwritev_part(s->data_file,
+                                   file_cluster_offset + offset_in_cluster,
+                                   bytes, qiov, qiov_offset, 0);
+        if (ret < 0) {
+            goto out_unlocked;
+        }
+    }
+
+    qemu_co_mutex_lock(&s->lock);
+
+    ret = qcow2_handle_l2meta(bs, &l2meta, true);
+    goto out_locked;
+
+out_unlocked:
+    qemu_co_mutex_lock(&s->lock);
+
+out_locked:
+    qcow2_handle_l2meta(bs, &l2meta, false);
+    qemu_co_mutex_unlock(&s->lock);
+
+    qemu_vfree(crypt_buf);
+
+    return ret;
+}
+
 static coroutine_fn int qcow2_co_pwritev_part(
         BlockDriverState *bs, uint64_t offset, uint64_t bytes,
         QEMUIOVector *qiov, size_t qiov_offset, int flags)
@@ -2251,15 +2333,10 @@ static coroutine_fn int qcow2_co_pwritev_part(
     int ret;
     unsigned int cur_bytes; /* number of sectors in current iteration */
     uint64_t cluster_offset;
-    QEMUIOVector encrypted_qiov;
-    uint64_t bytes_done = 0;
-    uint8_t *cluster_data = NULL;
     QCowL2Meta *l2meta = NULL;
 
     trace_qcow2_writev_start_req(qemu_coroutine_self(), offset, bytes);
 
-    qemu_co_mutex_lock(&s->lock);
-
     while (bytes != 0) {
 
         l2meta = NULL;
@@ -2273,6 +2350,8 @@ static coroutine_fn int qcow2_co_pwritev_part(
                             - offset_in_cluster);
         }
 
+        qemu_co_mutex_lock(&s->lock);
+
         ret = qcow2_alloc_cluster_offset(bs, offset, &cur_bytes,
                                          &cluster_offset, &l2meta);
         if (ret < 0) {
@@ -2290,73 +2369,20 @@ static coroutine_fn int qcow2_co_pwritev_part(
 
         qemu_co_mutex_unlock(&s->lock);
 
-        if (bs->encrypted) {
-            assert(s->crypto);
-            if (!cluster_data) {
-                cluster_data = qemu_try_blockalign(bs->file->bs,
-                                                   QCOW_MAX_CRYPT_CLUSTERS
-                                                   * s->cluster_size);
-                if (cluster_data == NULL) {
-                    ret = -ENOMEM;
-                    goto out_unlocked;
-                }
-            }
-
-            assert(cur_bytes <= QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size);
-            qemu_iovec_to_buf(qiov, qiov_offset + bytes_done,
-                              cluster_data, cur_bytes);
-
-            if (qcow2_co_encrypt(bs, cluster_offset + offset_in_cluster, offset,
-                                 cluster_data, cur_bytes) < 0) {
-                ret = -EIO;
-                goto out_unlocked;
-            }
-
-            qemu_iovec_init_buf(&encrypted_qiov, cluster_data, cur_bytes);
-        }
-
-        /* Try to efficiently initialize the physical space with zeroes */
-        ret = handle_alloc_space(bs, l2meta);
+        ret = qcow2_co_pwritev_task(bs, cluster_offset, offset, cur_bytes,
+                                    qiov, qiov_offset, l2meta);
+        l2meta = NULL; /* l2meta is consumed by qcow2_co_pwritev_task() */
         if (ret < 0) {
-            goto out_unlocked;
-        }
-
-        /* If we need to do COW, check if it's possible to merge the
-         * writing of the guest data together with that of the COW regions.
-         * If it's not possible (or not necessary) then write the
-         * guest data now. */
-        if (!merge_cow(offset, cur_bytes,
-                       bs->encrypted ? &encrypted_qiov : qiov,
-                       bs->encrypted ? 0 : qiov_offset + bytes_done, l2meta))
-        {
-            BLKDBG_EVENT(bs->file, BLKDBG_WRITE_AIO);
-            trace_qcow2_writev_data(qemu_coroutine_self(),
-                                    cluster_offset + offset_in_cluster);
-            ret = bdrv_co_pwritev_part(
-                    s->data_file, cluster_offset + offset_in_cluster, cur_bytes,
-                    bs->encrypted ? &encrypted_qiov : qiov,
-                    bs->encrypted ? 0 : qiov_offset + bytes_done, 0);
-            if (ret < 0) {
-                goto out_unlocked;
-            }
-        }
-
-        qemu_co_mutex_lock(&s->lock);
-
-        ret = qcow2_handle_l2meta(bs, &l2meta, true);
-        if (ret) {
-            goto out_locked;
+            goto fail_nometa;
         }
 
         bytes -= cur_bytes;
         offset += cur_bytes;
-        bytes_done += cur_bytes;
+        qiov_offset += cur_bytes;
         trace_qcow2_writev_done_part(qemu_coroutine_self(), cur_bytes);
     }
     ret = 0;
-    goto out_locked;
 
-out_unlocked:
     qemu_co_mutex_lock(&s->lock);
 
 out_locked:
@@ -2364,7 +2390,7 @@ out_locked:
 
     qemu_co_mutex_unlock(&s->lock);
 
-    qemu_vfree(cluster_data);
+fail_nometa:
     trace_qcow2_writev_done_req(qemu_coroutine_self(), ret);
 
     return ret;
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v5 5/5] block/qcow2: introduce parallel subrequest handling in read and write
  2019-09-16 17:53 [Qemu-devel] [PATCH v5 0/5] qcow2: async handling of fragmented io Vladimir Sementsov-Ogievskiy
                   ` (3 preceding siblings ...)
  2019-09-16 17:53 ` [Qemu-devel] [PATCH v5 4/5] block/qcow2: refactor qcow2_co_pwritev_part Vladimir Sementsov-Ogievskiy
@ 2019-09-16 17:53 ` Vladimir Sementsov-Ogievskiy
  2019-09-17  9:32 ` [Qemu-devel] [PATCH v5 0/5] qcow2: async handling of fragmented io Vladimir Sementsov-Ogievskiy
  2019-09-20 11:10 ` Max Reitz
  6 siblings, 0 replies; 16+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-09-16 17:53 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, den, vsementsov, qemu-devel, mreitz

It improves performance for fragmented qcow2 images.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/qcow2.h      |   3 ++
 block/qcow2.c      | 125 ++++++++++++++++++++++++++++++++++++++++-----
 block/trace-events |   1 +
 3 files changed, 117 insertions(+), 12 deletions(-)

diff --git a/block/qcow2.h b/block/qcow2.h
index a488d761ff..f51f478e34 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -65,6 +65,9 @@
 #define QCOW2_MAX_BITMAPS 65535
 #define QCOW2_MAX_BITMAP_DIRECTORY_SIZE (1024 * QCOW2_MAX_BITMAPS)
 
+/* Maximum of parallel sub-request per guest request */
+#define QCOW2_MAX_WORKERS 8
+
 /* indicate that the refcount of the referenced cluster is exactly one. */
 #define QCOW_OFLAG_COPIED     (1ULL << 63)
 /* indicate that the cluster is compressed (they never have the copied flag) */
diff --git a/block/qcow2.c b/block/qcow2.c
index 164b22e4a2..7961c05783 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -41,6 +41,7 @@
 #include "qapi/qobject-input-visitor.h"
 #include "qapi/qapi-visit-block-core.h"
 #include "crypto.h"
+#include "block/aio_task.h"
 
 /*
   Differences with QCOW:
@@ -2025,6 +2026,60 @@ fail:
     return ret;
 }
 
+typedef struct Qcow2AioTask {
+    AioTask task;
+
+    BlockDriverState *bs;
+    QCow2ClusterType cluster_type; /* only for read */
+    uint64_t file_cluster_offset;
+    uint64_t offset;
+    uint64_t bytes;
+    QEMUIOVector *qiov;
+    uint64_t qiov_offset;
+    QCowL2Meta *l2meta; /* only for write */
+} Qcow2AioTask;
+
+static coroutine_fn int qcow2_co_preadv_task_entry(AioTask *task);
+static coroutine_fn int qcow2_add_task(BlockDriverState *bs,
+                                       AioTaskPool *pool,
+                                       AioTaskFunc func,
+                                       QCow2ClusterType cluster_type,
+                                       uint64_t file_cluster_offset,
+                                       uint64_t offset,
+                                       uint64_t bytes,
+                                       QEMUIOVector *qiov,
+                                       size_t qiov_offset,
+                                       QCowL2Meta *l2meta)
+{
+    Qcow2AioTask local_task;
+    Qcow2AioTask *task = pool ? g_new(Qcow2AioTask, 1) : &local_task;
+
+    *task = (Qcow2AioTask) {
+        .task.func = func,
+        .bs = bs,
+        .cluster_type = cluster_type,
+        .qiov = qiov,
+        .file_cluster_offset = file_cluster_offset,
+        .offset = offset,
+        .bytes = bytes,
+        .qiov_offset = qiov_offset,
+        .l2meta = l2meta,
+    };
+
+    trace_qcow2_add_task(qemu_coroutine_self(), bs, pool,
+                         func == qcow2_co_preadv_task_entry ? "read" : "write",
+                         cluster_type, file_cluster_offset, offset, bytes,
+                         qiov, qiov_offset);
+
+    if (!pool) {
+        return func(&task->task);
+    }
+
+    aio_task_pool_start_task(pool, &task->task);
+
+    return 0;
+}
+
 static coroutine_fn int qcow2_co_preadv_task(BlockDriverState *bs,
                                              QCow2ClusterType cluster_type,
                                              uint64_t file_cluster_offset,
@@ -2074,18 +2129,28 @@ static coroutine_fn int qcow2_co_preadv_task(BlockDriverState *bs,
     g_assert_not_reached();
 }
 
+static coroutine_fn int qcow2_co_preadv_task_entry(AioTask *task)
+{
+    Qcow2AioTask *t = container_of(task, Qcow2AioTask, task);
+
+    assert(!t->l2meta);
+
+    return qcow2_co_preadv_task(t->bs, t->cluster_type, t->file_cluster_offset,
+                                t->offset, t->bytes, t->qiov, t->qiov_offset);
+}
+
 static coroutine_fn int qcow2_co_preadv_part(BlockDriverState *bs,
                                              uint64_t offset, uint64_t bytes,
                                              QEMUIOVector *qiov,
                                              size_t qiov_offset, int flags)
 {
     BDRVQcow2State *s = bs->opaque;
-    int ret;
+    int ret = 0;
     unsigned int cur_bytes; /* number of bytes in current iteration */
     uint64_t cluster_offset = 0;
+    AioTaskPool *aio = NULL;
 
-    while (bytes != 0) {
-
+    while (bytes != 0 && aio_task_pool_status(aio) == 0) {
         /* prepare next request */
         cur_bytes = MIN(bytes, INT_MAX);
         if (s->crypto) {
@@ -2097,7 +2162,7 @@ static coroutine_fn int qcow2_co_preadv_part(BlockDriverState *bs,
         ret = qcow2_get_cluster_offset(bs, offset, &cur_bytes, &cluster_offset);
         qemu_co_mutex_unlock(&s->lock);
         if (ret < 0) {
-            return ret;
+            goto out;
         }
 
         if (ret == QCOW2_CLUSTER_ZERO_PLAIN ||
@@ -2106,11 +2171,14 @@ static coroutine_fn int qcow2_co_preadv_part(BlockDriverState *bs,
         {
             qemu_iovec_memset(qiov, qiov_offset, 0, cur_bytes);
         } else {
-            ret = qcow2_co_preadv_task(bs, ret,
-                                       cluster_offset, offset, cur_bytes,
-                                       qiov, qiov_offset);
+            if (!aio && cur_bytes != bytes) {
+                aio = aio_task_pool_new(QCOW2_MAX_WORKERS);
+            }
+            ret = qcow2_add_task(bs, aio, qcow2_co_preadv_task_entry, ret,
+                                 cluster_offset, offset, cur_bytes,
+                                 qiov, qiov_offset, NULL);
             if (ret < 0) {
-                return ret;
+                goto out;
             }
         }
 
@@ -2119,7 +2187,16 @@ static coroutine_fn int qcow2_co_preadv_part(BlockDriverState *bs,
         qiov_offset += cur_bytes;
     }
 
-    return 0;
+out:
+    if (aio) {
+        aio_task_pool_wait_all(aio);
+        if (ret == 0) {
+            ret = aio_task_pool_status(aio);
+        }
+        g_free(aio);
+    }
+
+    return ret;
 }
 
 /* Check if it's possible to merge a write request with the writing of
@@ -2324,6 +2401,17 @@ out_locked:
     return ret;
 }
 
+static coroutine_fn int qcow2_co_pwritev_task_entry(AioTask *task)
+{
+    Qcow2AioTask *t = container_of(task, Qcow2AioTask, task);
+
+    assert(!t->cluster_type);
+
+    return qcow2_co_pwritev_task(t->bs, t->file_cluster_offset,
+                                 t->offset, t->bytes, t->qiov, t->qiov_offset,
+                                 t->l2meta);
+}
+
 static coroutine_fn int qcow2_co_pwritev_part(
         BlockDriverState *bs, uint64_t offset, uint64_t bytes,
         QEMUIOVector *qiov, size_t qiov_offset, int flags)
@@ -2334,10 +2422,11 @@ static coroutine_fn int qcow2_co_pwritev_part(
     unsigned int cur_bytes; /* number of sectors in current iteration */
     uint64_t cluster_offset;
     QCowL2Meta *l2meta = NULL;
+    AioTaskPool *aio = NULL;
 
     trace_qcow2_writev_start_req(qemu_coroutine_self(), offset, bytes);
 
-    while (bytes != 0) {
+    while (bytes != 0 && aio_task_pool_status(aio) == 0) {
 
         l2meta = NULL;
 
@@ -2369,8 +2458,12 @@ static coroutine_fn int qcow2_co_pwritev_part(
 
         qemu_co_mutex_unlock(&s->lock);
 
-        ret = qcow2_co_pwritev_task(bs, cluster_offset, offset, cur_bytes,
-                                    qiov, qiov_offset, l2meta);
+        if (!aio && cur_bytes != bytes) {
+            aio = aio_task_pool_new(QCOW2_MAX_WORKERS);
+        }
+        ret = qcow2_add_task(bs, aio, qcow2_co_pwritev_task_entry, 0,
+                             cluster_offset, offset, cur_bytes,
+                             qiov, qiov_offset, l2meta);
         l2meta = NULL; /* l2meta is consumed by qcow2_co_pwritev_task() */
         if (ret < 0) {
             goto fail_nometa;
@@ -2391,6 +2484,14 @@ out_locked:
     qemu_co_mutex_unlock(&s->lock);
 
 fail_nometa:
+    if (aio) {
+        aio_task_pool_wait_all(aio);
+        if (ret == 0) {
+            ret = aio_task_pool_status(aio);
+        }
+        g_free(aio);
+    }
+
     trace_qcow2_writev_done_req(qemu_coroutine_self(), ret);
 
     return ret;
diff --git a/block/trace-events b/block/trace-events
index 04209f058d..3aa27e6b1e 100644
--- a/block/trace-events
+++ b/block/trace-events
@@ -62,6 +62,7 @@ file_paio_submit(void *acb, void *opaque, int64_t offset, int count, int type) "
 file_copy_file_range(void *bs, int src, int64_t src_off, int dst, int64_t dst_off, int64_t bytes, int flags, int64_t ret) "bs %p src_fd %d offset %"PRIu64" dst_fd %d offset %"PRIu64" bytes %"PRIu64" flags %d ret %"PRId64
 
 # qcow2.c
+qcow2_add_task(void *co, void *bs, void *pool, const char *action, int cluster_type, uint64_t file_cluster_offset, uint64_t offset, uint64_t bytes, void *qiov, size_t qiov_offset) "co %p bs %p pool %p: %s: cluster_type %d file_cluster_offset %" PRIu64 " offset %" PRIu64 " bytes %" PRIu64 " qiov %p qiov_offset %zu"
 qcow2_writev_start_req(void *co, int64_t offset, int bytes) "co %p offset 0x%" PRIx64 " bytes %d"
 qcow2_writev_done_req(void *co, int ret) "co %p ret %d"
 qcow2_writev_start_part(void *co) "co %p"
-- 
2.21.0



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v5 0/5] qcow2: async handling of fragmented io
  2019-09-16 17:53 [Qemu-devel] [PATCH v5 0/5] qcow2: async handling of fragmented io Vladimir Sementsov-Ogievskiy
                   ` (4 preceding siblings ...)
  2019-09-16 17:53 ` [Qemu-devel] [PATCH v5 5/5] block/qcow2: introduce parallel subrequest handling in read and write Vladimir Sementsov-Ogievskiy
@ 2019-09-17  9:32 ` Vladimir Sementsov-Ogievskiy
  2019-09-20 11:10 ` Max Reitz
  6 siblings, 0 replies; 16+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-09-17  9:32 UTC (permalink / raw)
  To: qemu-block; +Cc: kwolf, Denis Lunev, qemu-devel, mreitz

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v5 0/5] qcow2: async handling of fragmented io
  2019-09-16 17:53 [Qemu-devel] [PATCH v5 0/5] qcow2: async handling of fragmented io Vladimir Sementsov-Ogievskiy
                   ` (5 preceding siblings ...)
  2019-09-17  9:32 ` [Qemu-devel] [PATCH v5 0/5] qcow2: async handling of fragmented io Vladimir Sementsov-Ogievskiy
@ 2019-09-20 11:10 ` Max Reitz
  2019-09-20 11:53   ` Vladimir Sementsov-Ogievskiy
  6 siblings, 1 reply; 16+ messages in thread
From: Max Reitz @ 2019-09-20 11:10 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-block; +Cc: kwolf, den, qemu-devel


[-- Attachment #1.1: Type: text/plain, Size: 1653 bytes --]

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v5 0/5] qcow2: async handling of fragmented io
  2019-09-20 11:10 ` Max Reitz
@ 2019-09-20 11:53   ` Vladimir Sementsov-Ogievskiy
  2019-09-20 12:40     ` Max Reitz
  0 siblings, 1 reply; 16+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-09-20 11:53 UTC (permalink / raw)
  To: Max Reitz, qemu-block; +Cc: kwolf, qemu-devel, Denis Lunev

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v5 0/5] qcow2: async handling of fragmented io
  2019-09-20 11:53   ` Vladimir Sementsov-Ogievskiy
@ 2019-09-20 12:40     ` Max Reitz
  2019-09-20 12:53       ` Vladimir Sementsov-Ogievskiy
  0 siblings, 1 reply; 16+ messages in thread
From: Max Reitz @ 2019-09-20 12:40 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-block; +Cc: kwolf, qemu-devel, Denis Lunev


[-- Attachment #1.1: Type: text/plain, Size: 2149 bytes --]

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v5 0/5] qcow2: async handling of fragmented io
  2019-09-20 12:40     ` Max Reitz
@ 2019-09-20 12:53       ` Vladimir Sementsov-Ogievskiy
  2019-09-20 13:10         ` Max Reitz
  2019-09-20 14:17         ` Eric Blake
  0 siblings, 2 replies; 16+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-09-20 12:53 UTC (permalink / raw)
  To: Max Reitz, qemu-block; +Cc: kwolf, qemu-devel, Denis Lunev

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v5 0/5] qcow2: async handling of fragmented io
  2019-09-20 12:53       ` Vladimir Sementsov-Ogievskiy
@ 2019-09-20 13:10         ` Max Reitz
  2019-09-20 13:26           ` Vladimir Sementsov-Ogievskiy
  2019-09-20 14:17         ` Eric Blake
  1 sibling, 1 reply; 16+ messages in thread
From: Max Reitz @ 2019-09-20 13:10 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-block; +Cc: kwolf, qemu-devel, Denis Lunev


[-- Attachment #1.1: Type: text/plain, Size: 2939 bytes --]

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v5 0/5] qcow2: async handling of fragmented io
  2019-09-20 13:10         ` Max Reitz
@ 2019-09-20 13:26           ` Vladimir Sementsov-Ogievskiy
  2019-09-20 13:29             ` Max Reitz
  0 siblings, 1 reply; 16+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-09-20 13:26 UTC (permalink / raw)
  To: Max Reitz, qemu-block; +Cc: kwolf, qemu-devel, Denis Lunev

[-- Attachment #1: Type: text/plain, Size: 3831 bytes --]

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

[-- Attachment #2: eat-diff-numbers --]
[-- Type: text/plain, Size: 170 bytes --]

#!/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>/'

[-- Attachment #3: git-check-rebase2 --]
[-- Type: text/plain, Size: 921 bytes --]

#!/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

[-- Attachment #4: git-check-rebase --]
[-- Type: text/plain, Size: 1002 bytes --]

#!/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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v5 0/5] qcow2: async handling of fragmented io
  2019-09-20 13:26           ` Vladimir Sementsov-Ogievskiy
@ 2019-09-20 13:29             ` Max Reitz
  0 siblings, 0 replies; 16+ messages in thread
From: Max Reitz @ 2019-09-20 13:29 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, qemu-block; +Cc: kwolf, qemu-devel, Denis Lunev


[-- Attachment #1.1: Type: text/plain, Size: 3302 bytes --]

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v5 0/5] qcow2: async handling of fragmented io
  2019-09-20 12:53       ` Vladimir Sementsov-Ogievskiy
  2019-09-20 13:10         ` Max Reitz
@ 2019-09-20 14:17         ` Eric Blake
  2019-09-20 14:39           ` Vladimir Sementsov-Ogievskiy
  1 sibling, 1 reply; 16+ messages in thread
From: Eric Blake @ 2019-09-20 14:17 UTC (permalink / raw)
  To: Vladimir Sementsov-Ogievskiy, Max Reitz, qemu-block
  Cc: kwolf, qemu-devel, Denis Lunev


[-- Attachment #1.1: Type: text/plain, Size: 755 bytes --]

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v5 0/5] qcow2: async handling of fragmented io
  2019-09-20 14:17         ` Eric Blake
@ 2019-09-20 14:39           ` Vladimir Sementsov-Ogievskiy
  0 siblings, 0 replies; 16+ messages in thread
From: Vladimir Sementsov-Ogievskiy @ 2019-09-20 14:39 UTC (permalink / raw)
  To: Eric Blake, Max Reitz, qemu-block; +Cc: kwolf, qemu-devel, Denis Lunev

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2019-09-20 14:56 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-16 17:53 [Qemu-devel] [PATCH v5 0/5] qcow2: async handling of fragmented io Vladimir Sementsov-Ogievskiy
2019-09-16 17:53 ` [Qemu-devel] [PATCH v5 1/5] qemu-iotests: ignore leaks on failure paths in 026 Vladimir Sementsov-Ogievskiy
2019-09-16 17:53 ` [Qemu-devel] [PATCH v5 2/5] block: introduce aio task pool Vladimir Sementsov-Ogievskiy
2019-09-16 17:53 ` [Qemu-devel] [PATCH v5 3/5] block/qcow2: refactor qcow2_co_preadv_part Vladimir Sementsov-Ogievskiy
2019-09-16 17:53 ` [Qemu-devel] [PATCH v5 4/5] block/qcow2: refactor qcow2_co_pwritev_part Vladimir Sementsov-Ogievskiy
2019-09-16 17:53 ` [Qemu-devel] [PATCH v5 5/5] block/qcow2: introduce parallel subrequest handling in read and write Vladimir Sementsov-Ogievskiy
2019-09-17  9:32 ` [Qemu-devel] [PATCH v5 0/5] qcow2: async handling of fragmented io Vladimir Sementsov-Ogievskiy
2019-09-20 11:10 ` Max Reitz
2019-09-20 11:53   ` Vladimir Sementsov-Ogievskiy
2019-09-20 12:40     ` Max Reitz
2019-09-20 12:53       ` Vladimir Sementsov-Ogievskiy
2019-09-20 13:10         ` Max Reitz
2019-09-20 13:26           ` Vladimir Sementsov-Ogievskiy
2019-09-20 13:29             ` Max Reitz
2019-09-20 14:17         ` Eric Blake
2019-09-20 14:39           ` Vladimir Sementsov-Ogievskiy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).