Hi all!

The aim of the series is to reduce code-duplication and writing
parameters structure-packing by hand around coroutine function wrappers.

Benefits:
 - no code duplication
 - less indirection

v9: Thanks to Eric, I used commit message tweaks and rebase-conflict solving from his git.
01: add Philippe's, Stefan's r-bs
02: - add Philippe's, Stefan's r-bs
    - commit message tweaks stolen from Eric's git :)
03: add Philippe's, Stefan's r-bs
04: - wording/grammar by Eric (partly, stolen from repo)
    - ref new file in docs/devel/index.rst
    - use 644 rights and recommended header for python script
    - call gen_header() once
    - rename gen_wrappers_file to gen_wrappers
05: add Stefan's r-b
06: add Philippe's, Stefan's r-bs
07: Stefan's r-b

Vladimir Sementsov-Ogievskiy (7):
  block: return error-code from bdrv_invalidate_cache
  block/io: refactor coroutine wrappers
  block: declare some coroutine functions in block/coroutines.h
  scripts: add block-coroutine-wrapper.py
  block: generate coroutine-wrapper code
  block: drop bdrv_prwv
  block/io: refactor save/load vmstate

 docs/devel/block-coroutine-wrapper.rst |  54 ++++
 docs/devel/index.rst                   |   1 +
 block/block-gen.h                      |  49 ++++
 block/coroutines.h                     |  65 +++++
 include/block/block.h                  |  34 ++-
 block.c                                |  97 +------
 block/io.c                             | 337 ++++---------------------
 tests/test-bdrv-drain.c                |   2 +-
 block/meson.build                      |   8 +
 scripts/block-coroutine-wrapper.py     | 188 ++++++++++++++
 10 files changed, 454 insertions(+), 381 deletions(-)
 create mode 100644 docs/devel/block-coroutine-wrapper.rst
 create mode 100644 block/block-gen.h
 create mode 100644 block/coroutines.h
 create mode 100644 scripts/block-coroutine-wrapper.py

-- 
2.21.3

On Thu, Sep 24, 2020 at 09:54:07PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> Hi all!
> 
> The aim of the series is to reduce code-duplication and writing
> parameters structure-packing by hand around coroutine function wrappers.
> 
> Benefits:
>  - no code duplication
>  - less indirection
> 
> v9: Thanks to Eric, I used commit message tweaks and rebase-conflict solving from his git.

Thanks, applied to my block tree with the encoding='utf-8' and
spelling/grammar fixes:
https://github.com/stefanha/qemu/commits/block

Stefan

On 9/24/20 1:54 PM, Vladimir Sementsov-Ogievskiy wrote:
> Hi all!
> 
> The aim of the series is to reduce code-duplication and writing
> parameters structure-packing by hand around coroutine function wrappers.
> 
> Benefits:
>   - no code duplication
>   - less indirection
> 
> v9: Thanks to Eric, I used commit message tweaks and rebase-conflict solving from his git.
> 01: add Philippe's, Stefan's r-bs
> 02: - add Philippe's, Stefan's r-bs
>      - commit message tweaks stolen from Eric's git :)

LOL: "stolen" makes it sound like a crime was committed ;)  I find it to 
be one of the joys of open source when my ideas show up in someone 
else's work when properly attributed, as you did here.  [Maybe the 
recent push towards a conscious language initiative has let me hone in 
on something that turned out humorous to me, but might not be as obvious 
to someone less fluent in English idioms]

At any rate, I'm glad my rebase efforts helped.

> 03: add Philippe's, Stefan's r-bs
> 04: - wording/grammar by Eric (partly, stolen from repo)
>      - ref new file in docs/devel/index.rst
>      - use 644 rights and recommended header for python script
>      - call gen_header() once
>      - rename gen_wrappers_file to gen_wrappers
> 05: add Stefan's r-b
> 06: add Philippe's, Stefan's r-bs
> 07: Stefan's r-b
> 

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

Patchew URL: https://patchew.org/QEMU/20200924185414.28642-1-vsementsov@virtuozzo.com/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

