Re: [PATCH v3 2/4] qemu-img: make --block-size optional for compare --stat

[Vladimir Sementsov-Ogievskiy]

29.10.2021 23:32, Eric Blake wrote:

On Thu, Oct 28, 2021 at 12:24:39PM +0200, Vladimir Sementsov-Ogievskiy wrote:

Let's detect block-size automatically if not specified by user:

  If both files define cluster-size, use minimum to be more precise.
  If both files don't specify cluster-size, use default of 64K
  If only one file specify cluster-size, just use it.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  docs/tools/qemu-img.rst |  7 +++-
  qemu-img.c  | 71 +
  qemu-img-cmds.hx|  4 +--
  3 files changed, 72 insertions(+), 10 deletions(-)



Reviewed-by: Eric Blake 


+if (cluster_size1 > 0 && cluster_size2 > 0) {
+if (cluster_size1 == cluster_size2) {
+block_size = cluster_size1;
+} else {
+block_size = MIN(cluster_size1, cluster_size2);
+qprintf(quiet, "%s and %s have different cluster sizes: %d and %d "
+"respectively. Using minimum as block-size for "
+"accuracy: %d. %s\n",
+fname1, fname2, cluster_size1,
+cluster_size2, block_size, note);


Results in a long line; I don't know if it's worth trying to wrap it
(if we had a generic utility function that took arbitrary text, then
outputs it wrapped to the user's current terminal column width, I'd
suggest using that instead - but that's NOT something I expect you to
write, and I don't know if glib has such a utility).



Hmm. But long lines printed to the terminal are wrapped by terminal 
automatically, so we don't need to wrap to terminal width by hand..

--
Best regards,
Vladimir

Re: [PATCH v3 3/4] qemu-img: add --shallow option for qemu-img compare

[Vladimir Sementsov-Ogievskiy]

29.10.2021 23:44, Eric Blake wrote:

On Thu, Oct 28, 2021 at 12:24:40PM +0200, Vladimir Sementsov-Ogievskiy wrote:

Allow compare only top images of backing chains. This is useful to


Allow the comparison of only the top image of backing chains.


compare images with same backing file or to compare incremental images
from the chain of incremental backups with "--stat" option.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Hanna Reitz 
---
  docs/tools/qemu-img.rst | 9 -
  qemu-img.c  | 8 ++--
  qemu-img-cmds.hx| 4 ++--
  3 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/docs/tools/qemu-img.rst b/docs/tools/qemu-img.rst
index 9bfdd93d6c..c6e9306c70 100644
--- a/docs/tools/qemu-img.rst
+++ b/docs/tools/qemu-img.rst
@@ -176,6 +176,13 @@ Parameters to compare subcommand:
  - If both files don't specify cluster-size, use default of 64K
  - If only one file specifies cluster-size, just use that.
  
+.. option:: --shallow

+
+  This option prevents opening and comparing any backing files.
+  This is useful to compare images with same backing file or to compare
+  incremental images from the chain of incremental backups with
+  ``--stat`` option.


If I understand correctly, your implementation makes --shallow an
all-or-none option (either both images are compared shallow, or
neither is).  Does it make sense to make it a per-image option
(--shallow-source, --shallow-dest), where --shallow is a synonym for
the more verbose --shallow-source --shallow-dest?



Usable, to compare incremental image with "everything below it".

But I'm not sure about source/dest terms in context of compare, where image 
roles are symmetrical.

I even start to thing, that it should be an option, used together with 
--image-opts, applied to image in person.. And actually we already have it, like

--image-opts ... driver=qcow2,file.filename=file.qcow2,backing=

At least this works with qemu-io. But prints warning:


qemu-io: warning: Use of "backing": "" is deprecated; use "backing": null 
instead


And of course, "null" doesn't work here.

May be we should drop the warning, keeping backing= syntax at least for 
--image-opts of qemu utilities. Or, another way, support json for --image-opts.

What do you think? driver=qcow2,file.filename=a.qcow2,backing=  doesn't look 
better than --shallow. But if we want to make shallow option per-image, we 
can't ignore the fact that we already have a way to specify per-image options.


--
Best regards,
Vladimir

Re: [PATCH v3 2/4] qemu-img: make --block-size optional for compare --stat

[Eric Blake]

On Mon, Nov 01, 2021 at 11:03:22AM +0300, Vladimir Sementsov-Ogievskiy wrote:
> 29.10.2021 23:32, Eric Blake wrote:
> > On Thu, Oct 28, 2021 at 12:24:39PM +0200, Vladimir Sementsov-Ogievskiy 
> > wrote:
> > > Let's detect block-size automatically if not specified by user:
> > > 
> > >   If both files define cluster-size, use minimum to be more precise.
> > >   If both files don't specify cluster-size, use default of 64K
> > >   If only one file specify cluster-size, just use it.
> > > 
> > > Signed-off-by: Vladimir Sementsov-Ogievskiy 
> > > ---
> > >   docs/tools/qemu-img.rst |  7 +++-
> > >   qemu-img.c  | 71 +
> > >   qemu-img-cmds.hx|  4 +--
> > >   3 files changed, 72 insertions(+), 10 deletions(-)
> > > 
> > 
> > Reviewed-by: Eric Blake 
> > 
> > > +if (cluster_size1 > 0 && cluster_size2 > 0) {
> > > +if (cluster_size1 == cluster_size2) {
> > > +block_size = cluster_size1;
> > > +} else {
> > > +block_size = MIN(cluster_size1, cluster_size2);
> > > +qprintf(quiet, "%s and %s have different cluster sizes: %d 
> > > and %d "
> > > +"respectively. Using minimum as block-size for "
> > > +"accuracy: %d. %s\n",
> > > +fname1, fname2, cluster_size1,
> > > +cluster_size2, block_size, note);
> > 
> > Results in a long line; I don't know if it's worth trying to wrap it
> > (if we had a generic utility function that took arbitrary text, then
> > outputs it wrapped to the user's current terminal column width, I'd
> > suggest using that instead - but that's NOT something I expect you to
> > write, and I don't know if glib has such a utility).
> > 
> 
> Hmm. But long lines printed to the terminal are wrapped by terminal 
> automatically, so we don't need to wrap to terminal width by hand..

/me using a short line length in the next paragraph on purpose...

There's a difference between the ter
minal wrapping when you hit the maxi
mum length, and in intelligent
wrapping the prefers natural word
breaks.

There are programs out there that do intelligent wrapping (for
example, the mutt mailer), but I'm not sure if those wrapping routines
are available through glib.  At any rate, I do NOT want to reimplement
intelligent wrapping from scratch (Unicode specifies a rather complex
algorithm for getting sane wrapping in the presence of various Unicode
characters, and it is not trivial https://unicode.org/reports/tr14/).
So unless someone knows of an easy-to-use library that does wrapping,
you are right that leaving long lines for the terminal to output, even
when that output has awkward mid-word breaks, is tolerable.

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

Re: [PATCH v2 2/5] hw/arm: Add Nuvoton SD module to board

[Peter Maydell]

On Fri, 8 Oct 2021 at 01:26, Hao Wu  wrote:
>
> From: Shengtan Mao 
>
> Signed-off-by: Shengtan Mao 
> Reviewed-by: Hao Wu 
> Reviewed-by: Chris Rauer 
> Reviewed-by: Tyrone Ting 
> Signed-off-by: Hao Wu 

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [PATCH v2 4/5] tests/qtest/libqos: add SDHCI commands

[Peter Maydell]

On Fri, 8 Oct 2021 at 01:26, Hao Wu  wrote:
>
> From: Shengtan Mao 
>
> Signed-off-by: Shengtan Mao 
> Reviewed-by: Hao Wu 
> Reviewed-by: Chris Rauer 
> Reviewed-by: Tyrone Ting 
> Signed-off-by: Hao Wu 
> ---
>  tests/qtest/libqos/meson.build |   1 +
>  tests/qtest/libqos/sdhci-cmd.c | 116 +
>  tests/qtest/libqos/sdhci-cmd.h |  70 
>  3 files changed, 187 insertions(+)
>  create mode 100644 tests/qtest/libqos/sdhci-cmd.c
>  create mode 100644 tests/qtest/libqos/sdhci-cmd.h

Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [PATCH v2 1/5] hw/sd: add nuvoton MMC

[Peter Maydell]

On Fri, 8 Oct 2021 at 01:26, Hao Wu  wrote:
>
> From: Shengtan Mao 
>
> Signed-off-by: Shengtan Mao 
> Reviewed-by: Hao Wu 
> Reviewed-by: Chris Rauer 
> Reviewed-by: Tyrone Ting 
> Signed-off-by:  Hao Wu 
> ---

> +default:
> +qemu_log_mask(LOG_GUEST_ERROR, "SDHCI read of nonexist reg: 0x%02"

"nonexistent"

> +  HWADDR_PRIx, addr);
> +break;
> +}
> +
> +return val;
> +}
> +
> +static void npcm7xx_sdhci_write(void *opaque, hwaddr addr, uint64_t val,
> +unsigned int size)
> +{
> +NPCM7xxSDHCIState *s = opaque;
> +
> +switch (addr) {
> +case NPCM7XX_BOOTTOCTRL:
> +s->regs.boottoctrl = val;
> +break;
> +default:
> +qemu_log_mask(LOG_GUEST_ERROR, "SDHCI write of nonexist reg: 0x%02"

ditto

Otherwise
Reviewed-by: Peter Maydell 

thanks
-- PMM

Re: [PATCH v2 5/5] tests/qtest: add qtests for npcm7xx sdhci

[Peter Maydell]

On Fri, 8 Oct 2021 at 01:26, Hao Wu  wrote:
>
> From: Shengtan Mao 
>
> Signed-off-by: Shengtan Mao 
> Reviewed-by: Hao Wu 
> Reviewed-by: Chris Rauer 
> Reviewed-by: Tyrone Ting 
> Signed-off-by: Hao Wu 

Reviewed-by: Peter Maydell 

thanks
-- PMM

[PULL 00/22] Python patches

[John Snow]

The following changes since commit af531756d25541a1b3b3d9a14e72e7fedd941a2e:

  Merge remote-tracking branch 'remotes/philmd/tags/renesas-20211030' into 
staging (2021-10-30 11:31:41 -0700)

are available in the Git repository at:

  https://gitlab.com/jsnow/qemu.git tags/python-pull-request

for you to fetch changes up to 76cd358671e6b8e7c435ec65b1c44200254514a9:

  python, iotests: replace qmp with aqmp (2021-11-01 11:54:59 -0400)


Pull request



John Snow (22):
  iotests/297: Move pylint config into pylintrc
  iotests/297: Split mypy configuration out into mypy.ini
  iotests/297: Add get_files() function
  iotests/297: Create main() function
  iotests/297: Don't rely on distro-specific linter binaries
  iotests/297: Split run_linters apart into run_pylint and run_mypy
  iotests/297: refactor run_[mypy|pylint] as generic execution shim
  iotests/297: Change run_linter() to raise an exception on failure
  iotests/297: update tool availability checks
  iotests/297: split test into sub-cases
  iotests: split linters.py out from 297
  iotests/linters: Add entry point for linting via Python CI
  iotests/linters: Add workaround for mypy bug #9852
  python: Add iotest linters to test suite
  python/machine: remove has_quit argument
  python/machine: Handle QMP errors on close more meticulously
  python/aqmp: Remove scary message
  iotests: Accommodate async QMP Exception classes
  iotests: Conditionally silence certain AQMP errors
  iotests/300: avoid abnormal shutdown race condition
  python/aqmp: Create sync QMP wrapper for iotests
  python, iotests: replace qmp with aqmp

 python/qemu/aqmp/__init__.py  |  12 --
 python/qemu/aqmp/legacy.py| 138 ++
 python/qemu/machine/machine.py|  85 +
 python/tests/iotests-mypy.sh  |   4 +
 python/tests/iotests-pylint.sh|   4 +
 scripts/simplebench/bench_block_job.py|   3 +-
 tests/qemu-iotests/040|   7 +-
 tests/qemu-iotests/218|   2 +-
 tests/qemu-iotests/255|   2 +-
 tests/qemu-iotests/297| 103 +++-
 tests/qemu-iotests/300|  13 +-
 tests/qemu-iotests/iotests.py |  20 +++-
 tests/qemu-iotests/linters.py | 105 
 tests/qemu-iotests/mypy.ini   |  12 ++
 tests/qemu-iotests/pylintrc   |  16 +++
 tests/qemu-iotests/tests/mirror-top-perms |  17 ++-
 16 files changed, 424 insertions(+), 119 deletions(-)
 create mode 100644 python/qemu/aqmp/legacy.py
 create mode 100755 python/tests/iotests-mypy.sh
 create mode 100755 python/tests/iotests-pylint.sh
 create mode 100644 tests/qemu-iotests/linters.py
 create mode 100644 tests/qemu-iotests/mypy.ini

-- 
2.31.1

[PULL 01/22] iotests/297: Move pylint config into pylintrc

[John Snow]

Move --score=n and --notes=XXX,FIXME into pylintrc. This pulls
configuration out of code, which I think is probably a good thing in
general.

Signed-off-by: John Snow 
Reviewed-by: Hanna Reitz 
Message-id: 20211019144918.3159078-2-js...@redhat.com
Signed-off-by: John Snow 
---
 tests/qemu-iotests/297  |  4 +---
 tests/qemu-iotests/pylintrc | 16 
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index 91ec34d9521..bc3a0ceb2aa 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -65,10 +65,8 @@ def run_linters():
 print('=== pylint ===')
 sys.stdout.flush()
 
-# Todo notes are fine, but fixme's or xxx's should probably just be
-# fixed (in tests, at least)
 env = os.environ.copy()
-subprocess.run(('pylint-3', '--score=n', '--notes=FIXME,XXX', *files),
+subprocess.run(('pylint-3', *files),
env=env, check=False)
 
 print('=== mypy ===')
diff --git a/tests/qemu-iotests/pylintrc b/tests/qemu-iotests/pylintrc
index 8cb4e1d6a6d..32ab77b8bb9 100644
--- a/tests/qemu-iotests/pylintrc
+++ b/tests/qemu-iotests/pylintrc
@@ -31,6 +31,22 @@ disable=invalid-name,
 too-many-statements,
 consider-using-f-string,
 
+
+[REPORTS]
+
+# Activate the evaluation score.
+score=no
+
+
+[MISCELLANEOUS]
+
+# List of note tags to take in consideration, separated by a comma.
+# TODO notes are fine, but FIXMEs or XXXs should probably just be
+# fixed (in tests, at least).
+notes=FIXME,
+  XXX,
+
+
 [FORMAT]
 
 # Maximum number of characters on a single line.
-- 
2.31.1

[PULL 02/22] iotests/297: Split mypy configuration out into mypy.ini

[John Snow]

More separation of code and configuration.

Signed-off-by: John Snow 
Reviewed-by: Hanna Reitz 
Message-id: 20211019144918.3159078-3-js...@redhat.com
Signed-off-by: John Snow 
---
 tests/qemu-iotests/297  | 14 +-
 tests/qemu-iotests/mypy.ini | 12 
 2 files changed, 13 insertions(+), 13 deletions(-)
 create mode 100644 tests/qemu-iotests/mypy.ini

diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index bc3a0ceb2aa..b8101e6024a 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -73,19 +73,7 @@ def run_linters():
 sys.stdout.flush()
 
 env['MYPYPATH'] = env['PYTHONPATH']
-p = subprocess.run(('mypy',
-'--warn-unused-configs',
-'--disallow-subclassing-any',
-'--disallow-any-generics',
-'--disallow-incomplete-defs',
-'--disallow-untyped-decorators',
-'--no-implicit-optional',
-'--warn-redundant-casts',
-'--warn-unused-ignores',
-'--no-implicit-reexport',
-'--namespace-packages',
-'--scripts-are-modules',
-*files),
+p = subprocess.run(('mypy', *files),
env=env,
check=False,
stdout=subprocess.PIPE,
diff --git a/tests/qemu-iotests/mypy.ini b/tests/qemu-iotests/mypy.ini
new file mode 100644
index 000..4c0339f5589
--- /dev/null
+++ b/tests/qemu-iotests/mypy.ini
@@ -0,0 +1,12 @@
+[mypy]
+disallow_any_generics = True
+disallow_incomplete_defs = True
+disallow_subclassing_any = True
+disallow_untyped_decorators = True
+implicit_reexport = False
+namespace_packages = True
+no_implicit_optional = True
+scripts_are_modules = True
+warn_redundant_casts = True
+warn_unused_configs = True
+warn_unused_ignores = True
-- 
2.31.1

[PULL 03/22] iotests/297: Add get_files() function

[John Snow]

Split out file discovery into its own method to begin separating out
configuration/setup and test execution.

Signed-off-by: John Snow 
Reviewed-by: Hanna Reitz 
Message-id: 20211019144918.3159078-4-js...@redhat.com
Signed-off-by: John Snow 
---
 tests/qemu-iotests/297 | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index b8101e6024a..15b54594c11 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -21,6 +21,7 @@ import re
 import shutil
 import subprocess
 import sys
+from typing import List
 
 import iotests
 
@@ -54,10 +55,14 @@ def is_python_file(filename):
 return False
 
 
-def run_linters():
+def get_test_files() -> List[str]:
 named_tests = [f'tests/{entry}' for entry in os.listdir('tests')]
 check_tests = set(os.listdir('.') + named_tests) - set(SKIP_FILES)
-files = [filename for filename in check_tests if is_python_file(filename)]
+return list(filter(is_python_file, check_tests))
+
+
+def run_linters():
+files = get_test_files()
 
 iotests.logger.debug('Files to be checked:')
 iotests.logger.debug(', '.join(sorted(files)))
-- 
2.31.1

[PULL 06/22] iotests/297: Split run_linters apart into run_pylint and run_mypy

[John Snow]

Move environment setup into main(), and split the actual linter
execution into run_pylint and run_mypy, respectively.

Signed-off-by: John Snow 
Reviewed-by: Hanna Reitz 
Message-id: 20211019144918.3159078-7-js...@redhat.com
Signed-off-by: John Snow 
---
 tests/qemu-iotests/297 | 38 --
 1 file changed, 24 insertions(+), 14 deletions(-)

diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index c1bddb9ce0e..189bcaf5f94 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -21,7 +21,7 @@ import re
 import shutil
 import subprocess
 import sys
-from typing import List
+from typing import List, Mapping, Optional
 
 import iotests
 
@@ -61,23 +61,19 @@ def get_test_files() -> List[str]:
 return list(filter(is_python_file, check_tests))
 
 
-def run_linters():
-files = get_test_files()
+def run_pylint(
+files: List[str],
+env: Optional[Mapping[str, str]] = None,
+) -> None:
 
-iotests.logger.debug('Files to be checked:')
-iotests.logger.debug(', '.join(sorted(files)))
-
-print('=== pylint ===')
-sys.stdout.flush()
-
-env = os.environ.copy()
 subprocess.run(('python3', '-m', 'pylint', *files),
env=env, check=False)
 
-print('=== mypy ===')
-sys.stdout.flush()
 
-env['MYPYPATH'] = env['PYTHONPATH']
+def run_mypy(
+files: List[str],
+env: Optional[Mapping[str, str]] = None,
+) -> None:
 p = subprocess.run(('python3', '-m', 'mypy', *files),
env=env,
check=False,
@@ -94,7 +90,21 @@ def main() -> None:
 if shutil.which(linter) is None:
 iotests.notrun(f'{linter} not found')
 
-run_linters()
+files = get_test_files()
+
+iotests.logger.debug('Files to be checked:')
+iotests.logger.debug(', '.join(sorted(files)))
+
+env = os.environ.copy()
+env['MYPYPATH'] = env['PYTHONPATH']
+
+print('=== pylint ===')
+sys.stdout.flush()
+run_pylint(files, env=env)
+
+print('=== mypy ===')
+sys.stdout.flush()
+run_mypy(files, env=env)
 
 
 iotests.script_main(main)
-- 
2.31.1

[PULL 04/22] iotests/297: Create main() function

[John Snow]

Instead of running "run_linters" directly, create a main() function that
will be responsible for environment setup, leaving run_linters()
responsible only for execution of the linters.

(That environment setup will be moved over in forthcoming commits.)

Signed-off-by: John Snow 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Hanna Reitz 
Message-id: 20211019144918.3159078-5-js...@redhat.com
Signed-off-by: John Snow 
---
 tests/qemu-iotests/297 | 12 
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index 15b54594c11..163ebc8ebfd 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -89,8 +89,12 @@ def run_linters():
 print(p.stdout)
 
 
-for linter in ('pylint-3', 'mypy'):
-if shutil.which(linter) is None:
-iotests.notrun(f'{linter} not found')
+def main() -> None:
+for linter in ('pylint-3', 'mypy'):
+if shutil.which(linter) is None:
+iotests.notrun(f'{linter} not found')
 
-iotests.script_main(run_linters)
+run_linters()
+
+
+iotests.script_main(main)
-- 
2.31.1

[PULL 08/22] iotests/297: Change run_linter() to raise an exception on failure

[John Snow]

Instead of using a process return code as the python function return
value (or just not returning anything at all), allow run_linter() to
raise an exception instead.

The responsibility for printing output on error shifts from the function
itself to the caller, who will know best how to present/format that
information. (Also, "suppress_output" is now a lot more accurate of a
parameter name.)

Signed-off-by: John Snow 
Reviewed-by: Hanna Reitz 
Message-id: 20211019144918.3159078-9-js...@redhat.com
Signed-off-by: John Snow 
---
 tests/qemu-iotests/297 | 24 ++--
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index d21673a2929..76d6a23f531 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -70,22 +70,18 @@ def run_linter(
 """
 Run a python-based linting tool.
 
-If suppress_output is True, capture stdout/stderr of the child
-process and only print that information back to stdout if the child
-process's return code was non-zero.
+:param suppress_output: If True, suppress all stdout/stderr output.
+:raise CalledProcessError: If the linter process exits with failure.
 """
-p = subprocess.run(
+subprocess.run(
 ('python3', '-m', tool, *args),
 env=env,
-check=False,
+check=True,
 stdout=subprocess.PIPE if suppress_output else None,
 stderr=subprocess.STDOUT if suppress_output else None,
 universal_newlines=True,
 )
 
-if suppress_output and p.returncode != 0:
-print(p.stdout)
-
 
 def main() -> None:
 for linter in ('pylint-3', 'mypy'):
@@ -102,11 +98,19 @@ def main() -> None:
 
 print('=== pylint ===')
 sys.stdout.flush()
-run_linter('pylint', files, env=env)
+try:
+run_linter('pylint', files, env=env)
+except subprocess.CalledProcessError:
+# pylint failure will be caught by diffing the IO.
+pass
 
 print('=== mypy ===')
 sys.stdout.flush()
-run_linter('mypy', files, env=env, suppress_output=True)
+try:
+run_linter('mypy', files, env=env, suppress_output=True)
+except subprocess.CalledProcessError as exc:
+if exc.output:
+print(exc.output)
 
 
 iotests.script_main(main)
-- 
2.31.1

[PULL 10/22] iotests/297: split test into sub-cases

[John Snow]

Take iotest 297's main() test function and split it into two sub-cases
that can be skipped individually. We can also drop custom environment
setup from the pylint test as it isn't needed.

Signed-off-by: John Snow 
Reviewed-by: Hanna Reitz 
Message-id: 20211019144918.3159078-11-js...@redhat.com
Signed-off-by: John Snow 
---
 tests/qemu-iotests/297 | 63 ++
 1 file changed, 39 insertions(+), 24 deletions(-)

diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index b2ad8d1cbe0..b7d9d6077b3 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -82,36 +82,51 @@ def run_linter(
 )
 
 
+def check_linter(linter: str) -> bool:
+try:
+run_linter(linter, ['--version'], suppress_output=True)
+except subprocess.CalledProcessError:
+iotests.case_notrun(f"'{linter}' not found")
+return False
+return True
+
+
+def test_pylint(files: List[str]) -> None:
+print('=== pylint ===')
+sys.stdout.flush()
+
+if not check_linter('pylint'):
+return
+
+run_linter('pylint', files)
+
+
+def test_mypy(files: List[str]) -> None:
+print('=== mypy ===')
+sys.stdout.flush()
+
+if not check_linter('mypy'):
+return
+
+env = os.environ.copy()
+env['MYPYPATH'] = env['PYTHONPATH']
+
+run_linter('mypy', files, env=env, suppress_output=True)
+
+
 def main() -> None:
-for linter in ('pylint', 'mypy'):
-try:
-run_linter(linter, ['--version'], suppress_output=True)
-except subprocess.CalledProcessError:
-iotests.notrun(f"'{linter}' not found")
-
 files = get_test_files()
 
 iotests.logger.debug('Files to be checked:')
 iotests.logger.debug(', '.join(sorted(files)))
 
-env = os.environ.copy()
-env['MYPYPATH'] = env['PYTHONPATH']
-
-print('=== pylint ===')
-sys.stdout.flush()
-try:
-run_linter('pylint', files, env=env)
-except subprocess.CalledProcessError:
-# pylint failure will be caught by diffing the IO.
-pass
-
-print('=== mypy ===')
-sys.stdout.flush()
-try:
-run_linter('mypy', files, env=env, suppress_output=True)
-except subprocess.CalledProcessError as exc:
-if exc.output:
-print(exc.output)
+for test in (test_pylint, test_mypy):
+try:
+test(files)
+except subprocess.CalledProcessError as exc:
+# Linter failure will be caught by diffing the IO.
+if exc.output:
+print(exc.output)
 
 
 iotests.script_main(main)
-- 
2.31.1

[PULL 11/22] iotests: split linters.py out from 297

[John Snow]

Now, 297 is just the iotests-specific incantations and linters.py is as
minimal as I can think to make it. The only remaining element in here
that ought to be configuration and not code is the list of skip files,
but they're still numerous enough that repeating them for mypy and
pylint configurations both would be ... a hassle.

Signed-off-by: John Snow 
Reviewed-by: Hanna Reitz 
Message-id: 20211019144918.3159078-12-js...@redhat.com
Signed-off-by: John Snow 
---
 tests/qemu-iotests/297| 72 +
 tests/qemu-iotests/linters.py | 76 +++
 2 files changed, 87 insertions(+), 61 deletions(-)
 create mode 100644 tests/qemu-iotests/linters.py

diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index b7d9d6077b3..ee78a627359 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -17,74 +17,24 @@
 # along with this program.  If not, see .
 
 import os
-import re
 import subprocess
 import sys
-from typing import List, Mapping, Optional
+from typing import List
 
 import iotests
+import linters
 
 
-# TODO: Empty this list!
-SKIP_FILES = (
-'030', '040', '041', '044', '045', '055', '056', '057', '065', '093',
-'096', '118', '124', '132', '136', '139', '147', '148', '149',
-'151', '152', '155', '163', '165', '194', '196', '202',
-'203', '205', '206', '207', '208', '210', '211', '212', '213', '216',
-'218', '219', '224', '228', '234', '235', '236', '237', '238',
-'240', '242', '245', '246', '248', '255', '256', '257', '258', '260',
-'262', '264', '266', '274', '277', '280', '281', '295', '296', '298',
-'299', '302', '303', '304', '307',
-'nbd-fault-injector.py', 'qcow2.py', 'qcow2_format.py', 'qed.py'
-)
-
-
-def is_python_file(filename):
-if not os.path.isfile(filename):
-return False
-
-if filename.endswith('.py'):
-return True
-
-with open(filename, encoding='utf-8') as f:
-try:
-first_line = f.readline()
-return re.match('^#!.*python', first_line) is not None
-except UnicodeDecodeError:  # Ignore binary files
-return False
-
-
-def get_test_files() -> List[str]:
-named_tests = [f'tests/{entry}' for entry in os.listdir('tests')]
-check_tests = set(os.listdir('.') + named_tests) - set(SKIP_FILES)
-return list(filter(is_python_file, check_tests))
-
-
-def run_linter(
-tool: str,
-args: List[str],
-env: Optional[Mapping[str, str]] = None,
-suppress_output: bool = False,
-) -> None:
-"""
-Run a python-based linting tool.
-
-:param suppress_output: If True, suppress all stdout/stderr output.
-:raise CalledProcessError: If the linter process exits with failure.
-"""
-subprocess.run(
-('python3', '-m', tool, *args),
-env=env,
-check=True,
-stdout=subprocess.PIPE if suppress_output else None,
-stderr=subprocess.STDOUT if suppress_output else None,
-universal_newlines=True,
-)
+# Looking for something?
+#
+#  List of files to exclude from linting: linters.py
+#  mypy configuration:mypy.ini
+#  pylint configuration:  pylintrc
 
 
 def check_linter(linter: str) -> bool:
 try:
-run_linter(linter, ['--version'], suppress_output=True)
+linters.run_linter(linter, ['--version'], suppress_output=True)
 except subprocess.CalledProcessError:
 iotests.case_notrun(f"'{linter}' not found")
 return False
@@ -98,7 +48,7 @@ def test_pylint(files: List[str]) -> None:
 if not check_linter('pylint'):
 return
 
-run_linter('pylint', files)
+linters.run_linter('pylint', files)
 
 
 def test_mypy(files: List[str]) -> None:
@@ -111,11 +61,11 @@ def test_mypy(files: List[str]) -> None:
 env = os.environ.copy()
 env['MYPYPATH'] = env['PYTHONPATH']
 
-run_linter('mypy', files, env=env, suppress_output=True)
+linters.run_linter('mypy', files, env=env, suppress_output=True)
 
 
 def main() -> None:
-files = get_test_files()
+files = linters.get_test_files()
 
 iotests.logger.debug('Files to be checked:')
 iotests.logger.debug(', '.join(sorted(files)))
diff --git a/tests/qemu-iotests/linters.py b/tests/qemu-iotests/linters.py
new file mode 100644
index 000..c515c7afe36
--- /dev/null
+++ b/tests/qemu-iotests/linters.py
@@ -0,0 +1,76 @@
+# Copyright (C) 2020 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+

[PULL 17/22] python/aqmp: Remove scary message

[John Snow]

The scary message interferes with the iotests output. Coincidentally, if
iotests works by removing this, then it's good evidence that we don't
really need to scare people away from using it.

Signed-off-by: John Snow 
Acked-by: Hanna Reitz 
Reviewed-by: Kevin Wolf 
Reviewed-by: Hanna Reitz 
Message-id: 20211026175612.4127598-4-js...@redhat.com
Signed-off-by: John Snow 
---
 python/qemu/aqmp/__init__.py | 12 
 1 file changed, 12 deletions(-)

diff --git a/python/qemu/aqmp/__init__.py b/python/qemu/aqmp/__init__.py
index d1b0e4dc3d3..880d5b6fa7f 100644
--- a/python/qemu/aqmp/__init__.py
+++ b/python/qemu/aqmp/__init__.py
@@ -22,7 +22,6 @@
 # the COPYING file in the top-level directory.
 
 import logging
-import warnings
 
 from .error import AQMPError
 from .events import EventListener
@@ -31,17 +30,6 @@
 from .qmp_client import ExecInterruptedError, ExecuteError, QMPClient
 
 
-_WMSG = """
-
-The Asynchronous QMP library is currently in development and its API
-should be considered highly fluid and subject to change. It should
-not be used by any other scripts checked into the QEMU tree.
-
-Proceed with caution!
-"""
-
-warnings.warn(_WMSG, FutureWarning)
-
 # Suppress logging unless an application engages it.
 logging.getLogger('qemu.aqmp').addHandler(logging.NullHandler())
 
-- 
2.31.1

[PULL 12/22] iotests/linters: Add entry point for linting via Python CI

[John Snow]

We need at least a tiny little shim here to join test file discovery
with test invocation. This logic could conceivably be hosted somewhere
in python/, but I felt it was strictly the least-rude thing to keep the
test logic here in iotests/, even if this small function isn't itself an
iotest.

Note that we don't actually even need the executable bit here, we'll be
relying on the ability to run this module as a script using Python CLI
arguments. No chance it gets misunderstood as an actual iotest that way.

(It's named, not in tests/, doesn't have the execute bit, and doesn't
have an execution shebang.)

Signed-off-by: John Snow 
Reviewed-by: Hanna Reitz 
Message-id: 20211019144918.3159078-13-js...@redhat.com
Signed-off-by: John Snow 
---
 tests/qemu-iotests/linters.py | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/tests/qemu-iotests/linters.py b/tests/qemu-iotests/linters.py
index c515c7afe36..46c28fdcda0 100644
--- a/tests/qemu-iotests/linters.py
+++ b/tests/qemu-iotests/linters.py
@@ -16,6 +16,7 @@
 import os
 import re
 import subprocess
+import sys
 from typing import List, Mapping, Optional
 
 
@@ -74,3 +75,29 @@ def run_linter(
 stderr=subprocess.STDOUT if suppress_output else None,
 universal_newlines=True,
 )
+
+
+def main() -> None:
+"""
+Used by the Python CI system as an entry point to run these linters.
+"""
+def show_usage() -> None:
+print(f"Usage: {sys.argv[0]} < --mypy | --pylint >", file=sys.stderr)
+sys.exit(1)
+
+if len(sys.argv) != 2:
+show_usage()
+
+files = get_test_files()
+
+if sys.argv[1] == '--pylint':
+run_linter('pylint', files)
+elif sys.argv[1] == '--mypy':
+run_linter('mypy', files)
+else:
+print(f"Unrecognized argument: '{sys.argv[1]}'", file=sys.stderr)
+show_usage()
+
+
+if __name__ == '__main__':
+main()
-- 
2.31.1

[PULL 05/22] iotests/297: Don't rely on distro-specific linter binaries

[John Snow]

'pylint-3' is another Fedora-ism. Use "python3 -m pylint" or "python3 -m
mypy" to access these scripts instead. This style of invocation will
prefer the "correct" tool when run in a virtual environment.

Note that we still check for "pylint-3" before the test begins -- this
check is now "overly strict", but shouldn't cause anything that was
already running correctly to start failing. This is addressed by a
commit later in this series;
  'iotests/297: update tool availability checks'.

Signed-off-by: John Snow 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Hanna Reitz 
Message-id: 20211019144918.3159078-6-js...@redhat.com
Signed-off-by: John Snow 
---
 tests/qemu-iotests/297 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index 163ebc8ebfd..c1bddb9ce0e 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -71,14 +71,14 @@ def run_linters():
 sys.stdout.flush()
 
 env = os.environ.copy()
-subprocess.run(('pylint-3', *files),
+subprocess.run(('python3', '-m', 'pylint', *files),
env=env, check=False)
 
 print('=== mypy ===')
 sys.stdout.flush()
 
 env['MYPYPATH'] = env['PYTHONPATH']
-p = subprocess.run(('mypy', *files),
+p = subprocess.run(('python3', '-m', 'mypy', *files),
env=env,
check=False,
stdout=subprocess.PIPE,
-- 
2.31.1

[PULL 07/22] iotests/297: refactor run_[mypy|pylint] as generic execution shim

[John Snow]

There's virtually nothing special here anymore; we can combine these
into a single, rather generic function.

Signed-off-by: John Snow 
Reviewed-by: Hanna Reitz 
Message-id: 20211019144918.3159078-8-js...@redhat.com
Signed-off-by: John Snow 
---
 tests/qemu-iotests/297 | 42 ++
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index 189bcaf5f94..d21673a2929 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -61,27 +61,29 @@ def get_test_files() -> List[str]:
 return list(filter(is_python_file, check_tests))
 
 
-def run_pylint(
-files: List[str],
-env: Optional[Mapping[str, str]] = None,
+def run_linter(
+tool: str,
+args: List[str],
+env: Optional[Mapping[str, str]] = None,
+suppress_output: bool = False,
 ) -> None:
+"""
+Run a python-based linting tool.
 
-subprocess.run(('python3', '-m', 'pylint', *files),
-   env=env, check=False)
+If suppress_output is True, capture stdout/stderr of the child
+process and only print that information back to stdout if the child
+process's return code was non-zero.
+"""
+p = subprocess.run(
+('python3', '-m', tool, *args),
+env=env,
+check=False,
+stdout=subprocess.PIPE if suppress_output else None,
+stderr=subprocess.STDOUT if suppress_output else None,
+universal_newlines=True,
+)
 
-
-def run_mypy(
-files: List[str],
-env: Optional[Mapping[str, str]] = None,
-) -> None:
-p = subprocess.run(('python3', '-m', 'mypy', *files),
-   env=env,
-   check=False,
-   stdout=subprocess.PIPE,
-   stderr=subprocess.STDOUT,
-   universal_newlines=True)
-
-if p.returncode != 0:
+if suppress_output and p.returncode != 0:
 print(p.stdout)
 
 
@@ -100,11 +102,11 @@ def main() -> None:
 
 print('=== pylint ===')
 sys.stdout.flush()
-run_pylint(files, env=env)
+run_linter('pylint', files, env=env)
 
 print('=== mypy ===')
 sys.stdout.flush()
-run_mypy(files, env=env)
+run_linter('mypy', files, env=env, suppress_output=True)
 
 
 iotests.script_main(main)
-- 
2.31.1

[PULL 14/22] python: Add iotest linters to test suite

[John Snow]

Run mypy and pylint on the iotests files directly from the Python CI
test infrastructure. This ensures that any accidental breakages to the
qemu.[qmp|aqmp|machine|utils] packages will be caught by that test
suite.

It also ensures that these linters are run with well-known versions and
test against a wide variety of python versions, which helps to find
accidental cross-version python compatibility issues.

Signed-off-by: John Snow 
Reviewed-by: Hanna Reitz 
Message-id: 20211019144918.3159078-15-js...@redhat.com
Signed-off-by: John Snow 
---
 python/tests/iotests-mypy.sh   | 4 
 python/tests/iotests-pylint.sh | 4 
 2 files changed, 8 insertions(+)
 create mode 100755 python/tests/iotests-mypy.sh
 create mode 100755 python/tests/iotests-pylint.sh

diff --git a/python/tests/iotests-mypy.sh b/python/tests/iotests-mypy.sh
new file mode 100755
index 000..ee764708199
--- /dev/null
+++ b/python/tests/iotests-mypy.sh
@@ -0,0 +1,4 @@
+#!/bin/sh -e
+
+cd ../tests/qemu-iotests/
+python3 -m linters --mypy
diff --git a/python/tests/iotests-pylint.sh b/python/tests/iotests-pylint.sh
new file mode 100755
index 000..4cae03424b4
--- /dev/null
+++ b/python/tests/iotests-pylint.sh
@@ -0,0 +1,4 @@
+#!/bin/sh -e
+
+cd ../tests/qemu-iotests/
+python3 -m linters --pylint
-- 
2.31.1

Re: [PATCH v2 1/5] hw/sd: add nuvoton MMC

[Richard Henderson]

On 11/1/21 1:18 PM, Peter Maydell wrote:

On Fri, 8 Oct 2021 at 01:26, Hao Wu  wrote:


From: Shengtan Mao 

Signed-off-by: Shengtan Mao 
Reviewed-by: Hao Wu 
Reviewed-by: Chris Rauer 
Reviewed-by: Tyrone Ting 
Signed-off-by:  Hao Wu 
---



+default:
+qemu_log_mask(LOG_GUEST_ERROR, "SDHCI read of nonexist reg: 0x%02"


"nonexistent"


+  HWADDR_PRIx, addr);
+break;
+}
+
+return val;
+}
+
+static void npcm7xx_sdhci_write(void *opaque, hwaddr addr, uint64_t val,
+unsigned int size)
+{
+NPCM7xxSDHCIState *s = opaque;
+
+switch (addr) {
+case NPCM7XX_BOOTTOCTRL:
+s->regs.boottoctrl = val;
+break;
+default:
+qemu_log_mask(LOG_GUEST_ERROR, "SDHCI write of nonexist reg: 0x%02"


ditto


Fixed while applying.


r~

[PULL 18/22] iotests: Accommodate async QMP Exception classes

[John Snow]

(But continue to support the old ones for now, too.)

There are very few cases of any user of QEMUMachine or a subclass
thereof relying on a QMP Exception type. If you'd like to check for
yourself, you want to grep for all of the derivatives of QMPError,
excluding 'AQMPError' and its derivatives. That'd be these:

- QMPError
- QMPConnectError
- QMPCapabilitiesError
- QMPTimeoutError
- QMPProtocolError
- QMPResponseError
- QMPBadPortError

Signed-off-by: John Snow 
Reviewed-by: Hanna Reitz 
Reviewed-by: Kevin Wolf 
Message-id: 20211026175612.4127598-5-js...@redhat.com
Signed-off-by: John Snow 
---
 scripts/simplebench/bench_block_job.py| 3 ++-
 tests/qemu-iotests/tests/mirror-top-perms | 5 +++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/scripts/simplebench/bench_block_job.py 
b/scripts/simplebench/bench_block_job.py
index 4f03c121697..a403c35b08f 100755
--- a/scripts/simplebench/bench_block_job.py
+++ b/scripts/simplebench/bench_block_job.py
@@ -28,6 +28,7 @@
 sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
 from qemu.machine import QEMUMachine
 from qemu.qmp import QMPConnectError
+from qemu.aqmp import ConnectError
 
 
 def bench_block_job(cmd, cmd_args, qemu_args):
@@ -49,7 +50,7 @@ def bench_block_job(cmd, cmd_args, qemu_args):
 vm.launch()
 except OSError as e:
 return {'error': 'popen failed: ' + str(e)}
-except (QMPConnectError, socket.timeout):
+except (QMPConnectError, ConnectError, socket.timeout):
 return {'error': 'qemu failed: ' + str(vm.get_log())}
 
 try:
diff --git a/tests/qemu-iotests/tests/mirror-top-perms 
b/tests/qemu-iotests/tests/mirror-top-perms
index 3d475aa3a54..a2d5c269d7a 100755
--- a/tests/qemu-iotests/tests/mirror-top-perms
+++ b/tests/qemu-iotests/tests/mirror-top-perms
@@ -21,8 +21,9 @@
 
 import os
 
-from qemu import qmp
+from qemu.aqmp import ConnectError
 from qemu.machine import machine
+from qemu.qmp import QMPConnectError
 
 import iotests
 from iotests import qemu_img
@@ -102,7 +103,7 @@ class TestMirrorTopPerms(iotests.QMPTestCase):
 self.vm_b.launch()
 print('ERROR: VM B launched successfully, this should not have '
   'happened')
-except qmp.QMPConnectError:
+except (QMPConnectError, ConnectError):
 assert 'Is another process using the image' in self.vm_b.get_log()
 
 result = self.vm.qmp('block-job-cancel',
-- 
2.31.1

[PULL 19/22] iotests: Conditionally silence certain AQMP errors

[John Snow]

AQMP likes to be very chatty about errors it encounters. In general,
this is good because it allows us to get good diagnostic information for
otherwise complex async failures.

For example, during a failed QMP connection attempt, we might see:

+ERROR:qemu.aqmp.qmp_client.qemub-2536319:Negotiation failed: EOFError
+ERROR:qemu.aqmp.qmp_client.qemub-2536319:Failed to establish session: EOFError

This might be nice in iotests output, because failure scenarios
involving the new QMP library will be spelled out plainly in the output
diffs.

For tests that are intentionally causing this scenario though, filtering
that log output could be a hassle. For now, add a context manager that
simply lets us toggle this output off during a critical region.

(Additionally, a forthcoming patch allows the use of either legacy or
async QMP to be toggled with an environment variable. In this
circumstance, we can't amend the iotest output to just always expect the
error message, either. Just suppress it for now. More rigorous log
filtering can be investigated later if/when it is deemed safe to
permanently replace the legacy QMP library.)

Signed-off-by: John Snow 
Reviewed-by: Hanna Reitz 
Reviewed-by: Kevin Wolf 
Message-id: 20211026175612.4127598-6-js...@redhat.com
Signed-off-by: John Snow 
---
 tests/qemu-iotests/iotests.py | 20 +++-
 tests/qemu-iotests/tests/mirror-top-perms | 12 
 2 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index e5fff6ddcfc..e2f9d873ada 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -30,7 +30,7 @@
 import subprocess
 import sys
 import time
-from typing import (Any, Callable, Dict, Iterable,
+from typing import (Any, Callable, Dict, Iterable, Iterator,
 List, Optional, Sequence, TextIO, Tuple, Type, TypeVar)
 import unittest
 
@@ -114,6 +114,24 @@
 sample_img_dir = os.environ['SAMPLE_IMG_DIR']
 
 
+@contextmanager
+def change_log_level(
+logger_name: str, level: int = logging.CRITICAL) -> Iterator[None]:
+"""
+Utility function for temporarily changing the log level of a logger.
+
+This can be used to silence errors that are expected or uninteresting.
+"""
+_logger = logging.getLogger(logger_name)
+current_level = _logger.level
+_logger.setLevel(level)
+
+try:
+yield
+finally:
+_logger.setLevel(current_level)
+
+
 def unarchive_sample_image(sample, fname):
 sample_fname = os.path.join(sample_img_dir, sample + '.bz2')
 with bz2.open(sample_fname) as f_in, open(fname, 'wb') as f_out:
diff --git a/tests/qemu-iotests/tests/mirror-top-perms 
b/tests/qemu-iotests/tests/mirror-top-perms
index a2d5c269d7a..0a51a613f39 100755
--- a/tests/qemu-iotests/tests/mirror-top-perms
+++ b/tests/qemu-iotests/tests/mirror-top-perms
@@ -26,7 +26,7 @@ from qemu.machine import machine
 from qemu.qmp import QMPConnectError
 
 import iotests
-from iotests import qemu_img
+from iotests import change_log_level, qemu_img
 
 
 image_size = 1 * 1024 * 1024
@@ -100,9 +100,13 @@ class TestMirrorTopPerms(iotests.QMPTestCase):
 self.vm_b.add_blockdev(f'file,node-name=drive0,filename={source}')
 self.vm_b.add_device('virtio-blk,drive=drive0,share-rw=on')
 try:
-self.vm_b.launch()
-print('ERROR: VM B launched successfully, this should not have '
-  'happened')
+# Silence AQMP errors temporarily.
+# TODO: Remove this and just allow the errors to be logged when
+# AQMP fully replaces QMP.
+with change_log_level('qemu.aqmp'):
+self.vm_b.launch()
+print('ERROR: VM B launched successfully, '
+  'this should not have happened')
 except (QMPConnectError, ConnectError):
 assert 'Is another process using the image' in self.vm_b.get_log()
 
-- 
2.31.1

[PULL 20/22] iotests/300: avoid abnormal shutdown race condition

[John Snow]

Wait for the destination VM to close itself instead of racing to shut it
down first, which produces different error log messages from AQMP
depending on precisely when we tried to shut it down.

(For example: We may try to issue 'quit' immediately prior to the target
VM closing its QMP socket, which will cause an ECONNRESET error to be
logged. Waiting for the VM to exit itself avoids the race on shutdown
behavior.)

Reported-by: Hanna Reitz 
Signed-off-by: John Snow 
Reviewed-by: Kevin Wolf 
Reviewed-by: Hanna Reitz 
Message-id: 20211026175612.4127598-7-js...@redhat.com
Signed-off-by: John Snow 
---
 tests/qemu-iotests/300 | 13 +
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/tests/qemu-iotests/300 b/tests/qemu-iotests/300
index 10f9f2a8da6..dbd28384ec3 100755
--- a/tests/qemu-iotests/300
+++ b/tests/qemu-iotests/300
@@ -24,8 +24,6 @@ import random
 import re
 from typing import Dict, List, Optional
 
-from qemu.machine import machine
-
 import iotests
 
 
@@ -461,12 +459,11 @@ class 
TestBlockBitmapMappingErrors(TestDirtyBitmapMigration):
   f"'{self.src_node_name}': Name is longer than 255 bytes",
   log)
 
-# Expect abnormal shutdown of the destination VM because of
-# the failed migration
-try:
-self.vm_b.shutdown()
-except machine.AbnormalShutdown:
-pass
+# Destination VM will terminate w/ error of its own accord
+# due to the failed migration.
+self.vm_b.wait()
+rc = self.vm_b.exitcode()
+assert rc is not None and rc > 0
 
 def test_aliased_bitmap_name_too_long(self) -> None:
 # Longer than the maximum for bitmap names
-- 
2.31.1

[PULL 09/22] iotests/297: update tool availability checks

[John Snow]

As mentioned in 'iotests/297: Don't rely on distro-specific linter
binaries', these checks are overly strict. Update them to be in-line
with how we actually invoke the linters themselves.

Signed-off-by: John Snow 
Reviewed-by: Hanna Reitz 
Message-id: 20211019144918.3159078-10-js...@redhat.com
Signed-off-by: John Snow 
---
 tests/qemu-iotests/297 | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index 76d6a23f531..b2ad8d1cbe0 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -18,7 +18,6 @@
 
 import os
 import re
-import shutil
 import subprocess
 import sys
 from typing import List, Mapping, Optional
@@ -84,9 +83,11 @@ def run_linter(
 
 
 def main() -> None:
-for linter in ('pylint-3', 'mypy'):
-if shutil.which(linter) is None:
-iotests.notrun(f'{linter} not found')
+for linter in ('pylint', 'mypy'):
+try:
+run_linter(linter, ['--version'], suppress_output=True)
+except subprocess.CalledProcessError:
+iotests.notrun(f"'{linter}' not found")
 
 files = get_test_files()
 
-- 
2.31.1

[PULL 13/22] iotests/linters: Add workaround for mypy bug #9852

[John Snow]

This one is insidious: if you write an import as "from {namespace}
import {subpackage}" as mirror-top-perms (now) does, mypy will fail on
every-other invocation *if* the package being imported is a typed,
installed, namespace-scoped package.

Upsettingly, that's exactly what 'qemu.[aqmp|qmp|machine]' et al are in
the context of Python CI tests.

Now, I could just edit mirror-top-perms to avoid this invocation, but
since I tripped on a landmine, I might as well head it off at the pass
and make sure nobody else trips on that same landmine.

It seems to have something to do with the order in which files are
checked as well, meaning the random order in which set(os.listdir())
produces the list of files to test will cause problems intermittently
and not just strictly "every other run".

This will be fixed in mypy >= 0.920, which is not released yet. The
workaround for now is to disable incremental checking, which avoids the
issue.

Note: This workaround is not applied when running iotest 297 directly,
because the bug does not surface there! Given the nature of CI jobs not
starting with any stale cache to begin with, this really only has a
half-second impact on manual runs of the Python test suite when executed
directly by a developer on their local machine. The workaround may be
removed when the Python package requirements can stipulate mypy 0.920 or
higher, which can happen as soon as it is released. (Barring any
unforseen compatibility issues that 0.920 may bring with it.)

See also:
 https://github.com/python/mypy/issues/11010
 https://github.com/python/mypy/issues/9852

Signed-off-by: John Snow 
Reviewed-by: Hanna Reitz 
Message-id: 20211019144918.3159078-14-js...@redhat.com
Signed-off-by: John Snow 
---
 tests/qemu-iotests/linters.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/qemu-iotests/linters.py b/tests/qemu-iotests/linters.py
index 46c28fdcda0..65c4c4e8272 100644
--- a/tests/qemu-iotests/linters.py
+++ b/tests/qemu-iotests/linters.py
@@ -93,7 +93,9 @@ def show_usage() -> None:
 if sys.argv[1] == '--pylint':
 run_linter('pylint', files)
 elif sys.argv[1] == '--mypy':
-run_linter('mypy', files)
+# mypy bug #9852; disable incremental checking as a workaround.
+args = ['--no-incremental'] + files
+run_linter('mypy', args)
 else:
 print(f"Unrecognized argument: '{sys.argv[1]}'", file=sys.stderr)
 show_usage()
-- 
2.31.1

[PULL 15/22] python/machine: remove has_quit argument

[John Snow]

If we spy on the QMP commands instead, we don't need callers to remember
to pass it. Seems like a fair trade-off.

The one slightly weird bit is overloading this instance variable for
wait(), where we use it to mean "don't issue the qmp 'quit'
command". This means that wait() will "fail" if the QEMU process does
not terminate of its own accord.

In most cases, we probably did already actually issue quit -- some
iotests do this -- but in some others, we may be waiting for QEMU to
terminate for some other reason, such as a test wherein we tell the
guest (directly) to shut down.

Signed-off-by: John Snow 
Reviewed-by: Hanna Reitz 
Reviewed-by: Kevin Wolf 
Message-id: 20211026175612.4127598-2-js...@redhat.com
Signed-off-by: John Snow 
---
 python/qemu/machine/machine.py | 34 +++---
 tests/qemu-iotests/040 |  7 +--
 tests/qemu-iotests/218 |  2 +-
 tests/qemu-iotests/255 |  2 +-
 4 files changed, 22 insertions(+), 23 deletions(-)

diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
index 056d340e355..0bd40bc2f76 100644
--- a/python/qemu/machine/machine.py
+++ b/python/qemu/machine/machine.py
@@ -170,6 +170,7 @@ def __init__(self,
 self._console_socket: Optional[socket.socket] = None
 self._remove_files: List[str] = []
 self._user_killed = False
+self._quit_issued = False
 
 def __enter__(self: _T) -> _T:
 return self
@@ -368,6 +369,7 @@ def _post_shutdown(self) -> None:
 command = ''
 LOG.warning(msg, -int(exitcode), command)
 
+self._quit_issued = False
 self._user_killed = False
 self._launched = False
 
@@ -443,15 +445,13 @@ def _hard_shutdown(self) -> None:
 self._subp.kill()
 self._subp.wait(timeout=60)
 
-def _soft_shutdown(self, timeout: Optional[int],
-   has_quit: bool = False) -> None:
+def _soft_shutdown(self, timeout: Optional[int]) -> None:
 """
 Perform early cleanup, attempt to gracefully shut down the VM, and wait
 for it to terminate.
 
 :param timeout: Timeout in seconds for graceful shutdown.
 A value of None is an infinite wait.
-:param has_quit: When True, don't attempt to issue 'quit' QMP command
 
 :raise ConnectionReset: On QMP communication errors
 :raise subprocess.TimeoutExpired: When timeout is exceeded waiting for
@@ -460,21 +460,19 @@ def _soft_shutdown(self, timeout: Optional[int],
 self._early_cleanup()
 
 if self._qmp_connection:
-if not has_quit:
+if not self._quit_issued:
 # Might raise ConnectionReset
-self._qmp.cmd('quit')
+self.qmp('quit')
 
 # May raise subprocess.TimeoutExpired
 self._subp.wait(timeout=timeout)
 
-def _do_shutdown(self, timeout: Optional[int],
- has_quit: bool = False) -> None:
+def _do_shutdown(self, timeout: Optional[int]) -> None:
 """
 Attempt to shutdown the VM gracefully; fallback to a hard shutdown.
 
 :param timeout: Timeout in seconds for graceful shutdown.
 A value of None is an infinite wait.
-:param has_quit: When True, don't attempt to issue 'quit' QMP command
 
 :raise AbnormalShutdown: When the VM could not be shut down gracefully.
 The inner exception will likely be ConnectionReset or
@@ -482,13 +480,13 @@ def _do_shutdown(self, timeout: Optional[int],
 may result in its own exceptions, likely subprocess.TimeoutExpired.
 """
 try:
-self._soft_shutdown(timeout, has_quit)
+self._soft_shutdown(timeout)
 except Exception as exc:
 self._hard_shutdown()
 raise AbnormalShutdown("Could not perform graceful shutdown") \
 from exc
 
-def shutdown(self, has_quit: bool = False,
+def shutdown(self,
  hard: bool = False,
  timeout: Optional[int] = 30) -> None:
 """
@@ -498,7 +496,6 @@ def shutdown(self, has_quit: bool = False,
 If the VM has not yet been launched, or shutdown(), wait(), or kill()
 have already been called, this method does nothing.
 
-:param has_quit: When true, do not attempt to issue 'quit' QMP command.
 :param hard: When true, do not attempt graceful shutdown, and
  suppress the SIGKILL warning log message.
 :param timeout: Optional timeout in seconds for graceful shutdown.
@@ -512,7 +509,7 @@ def shutdown(self, has_quit: bool = False,
 self._user_killed = True
 self._hard_shutdown()
 else:
-self._do_shutdown(timeout, has_quit)
+self._do_shutdown(timeout)
 finally:
 self._post_shutdown()
 
@@ -529,7 +526,8 @@ def wait(self, timeout: Opti

[PULL 16/22] python/machine: Handle QMP errors on close more meticulously

[John Snow]

To use the AQMP backend, Machine just needs to be a little more diligent
about what happens when closing a QMP connection. The operation is no
longer a freebie in the async world; it may return errors encountered in
the async bottom half on incoming message receipt, etc.

(AQMP's disconnect, ultimately, serves as the quiescence point where all
async contexts are gathered together, and any final errors reported at
that point.)

Because async QMP continues to check for messages asynchronously, it's
almost certainly likely that the loop will have exited due to EOF after
issuing the last 'quit' command. That error will ultimately be bubbled
up when attempting to close the QMP connection. The manager class here
then is free to discard it -- if it was expected.

Signed-off-by: John Snow 
Reviewed-by: Hanna Reitz 
Reviewed-by: Kevin Wolf 
Message-id: 20211026175612.4127598-3-js...@redhat.com
Signed-off-by: John Snow 
---
 python/qemu/machine/machine.py | 48 +-
 1 file changed, 42 insertions(+), 6 deletions(-)

diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
index 0bd40bc2f76..a0cf69786b4 100644
--- a/python/qemu/machine/machine.py
+++ b/python/qemu/machine/machine.py
@@ -342,9 +342,15 @@ def _post_shutdown(self) -> None:
 # Comprehensive reset for the failed launch case:
 self._early_cleanup()
 
-if self._qmp_connection:
-self._qmp.close()
-self._qmp_connection = None
+try:
+self._close_qmp_connection()
+except Exception as err:  # pylint: disable=broad-except
+LOG.warning(
+"Exception closing QMP connection: %s",
+str(err) if str(err) else type(err).__name__
+)
+finally:
+assert self._qmp_connection is None
 
 self._close_qemu_log_file()
 
@@ -420,6 +426,31 @@ def _launch(self) -> None:
close_fds=False)
 self._post_launch()
 
+def _close_qmp_connection(self) -> None:
+"""
+Close the underlying QMP connection, if any.
+
+Dutifully report errors that occurred while closing, but assume
+that any error encountered indicates an abnormal termination
+process and not a failure to close.
+"""
+if self._qmp_connection is None:
+return
+
+try:
+self._qmp.close()
+except EOFError:
+# EOF can occur as an Exception here when using the Async
+# QMP backend. It indicates that the server closed the
+# stream. If we successfully issued 'quit' at any point,
+# then this was expected. If the remote went away without
+# our permission, it's worth reporting that as an abnormal
+# shutdown case.
+if not (self._user_killed or self._quit_issued):
+raise
+finally:
+self._qmp_connection = None
+
 def _early_cleanup(self) -> None:
 """
 Perform any cleanup that needs to happen before the VM exits.
@@ -460,9 +491,14 @@ def _soft_shutdown(self, timeout: Optional[int]) -> None:
 self._early_cleanup()
 
 if self._qmp_connection:
-if not self._quit_issued:
-# Might raise ConnectionReset
-self.qmp('quit')
+try:
+if not self._quit_issued:
+# May raise ExecInterruptedError or StateError if the
+# connection dies or has *already* died.
+self.qmp('quit')
+finally:
+# Regardless, we want to quiesce the connection.
+self._close_qmp_connection()
 
 # May raise subprocess.TimeoutExpired
 self._subp.wait(timeout=timeout)
-- 
2.31.1

[PULL 21/22] python/aqmp: Create sync QMP wrapper for iotests

[John Snow]

This is a wrapper around the async QMPClient that mimics the old,
synchronous QEMUMonitorProtocol class. It is designed to be
interchangeable with the old implementation.

It does not, however, attempt to mimic Exception compatibility.

Signed-off-by: John Snow 
Acked-by: Hanna Reitz 
Reviewed-by: Kevin Wolf 
Reviewed-by: Hanna Reitz 
Message-id: 20211026175612.4127598-8-js...@redhat.com
Signed-off-by: John Snow 
---
 python/qemu/aqmp/legacy.py | 138 +
 1 file changed, 138 insertions(+)
 create mode 100644 python/qemu/aqmp/legacy.py

diff --git a/python/qemu/aqmp/legacy.py b/python/qemu/aqmp/legacy.py
new file mode 100644
index 000..9e7b9fb80b9
--- /dev/null
+++ b/python/qemu/aqmp/legacy.py
@@ -0,0 +1,138 @@
+"""
+Sync QMP Wrapper
+
+This class pretends to be qemu.qmp.QEMUMonitorProtocol.
+"""
+
+import asyncio
+from typing import (
+Awaitable,
+List,
+Optional,
+TypeVar,
+Union,
+)
+
+import qemu.qmp
+from qemu.qmp import QMPMessage, QMPReturnValue, SocketAddrT
+
+from .qmp_client import QMPClient
+
+
+# pylint: disable=missing-docstring
+
+
+class QEMUMonitorProtocol(qemu.qmp.QEMUMonitorProtocol):
+def __init__(self, address: SocketAddrT,
+ server: bool = False,
+ nickname: Optional[str] = None):
+
+# pylint: disable=super-init-not-called
+self._aqmp = QMPClient(nickname)
+self._aloop = asyncio.get_event_loop()
+self._address = address
+self._timeout: Optional[float] = None
+
+_T = TypeVar('_T')
+
+def _sync(
+self, future: Awaitable[_T], timeout: Optional[float] = None
+) -> _T:
+return self._aloop.run_until_complete(
+asyncio.wait_for(future, timeout=timeout)
+)
+
+def _get_greeting(self) -> Optional[QMPMessage]:
+if self._aqmp.greeting is not None:
+# pylint: disable=protected-access
+return self._aqmp.greeting._asdict()
+return None
+
+# __enter__ and __exit__ need no changes
+# parse_address needs no changes
+
+def connect(self, negotiate: bool = True) -> Optional[QMPMessage]:
+self._aqmp.await_greeting = negotiate
+self._aqmp.negotiate = negotiate
+
+self._sync(
+self._aqmp.connect(self._address)
+)
+return self._get_greeting()
+
+def accept(self, timeout: Optional[float] = 15.0) -> QMPMessage:
+self._aqmp.await_greeting = True
+self._aqmp.negotiate = True
+
+self._sync(
+self._aqmp.accept(self._address),
+timeout
+)
+
+ret = self._get_greeting()
+assert ret is not None
+return ret
+
+def cmd_obj(self, qmp_cmd: QMPMessage) -> QMPMessage:
+return dict(
+self._sync(
+# pylint: disable=protected-access
+
+# _raw() isn't a public API, because turning off
+# automatic ID assignment is discouraged. For
+# compatibility with iotests *only*, do it anyway.
+self._aqmp._raw(qmp_cmd, assign_id=False),
+self._timeout
+)
+)
+
+# Default impl of cmd() delegates to cmd_obj
+
+def command(self, cmd: str, **kwds: object) -> QMPReturnValue:
+return self._sync(
+self._aqmp.execute(cmd, kwds),
+self._timeout
+)
+
+def pull_event(self,
+   wait: Union[bool, float] = False) -> Optional[QMPMessage]:
+if not wait:
+# wait is False/0: "do not wait, do not except."
+if self._aqmp.events.empty():
+return None
+
+# If wait is 'True', wait forever. If wait is False/0, the events
+# queue must not be empty; but it still needs some real amount
+# of time to complete.
+timeout = None
+if wait and isinstance(wait, float):
+timeout = wait
+
+return dict(
+self._sync(
+self._aqmp.events.get(),
+timeout
+)
+)
+
+def get_events(self, wait: Union[bool, float] = False) -> List[QMPMessage]:
+events = [dict(x) for x in self._aqmp.events.clear()]
+if events:
+return events
+
+event = self.pull_event(wait)
+return [event] if event is not None else []
+
+def clear_events(self) -> None:
+self._aqmp.events.clear()
+
+def close(self) -> None:
+self._sync(
+self._aqmp.disconnect()
+)
+
+def settimeout(self, timeout: Optional[float]) -> None:
+self._timeout = timeout
+
+def send_fd_scm(self, fd: int) -> None:
+self._aqmp.send_fd_scm(fd)
-- 
2.31.1

[PULL 22/22] python, iotests: replace qmp with aqmp

[John Snow]

Swap out the synchronous QEMUMonitorProtocol from qemu.qmp with the sync
wrapper from qemu.aqmp instead.

Add an escape hatch in the form of the environment variable
QEMU_PYTHON_LEGACY_QMP which allows you to cajole QEMUMachine into using
the old implementation, proving that both implementations work
concurrently.

Signed-off-by: John Snow 
Reviewed-by: Kevin Wolf 
Reviewed-by: Hanna Reitz 
Message-id: 20211026175612.4127598-9-js...@redhat.com
Signed-off-by: John Snow 
---
 python/qemu/machine/machine.py | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
index a0cf69786b4..a487c397459 100644
--- a/python/qemu/machine/machine.py
+++ b/python/qemu/machine/machine.py
@@ -41,7 +41,6 @@
 )
 
 from qemu.qmp import (  # pylint: disable=import-error
-QEMUMonitorProtocol,
 QMPMessage,
 QMPReturnValue,
 SocketAddrT,
@@ -50,6 +49,12 @@
 from . import console_socket
 
 
+if os.environ.get('QEMU_PYTHON_LEGACY_QMP'):
+from qemu.qmp import QEMUMonitorProtocol
+else:
+from qemu.aqmp.legacy import QEMUMonitorProtocol
+
+
 LOG = logging.getLogger(__name__)
 
 
-- 
2.31.1

Re: [PATCH v2 0/3] virtio: increase VIRTQUEUE_MAX_SIZE to 32k

[Christian Schoenebeck]

On Donnerstag, 28. Oktober 2021 11:00:48 CET Stefan Hajnoczi wrote:
> On Mon, Oct 25, 2021 at 05:03:25PM +0200, Christian Schoenebeck wrote:
> > On Montag, 25. Oktober 2021 12:30:41 CEST Stefan Hajnoczi wrote:
> > > On Thu, Oct 21, 2021 at 05:39:28PM +0200, Christian Schoenebeck wrote:
> > > > On Freitag, 8. Oktober 2021 18:08:48 CEST Christian Schoenebeck wrote:
> > > > > On Freitag, 8. Oktober 2021 16:24:42 CEST Christian Schoenebeck wrote:
> > > > > > On Freitag, 8. Oktober 2021 09:25:33 CEST Greg Kurz wrote:
> > > > > > > On Thu, 7 Oct 2021 16:42:49 +0100
> > > > > > > 
> > > > > > > Stefan Hajnoczi  wrote:
> > > > > > > > On Thu, Oct 07, 2021 at 02:51:55PM +0200, Christian Schoenebeck 
> > > > > > > > wrote:
> > > > > > > > > On Donnerstag, 7. Oktober 2021 07:23:59 CEST Stefan Hajnoczi 
> > > > > > > > > wrote:
> > > > > > > > > > On Mon, Oct 04, 2021 at 09:38:00PM +0200, Christian
> > > > > > > > > > Schoenebeck
> > > > > 
> > > > > wrote:
> > > > > > > > > > > At the moment the maximum transfer size with virtio is
> > > > > > > > > > > limited
> > > > > > > > > > > to
> > > > > > > > > > > 4M
> > > > > > > > > > > (1024 * PAGE_SIZE). This series raises this limit to its
> > > > > > > > > > > maximum
> > > > > > > > > > > theoretical possible transfer size of 128M (32k pages)
> > > > > > > > > > > according
> > > > > > > > > > > to
> > > > > > > > > > > the
> > > > > > > > > > > virtio specs:
> > > > > > > > > > > 
> > > > > > > > > > > https://docs.oasis-open.org/virtio/virtio/v1.1/cs01/virt
> > > > > > > > > > > io-v
> > > > > > > > > > > 1.1-
> > > > > > > > > > > cs
> > > > > > > > > > > 01
> > > > > > > > > > > .html#
> > > > > > > > > > > x1-240006
> > > > > > > > > > 
> > > > > > > > > > Hi Christian,
> > > > > > > 
> > > > > > > > > > I took a quick look at the code:
> > > > > > > Hi,
> > > > > > > 
> > > > > > > Thanks Stefan for sharing virtio expertise and helping Christian
> > > > > > > !
> > > > > > > 
> > > > > > > > > > - The Linux 9p driver restricts descriptor chains to 128
> > > > > > > > > > elements
> > > > > > > > > > 
> > > > > > > > > >   (net/9p/trans_virtio.c:VIRTQUEUE_NUM)
> > > > > > > > > 
> > > > > > > > > Yes, that's the limitation that I am about to remove (WIP);
> > > > > > > > > current
> > > > > > > > > kernel
> > > > > > > > > patches:
> > > > > > > > > https://lore.kernel.org/netdev/cover.1632327421.git.linux_os
> > > > > > > > > s@cr
> > > > > > > > > udeb
> > > > > > > > > yt
> > > > > > > > > e.
> > > > > > > > > com/>
> > > > > > > > 
> > > > > > > > I haven't read the patches yet but I'm concerned that today
> > > > > > > > the
> > > > > > > > driver
> > > > > > > > is pretty well-behaved and this new patch series introduces a
> > > > > > > > spec
> > > > > > > > violation. Not fixing existing spec violations is okay, but
> > > > > > > > adding
> > > > > > > > new
> > > > > > > > ones is a red flag. I think we need to figure out a clean
> > > > > > > > solution.
> > > > > > 
> > > > > > Nobody has reviewed the kernel patches yet. My main concern
> > > > > > therefore
> > > > > > actually is that the kernel patches are already too complex,
> > > > > > because
> > > > > > the
> > > > > > current situation is that only Dominique is handling 9p patches on
> > > > > > kernel
> > > > > > side, and he barely has time for 9p anymore.
> > > > > > 
> > > > > > Another reason for me to catch up on reading current kernel code
> > > > > > and
> > > > > > stepping in as reviewer of 9p on kernel side ASAP, independent of
> > > > > > this
> > > > > > issue.
> > > > > > 
> > > > > > As for current kernel patches' complexity: I can certainly drop
> > > > > > patch
> > > > > > 7
> > > > > > entirely as it is probably just overkill. Patch 4 is then the
> > > > > > biggest
> > > > > > chunk, I have to see if I can simplify it, and whether it would
> > > > > > make
> > > > > > sense to squash with patch 3.
> > > > > > 
> > > > > > > > > > - The QEMU 9pfs code passes iovecs directly to preadv(2)
> > > > > > > > > > and
> > > > > > > > > > will
> > > > > > > > > > fail
> > > > > > > > > > 
> > > > > > > > > >   with EINVAL when called with more than IOV_MAX iovecs
> > > > > > > > > >   (hw/9pfs/9p.c:v9fs_read())
> > > > > > > > > 
> > > > > > > > > Hmm, which makes me wonder why I never encountered this
> > > > > > > > > error
> > > > > > > > > during
> > > > > > > > > testing.
> > > > > > > > > 
> > > > > > > > > Most people will use the 9p qemu 'local' fs driver backend
> > > > > > > > > in
> > > > > > > > > practice,
> > > > > > > > > so
> > > > > > > > > that v9fs_read() call would translate for most people to
> > > > > > > > > this
> > > > > > > > > implementation on QEMU side (hw/9p/9p-local.c):
> > > > > > > > > 
> > > > > > > > > static ssize_t local_preadv(FsContext *ctx, V9fsFidOpenState
> > > > > > > > > *fs,
> > > > > > > > > 
> > > > > > > > > const struct iovec *iov,
> > > > > > > > > int iovcnt, off_t offset)
> >