C linker for the host machine: cc ld.bfd 2.27-43
Host machine cpu family: x86_64
Host machine cpu: x86_64
../src/meson.build:10: WARNING: Module unstable-keyval has no backwards or forwards compatibility and might not exist in future releases.
Program sh found: YES
Program python3 found: YES (/usr/bin/python3)
Configuring ninjatool using configuration
---
    return codecs.ascii_decode(input, self.errors)[0]
UnicodeDecodeError: 'ascii' codec can't decode byte 0xe2 in position 11406: ordinal not in range(128)
Generating 'libqemu-aarch64-softmmu.fa.p/decode-vfp-uncond.c.inc'.
make: *** [block/block-gen.c.stamp] Error 1
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
  File "./tests/docker/docker.py", line 709, in <module>
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--rm', '--label', 'com.qemu.instance.uuid=ea922a0a6ce34dfc90f59bf1b059b9d5', '-u', '1001', '--security-opt', 'seccomp=unconfined', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-rctf_xsm/src/docker-src.2020-09-24-16.29.53.14429:/var/tmp/qemu:z,ro', 'qemu/centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=ea922a0a6ce34dfc90f59bf1b059b9d5
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-rctf_xsm/src'
make: *** [docker-run-test-quick@centos7] Error 2

real    3m4.047s
user    0m21.648s


The full log is available at
http://patchew.org/logs/20200924185414.28642-1-vsementsov@virtuozzo.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

24.09.2020 23:32, no-reply@patchew.org wrote:
> Patchew URL: https://patchew.org/QEMU/20200924185414.28642-1-vsementsov@virtuozzo.com/
> 
> 
> 
> Hi,
> 
> This series failed the docker-quick@centos7 build test. Please find the testing commands and
> their output below. If you have Docker installed, you can probably reproduce it
> locally.
> 
> === TEST SCRIPT BEGIN ===
> #!/bin/bash
> make docker-image-centos7 V=1 NETWORK=1
> time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
> === TEST SCRIPT END ===
> 
> C linker for the host machine: cc ld.bfd 2.27-43
> Host machine cpu family: x86_64
> Host machine cpu: x86_64
> ../src/meson.build:10: WARNING: Module unstable-keyval has no backwards or forwards compatibility and might not exist in future releases.
> Program sh found: YES
> Program python3 found: YES (/usr/bin/python3)
> Configuring ninjatool using configuration
> ---
>      return codecs.ascii_decode(input, self.errors)[0]
> UnicodeDecodeError: 'ascii' codec can't decode byte 0xe2 in position 11406: ordinal not in range(128)
> Generating 'libqemu-aarch64-softmmu.fa.p/decode-vfp-uncond.c.inc'.
> make: *** [block/block-gen.c.stamp] Error 1
> make: *** Waiting for unfinished jobs....
> Traceback (most recent call last):
>    File "./tests/docker/docker.py", line 709, in <module>
> ---
>      raise CalledProcessError(retcode, cmd)
> subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--rm', '--label', 'com.qemu.instance.uuid=ea922a0a6ce34dfc90f59bf1b059b9d5', '-u', '1001', '--security-opt', 'seccomp=unconfined', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-rctf_xsm/src/docker-src.2020-09-24-16.29.53.14429:/var/tmp/qemu:z,ro', 'qemu/centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
> filter=--filter=label=com.qemu.instance.uuid=ea922a0a6ce34dfc90f59bf1b059b9d5
> make[1]: *** [docker-run] Error 1
> make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-rctf_xsm/src'
> make: *** [docker-run-test-quick@centos7] Error 2
> 
> real    3m4.047s
> user    0m21.648s
> 
> 
> The full log is available at
> http://patchew.org/logs/20200924185414.28642-1-vsementsov@virtuozzo.com/testing.docker-quick@centos7/?type=message.
> ---
> Email generated automatically by Patchew [https://patchew.org/].
> Please send your feedback to patchew-devel@redhat.com
> 

Generating 'libqemu-aarch64-softmmu.fa.p/decode-vfp.c.inc'.
Traceback (most recent call last):
   File "/tmp/qemu-test/src/block/../scripts/block-coroutine-wrapper.py", line 187, in <module>
     f_out.write(gen_wrappers(f_in.read()))
   File "/usr/lib64/python3.6/encodings/ascii.py", line 26, in decode
     return codecs.ascii_decode(input, self.errors)[0]
UnicodeDecodeError: 'ascii' codec can't decode byte 0xe2 in position 11406: ordinal not in range(128)


Interesting:

[root@kvm up-coroutine-wrapper]# grep --color='auto' -P -n '[^\x00-\x7F]' include/block/block.h
307:     * Child from which to read all data that isn’t allocated in the
                                                      ^

The file really contains one non-ascii symbol. I think it worth a separate patch. Still, it shouldn't break build process. On my system it works as is, probably unicode is default for me.

Aha, from "open" specification:

    if encoding is not specified the encoding used is platform dependent: locale.getpreferredencoding(False) is called to get the current locale encoding.



Is it ok, that utf-8 is not default on test system?

So, possible solutions are:

1. Enforce utf-8 io in scripts/block-coroutine-wrapper.py (patch 4)
2. Drop non-ascii quotation mark from block.h
3. Fix the test system default to be utf-8

Do we want them all?

-- 
Best regards,
Vladimir

On 9/25/20 3:04 AM, Vladimir Sementsov-Ogievskiy wrote:
> 24.09.2020 23:32, no-reply@patchew.org wrote:
>> Patchew URL: 
>> https://patchew.org/QEMU/20200924185414.28642-1-vsementsov@virtuozzo.com/
>>

>> Program python3 found: YES (/usr/bin/python3)
>> Configuring ninjatool using configuration
>> ---
>>      return codecs.ascii_decode(input, self.errors)[0]
>> UnicodeDecodeError: 'ascii' codec can't decode byte 0xe2 in position 
>> 11406: ordinal not in range(128)

> Generating 'libqemu-aarch64-softmmu.fa.p/decode-vfp.c.inc'.
> Traceback (most recent call last):
>    File 
> "/tmp/qemu-test/src/block/../scripts/block-coroutine-wrapper.py", line 
> 187, in <module>
>      f_out.write(gen_wrappers(f_in.read()))
>    File "/usr/lib64/python3.6/encodings/ascii.py", line 26, in decode
>      return codecs.ascii_decode(input, self.errors)[0]
> UnicodeDecodeError: 'ascii' codec can't decode byte 0xe2 in position 
> 11406: ordinal not in range(128)
> 
> 
> Interesting:
> 
> [root@kvm up-coroutine-wrapper]# grep --color='auto' -P -n 
> '[^\x00-\x7F]' include/block/block.h
> 307:     * Child from which to read all data that isn’t allocated in the
>                                                       ^
> 
> The file really contains one non-ascii symbol. I think it worth a 
> separate patch. Still, it shouldn't break build process. On my system it 
> works as is, probably unicode is default for me.

Python 3 has had an interesting history when it comes to 8-bit cleanness 
by default.  Which means we DO have to be explicit about utf8.

> 
> Aha, from "open" specification:
> 
>     if encoding is not specified the encoding used is platform 
> dependent: locale.getpreferredencoding(False) is called to get the 
> current locale encoding.
> 
> 
> 
> Is it ok, that utf-8 is not default on test system?

It's intentional.

> 
> So, possible solutions are:
> 
> 1. Enforce utf-8 io in scripts/block-coroutine-wrapper.py (patch 4)

Yes, we should do that regardless (we do it in our other python scripts).

> 2. Drop non-ascii quotation mark from block.h

Yes, we should do that as well (it's only in a comment, but it is 
inconsistent).

> 3. Fix the test system default to be utf-8

No.  That one we want to keep where it is, because it helps us flush out 
these sorts of issues.

> 
> Do we want them all?
> 

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