Re: [RUST] Add crate for generic vhost-user-i2c backend daemon

2021-05-27 Thread Viresh Kumar
On 28-04-21, 17:52, Viresh Kumar wrote:
> Hello,
> 
> In my earlier attempt [1], I implemented the vhost-user-i2c backend
> deamon for QEMU (though the code was generic enough to be used with
> any hypervisor).
> 
> And here is a Rust implementation of the vhost-user-i2c backend
> daemon. Again this is generic enough to be used with any hypervisor
> and can live in its own repository now:
> 
>   https://github.com/vireshk/vhost-user-i2c

A new crate is added in rust-vmm for this and here is the new repo I am using:

https://github.com/vireshk/vhost-device

And here is the discussion happening on my PULL request.

https://github.com/rust-vmm/vhost-device/pull/1

-- 
viresh



Re: [PATCH v1 3/6] tests/tcg/configure.sh: tweak quoting of target_compiler

2021-05-27 Thread Thomas Huth

On 27/05/2021 18.03, Alex Bennée wrote:

If you configure the host compiler with a multi-command stanza like:

   --cc="ccache gcc"

then the configure.sh machinery falls over with confusion. Work around
this by ensuring we correctly quote so where we need a complete
evaluation we get it. Of course the has() check needs single variable
so we need to unquote that. This does mean it essentially checks that
just the ccache command exits but if we got past that step we still
check the compiler actually does something.

Signed-off-by: Alex Bennée 
Cc: Thomas Huth 
---
  tests/tcg/configure.sh | 6 +++---
  1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/tcg/configure.sh b/tests/tcg/configure.sh
index ed6492ce59..aa7c24328a 100755
--- a/tests/tcg/configure.sh
+++ b/tests/tcg/configure.sh
@@ -222,10 +222,10 @@ for target in $target_list; do
  
got_cross_cc=no
  
-  if eval test "x\${cross_cc_$arch}" != xyes; then

-  eval "target_compiler=\${cross_cc_$arch}"
+  if eval test "x\"\${cross_cc_$arch}\"" != xyes; then
+  eval "target_compiler=\"\${cross_cc_$arch}\""
  
-  if has "$target_compiler"; then

+  if has $target_compiler; then
if test "$supress_clang" = yes &&
$target_compiler --version | grep -qi "clang"; then
got_cross_cc=no



Reviewed-by: Thomas Huth 




[PULL 39/44] python: add devel package requirements to setuptools

2021-05-27 Thread John Snow
setuptools doesn't have a formal understanding of development requires,
but it has an optional feataures section. Fine; add a "devel" feature
and add the requirements to it.

To avoid duplication, we can modify pipenv to install qemu[devel]
instead. This enables us to run invocations like "pip install -e
.[devel]" and test the package on bleeding-edge packages beyond those
specified in Pipfile.lock.

Importantly, this also allows us to install the qemu development
packages in a non-networked mode: `pip3 install --no-index -e .[devel]`
will now fail if the proper development dependencies are not already
met. This can be useful for automated build scripts where fetching
network packages may be undesirable.

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
Message-id: 20210527211715.394144-27-js...@redhat.com
Signed-off-by: John Snow 
---
 python/PACKAGE.rst  |  4 
 python/README.rst   |  4 
 python/Pipfile  |  5 +
 python/Pipfile.lock | 14 +-
 python/setup.cfg|  9 +
 5 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/python/PACKAGE.rst b/python/PACKAGE.rst
index 1bbfe1b58e2..05ea7789fc1 100644
--- a/python/PACKAGE.rst
+++ b/python/PACKAGE.rst
@@ -31,3 +31,7 @@ official `GitLab mirror 
`_.
 Please report bugs on the `QEMU issue tracker
 `_ and tag ``@jsnow`` in
 the report.
+
+Optional packages necessary for running code quality analysis for this
+package can be installed with the optional dependency group "devel":
+``pip install qemu[devel]``.
diff --git a/python/README.rst b/python/README.rst
index bf9bbca979a..954870973d0 100644
--- a/python/README.rst
+++ b/python/README.rst
@@ -24,6 +24,10 @@ which installs a version of the package that installs a 
forwarder
 pointing to these files, such that the package always reflects the
 latest version in your git tree.
 
+Installing ".[devel]" instead of "." will additionally pull in required
+packages for testing this package. They are not runtime requirements,
+and are not needed to simply use these libraries.
+
 See `Installing packages using pip and virtual environments
 
`_
 for more information.
diff --git a/python/Pipfile b/python/Pipfile
index dbe96f71c48..e7acb8cefa4 100644
--- a/python/Pipfile
+++ b/python/Pipfile
@@ -4,10 +4,7 @@ url = "https://pypi.org/simple;
 verify_ssl = true
 
 [dev-packages]
-flake8 = ">=3.6.0"
-isort = ">=5.1.2"
-mypy = ">=0.770"
-pylint = ">=2.8.0"
+qemu = {editable = true, extras = ["devel"], path = "."}
 
 [packages]
 qemu = {editable = true,path = "."}
diff --git a/python/Pipfile.lock b/python/Pipfile.lock
index f0bf576c31e..a2cdc1c50ea 100644
--- a/python/Pipfile.lock
+++ b/python/Pipfile.lock
@@ -1,7 +1,7 @@
 {
 "_meta": {
 "hash": {
-"sha256": 
"7c74cc4c2db3a75c954a6686411cda6fd60e464620bb6d5f1ed9a54be61db4cc"
+"sha256": 
"eff562a688ebc6f3ffe67494dbb804b883e2159ad81c4d55d96da9f7aec13e91"
 },
 "pipfile-spec": 6,
 "requires": {
@@ -35,7 +35,7 @@
 
"sha256:07528381786f2a6237b061f6e96610a4167b226cb926e2aa2b6b1d78057c576b",
 
"sha256:bf8fd46d844f616e8d47905ef3a3384edae6b4e9beb0c5101e25e3110907"
 ],
-"index": "pypi",
+"markers": "python_version >= '2.7' and python_version not in 
'3.0, 3.1, 3.2, 3.3, 3.4'",
 "version": "==3.9.2"
 },
 "importlib-metadata": {
@@ -51,7 +51,7 @@
 
"sha256:0a943902919f65c5684ac4e0154b1ad4fac6dcaa5d9f3426b732f1c8b5419be6",
 
"sha256:2bb1680aad211e3c9944dbce1d4ba09a989f04e238296c87fe2139faa26d655d"
 ],
-"index": "pypi",
+"markers": "python_version >= '3.6' and python_version < '4.0'",
 "version": "==5.8.0"
 },
 "lazy-object-proxy": {
@@ -114,7 +114,7 @@
 
"sha256:d65cc1df038ef55a99e617431f0553cd77763869eebdf9042403e16089fe746c",
 
"sha256:d7da2e1d5f558c37d6e8c1246f1aec1e7349e4913d8fb3cb289a35de573fe2eb"
 ],
-"index": "pypi",
+"markers": "python_version >= '3.5'",
 "version": "==0.812"
 },
 "mypy-extensions": {
@@ -145,9 +145,13 @@
 
"sha256:586d8fa9b1891f4b725f587ef267abe2a1bad89d6b184520c7f07a253dd6e217",
 
"sha256:f7e2072654a6b6afdf5e2fb38147d3e2d2d43c89f648637baab63e026481279b"
 ],
-"index": "pypi",
+"markers": "python_version ~= '3.6'",
 "version": "==2.8.2"
 },
+"qemu": {
+"editable": true,
+"path": "."
+},
 "toml": {
 "hashes": [
 
"sha256:806143ae5bfb6a3c6e736a764057db0e6a0e05e338b5630894a5f779cabb4f9b",
diff --git a/python/setup.cfg b/python/setup.cfg
index 

[PULL 44/44] gitlab: add python linters to CI

2021-05-27 Thread John Snow
Add a Python container that has just enough juice for us to run the
Python code quality analysis tools. Base this container on Fedora,
because Fedora has very convenient packaging for testing multiple Python
versions.

We need python3, pip (for pulling packages), pipenv and virtualenv for
creating virtual environments, and tox for running tests. make is needed
for running 'make check-tox' and 'make venv-check' targets. Python3.10
is needed explicitly because the tox package only pulls in 3.6-3.9, but
we wish to test the forthcoming release of Python as well to help
predict any problems. Lastly, we need gcc to compile PyPI packages that
may not have a binary distribution available.

Add two tests:

check-python-pipenv uses pipenv to test a frozen, very explicit set of
packages against our minimum supported python version, Python 3.6. This
test is not allowed to fail. The dependencies this test uses do not
change unless python/Pipfile.lock is changed.

check-python-tox uses tox to install the latest versions of required
python dependencies against a wide array of Python versions from 3.6 to
3.9, even including the yet-to-be-released Python 3.10. This test is
allowed to fail with a warning.

Signed-off-by: John Snow 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Alex Bennée 
Reviewed-by: Cleber Rosa 
Message-id: 20210527211715.394144-32-js...@redhat.com
[Fix rebase conflict over .gitlab-ci.yml --js]
Signed-off-by: John Snow 
---
 .gitlab-ci.d/containers.yml|  5 +
 .gitlab-ci.d/static_checks.yml | 21 +
 tests/docker/dockerfiles/python.docker | 18 ++
 3 files changed, 44 insertions(+)
 create mode 100644 tests/docker/dockerfiles/python.docker

diff --git a/.gitlab-ci.d/containers.yml b/.gitlab-ci.d/containers.yml
index bd01ae8f802..1ca455f8e10 100644
--- a/.gitlab-ci.d/containers.yml
+++ b/.gitlab-ci.d/containers.yml
@@ -43,3 +43,8 @@ amd64-opensuse-leap-container:
   extends: .container_job_template
   variables:
 NAME: opensuse-leap
+
+python-container:
+  extends: .container_job_template
+  variables:
+NAME: python
diff --git a/.gitlab-ci.d/static_checks.yml b/.gitlab-ci.d/static_checks.yml
index 91247a6f670..8e308721640 100644
--- a/.gitlab-ci.d/static_checks.yml
+++ b/.gitlab-ci.d/static_checks.yml
@@ -24,3 +24,24 @@ check-dco:
 - if: '$CI_PROJECT_NAMESPACE == "qemu-project" && $CI_COMMIT_BRANCH == 
$CI_DEFAULT_BRANCH'
   when: never
 - when: on_success
+
+check-python-pipenv:
+  stage: test
+  image: $CI_REGISTRY_IMAGE/qemu/python:latest
+  script:
+- make -C python venv-check
+  variables:
+GIT_DEPTH: 1
+  needs:
+job: python-container
+
+check-python-tox:
+  stage: test
+  image: $CI_REGISTRY_IMAGE/qemu/python:latest
+  script:
+- make -C python check-tox
+  variables:
+GIT_DEPTH: 1
+  needs:
+job: python-container
+  allow_failure: true
diff --git a/tests/docker/dockerfiles/python.docker 
b/tests/docker/dockerfiles/python.docker
new file mode 100644
index 000..56d88417df4
--- /dev/null
+++ b/tests/docker/dockerfiles/python.docker
@@ -0,0 +1,18 @@
+# Python library testing environment
+
+FROM fedora:latest
+MAINTAINER John Snow 
+
+# Please keep this list sorted alphabetically
+ENV PACKAGES \
+gcc \
+make \
+pipenv \
+python3 \
+python3-pip \
+python3-tox \
+python3-virtualenv \
+python3.10
+
+RUN dnf install -y $PACKAGES
+RUN rpm -q $PACKAGES | sort > /packages.txt
-- 
2.31.1




[PULL 42/44] python: add .gitignore

2021-05-27 Thread John Snow
Ignore *Python* build and package output (build, dist, qemu.egg-info);
these files are not created as part of a QEMU build. They are created by
running the commands 'python3 setup.py ' when preparing
tarballs to upload to e.g. PyPI.

Ignore miscellaneous cached python confetti (mypy, pylint, et al)

Ignore .idea (pycharm) .vscode, and .venv (pipenv et al).

Signed-off-by: John Snow 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Cleber Rosa 
Message-id: 20210527211715.394144-30-js...@redhat.com
Signed-off-by: John Snow 
---
 python/.gitignore | 15 +++
 1 file changed, 15 insertions(+)
 create mode 100644 python/.gitignore

diff --git a/python/.gitignore b/python/.gitignore
new file mode 100644
index 000..4ed144ceac3
--- /dev/null
+++ b/python/.gitignore
@@ -0,0 +1,15 @@
+# linter/tooling cache
+.mypy_cache/
+.cache/
+
+# python packaging
+build/
+dist/
+qemu.egg-info/
+
+# editor config
+.idea/
+.vscode/
+
+# virtual environments (pipenv et al)
+.venv/
-- 
2.31.1




[PULL 38/44] python/qemu: add qemu package itself to pipenv

2021-05-27 Thread John Snow
This adds the python qemu packages themselves to the pipenv manifest.
'pipenv sync' will create a virtual environment sufficient to use the SDK.
'pipenv sync --dev' will create a virtual environment sufficient to use
and test the SDK (with pylint, mypy, isort, flake8, etc.)

The qemu packages are installed in 'editable' mode; all changes made to
the python package inside the git tree will be reflected in the
installed package without reinstallation. This includes changes made
via git pull and so on.

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
Tested-by: Cleber Rosa 
Message-id: 20210527211715.394144-26-js...@redhat.com
Signed-off-by: John Snow 
---
 python/Pipfile  | 1 +
 python/Pipfile.lock | 9 +++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/python/Pipfile b/python/Pipfile
index 79c74dd8db4..dbe96f71c48 100644
--- a/python/Pipfile
+++ b/python/Pipfile
@@ -10,6 +10,7 @@ mypy = ">=0.770"
 pylint = ">=2.8.0"
 
 [packages]
+qemu = {editable = true,path = "."}
 
 [requires]
 python_version = "3.6"
diff --git a/python/Pipfile.lock b/python/Pipfile.lock
index 57a5befb104..f0bf576c31e 100644
--- a/python/Pipfile.lock
+++ b/python/Pipfile.lock
@@ -1,7 +1,7 @@
 {
 "_meta": {
 "hash": {
-"sha256": 
"8173290ad57aab0b722c9b4f109519de4e0dd7cd1bad1e16806b78bb169bce08"
+"sha256": 
"7c74cc4c2db3a75c954a6686411cda6fd60e464620bb6d5f1ed9a54be61db4cc"
 },
 "pipfile-spec": 6,
 "requires": {
@@ -15,7 +15,12 @@
 }
 ]
 },
-"default": {},
+"default": {
+"qemu": {
+"editable": true,
+"path": "."
+}
+},
 "develop": {
 "astroid": {
 "hashes": [
-- 
2.31.1




[PULL 36/44] python: move .isort.cfg into setup.cfg

2021-05-27 Thread John Snow
Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
Message-id: 20210527211715.394144-24-js...@redhat.com
Signed-off-by: John Snow 
---
 python/.isort.cfg | 7 ---
 python/setup.cfg  | 8 
 2 files changed, 8 insertions(+), 7 deletions(-)
 delete mode 100644 python/.isort.cfg

diff --git a/python/.isort.cfg b/python/.isort.cfg
deleted file mode 100644
index 6d0fd6cc0d3..000
--- a/python/.isort.cfg
+++ /dev/null
@@ -1,7 +0,0 @@
-[settings]
-force_grid_wrap=4
-force_sort_within_sections=True
-include_trailing_comma=True
-line_length=72
-lines_after_imports=2
-multi_line_output=3
\ No newline at end of file
diff --git a/python/setup.cfg b/python/setup.cfg
index b485d6161d5..3f07bd2752d 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -61,3 +61,11 @@ good-names=i,
 [pylint.similarities]
 # Ignore imports when computing similarities.
 ignore-imports=yes
+
+[isort]
+force_grid_wrap=4
+force_sort_within_sections=True
+include_trailing_comma=True
+line_length=72
+lines_after_imports=2
+multi_line_output=3
-- 
2.31.1




[PULL 40/44] python: add avocado-framework and tests

2021-05-27 Thread John Snow
Try using avocado to manage our various tests; even though right now
they're only invoking shell scripts and not really running any
python-native code.

Create tests/, and add shell scripts which call out to mypy, flake8,
pylint and isort to enforce the standards in this directory.

Add avocado-framework to the setup.cfg development dependencies, and add
avocado.cfg to store some preferences for how we'd like the test output
to look.

Finally, add avocado-framework to the Pipfile environment and lock the
new dependencies. We are using avocado >= 87.0 here to take advantage of
some features that Cleber has helpfully added to make the test output
here *very* friendly and easy to read for developers that might chance
upon the output in Gitlab CI.

[Note: ALL of the dependencies get updated to the most modern versions
that exist at the time of this writing. No way around it that I have
seen. Not ideal, but so it goes.]

Provided you have the right development dependencies (mypy, flake8,
isort, pylint, and now avocado-framework) You should be able to run
"avocado --config avocado.cfg run tests/" from the python folder to run
all of these linters with the correct arguments.

(A forthcoming commit adds the much easier 'make check'.)

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
Tested-by: Cleber Rosa 
Message-id: 20210527211715.394144-28-js...@redhat.com
Signed-off-by: John Snow 
---
 python/README.rst  |  2 ++
 python/Pipfile.lock|  8 
 python/avocado.cfg | 10 ++
 python/setup.cfg   |  1 +
 python/tests/flake8.sh |  2 ++
 python/tests/isort.sh  |  2 ++
 python/tests/mypy.sh   |  2 ++
 python/tests/pylint.sh |  2 ++
 8 files changed, 29 insertions(+)
 create mode 100644 python/avocado.cfg
 create mode 100755 python/tests/flake8.sh
 create mode 100755 python/tests/isort.sh
 create mode 100755 python/tests/mypy.sh
 create mode 100755 python/tests/pylint.sh

diff --git a/python/README.rst b/python/README.rst
index 954870973d0..6bd2c6b3547 100644
--- a/python/README.rst
+++ b/python/README.rst
@@ -37,6 +37,8 @@ Files in this directory
 ---
 
 - ``qemu/`` Python package source directory.
+- ``tests/`` Python package tests directory.
+- ``avocado.cfg`` Configuration for the Avocado test-runner.
 - ``MANIFEST.in`` is read by python setuptools, it specifies additional files
   that should be included by a source distribution.
 - ``PACKAGE.rst`` is used as the README file that is visible on PyPI.org.
diff --git a/python/Pipfile.lock b/python/Pipfile.lock
index a2cdc1c50ea..6e344f5fadf 100644
--- a/python/Pipfile.lock
+++ b/python/Pipfile.lock
@@ -30,6 +30,14 @@
 "markers": "python_version ~= '3.6'",
 "version": "==2.5.6"
 },
+"avocado-framework": {
+"hashes": [
+
"sha256:42aa7962df98d6b78d4efd9afa2177226dc630f3d83a2a7d5baf7a0a7da7fa1b",
+
"sha256:d96ae343abf890e1ef3b3a6af5ce49e35f6bded0715770c4acb325bca555c515"
+],
+"markers": "python_version >= '3.6'",
+"version": "==88.1"
+},
 "flake8": {
 "hashes": [
 
"sha256:07528381786f2a6237b061f6e96610a4167b226cb926e2aa2b6b1d78057c576b",
diff --git a/python/avocado.cfg b/python/avocado.cfg
new file mode 100644
index 000..10dc6fb6054
--- /dev/null
+++ b/python/avocado.cfg
@@ -0,0 +1,10 @@
+[simpletests]
+# Don't show stdout/stderr in the test *summary*
+status.failure_fields = ['status']
+
+[job]
+# Don't show the full debug.log output; only select stdout/stderr.
+output.testlogs.logfiles = ['stdout', 'stderr']
+
+# Show full stdout/stderr only on tests that FAIL
+output.testlogs.statuses = ['FAIL']
diff --git a/python/setup.cfg b/python/setup.cfg
index 39dc135e601..fd325194901 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -25,6 +25,7 @@ packages =
 [options.extras_require]
 # Run `pipenv lock --dev` when changing these requirements.
 devel =
+avocado-framework >= 87.0
 flake8 >= 3.6.0
 isort >= 5.1.2
 mypy >= 0.770
diff --git a/python/tests/flake8.sh b/python/tests/flake8.sh
new file mode 100755
index 000..51e0788462b
--- /dev/null
+++ b/python/tests/flake8.sh
@@ -0,0 +1,2 @@
+#!/bin/sh -e
+python3 -m flake8
diff --git a/python/tests/isort.sh b/python/tests/isort.sh
new file mode 100755
index 000..4480405bfb0
--- /dev/null
+++ b/python/tests/isort.sh
@@ -0,0 +1,2 @@
+#!/bin/sh -e
+python3 -m isort -c qemu/
diff --git a/python/tests/mypy.sh b/python/tests/mypy.sh
new file mode 100755
index 000..5f980f563bb
--- /dev/null
+++ b/python/tests/mypy.sh
@@ -0,0 +1,2 @@
+#!/bin/sh -e
+python3 -m mypy -p qemu
diff --git a/python/tests/pylint.sh b/python/tests/pylint.sh
new file mode 100755
index 000..4b10b34db7c
--- /dev/null
+++ b/python/tests/pylint.sh
@@ -0,0 +1,2 @@
+#!/bin/sh -e
+python3 -m pylint qemu/
-- 
2.31.1




[PULL 37/44] python/qemu: add isort to pipenv

2021-05-27 Thread John Snow
isort 5.0.0 through 5.0.4 has a bug that causes it to misinterpret
certain "from ..." clauses that are not related to imports.

isort < 5.1.1 has a bug where it does not handle comments near import
statements correctly.

Require 5.1.2 or greater.

isort can be run (in "check" mode) with 'isort -c qemu' from the python
root. isort can also be used to fix/rewrite import order automatically
by using 'isort qemu'.

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
Message-id: 20210527211715.394144-25-js...@redhat.com
Signed-off-by: John Snow 
---
 python/Pipfile  | 1 +
 python/Pipfile.lock | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/python/Pipfile b/python/Pipfile
index 796c6282e17..79c74dd8db4 100644
--- a/python/Pipfile
+++ b/python/Pipfile
@@ -5,6 +5,7 @@ verify_ssl = true
 
 [dev-packages]
 flake8 = ">=3.6.0"
+isort = ">=5.1.2"
 mypy = ">=0.770"
 pylint = ">=2.8.0"
 
diff --git a/python/Pipfile.lock b/python/Pipfile.lock
index 626e68403f7..57a5befb104 100644
--- a/python/Pipfile.lock
+++ b/python/Pipfile.lock
@@ -1,7 +1,7 @@
 {
 "_meta": {
 "hash": {
-"sha256": 
"14d171b3d86759e1fdfb9e55f66be4a696b6afa8f627d6c4778f8398c6a66b98"
+"sha256": 
"8173290ad57aab0b722c9b4f109519de4e0dd7cd1bad1e16806b78bb169bce08"
 },
 "pipfile-spec": 6,
 "requires": {
@@ -46,7 +46,7 @@
 
"sha256:0a943902919f65c5684ac4e0154b1ad4fac6dcaa5d9f3426b732f1c8b5419be6",
 
"sha256:2bb1680aad211e3c9944dbce1d4ba09a989f04e238296c87fe2139faa26d655d"
 ],
-"markers": "python_version >= '3.6' and python_version < '4.0'",
+"index": "pypi",
 "version": "==5.8.0"
 },
 "lazy-object-proxy": {
-- 
2.31.1




[PULL 35/44] python: add mypy to pipenv

2021-05-27 Thread John Snow
0.730 appears to be about the oldest version that works with the
features we want, including nice human readable output (to make sure
iotest 297 passes), and type-parameterized Popen generics.

0.770, however, supports adding 'strict' to the config file, so require
at least 0.770.

Now that we are checking a namespace package, we need to tell mypy to
allow PEP420 namespaces, so modify the mypy config as part of the move.

mypy can now be run from the python root by typing 'mypy -p qemu'.

A note on mypy invocation: Running it as "mypy qemu/" changes the import
path detection mechanisms in mypy slightly, and it will fail. See
https://github.com/python/mypy/issues/8584 for a decent entry point with
more breadcrumbs on the various behaviors that contribute to this subtle
difference.

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
Tested-by: Cleber Rosa 
Message-id: 20210527211715.394144-23-js...@redhat.com
Signed-off-by: John Snow 
---
 python/Pipfile  |  1 +
 python/Pipfile.lock | 37 -
 python/setup.cfg|  1 +
 3 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/python/Pipfile b/python/Pipfile
index 053f344dcbe..796c6282e17 100644
--- a/python/Pipfile
+++ b/python/Pipfile
@@ -5,6 +5,7 @@ verify_ssl = true
 
 [dev-packages]
 flake8 = ">=3.6.0"
+mypy = ">=0.770"
 pylint = ">=2.8.0"
 
 [packages]
diff --git a/python/Pipfile.lock b/python/Pipfile.lock
index 5c34019060a..626e68403f7 100644
--- a/python/Pipfile.lock
+++ b/python/Pipfile.lock
@@ -1,7 +1,7 @@
 {
 "_meta": {
 "hash": {
-"sha256": 
"3c842ab9c72c40d24d146349aa144e00e4dec1c358c812cfa96489411f5b3f87"
+"sha256": 
"14d171b3d86759e1fdfb9e55f66be4a696b6afa8f627d6c4778f8398c6a66b98"
 },
 "pipfile-spec": 6,
 "requires": {
@@ -84,6 +84,41 @@
 ],
 "version": "==0.6.1"
 },
+"mypy": {
+"hashes": [
+
"sha256:0d0a87c0e7e3a9becdfbe936c981d32e5ee0ccda3e0f07e1ef2c3d1a817cf73e",
+
"sha256:25adde9b862f8f9aac9d2d11971f226bd4c8fbaa89fb76bdadb267ef22d10064",
+
"sha256:28fb5479c494b1bab244620685e2eb3c3f988d71fd5d64cc753195e8ed53df7c",
+
"sha256:2f9b3407c58347a452fc0736861593e105139b905cca7d097e413453a1d650b4",
+
"sha256:33f159443db0829d16f0a8d83d94df3109bb6dd801975fe86bacb9bf71628e97",
+
"sha256:3f2aca7f68580dc2508289c729bd49ee929a436208d2b2b6aab15745a70a57df",
+
"sha256:499c798053cdebcaa916eef8cd733e5584b5909f789de856b482cd7d069bdad8",
+
"sha256:4eec37370483331d13514c3f55f446fc5248d6373e7029a29ecb7b7494851e7a",
+
"sha256:552a815579aa1e995f39fd05dde6cd378e191b063f031f2acfe73ce9fb7f9e56",
+
"sha256:5873888fff1c7cf5b71efbe80e0e73153fe9212fafdf8e44adfe4c20ec9f82d7",
+
"sha256:61a3d5b97955422964be6b3baf05ff2ce7f26f52c85dd88db11d5e03e146a3a6",
+
"sha256:674e822aa665b9fd75130c6c5f5ed9564a38c6cea6a6432ce47eafb68ee578c5",
+
"sha256:7ce3175801d0ae5fdfa79b4f0cfed08807af4d075b402b7e294e6aa72af9aa2a",
+
"sha256:9743c91088d396c1a5a3c9978354b61b0382b4e3c440ce83cf77994a43e8c521",
+
"sha256:9f94aac67a2045ec719ffe6111df543bac7874cee01f41928f6969756e030564",
+
"sha256:a26f8ec704e5a7423c8824d425086705e381b4f1dfdef6e3a1edab7ba174ec49",
+
"sha256:abf7e0c3cf117c44d9285cc6128856106183938c68fd4944763003decdcfeb66",
+
"sha256:b09669bcda124e83708f34a94606e01b614fa71931d356c1f1a5297ba11f110a",
+
"sha256:cd07039aa5df222037005b08fbbfd69b3ab0b0bd7a07d7906de75ae52c4e3119",
+
"sha256:d23e0ea196702d918b60c8288561e722bf437d82cb7ef2edcd98cfa38905d506",
+
"sha256:d65cc1df038ef55a99e617431f0553cd77763869eebdf9042403e16089fe746c",
+
"sha256:d7da2e1d5f558c37d6e8c1246f1aec1e7349e4913d8fb3cb289a35de573fe2eb"
+],
+"index": "pypi",
+"version": "==0.812"
+},
+"mypy-extensions": {
+"hashes": [
+
"sha256:090fedd75945a69ae91ce1303b5824f428daf5a028d2f6ab8a299250a846f15d",
+
"sha256:2d82818f5bb3e369420cb3c4060a7970edba416647068eb4c5343488a6c604a8"
+],
+"version": "==0.4.3"
+},
 "pycodestyle": {
 "hashes": [
 
"sha256:514f76d918fcc0b55c6680472f0a37970994e07bbb80725808c17089be302068",
diff --git a/python/setup.cfg b/python/setup.cfg
index bd88b44ad80..b485d6161d5 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -31,6 +31,7 @@ exclude = __pycache__,
 strict = True
 python_version = 3.6
 warn_unused_configs = True
+namespace_packages = True
 
 [pylint.messages control]
 # Disable the message, report, category or checker with the given id(s). You
-- 
2.31.1




[PULL 34/44] python: move mypy.ini into setup.cfg

2021-05-27 Thread John Snow
mypy supports reading its configuration values from a central project
configuration file; do so.

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
Message-id: 20210527211715.394144-22-js...@redhat.com
Signed-off-by: John Snow 
---
 python/mypy.ini  | 4 
 python/setup.cfg | 5 +
 2 files changed, 5 insertions(+), 4 deletions(-)
 delete mode 100644 python/mypy.ini

diff --git a/python/mypy.ini b/python/mypy.ini
deleted file mode 100644
index 1a581c5f1ea..000
--- a/python/mypy.ini
+++ /dev/null
@@ -1,4 +0,0 @@
-[mypy]
-strict = True
-python_version = 3.6
-warn_unused_configs = True
diff --git a/python/setup.cfg b/python/setup.cfg
index 9aeab2bb0d3..bd88b44ad80 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -27,6 +27,11 @@ extend-ignore = E722  # Prefer pylint's bare-except checks 
to flake8's
 exclude = __pycache__,
   .venv,
 
+[mypy]
+strict = True
+python_version = 3.6
+warn_unused_configs = True
+
 [pylint.messages control]
 # Disable the message, report, category or checker with the given id(s). You
 # can either give multiple identifiers separated by comma (,) or put this
-- 
2.31.1




[PULL 27/44] python: Add pipenv support

2021-05-27 Thread John Snow
pipenv is a tool used for managing virtual environments with pinned,
explicit dependencies. It is used for precisely recreating python
virtual environments.

pipenv uses two files to do this:

(1) Pipfile, which is similar in purpose and scope to what setup.cfg
lists. It specifies the requisite minimum to get a functional
environment for using this package.

(2) Pipfile.lock, which is similar in purpose to `pip freeze >
requirements.txt`. It specifies a canonical virtual environment used for
deployment or testing. This ensures that all users have repeatable
results.

The primary benefit of using this tool is to ensure *rock solid*
repeatable CI results with a known set of packages. Although I endeavor
to support as many versions as I can, the fluid nature of the Python
toolchain often means tailoring code for fairly specific versions.

Note that pipenv is *not* required to install or use this module; this is
purely for the sake of repeatable testing by CI or developers.

Here, a "blank" pipfile is added with no dependencies, but specifies
Python 3.6 for the virtual environment.

Pipfile will specify our version minimums, while Pipfile.lock specifies
an exact loadout of packages that were known to operate correctly. This
latter file provides the real value for easy setup of container images
and CI environments.

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
Message-id: 20210527211715.394144-15-js...@redhat.com
Signed-off-by: John Snow 
---
 python/README.rst |  3 +++
 python/Pipfile| 11 +++
 2 files changed, 14 insertions(+)
 create mode 100644 python/Pipfile

diff --git a/python/README.rst b/python/README.rst
index 0099646ae2f..bf9bbca979a 100644
--- a/python/README.rst
+++ b/python/README.rst
@@ -36,6 +36,9 @@ Files in this directory
 - ``MANIFEST.in`` is read by python setuptools, it specifies additional files
   that should be included by a source distribution.
 - ``PACKAGE.rst`` is used as the README file that is visible on PyPI.org.
+- ``Pipfile`` is used by Pipenv to generate ``Pipfile.lock``.
+- ``Pipfile.lock`` is a set of pinned package dependencies that this package
+  is tested under in our CI suite. It is used by ``make venv-check``.
 - ``README.rst`` you are here!
 - ``VERSION`` contains the PEP-440 compliant version used to describe
   this package; it is referenced by ``setup.cfg``.
diff --git a/python/Pipfile b/python/Pipfile
new file mode 100644
index 000..9534830b5eb
--- /dev/null
+++ b/python/Pipfile
@@ -0,0 +1,11 @@
+[[source]]
+name = "pypi"
+url = "https://pypi.org/simple;
+verify_ssl = true
+
+[dev-packages]
+
+[packages]
+
+[requires]
+python_version = "3.6"
-- 
2.31.1




[PULL 30/44] python: add pylint to pipenv

2021-05-27 Thread John Snow
We are specifying >= pylint 2.8.x for several reasons:

1. For setup.cfg support, added in pylint 2.5.x
2. To specify a version that has incompatibly dropped
   bad-whitespace checks (2.6.x)
3. 2.7.x fixes "unsubscriptable" warnings in Python 3.9
4. 2.8.x adds a new, incompatible 'consider-using-with'
   warning that must be disabled in some cases.
   These pragmas cause warnings themselves in 2.7.x.

Signed-off-by: John Snow 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Cleber Rosa 
Tested-by: Cleber Rosa 
Message-id: 20210527211715.394144-18-js...@redhat.com
Signed-off-by: John Snow 
---
 python/Pipfile  |   1 +
 python/Pipfile.lock | 130 
 2 files changed, 131 insertions(+)
 create mode 100644 python/Pipfile.lock

diff --git a/python/Pipfile b/python/Pipfile
index 9534830b5eb..285e2c8e671 100644
--- a/python/Pipfile
+++ b/python/Pipfile
@@ -4,6 +4,7 @@ url = "https://pypi.org/simple;
 verify_ssl = true
 
 [dev-packages]
+pylint = ">=2.8.0"
 
 [packages]
 
diff --git a/python/Pipfile.lock b/python/Pipfile.lock
new file mode 100644
index 000..c9debd09503
--- /dev/null
+++ b/python/Pipfile.lock
@@ -0,0 +1,130 @@
+{
+"_meta": {
+"hash": {
+"sha256": 
"bd4fb76fcdd145bbf23c3a9dd7ad966113c5ce43ca51cc2d828aa7e73d572901"
+},
+"pipfile-spec": 6,
+"requires": {
+"python_version": "3.6"
+},
+"sources": [
+{
+"name": "pypi",
+"url": "https://pypi.org/simple;,
+"verify_ssl": true
+}
+]
+},
+"default": {},
+"develop": {
+"astroid": {
+"hashes": [
+
"sha256:4db03ab5fc3340cf619dbc25e42c2cc3755154ce6009469766d7143d1fc2ee4e",
+
"sha256:8a398dfce302c13f14bab13e2b14fe385d32b73f4e4853b9bdfb64598baa1975"
+],
+"markers": "python_version ~= '3.6'",
+"version": "==2.5.6"
+},
+"isort": {
+"hashes": [
+
"sha256:0a943902919f65c5684ac4e0154b1ad4fac6dcaa5d9f3426b732f1c8b5419be6",
+
"sha256:2bb1680aad211e3c9944dbce1d4ba09a989f04e238296c87fe2139faa26d655d"
+],
+"markers": "python_version >= '3.6' and python_version < '4.0'",
+"version": "==5.8.0"
+},
+"lazy-object-proxy": {
+"hashes": [
+
"sha256:17e0967ba374fc24141738c69736da90e94419338fd4c7c7bef01ee26b339653",
+
"sha256:1fee665d2638491f4d6e55bd483e15ef21f6c8c2095f235fef72601021e64f61",
+
"sha256:22ddd618cefe54305df49e4c069fa65715be4ad0e78e8d252a33debf00f6ede2",
+
"sha256:24a5045889cc2729033b3e604d496c2b6f588c754f7a62027ad4437a7ecc4837",
+
"sha256:410283732af311b51b837894fa2f24f2c0039aa7f220135192b38fcc42bd43d3",
+
"sha256:4732c765372bd78a2d6b2150a6e99d00a78ec963375f236979c0626b97ed8e43",
+
"sha256:489000d368377571c6f982fba6497f2aa13c6d1facc40660963da62f5c379726",
+
"sha256:4f60460e9f1eb632584c9685bccea152f4ac2130e299784dbaf9fae9f49891b3",
+
"sha256:5743a5ab42ae40caa8421b320ebf3a998f89c85cdc8376d6b2e00bd12bd1b587",
+
"sha256:85fb7608121fd5621cc4377a8961d0b32ccf84a7285b4f1d21988b2eae2868e8",
+
"sha256:9698110e36e2df951c7c36b6729e96429c9c32b3331989ef19976592c5f3c77a",
+
"sha256:9d397bf41caad3f489e10774667310d73cb9c4258e9aed94b9ec734b34b495fd",
+
"sha256:b579f8acbf2bdd9ea200b1d5dea36abd93cabf56cf626ab9c744a432e15c815f",
+
"sha256:b865b01a2e7f96db0c5d12cfea590f98d8c5ba64ad222300d93ce6ff9138bcad",
+
"sha256:bf34e368e8dd976423396555078def5cfc3039ebc6fc06d1ae2c5a65eebbcde4",
+
"sha256:c6938967f8528b3668622a9ed3b31d145fab161a32f5891ea7b84f6b790be05b",
+
"sha256:d1c2676e3d840852a2de7c7d5d76407c772927addff8d742b9808fe0afccebdf",
+
"sha256:d7124f52f3bd259f510651450e18e0fd081ed82f3c08541dffc7b94b883aa981",
+
"sha256:d900d949b707778696fdf01036f58c9876a0d8bfe116e8d220cfd4b15f14e741",
+
"sha256:ebfd274dcd5133e0afae738e6d9da4323c3eb021b3e13052d8cbd0e457b1256e",
+
"sha256:ed361bb83436f117f9917d282a456f9e5009ea12fd6de8742d1a4752c3017e93",
+
"sha256:f5144c75445ae3ca2057faac03fda5a902eff196702b0a24daf1d6ce0650514b"
+],
+"markers": "python_version >= '2.7' and python_version not in 
'3.0, 3.1, 3.2, 3.3, 3.4, 3.5'",
+"version": "==1.6.0"
+},
+"mccabe": {
+"hashes": [
+
"sha256:ab8a6258860da4b6677da4bd2fe5dc2c659cff31b3ee4f7f5d64e79735b80d42",
+
"sha256:dd8d182285a0fe56bace7f45b5e7d1a6ebcbf524e8f3bd87eb0f125271b8831f"
+],
+"version": "==0.6.1"
+},
+"pylint": {
+"hashes": [
+

[PULL 24/44] python: add VERSION file

2021-05-27 Thread John Snow
Python infrastructure as it exists today is not capable reliably of
single-sourcing a package version from a parent directory. The authors
of pip are working to correct this, but as of today this is not possible.

The problem is that when using pip to build and install a python
package, it copies files over to a temporary directory and performs its
build there. This loses access to any information in the parent
directory, including git itself.

Further, Python versions have a standard (PEP 440) that may or may not
follow QEMU's versioning. In general, it does; but naturally QEMU does
not follow PEP 440. To avoid any automatically-generated conflict, a
manual version file is preferred.

I am proposing:

- Python tooling follows the QEMU version, indirectly, but with a major
  version of 0 to indicate that the API is not expected to be
  stable. This would mean version 0.5.2.0, 0.5.1.1, 0.5.3.0, etc.

- In the event that a Python package needs to be updated independently
  of the QEMU version, a pre-release alpha version should be preferred,
  but *only* after inclusion to the qemu development or stable branches.

  e.g. 0.5.2.0a1, 0.5.2.0a2, and so on should be preferred prior to
  5.2.0's release.

- The Python core tooling makes absolutely no version compatibility
  checks or constraints. It *may* work with releases of QEMU from the
  past or future, but it is not required to.

  i.e., "qemu.machine" will, for now, remain in lock-step with QEMU.

- We reserve the right to split the qemu package into independently
  versioned subpackages at a later date. This might allow for us to
  begin versioning QMP independently from QEMU at a later date, if
  we so choose.

Implement this versioning scheme by adding a VERSION file and setting it
to 0.6.0.0a1.

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
Message-id: 20210527211715.394144-12-js...@redhat.com
Signed-off-by: John Snow 
---
 python/VERSION   | 1 +
 python/setup.cfg | 1 +
 2 files changed, 2 insertions(+)
 create mode 100644 python/VERSION

diff --git a/python/VERSION b/python/VERSION
new file mode 100644
index 000..c19f3b832b7
--- /dev/null
+++ b/python/VERSION
@@ -0,0 +1 @@
+0.6.1.0a1
diff --git a/python/setup.cfg b/python/setup.cfg
index 3fa92a2e73f..b0010e0188f 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -1,5 +1,6 @@
 [metadata]
 name = qemu
+version = file:VERSION
 maintainer = QEMU Developer Team
 maintainer_email = qemu-devel@nongnu.org
 url = https://www.qemu.org/
-- 
2.31.1




[PULL 31/44] python: move flake8 config to setup.cfg

2021-05-27 Thread John Snow
Update the comment concerning the flake8 exception to match commit
42c0dd12, whose commit message stated:

A note on the flake8 exception: flake8 will warn on *any* bare except,
but pylint's is context-aware and will suppress the warning if you
re-raise the exception.

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
Message-id: 20210527211715.394144-19-js...@redhat.com
Signed-off-by: John Snow 
---
 python/qemu/machine/.flake8 | 2 --
 python/setup.cfg| 3 +++
 2 files changed, 3 insertions(+), 2 deletions(-)
 delete mode 100644 python/qemu/machine/.flake8

diff --git a/python/qemu/machine/.flake8 b/python/qemu/machine/.flake8
deleted file mode 100644
index 45d8146f3f5..000
--- a/python/qemu/machine/.flake8
+++ /dev/null
@@ -1,2 +0,0 @@
-[flake8]
-extend-ignore = E722  # Pylint handles this, but smarter.
\ No newline at end of file
diff --git a/python/setup.cfg b/python/setup.cfg
index 36b4253e939..52a89a0a290 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -22,6 +22,9 @@ packages =
 qemu.machine
 qemu.utils
 
+[flake8]
+extend-ignore = E722  # Prefer pylint's bare-except checks to flake8's
+
 [pylint.messages control]
 # Disable the message, report, category or checker with the given id(s). You
 # can either give multiple identifiers separated by comma (,) or put this
-- 
2.31.1




[PULL 25/44] python: add directory structure README.rst files

2021-05-27 Thread John Snow
Add short readmes to python/, python/qemu/, python/qemu/machine,
python/qemu/qmp, and python/qemu/utils that explain the directory
hierarchy. These readmes are visible when browsing the source on
e.g. gitlab/github and are designed to help new developers/users quickly
make sense of the source tree.

They are not designed for inclusion in a published manual.

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
Message-id: 20210527211715.394144-13-js...@redhat.com
Signed-off-by: John Snow 
---
 python/README.rst  | 41 ++
 python/qemu/README.rst |  8 +++
 python/qemu/machine/README.rst |  9 
 python/qemu/qmp/README.rst |  9 
 python/qemu/utils/README.rst   |  7 ++
 5 files changed, 74 insertions(+)
 create mode 100644 python/README.rst
 create mode 100644 python/qemu/README.rst
 create mode 100644 python/qemu/machine/README.rst
 create mode 100644 python/qemu/qmp/README.rst
 create mode 100644 python/qemu/utils/README.rst

diff --git a/python/README.rst b/python/README.rst
new file mode 100644
index 000..38b0c83f321
--- /dev/null
+++ b/python/README.rst
@@ -0,0 +1,41 @@
+QEMU Python Tooling
+===
+
+This directory houses Python tooling used by the QEMU project to build,
+configure, and test QEMU. It is organized by namespace (``qemu``), and
+then by package (e.g. ``qemu/machine``, ``qemu/qmp``, etc).
+
+``setup.py`` is used by ``pip`` to install this tooling to the current
+environment. ``setup.cfg`` provides the packaging configuration used by
+``setup.py`` in a setuptools specific format. You will generally invoke
+it by doing one of the following:
+
+1. ``pip3 install .`` will install these packages to your current
+   environment. If you are inside a virtual environment, they will
+   install there. If you are not, it will attempt to install to the
+   global environment, which is **not recommended**.
+
+2. ``pip3 install --user .`` will install these packages to your user's
+   local python packages. If you are inside of a virtual environment,
+   this will fail; you likely want the first invocation above.
+
+If you append the ``-e`` argument, pip will install in "editable" mode;
+which installs a version of the package that installs a forwarder
+pointing to these files, such that the package always reflects the
+latest version in your git tree.
+
+See `Installing packages using pip and virtual environments
+`_
+for more information.
+
+
+Files in this directory
+---
+
+- ``qemu/`` Python package source directory.
+- ``PACKAGE.rst`` is used as the README file that is visible on PyPI.org.
+- ``README.rst`` you are here!
+- ``VERSION`` contains the PEP-440 compliant version used to describe
+  this package; it is referenced by ``setup.cfg``.
+- ``setup.cfg`` houses setuptools package configuration.
+- ``setup.py`` is the setuptools installer used by pip; See above.
diff --git a/python/qemu/README.rst b/python/qemu/README.rst
new file mode 100644
index 000..d04943f526c
--- /dev/null
+++ b/python/qemu/README.rst
@@ -0,0 +1,8 @@
+QEMU Python Namespace
+=
+
+This directory serves as the root of a `Python PEP 420 implicit
+namespace package `_.
+
+Each directory below is assumed to be an installable Python package that
+is available under the ``qemu.`` namespace.
diff --git a/python/qemu/machine/README.rst b/python/qemu/machine/README.rst
new file mode 100644
index 000..ac2b4fffb42
--- /dev/null
+++ b/python/qemu/machine/README.rst
@@ -0,0 +1,9 @@
+qemu.machine package
+
+
+This package provides core utilities used for testing and debugging
+QEMU. It is used by the iotests, vm tests, acceptance tests, and several
+other utilities in the ./scripts directory. It is not a fully-fledged
+SDK and it is subject to change at any time.
+
+See the documentation in ``__init__.py`` for more information.
diff --git a/python/qemu/qmp/README.rst b/python/qemu/qmp/README.rst
new file mode 100644
index 000..c21951491cf
--- /dev/null
+++ b/python/qemu/qmp/README.rst
@@ -0,0 +1,9 @@
+qemu.qmp package
+
+
+This package provides a library used for connecting to and communicating
+with QMP servers. It is used extensively by iotests, vm tests,
+acceptance tests, and other utilities in the ./scripts directory. It is
+not a fully-fledged SDK and is subject to change at any time.
+
+See the documentation in ``__init__.py`` for more information.
diff --git a/python/qemu/utils/README.rst b/python/qemu/utils/README.rst
new file mode 100644
index 000..975fbf4d7de
--- /dev/null
+++ b/python/qemu/utils/README.rst
@@ -0,0 +1,7 @@
+qemu.utils package
+==
+
+This package provides miscellaneous utilities used for testing and
+debugging QEMU. It is used primarily by the vm and acceptance tests.
+

[PULL 29/44] python: move pylintrc into setup.cfg

2021-05-27 Thread John Snow
Delete the empty settings now that it's sharing a home with settings for
other tools.

pylint can now be run from this folder as "pylint qemu".

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
Tested-by: Cleber Rosa 
Message-id: 20210527211715.394144-17-js...@redhat.com
Signed-off-by: John Snow 
---
 python/qemu/machine/pylintrc | 58 
 python/setup.cfg | 29 ++
 2 files changed, 29 insertions(+), 58 deletions(-)
 delete mode 100644 python/qemu/machine/pylintrc

diff --git a/python/qemu/machine/pylintrc b/python/qemu/machine/pylintrc
deleted file mode 100644
index 3f69205000d..000
--- a/python/qemu/machine/pylintrc
+++ /dev/null
@@ -1,58 +0,0 @@
-[MASTER]
-
-[MESSAGES CONTROL]
-
-# Disable the message, report, category or checker with the given id(s). You
-# can either give multiple identifiers separated by comma (,) or put this
-# option multiple times (only on the command line, not in the configuration
-# file where it should appear only once). You can also use "--disable=all" to
-# disable everything first and then reenable specific checks. For example, if
-# you want to run only the similarities checker, you can use "--disable=all
-# --enable=similarities". If you want to run only the classes checker, but have
-# no Warning level messages displayed, use "--disable=all --enable=classes
-# --disable=W".
-disable=too-many-arguments,
-too-many-instance-attributes,
-too-many-public-methods,
-
-[REPORTS]
-
-[REFACTORING]
-
-[MISCELLANEOUS]
-
-[LOGGING]
-
-[BASIC]
-
-# Good variable names which should always be accepted, separated by a comma.
-good-names=i,
-   j,
-   k,
-   ex,
-   Run,
-   _,
-   fd,
-   c,
-[VARIABLES]
-
-[STRING]
-
-[SPELLING]
-
-[FORMAT]
-
-[SIMILARITIES]
-
-# Ignore imports when computing similarities.
-ignore-imports=yes
-
-[TYPECHECK]
-
-[CLASSES]
-
-[IMPORTS]
-
-[DESIGN]
-
-[EXCEPTIONS]
diff --git a/python/setup.cfg b/python/setup.cfg
index b0010e0188f..36b4253e939 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -21,3 +21,32 @@ packages =
 qemu.qmp
 qemu.machine
 qemu.utils
+
+[pylint.messages control]
+# Disable the message, report, category or checker with the given id(s). You
+# can either give multiple identifiers separated by comma (,) or put this
+# option multiple times (only on the command line, not in the configuration
+# file where it should appear only once). You can also use "--disable=all" to
+# disable everything first and then reenable specific checks. For example, if
+# you want to run only the similarities checker, you can use "--disable=all
+# --enable=similarities". If you want to run only the classes checker, but have
+# no Warning level messages displayed, use "--disable=all --enable=classes
+# --disable=W".
+disable=too-many-arguments,
+too-many-instance-attributes,
+too-many-public-methods,
+
+[pylint.basic]
+# Good variable names which should always be accepted, separated by a comma.
+good-names=i,
+   j,
+   k,
+   ex,
+   Run,
+   _,
+   fd,
+   c,
+
+[pylint.similarities]
+# Ignore imports when computing similarities.
+ignore-imports=yes
-- 
2.31.1




[PULL 26/44] python: add MANIFEST.in

2021-05-27 Thread John Snow
When creating a source or binary distribution via 'python3 setup.py
', the VERSION and PACKAGE.rst files aren't bundled by
default. Create a MANIFEST.in file that instructs the build tools to
include these so that installation from these files won't fail.

This is required by 'tox', as well as by the tooling needed to upload
packages to PyPI.

Exclude the 'README.rst' file -- that's intended as a guidebook to our
source tree, not a file that needs to be distributed.

Signed-off-by: John Snow 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Cleber Rosa 
Message-id: 20210527211715.394144-14-js...@redhat.com
Signed-off-by: John Snow 
---
 python/README.rst  | 2 ++
 python/MANIFEST.in | 3 +++
 2 files changed, 5 insertions(+)
 create mode 100644 python/MANIFEST.in

diff --git a/python/README.rst b/python/README.rst
index 38b0c83f321..0099646ae2f 100644
--- a/python/README.rst
+++ b/python/README.rst
@@ -33,6 +33,8 @@ Files in this directory
 ---
 
 - ``qemu/`` Python package source directory.
+- ``MANIFEST.in`` is read by python setuptools, it specifies additional files
+  that should be included by a source distribution.
 - ``PACKAGE.rst`` is used as the README file that is visible on PyPI.org.
 - ``README.rst`` you are here!
 - ``VERSION`` contains the PEP-440 compliant version used to describe
diff --git a/python/MANIFEST.in b/python/MANIFEST.in
new file mode 100644
index 000..7059ad28221
--- /dev/null
+++ b/python/MANIFEST.in
@@ -0,0 +1,3 @@
+include VERSION
+include PACKAGE.rst
+exclude README.rst
-- 
2.31.1




[PULL 22/44] python: create qemu packages

2021-05-27 Thread John Snow
move python/qemu/*.py to python/qemu/[machine, qmp, utils]/*.py and
update import directives across the tree.

This is done to create a PEP420 namespace package, in which we may
create subpackages. To do this, the namespace directory ("qemu") should
not have any modules in it. Those files will go into new 'machine',
'qmp' and 'utils' subpackages instead.

Implement machine/__init__.py making the top-level classes and functions
from its various modules available directly inside the package. Change
qmp.py to qmp/__init__.py similarly, such that all of the useful QMP
library classes are available directly from "qemu.qmp" instead of
"qemu.qmp.qmp".

Signed-off-by: John Snow 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Cleber Rosa 
Message-id: 20210527211715.394144-10-js...@redhat.com
Signed-off-by: John Snow 
---
 python/{qemu => }/.isort.cfg|  0
 python/qemu/__init__.py | 11 ---
 python/qemu/{ => machine}/.flake8   |  0
 python/qemu/machine/__init__.py | 33 +
 python/qemu/{ => machine}/console_socket.py |  0
 python/qemu/{ => machine}/machine.py| 16 ++
 python/qemu/{ => machine}/pylintrc  |  0
 python/qemu/{ => machine}/qtest.py  |  3 +-
 python/qemu/{qmp.py => qmp/__init__.py} | 12 +++-
 python/qemu/{utils.py => utils/__init__.py} | 18 +--
 python/qemu/{ => utils}/accel.py|  0
 tests/acceptance/avocado_qemu/__init__.py   |  9 +++---
 tests/acceptance/virtio-gpu.py  |  2 +-
 tests/qemu-iotests/300  |  4 +--
 tests/qemu-iotests/iotests.py   |  2 +-
 tests/vm/aarch64vm.py   |  2 +-
 tests/vm/basevm.py  |  3 +-
 17 files changed, 83 insertions(+), 32 deletions(-)
 rename python/{qemu => }/.isort.cfg (100%)
 delete mode 100644 python/qemu/__init__.py
 rename python/qemu/{ => machine}/.flake8 (100%)
 create mode 100644 python/qemu/machine/__init__.py
 rename python/qemu/{ => machine}/console_socket.py (100%)
 rename python/qemu/{ => machine}/machine.py (98%)
 rename python/qemu/{ => machine}/pylintrc (100%)
 rename python/qemu/{ => machine}/qtest.py (99%)
 rename python/qemu/{qmp.py => qmp/__init__.py} (96%)
 rename python/qemu/{utils.py => utils/__init__.py} (66%)
 rename python/qemu/{ => utils}/accel.py (100%)

diff --git a/python/qemu/.isort.cfg b/python/.isort.cfg
similarity index 100%
rename from python/qemu/.isort.cfg
rename to python/.isort.cfg
diff --git a/python/qemu/__init__.py b/python/qemu/__init__.py
deleted file mode 100644
index 4ca06c34a41..000
--- a/python/qemu/__init__.py
+++ /dev/null
@@ -1,11 +0,0 @@
-# QEMU library
-#
-# Copyright (C) 2015-2016 Red Hat Inc.
-# Copyright (C) 2012 IBM Corp.
-#
-# Authors:
-#  Fam Zheng 
-#
-# This work is licensed under the terms of the GNU GPL, version 2.  See
-# the COPYING file in the top-level directory.
-#
diff --git a/python/qemu/.flake8 b/python/qemu/machine/.flake8
similarity index 100%
rename from python/qemu/.flake8
rename to python/qemu/machine/.flake8
diff --git a/python/qemu/machine/__init__.py b/python/qemu/machine/__init__.py
new file mode 100644
index 000..98302ea31e7
--- /dev/null
+++ b/python/qemu/machine/__init__.py
@@ -0,0 +1,33 @@
+"""
+QEMU development and testing library.
+
+This library provides a few high-level classes for driving QEMU from a
+test suite, not intended for production use.
+
+- QEMUMachine: Configure and Boot a QEMU VM
+ - QEMUQtestMachine: VM class, with a qtest socket.
+
+- QEMUQtestProtocol: Connect to, send/receive qtest messages.
+"""
+
+# Copyright (C) 2020-2021 John Snow for Red Hat Inc.
+# Copyright (C) 2015-2016 Red Hat Inc.
+# Copyright (C) 2012 IBM Corp.
+#
+# Authors:
+#  John Snow 
+#  Fam Zheng 
+#
+# This work is licensed under the terms of the GNU GPL, version 2.  See
+# the COPYING file in the top-level directory.
+#
+
+from .machine import QEMUMachine
+from .qtest import QEMUQtestMachine, QEMUQtestProtocol
+
+
+__all__ = (
+'QEMUMachine',
+'QEMUQtestProtocol',
+'QEMUQtestMachine',
+)
diff --git a/python/qemu/console_socket.py 
b/python/qemu/machine/console_socket.py
similarity index 100%
rename from python/qemu/console_socket.py
rename to python/qemu/machine/console_socket.py
diff --git a/python/qemu/machine.py b/python/qemu/machine/machine.py
similarity index 98%
rename from python/qemu/machine.py
rename to python/qemu/machine/machine.py
index a8837b36e47..d33b02d2ce6 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine/machine.py
@@ -38,8 +38,14 @@
 Type,
 )
 
-from . import console_socket, qmp
-from .qmp import QMPMessage, QMPReturnValue, SocketAddrT
+from qemu.qmp import (
+QEMUMonitorProtocol,
+QMPMessage,
+QMPReturnValue,
+SocketAddrT,
+)
+
+from . import console_socket
 
 
 LOG = logging.getLogger(__name__)
@@ -139,7 +145,7 @@ def __init__(self,
 self._events: List[QMPMessage] = []
 

[PULL 43/44] python: add tox support

2021-05-27 Thread John Snow
This is intended to be a manually run, non-CI script.

Use tox to test the linters against all python versions from 3.6 to
3.10. This will only work if you actually have those versions installed
locally, but Fedora makes this easy:

> sudo dnf install python3.6 python3.7 python3.8 python3.9 python3.10

Unlike the pipenv tests (make venv-check), this pulls "whichever"
versions of the python packages, so they are unpinned and may break as
time goes on. In the case that breakages are found, setup.cfg should be
amended accordingly to avoid the bad dependant versions, or the code
should be amended to work around the issue.

With confidence that the tests pass on 3.6 through 3.10 inclusive, add
the appropriate classifiers to setup.cfg to indicate which versions we
claim to support.

Tox 3.18.0 or above is required to use the 'allowlist_externals' option.

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
Tested-by: Cleber Rosa 
Message-id: 20210527211715.394144-31-js...@redhat.com
Signed-off-by: John Snow 
---
 python/.gitignore |  1 +
 python/Makefile   |  7 ++-
 python/setup.cfg  | 23 ++-
 3 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/python/.gitignore b/python/.gitignore
index 4ed144ceac3..272ed223a84 100644
--- a/python/.gitignore
+++ b/python/.gitignore
@@ -13,3 +13,4 @@ qemu.egg-info/
 
 # virtual environments (pipenv et al)
 .venv/
+.tox/
diff --git a/python/Makefile b/python/Makefile
index a9da1689558..b5621b0d540 100644
--- a/python/Makefile
+++ b/python/Makefile
@@ -16,6 +16,8 @@ help:
@echo ""
@echo "make check:  run linters using the current environment."
@echo ""
+   @echo "make check-tox:  run linters using multiple python versions."
+   @echo ""
@echo "make clean:  remove package build output."
@echo ""
@echo "make distclean:  remove venv files, qemu package forwarder,"
@@ -36,8 +38,11 @@ develop:
 check:
@avocado --config avocado.cfg run tests/
 
+check-tox:
+   @tox
+
 clean:
python3 setup.py clean --all
 
 distclean: clean
-   rm -rf qemu.egg-info/ .venv/ dist/
+   rm -rf qemu.egg-info/ .venv/ .tox/ dist/
diff --git a/python/setup.cfg b/python/setup.cfg
index fd325194901..0fcdec6f322 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -14,6 +14,11 @@ classifiers =
 Natural Language :: English
 Operating System :: OS Independent
 Programming Language :: Python :: 3 :: Only
+Programming Language :: Python :: 3.6
+Programming Language :: Python :: 3.7
+Programming Language :: Python :: 3.8
+Programming Language :: Python :: 3.9
+Programming Language :: Python :: 3.10
 
 [options]
 python_requires = >= 3.6
@@ -30,12 +35,13 @@ devel =
 isort >= 5.1.2
 mypy >= 0.770
 pylint >= 2.8.0
-
+tox >= 3.18.0
 
 [flake8]
 extend-ignore = E722  # Prefer pylint's bare-except checks to flake8's
 exclude = __pycache__,
   .venv,
+  .tox,
 
 [mypy]
 strict = True
@@ -79,3 +85,18 @@ include_trailing_comma=True
 line_length=72
 lines_after_imports=2
 multi_line_output=3
+
+# tox (https://tox.readthedocs.io/) is a tool for running tests in
+# multiple virtualenvs. This configuration file will run the test suite
+# on all supported python versions. To use it, "pip install tox" and
+# then run "tox" from this directory. You will need all of these versions
+# of python available on your system to run this test.
+
+[tox:tox]
+envlist = py36, py37, py38, py39, py310
+
+[testenv]
+allowlist_externals = make
+deps = .[devel]
+commands =
+make check
-- 
2.31.1




[PULL 19/44] python/machine: disable warning for Popen in _launch()

2021-05-27 Thread John Snow
We handle this resource rather meticulously in
shutdown/kill/wait/__exit__ et al, through the laborious mechanisms in
_do_shutdown().

Quiet this pylint warning here.

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
Message-id: 20210527211715.394144-7-js...@redhat.com
Message-id: 20210517184808.3562549-7-js...@redhat.com
Signed-off-by: John Snow 
---
 python/qemu/machine.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index c66bc6a9c69..5d72c4ca369 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -405,6 +405,9 @@ def _launch(self) -> None:
   self._args)
 )
 LOG.debug('VM launch command: %r', ' '.join(self._qemu_full_args))
+
+# Cleaning up of this subprocess is guaranteed by _do_shutdown.
+# pylint: disable=consider-using-with
 self._popen = subprocess.Popen(self._qemu_full_args,
stdin=subprocess.DEVNULL,
stdout=self._qemu_log_file,
-- 
2.31.1




[PULL 23/44] python: add qemu package installer

2021-05-27 Thread John Snow
Add setup.cfg and setup.py, necessary for installing a package via
pip. Add a ReST document (PACKAGE.rst) explaining the basics of what
this package is for and who to contact for more information. This
document will be used as the landing page for the package on PyPI.

List the subpackages we intend to package by name instead of using
find_namespace because find_namespace will naively also packages tests,
things it finds in the dist/ folder, etc. I could not figure out how to
modify this behavior; adding allow/deny lists to setuptools kept
changing the packaged hierarchy. This works, roll with it.

I am not yet using a pyproject.toml style package manifest, because
"editable" installs are not defined (yet?) by PEP-517/518.

I consider editable installs crucial for development, though they have
(apparently) always been somewhat poorly defined.

Pip now (19.2 and later) now supports editable installs for projects
using pyproject.toml manifests, but might require the use of the
--no-use-pep517 flag, which somewhat defeats the point. Full support for
setup.py-less editable installs was not introduced until pip 21.1.1:
https://github.com/pypa/pip/pull/9547/commits/7a95720e796a5e56481c1cc20b6ce6249c50f357

For now, while the dust settles, stick with the de-facto
setup.py/setup.cfg combination supported by setuptools. It will be worth
re-evaluating this point again in the future when our supported build
platforms all ship a fairly modern pip.

Additional reading on this matter:

https://github.com/pypa/packaging-problems/issues/256
https://github.com/pypa/pip/issues/6334
https://github.com/pypa/pip/issues/6375
https://github.com/pypa/pip/issues/6434
https://github.com/pypa/pip/issues/6438

Signed-off-by: John Snow 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Cleber Rosa 
Message-id: 20210527211715.394144-11-js...@redhat.com
Signed-off-by: John Snow 
---
 python/PACKAGE.rst | 33 +
 python/setup.cfg   | 22 ++
 python/setup.py| 23 +++
 3 files changed, 78 insertions(+)
 create mode 100644 python/PACKAGE.rst
 create mode 100644 python/setup.cfg
 create mode 100755 python/setup.py

diff --git a/python/PACKAGE.rst b/python/PACKAGE.rst
new file mode 100644
index 000..1bbfe1b58e2
--- /dev/null
+++ b/python/PACKAGE.rst
@@ -0,0 +1,33 @@
+QEMU Python Tooling
+===
+
+This package provides QEMU tooling used by the QEMU project to build,
+configure, and test QEMU. It is not a fully-fledged SDK and it is subject
+to change at any time.
+
+Usage
+-
+
+The ``qemu.qmp`` subpackage provides a library for communicating with
+QMP servers. The ``qemu.machine`` subpackage offers rudimentary
+facilities for launching and managing QEMU processes. Refer to each
+package's documentation
+(``>>> help(qemu.qmp)``, ``>>> help(qemu.machine)``)
+for more information.
+
+Contributing
+
+
+This package is maintained by John Snow  as part of
+the QEMU source tree. Contributions are welcome and follow the `QEMU
+patch submission process
+`_, which involves
+sending patches to the QEMU development mailing list.
+
+John maintains a `GitLab staging branch
+`_, and there is an
+official `GitLab mirror `_.
+
+Please report bugs on the `QEMU issue tracker
+`_ and tag ``@jsnow`` in
+the report.
diff --git a/python/setup.cfg b/python/setup.cfg
new file mode 100644
index 000..3fa92a2e73f
--- /dev/null
+++ b/python/setup.cfg
@@ -0,0 +1,22 @@
+[metadata]
+name = qemu
+maintainer = QEMU Developer Team
+maintainer_email = qemu-devel@nongnu.org
+url = https://www.qemu.org/
+download_url = https://www.qemu.org/download/
+description = QEMU Python Build, Debug and SDK tooling.
+long_description = file:PACKAGE.rst
+long_description_content_type = text/x-rst
+classifiers =
+Development Status :: 3 - Alpha
+License :: OSI Approved :: GNU General Public License v2 (GPLv2)
+Natural Language :: English
+Operating System :: OS Independent
+Programming Language :: Python :: 3 :: Only
+
+[options]
+python_requires = >= 3.6
+packages =
+qemu.qmp
+qemu.machine
+qemu.utils
diff --git a/python/setup.py b/python/setup.py
new file mode 100755
index 000..2014f81b757
--- /dev/null
+++ b/python/setup.py
@@ -0,0 +1,23 @@
+#!/usr/bin/env python3
+"""
+QEMU tooling installer script
+Copyright (c) 2020-2021 John Snow for Red Hat, Inc.
+"""
+
+import setuptools
+import pkg_resources
+
+
+def main():
+"""
+QEMU tooling installer
+"""
+
+# 
https://medium.com/@daveshawley/safely-using-setup-cfg-for-metadata-1babbe54c108
+pkg_resources.require('setuptools>=39.2')
+
+setuptools.setup()
+
+
+if __name__ == '__main__':
+main()
-- 
2.31.1




[PULL 20/44] python/machine: Trim line length to below 80 chars

2021-05-27 Thread John Snow
One more little delinting fix that snuck in.

Signed-off-by: John Snow 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Cleber Rosa 
Message-id: 20210527211715.394144-8-js...@redhat.com
Signed-off-by: John Snow 
---
 python/qemu/machine.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index 5d72c4ca369..a8837b36e47 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -97,7 +97,7 @@ def __init__(self,
 @param args: list of extra arguments
 @param wrapper: list of arguments used as prefix to qemu binary
 @param name: prefix for socket and log file names (default: qemu-PID)
-@param base_temp_dir: default location where temporary files are 
created
+@param base_temp_dir: default location where temp files are created
 @param monitor_address: address for QMP monitor
 @param socket_scm_helper: helper program, required for send_fd_scm()
 @param sock_dir: where to create socket (defaults to base_temp_dir)
-- 
2.31.1




[PULL 41/44] python: add Makefile for some common tasks

2021-05-27 Thread John Snow
Add "make venv" to create the pipenv-managed virtual environment that
contains our explicitly pinned dependencies.

Add "make check" to run the python linters [in the host execution
environment].

Add "make venv-check" which combines the above two: create/update the
venv, then run the linters in that explicitly managed environment.

Add "make develop" which canonizes the runes needed to get both the
linting pre-requisites (the "[devel]" part), and the editable
live-install (the "-e" part) of these python libraries.

make clean: delete miscellaneous python packaging output possibly
created by pipenv, pip, or other python packaging utilities

make distclean: delete the above, the .venv, and the editable "qemu"
package forwarder (qemu.egg-info) if there is one.

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
Tested-by: Cleber Rosa 
Message-id: 20210527211715.394144-29-js...@redhat.com
Signed-off-by: John Snow 
---
 python/PACKAGE.rst |  6 ++
 python/README.rst  |  6 ++
 python/Makefile| 43 +++
 3 files changed, 55 insertions(+)
 create mode 100644 python/Makefile

diff --git a/python/PACKAGE.rst b/python/PACKAGE.rst
index 05ea7789fc1..b0b86cc4c31 100644
--- a/python/PACKAGE.rst
+++ b/python/PACKAGE.rst
@@ -35,3 +35,9 @@ the report.
 Optional packages necessary for running code quality analysis for this
 package can be installed with the optional dependency group "devel":
 ``pip install qemu[devel]``.
+
+``make develop`` can be used to install this package in editable mode
+(to the current environment) *and* bring in testing dependencies in one
+command.
+
+``make check`` can be used to run the available tests.
diff --git a/python/README.rst b/python/README.rst
index 6bd2c6b3547..dcf993819db 100644
--- a/python/README.rst
+++ b/python/README.rst
@@ -28,6 +28,9 @@ Installing ".[devel]" instead of "." will additionally pull 
in required
 packages for testing this package. They are not runtime requirements,
 and are not needed to simply use these libraries.
 
+Running ``make develop`` will pull in all testing dependencies and
+install QEMU in editable mode to the current environment.
+
 See `Installing packages using pip and virtual environments
 
`_
 for more information.
@@ -39,6 +42,9 @@ Files in this directory
 - ``qemu/`` Python package source directory.
 - ``tests/`` Python package tests directory.
 - ``avocado.cfg`` Configuration for the Avocado test-runner.
+  Used by ``make check`` et al.
+- ``Makefile`` provides some common testing/installation invocations.
+  Try ``make help`` to see available targets.
 - ``MANIFEST.in`` is read by python setuptools, it specifies additional files
   that should be included by a source distribution.
 - ``PACKAGE.rst`` is used as the README file that is visible on PyPI.org.
diff --git a/python/Makefile b/python/Makefile
new file mode 100644
index 000..a9da1689558
--- /dev/null
+++ b/python/Makefile
@@ -0,0 +1,43 @@
+.PHONY: help venv venv-check check clean distclean develop
+
+help:
+   @echo "python packaging help:"
+   @echo ""
+   @echo "make venv:   Create pipenv's virtual environment."
+   @echo "NOTE: Requires Python 3.6 and pipenv."
+   @echo "  Will download packages from PyPI."
+   @echo "Hint: (On Fedora): 'sudo dnf install python36 pipenv'"
+   @echo ""
+   @echo "make venv-check: run linters using pipenv's virtual environment."
+   @echo "Hint: If you don't know which test to run, run this one!"
+   @echo ""
+   @echo "make develop:Install deps for 'make check', and"
+   @echo " the qemu libs in editable/development mode."
+   @echo ""
+   @echo "make check:  run linters using the current environment."
+   @echo ""
+   @echo "make clean:  remove package build output."
+   @echo ""
+   @echo "make distclean:  remove venv files, qemu package forwarder,"
+   @echo " built distribution files, and everything"
+   @echo " from 'make clean'."
+
+venv: .venv
+.venv: Pipfile.lock
+   @PIPENV_VENV_IN_PROJECT=1 pipenv sync --dev --keep-outdated
+   @touch .venv
+
+venv-check: venv
+   @pipenv run make check
+
+develop:
+   pip3 install -e .[devel]
+
+check:
+   @avocado --config avocado.cfg run tests/
+
+clean:
+   python3 setup.py clean --all
+
+distclean: clean
+   rm -rf qemu.egg-info/ .venv/ dist/
-- 
2.31.1




[PULL 21/44] iotests/297: add --namespace-packages to mypy arguments

2021-05-27 Thread John Snow
mypy is kind of weird about how it handles imports. For legacy reasons,
it won't load PEP 420 namespaces, because of logic implemented prior to
that becoming a standard.

So, if you plan on using any, you have to pass
--namespace-packages. Alright, fine.

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
Message-id: 20210527211715.394144-9-js...@redhat.com
Signed-off-by: John Snow 
---
 tests/qemu-iotests/297 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index a37910b42d9..433b7323368 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -95,6 +95,7 @@ def run_linters():
 '--warn-redundant-casts',
 '--warn-unused-ignores',
 '--no-implicit-reexport',
+'--namespace-packages',
 filename),
env=env,
check=False,
-- 
2.31.1




[PULL 14/44] python/console_socket: avoid one-letter variable

2021-05-27 Thread John Snow
Fixes pylint warnings.

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
Reviewed-by: Philippe Mathieu-Daudé 
Message-id: 20210527211715.394144-2-js...@redhat.com
Message-id: 20210517184808.3562549-2-js...@redhat.com
Signed-off-by: John Snow 
---
 python/qemu/console_socket.py | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/python/qemu/console_socket.py b/python/qemu/console_socket.py
index ac21130e446..87237bebef7 100644
--- a/python/qemu/console_socket.py
+++ b/python/qemu/console_socket.py
@@ -46,11 +46,11 @@ def __init__(self, address: str, file: Optional[str] = None,
 self._drain_thread = self._thread_start()
 
 def __repr__(self) -> str:
-s = super().__repr__()
-s = s.rstrip(">")
-s = "%s,  logfile=%s, drain_thread=%s>" % (s, self._logfile,
-   self._drain_thread)
-return s
+tmp = super().__repr__()
+tmp = tmp.rstrip(">")
+tmp = "%s,  logfile=%s, drain_thread=%s>" % (tmp, self._logfile,
+ self._drain_thread)
+return tmp
 
 def _drain_fn(self) -> None:
 """Drains the socket and runs while the socket is open."""
-- 
2.31.1




[PULL 16/44] python/machine: use subprocess.run instead of subprocess.Popen

2021-05-27 Thread John Snow
use run() instead of Popen() -- to assert to pylint that we are not
forgetting to close a long-running program.

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
Tested-by: Cleber Rosa 
Message-id: 20210527211715.394144-4-js...@redhat.com
Message-id: 20210517184808.3562549-4-js...@redhat.com
Signed-off-by: John Snow 
---
 python/qemu/machine.py | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index 5b87e9ce024..04e005f3811 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -223,13 +223,16 @@ def send_fd_scm(self, fd: Optional[int] = None,
 assert fd is not None
 fd_param.append(str(fd))
 
-proc = subprocess.Popen(
-fd_param, stdin=subprocess.DEVNULL, stdout=subprocess.PIPE,
-stderr=subprocess.STDOUT, close_fds=False
+proc = subprocess.run(
+fd_param,
+stdin=subprocess.DEVNULL,
+stdout=subprocess.PIPE,
+stderr=subprocess.STDOUT,
+check=False,
+close_fds=False,
 )
-output = proc.communicate()[0]
-if output:
-LOG.debug(output)
+if proc.stdout:
+LOG.debug(proc.stdout)
 
 return proc.returncode
 
-- 
2.31.1




[PULL 33/44] python: Add flake8 to pipenv

2021-05-27 Thread John Snow
flake8 3.5.x does not support the --extend-ignore syntax used in the
.flake8 file to gracefully extend default ignores, so 3.6.x is our
minimum requirement. There is no known upper bound.

flake8 can be run from the python/ directory with no arguments.

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
Tested-by: Cleber Rosa 
Message-id: 20210527211715.394144-21-js...@redhat.com
Signed-off-by: John Snow 
---
 python/Pipfile  |  1 +
 python/Pipfile.lock | 51 -
 2 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/python/Pipfile b/python/Pipfile
index 285e2c8e671..053f344dcbe 100644
--- a/python/Pipfile
+++ b/python/Pipfile
@@ -4,6 +4,7 @@ url = "https://pypi.org/simple;
 verify_ssl = true
 
 [dev-packages]
+flake8 = ">=3.6.0"
 pylint = ">=2.8.0"
 
 [packages]
diff --git a/python/Pipfile.lock b/python/Pipfile.lock
index c9debd09503..5c34019060a 100644
--- a/python/Pipfile.lock
+++ b/python/Pipfile.lock
@@ -1,7 +1,7 @@
 {
 "_meta": {
 "hash": {
-"sha256": 
"bd4fb76fcdd145bbf23c3a9dd7ad966113c5ce43ca51cc2d828aa7e73d572901"
+"sha256": 
"3c842ab9c72c40d24d146349aa144e00e4dec1c358c812cfa96489411f5b3f87"
 },
 "pipfile-spec": 6,
 "requires": {
@@ -25,6 +25,22 @@
 "markers": "python_version ~= '3.6'",
 "version": "==2.5.6"
 },
+"flake8": {
+"hashes": [
+
"sha256:07528381786f2a6237b061f6e96610a4167b226cb926e2aa2b6b1d78057c576b",
+
"sha256:bf8fd46d844f616e8d47905ef3a3384edae6b4e9beb0c5101e25e3110907"
+],
+"index": "pypi",
+"version": "==3.9.2"
+},
+"importlib-metadata": {
+"hashes": [
+
"sha256:8c501196e49fb9df5df43833bdb1e4328f64847763ec8a50703148b73784d581",
+
"sha256:d7eb1dea6d6a6086f8be21784cc9e3bcfa55872b52309bc5fad53a8ea65d"
+],
+"markers": "python_version < '3.8'",
+"version": "==4.0.1"
+},
 "isort": {
 "hashes": [
 
"sha256:0a943902919f65c5684ac4e0154b1ad4fac6dcaa5d9f3426b732f1c8b5419be6",
@@ -68,6 +84,22 @@
 ],
 "version": "==0.6.1"
 },
+"pycodestyle": {
+"hashes": [
+
"sha256:514f76d918fcc0b55c6680472f0a37970994e07bbb80725808c17089be302068",
+
"sha256:c389c1d06bf7904078ca03399a4816f974a1d590090fecea0c63ec26ebaf1cef"
+],
+"markers": "python_version >= '2.7' and python_version not in 
'3.0, 3.1, 3.2, 3.3'",
+"version": "==2.7.0"
+},
+"pyflakes": {
+"hashes": [
+
"sha256:7893783d01b8a89811dd72d7dfd4d84ff098e5eed95cfa8905b22bbffe52efc3",
+
"sha256:f5bc8ecabc05bb9d291eb5203d6810b49040f6ff446a756326104746cc00c1db"
+],
+"markers": "python_version >= '2.7' and python_version not in 
'3.0, 3.1, 3.2, 3.3'",
+"version": "==2.3.1"
+},
 "pylint": {
 "hashes": [
 
"sha256:586d8fa9b1891f4b725f587ef267abe2a1bad89d6b184520c7f07a253dd6e217",
@@ -120,11 +152,28 @@
 "markers": "implementation_name == 'cpython' and python_version < 
'3.8'",
 "version": "==1.4.3"
 },
+"typing-extensions": {
+"hashes": [
+
"sha256:0ac0f89795dd19de6b97debb0c6af1c70987fd80a2d62d1958f7e56fcc31b497",
+
"sha256:50b6f157849174217d0656f99dc82fe932884fb250826c18350e159ec6cdf342",
+
"sha256:779383f6086d90c99ae41cf0ff39aac8a7937a9283ce0a414e5dd782f4c94a84"
+],
+"markers": "python_version < '3.8'",
+"version": "==3.10.0.0"
+},
 "wrapt": {
 "hashes": [
 
"sha256:b62ffa81fb85f4332a4f609cab4ac40709470da05643a082ec1eb88e6d9b97d7"
 ],
 "version": "==1.12.1"
+},
+"zipp": {
+"hashes": [
+
"sha256:3607921face881ba3e026887d8150cca609d517579abe052ac81fc5aeffdbd76",
+
"sha256:51cb66cc54621609dd593d1787f286ee42a5c0adbb4b29abea5a63edc3e03098"
+],
+"markers": "python_version >= '3.6'",
+"version": "==3.4.1"
 }
 }
 }
-- 
2.31.1




[PULL 13/44] acceptance tests: bump Avocado version to 88.1

2021-05-27 Thread John Snow
From: Willian Rampazzo 

Besides some internal changes, new features, and bug fixes, on the QEMU side,
this version fixes the following message seen when running the acceptance
tests: "Error running method "pre_tests" of plugin "fetchasset": 'bytes'
object has no attribute 'encode'".

The release notes are available at
https://avocado-framework.readthedocs.io/en/latest/releases/88_0.html.

Signed-off-by: Willian Rampazzo 
Message-Id: <20210520204747.210764-2-willi...@redhat.com>
Acked-by: Cleber Rosa 
Signed-off-by: John Snow 
---
 tests/requirements.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/requirements.txt b/tests/requirements.txt
index 91f3a343b95..a21b59b4439 100644
--- a/tests/requirements.txt
+++ b/tests/requirements.txt
@@ -1,5 +1,5 @@
 # Add Python module requirements, one per line, to be installed
 # in the tests/venv Python virtual environment. For more info,
 # refer to: https://pip.pypa.io/en/stable/user_guide/#id1
-avocado-framework==85.0
+avocado-framework==88.1
 pycdlib==1.11.0
-- 
2.31.1




[PULL 18/44] python/machine: Disable pylint warning for open() in _pre_launch

2021-05-27 Thread John Snow
Shift the open() call later so that the pylint pragma applies *only* to
that one open() call. Add a note that suggests why this is safe: the
resource is unconditionally cleaned up in _post_shutdown().

_post_shutdown is called after failed launches (see launch()), and
unconditionally after every call to shutdown(), and therefore also on
__exit__.

Signed-off-by: John Snow 
Reviewed-by: Wainer dos Santos Moschetta 
Reviewed-by: Cleber Rosa 
Message-id: 20210527211715.394144-6-js...@redhat.com
Message-id: 20210517184808.3562549-6-js...@redhat.com
Signed-off-by: John Snow 
---
 python/qemu/machine.py | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index 04e005f3811..c66bc6a9c69 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -306,7 +306,6 @@ def _base_args(self) -> List[str]:
 
 def _pre_launch(self) -> None:
 self._qemu_log_path = os.path.join(self.temp_dir, self._name + ".log")
-self._qemu_log_file = open(self._qemu_log_path, 'wb')
 
 if self._console_set:
 self._remove_files.append(self._console_address)
@@ -321,6 +320,11 @@ def _pre_launch(self) -> None:
 nickname=self._name
 )
 
+# NOTE: Make sure any opened resources are *definitely* freed in
+# _post_shutdown()!
+# pylint: disable=consider-using-with
+self._qemu_log_file = open(self._qemu_log_path, 'wb')
+
 def _post_launch(self) -> None:
 if self._qmp_connection:
 self._qmp.accept()
-- 
2.31.1




[PULL 32/44] python: add excluded dirs to flake8 config

2021-05-27 Thread John Snow
Instruct flake8 to avoid certain well-known directories created by
python tooling that it ought not check.

Note that at-present, nothing actually creates a ".venv" directory; but
it is in such widespread usage as a de-facto location for a developer's
virtual environment that it should be excluded anyway. A forthcoming
commit canonizes this with a "make venv" command.

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
Message-id: 20210527211715.394144-20-js...@redhat.com
Signed-off-by: John Snow 
---
 python/setup.cfg | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/python/setup.cfg b/python/setup.cfg
index 52a89a0a290..9aeab2bb0d3 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -24,6 +24,8 @@ packages =
 
 [flake8]
 extend-ignore = E722  # Prefer pylint's bare-except checks to flake8's
+exclude = __pycache__,
+  .venv,
 
 [pylint.messages control]
 # Disable the message, report, category or checker with the given id(s). You
-- 
2.31.1




[PULL 11/44] Acceptance Tests: introduce CPU hotplug test

2021-05-27 Thread John Snow
From: Cleber Rosa 

Even though there are qtest based tests for hotplugging CPUs (from
which this test took some inspiration from), this one adds checks
from a Linux guest point of view.

It should also serve as an example for tests that follow a similar
pattern and need to interact with QEMU (via qmp) and with the Linux
guest via SSH.

Signed-off-by: Cleber Rosa 
Reviewed-by: Marc-André Lureau 
Reviewed-by: Willian Rampazzo 
Reviewed-by: Eric Auger 
Message-Id: <20210412044644.55083-11-cr...@redhat.com>
Signed-off-by: John Snow 
---
 tests/acceptance/hotplug_cpu.py | 37 +
 1 file changed, 37 insertions(+)
 create mode 100644 tests/acceptance/hotplug_cpu.py

diff --git a/tests/acceptance/hotplug_cpu.py b/tests/acceptance/hotplug_cpu.py
new file mode 100644
index 000..6374bf1b546
--- /dev/null
+++ b/tests/acceptance/hotplug_cpu.py
@@ -0,0 +1,37 @@
+# Functional test that hotplugs a CPU and checks it on a Linux guest
+#
+# Copyright (c) 2021 Red Hat, Inc.
+#
+# Author:
+#  Cleber Rosa 
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# later.  See the COPYING file in the top-level directory.
+
+from avocado_qemu import LinuxTest
+
+
+class HotPlugCPU(LinuxTest):
+
+def test(self):
+"""
+:avocado: tags=arch:x86_64
+:avocado: tags=machine:q35
+:avocado: tags=accel:kvm
+"""
+self.require_accelerator('kvm')
+self.vm.add_args('-accel', 'kvm')
+self.vm.add_args('-cpu', 'Haswell')
+self.vm.add_args('-smp', '1,sockets=1,cores=2,threads=1,maxcpus=2')
+self.launch_and_wait()
+
+self.ssh_command('test -e /sys/devices/system/cpu/cpu0')
+with self.assertRaises(AssertionError):
+self.ssh_command('test -e /sys/devices/system/cpu/cpu1')
+
+self.vm.command('device_add',
+driver='Haswell-x86_64-cpu',
+socket_id=0,
+core_id=1,
+thread_id=0)
+self.ssh_command('test -e /sys/devices/system/cpu/cpu1')
-- 
2.31.1




[PULL 17/44] python/console_socket: Add a pylint ignore

2021-05-27 Thread John Snow
We manage cleaning up this resource ourselves. Pylint should shush.

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
Message-id: 20210527211715.394144-5-js...@redhat.com
Message-id: 20210517184808.3562549-5-js...@redhat.com
Signed-off-by: John Snow 
---
 python/qemu/console_socket.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/python/qemu/console_socket.py b/python/qemu/console_socket.py
index 87237bebef7..8c4ff598ad7 100644
--- a/python/qemu/console_socket.py
+++ b/python/qemu/console_socket.py
@@ -39,6 +39,7 @@ def __init__(self, address: str, file: Optional[str] = None,
 self.connect(address)
 self._logfile = None
 if file:
+# pylint: disable=consider-using-with
 self._logfile = open(file, "bw")
 self._open = True
 self._drain_thread = None
-- 
2.31.1




[PULL 12/44] tests/acceptance/virtiofs_submounts.py: fix setup of SSH pubkey

2021-05-27 Thread John Snow
From: Cleber Rosa 

The public key argument should be a path to a file, and not the
public key data.

Reported-by: Wainer dos Santos Moschetta 
Signed-off-by: Cleber Rosa 
Message-Id: <20210412044644.55083-12-cr...@redhat.com>
Reviewed-by: Willian Rampazzo 
Reviewed-by: Wainer dos Santos Moschetta 
Signed-off-by: John Snow 
---
 tests/acceptance/virtiofs_submounts.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/acceptance/virtiofs_submounts.py 
b/tests/acceptance/virtiofs_submounts.py
index d77ee356740..21ad7d792e7 100644
--- a/tests/acceptance/virtiofs_submounts.py
+++ b/tests/acceptance/virtiofs_submounts.py
@@ -195,7 +195,7 @@ def setUp(self):
 
 self.run(('ssh-keygen', '-N', '', '-t', 'ed25519', '-f', self.ssh_key))
 
-pubkey = open(self.ssh_key + '.pub').read()
+pubkey = self.ssh_key + '.pub'
 
 super(VirtiofsSubmountsTest, self).setUp(pubkey)
 
-- 
2.31.1




[PULL 08/44] Acceptance Tests: set up SSH connection by default after boot for LinuxTest

2021-05-27 Thread John Snow
From: Cleber Rosa 

The LinuxTest specifically targets users that need to interact with Linux
guests.  So, it makes sense to give a connection by default, and avoid
requiring it as boiler-plate code.

Signed-off-by: Cleber Rosa 
Reviewed-by: Marc-André Lureau 
Reviewed-by: Willian Rampazzo 
Message-Id: <20210412044644.55083-8-cr...@redhat.com>
Signed-off-by: John Snow 
---
 tests/acceptance/avocado_qemu/__init__.py |  5 -
 tests/acceptance/boot_linux.py| 18 +-
 tests/acceptance/virtiofs_submounts.py|  1 -
 3 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/tests/acceptance/avocado_qemu/__init__.py 
b/tests/acceptance/avocado_qemu/__init__.py
index 25f871f5bc6..1062a851b97 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -391,7 +391,7 @@ def set_up_cloudinit(self, ssh_pubkey=None):
 cloudinit_iso = self.prepare_cloudinit(ssh_pubkey)
 self.vm.add_args('-drive', 'file=%s,format=raw' % cloudinit_iso)
 
-def launch_and_wait(self):
+def launch_and_wait(self, set_up_ssh_connection=True):
 self.vm.set_console()
 self.vm.launch()
 console_drainer = 
datadrainer.LineLogger(self.vm.console_socket.fileno(),
@@ -399,3 +399,6 @@ def launch_and_wait(self):
 console_drainer.start()
 self.log.info('VM launched, waiting for boot confirmation from guest')
 cloudinit.wait_for_phone_home(('0.0.0.0', self.phone_home_port), 
self.name)
+if set_up_ssh_connection:
+self.log.info('Setting up the SSH connection')
+self.ssh_connect(self.username, self.ssh_key)
diff --git a/tests/acceptance/boot_linux.py b/tests/acceptance/boot_linux.py
index 0d178038a09..314370fd1f5 100644
--- a/tests/acceptance/boot_linux.py
+++ b/tests/acceptance/boot_linux.py
@@ -29,7 +29,7 @@ def test_pc_i440fx_tcg(self):
 """
 self.require_accelerator("tcg")
 self.vm.add_args("-accel", "tcg")
-self.launch_and_wait()
+self.launch_and_wait(set_up_ssh_connection=False)
 
 def test_pc_i440fx_kvm(self):
 """
@@ -38,7 +38,7 @@ def test_pc_i440fx_kvm(self):
 """
 self.require_accelerator("kvm")
 self.vm.add_args("-accel", "kvm")
-self.launch_and_wait()
+self.launch_and_wait(set_up_ssh_connection=False)
 
 def test_pc_q35_tcg(self):
 """
@@ -47,7 +47,7 @@ def test_pc_q35_tcg(self):
 """
 self.require_accelerator("tcg")
 self.vm.add_args("-accel", "tcg")
-self.launch_and_wait()
+self.launch_and_wait(set_up_ssh_connection=False)
 
 def test_pc_q35_kvm(self):
 """
@@ -56,7 +56,7 @@ def test_pc_q35_kvm(self):
 """
 self.require_accelerator("kvm")
 self.vm.add_args("-accel", "kvm")
-self.launch_and_wait()
+self.launch_and_wait(set_up_ssh_connection=False)
 
 
 class BootLinuxAarch64(LinuxTest):
@@ -85,7 +85,7 @@ def test_virt_tcg(self):
 self.vm.add_args("-cpu", "max")
 self.vm.add_args("-machine", "virt,gic-version=2")
 self.add_common_args()
-self.launch_and_wait()
+self.launch_and_wait(set_up_ssh_connection=False)
 
 def test_virt_kvm_gicv2(self):
 """
@@ -98,7 +98,7 @@ def test_virt_kvm_gicv2(self):
 self.vm.add_args("-cpu", "host")
 self.vm.add_args("-machine", "virt,gic-version=2")
 self.add_common_args()
-self.launch_and_wait()
+self.launch_and_wait(set_up_ssh_connection=False)
 
 def test_virt_kvm_gicv3(self):
 """
@@ -111,7 +111,7 @@ def test_virt_kvm_gicv3(self):
 self.vm.add_args("-cpu", "host")
 self.vm.add_args("-machine", "virt,gic-version=3")
 self.add_common_args()
-self.launch_and_wait()
+self.launch_and_wait(set_up_ssh_connection=False)
 
 
 class BootLinuxPPC64(LinuxTest):
@@ -128,7 +128,7 @@ def test_pseries_tcg(self):
 """
 self.require_accelerator("tcg")
 self.vm.add_args("-accel", "tcg")
-self.launch_and_wait()
+self.launch_and_wait(set_up_ssh_connection=False)
 
 
 class BootLinuxS390X(LinuxTest):
@@ -146,4 +146,4 @@ def test_s390_ccw_virtio_tcg(self):
 """
 self.require_accelerator("tcg")
 self.vm.add_args("-accel", "tcg")
-self.launch_and_wait()
+self.launch_and_wait(set_up_ssh_connection=False)
diff --git a/tests/acceptance/virtiofs_submounts.py 
b/tests/acceptance/virtiofs_submounts.py
index e10a935ac4e..e019d3b896b 100644
--- a/tests/acceptance/virtiofs_submounts.py
+++ b/tests/acceptance/virtiofs_submounts.py
@@ -136,7 +136,6 @@ def set_up_virtiofs(self):
 
 def launch_vm(self):
 self.launch_and_wait()
-self.ssh_connect('root', self.ssh_key)
 
 def set_up_nested_mounts(self):
 scratch_dir = os.path.join(self.shared_dir, 'scratch')
-- 
2.31.1




[PULL 28/44] python: add pylint import exceptions

2021-05-27 Thread John Snow
Pylint 2.5.x - 2.7.x have regressions that make import checking
inconsistent, see:

https://github.com/PyCQA/pylint/issues/3609
https://github.com/PyCQA/pylint/issues/3624
https://github.com/PyCQA/pylint/issues/3651

Pinning to 2.4.4 is worse, because it mandates versions of shared
dependencies that are too old for features we want in isort and mypy.
Oh well.

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
Message-id: 20210527211715.394144-16-js...@redhat.com
Signed-off-by: John Snow 
---
 python/qemu/machine/__init__.py | 3 +++
 python/qemu/machine/machine.py  | 2 +-
 python/qemu/machine/qtest.py| 2 +-
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/python/qemu/machine/__init__.py b/python/qemu/machine/__init__.py
index 98302ea31e7..728f27adbed 100644
--- a/python/qemu/machine/__init__.py
+++ b/python/qemu/machine/__init__.py
@@ -22,6 +22,9 @@
 # the COPYING file in the top-level directory.
 #
 
+# pylint: disable=import-error
+# see: https://github.com/PyCQA/pylint/issues/3624
+# see: https://github.com/PyCQA/pylint/issues/3651
 from .machine import QEMUMachine
 from .qtest import QEMUQtestMachine, QEMUQtestProtocol
 
diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
index d33b02d2ce6..b62435528e2 100644
--- a/python/qemu/machine/machine.py
+++ b/python/qemu/machine/machine.py
@@ -38,7 +38,7 @@
 Type,
 )
 
-from qemu.qmp import (
+from qemu.qmp import (  # pylint: disable=import-error
 QEMUMonitorProtocol,
 QMPMessage,
 QMPReturnValue,
diff --git a/python/qemu/machine/qtest.py b/python/qemu/machine/qtest.py
index e893ca3697a..93700684d1c 100644
--- a/python/qemu/machine/qtest.py
+++ b/python/qemu/machine/qtest.py
@@ -26,7 +26,7 @@
 TextIO,
 )
 
-from qemu.qmp import SocketAddrT
+from qemu.qmp import SocketAddrT  # pylint: disable=import-error
 
 from .machine import QEMUMachine
 
-- 
2.31.1




[PULL 15/44] python/machine: use subprocess.DEVNULL instead of open(os.path.devnull)

2021-05-27 Thread John Snow
One less file resource to manage, and it helps quiet some pylint >=
2.8.0 warnings about not using a with-context manager for the open call.

Signed-off-by: John Snow 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Cleber Rosa 
Message-id: 20210527211715.394144-3-js...@redhat.com
Message-id: 20210517184808.3562549-3-js...@redhat.com
Signed-off-by: John Snow 
---
 python/qemu/machine.py | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index b379fcbe726..5b87e9ce024 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -223,9 +223,8 @@ def send_fd_scm(self, fd: Optional[int] = None,
 assert fd is not None
 fd_param.append(str(fd))
 
-devnull = open(os.path.devnull, 'rb')
 proc = subprocess.Popen(
-fd_param, stdin=devnull, stdout=subprocess.PIPE,
+fd_param, stdin=subprocess.DEVNULL, stdout=subprocess.PIPE,
 stderr=subprocess.STDOUT, close_fds=False
 )
 output = proc.communicate()[0]
@@ -391,7 +390,6 @@ def _launch(self) -> None:
 """
 Launch the VM and establish a QMP connection
 """
-devnull = open(os.path.devnull, 'rb')
 self._pre_launch()
 self._qemu_full_args = tuple(
 chain(self._wrapper,
@@ -401,7 +399,7 @@ def _launch(self) -> None:
 )
 LOG.debug('VM launch command: %r', ' '.join(self._qemu_full_args))
 self._popen = subprocess.Popen(self._qemu_full_args,
-   stdin=devnull,
+   stdin=subprocess.DEVNULL,
stdout=self._qemu_log_file,
stderr=subprocess.STDOUT,
shell=False,
-- 
2.31.1




[PULL 09/44] tests/acceptance/virtiofs_submounts.py: remove launch_vm()

2021-05-27 Thread John Snow
From: Cleber Rosa 

The LinuxTest class' launch_and_wait() method now behaves the same way
as this test's custom launch_vm(), so let's just use the upper layer
(common) method.

Signed-off-by: Cleber Rosa 
Reviewed-by: Marc-André Lureau 
Reviewed-by: Eric Auger 
Reviewed-by: Willian Rampazzo 
Message-Id: <20210412044644.55083-9-cr...@redhat.com>
Signed-off-by: John Snow 
---
 tests/acceptance/virtiofs_submounts.py | 13 +
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/tests/acceptance/virtiofs_submounts.py 
b/tests/acceptance/virtiofs_submounts.py
index e019d3b896b..d77ee356740 100644
--- a/tests/acceptance/virtiofs_submounts.py
+++ b/tests/acceptance/virtiofs_submounts.py
@@ -134,9 +134,6 @@ def set_up_virtiofs(self):
  '-numa',
  'node,memdev=mem')
 
-def launch_vm(self):
-self.launch_and_wait()
-
 def set_up_nested_mounts(self):
 scratch_dir = os.path.join(self.shared_dir, 'scratch')
 try:
@@ -225,7 +222,7 @@ def test_pre_virtiofsd_set_up(self):
 self.set_up_nested_mounts()
 
 self.set_up_virtiofs()
-self.launch_vm()
+self.launch_and_wait()
 self.mount_in_guest()
 self.check_in_guest()
 
@@ -235,14 +232,14 @@ def test_pre_launch_set_up(self):
 
 self.set_up_nested_mounts()
 
-self.launch_vm()
+self.launch_and_wait()
 self.mount_in_guest()
 self.check_in_guest()
 
 def test_post_launch_set_up(self):
 self.set_up_shared_dir()
 self.set_up_virtiofs()
-self.launch_vm()
+self.launch_and_wait()
 
 self.set_up_nested_mounts()
 
@@ -252,7 +249,7 @@ def test_post_launch_set_up(self):
 def test_post_mount_set_up(self):
 self.set_up_shared_dir()
 self.set_up_virtiofs()
-self.launch_vm()
+self.launch_and_wait()
 self.mount_in_guest()
 
 self.set_up_nested_mounts()
@@ -265,7 +262,7 @@ def test_two_runs(self):
 self.set_up_nested_mounts()
 
 self.set_up_virtiofs()
-self.launch_vm()
+self.launch_and_wait()
 self.mount_in_guest()
 self.check_in_guest()
 
-- 
2.31.1




[PULL 02/44] tests/acceptance/virtiofs_submounts.py: add missing accel tag

2021-05-27 Thread John Snow
From: Cleber Rosa 

The tag is useful to select tests that depend/use a particular
feature.

Signed-off-by: Cleber Rosa 
Reviewed-by: Wainer dos Santos Moschetta 
Reviewed-by: Willian Rampazzo 
Reviewed-by: Eric Auger 
Message-Id: <20210412044644.55083-2-cr...@redhat.com>
Signed-off-by: John Snow 
---
 tests/acceptance/virtiofs_submounts.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/acceptance/virtiofs_submounts.py 
b/tests/acceptance/virtiofs_submounts.py
index 46fa65392a1..5b74ce2929b 100644
--- a/tests/acceptance/virtiofs_submounts.py
+++ b/tests/acceptance/virtiofs_submounts.py
@@ -70,6 +70,7 @@ def test_something_that_needs_cmd1_and_cmd2(self):
 class VirtiofsSubmountsTest(LinuxTest):
 """
 :avocado: tags=arch:x86_64
+:avocado: tags=accel:kvm
 """
 
 def get_portfwd(self):
-- 
2.31.1




[PULL 10/44] Acceptance Tests: add basic documentation on LinuxTest base class

2021-05-27 Thread John Snow
From: Cleber Rosa 

Signed-off-by: Cleber Rosa 
Reviewed-by: Marc-André Lureau 
Reviewed-by: Willian Rampazzo 
Reviewed-by: Eric Auger 
Reviewed-by: Wainer dos Santos Moschetta 
Message-Id: <20210412044644.55083-10-cr...@redhat.com>
Signed-off-by: John Snow 
---
 docs/devel/testing.rst | 26 ++
 1 file changed, 26 insertions(+)

diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
index 1da4c4e4c4e..4e423928106 100644
--- a/docs/devel/testing.rst
+++ b/docs/devel/testing.rst
@@ -810,6 +810,32 @@ and hypothetical example follows:
 At test "tear down", ``avocado_qemu.Test`` handles all the QEMUMachines
 shutdown.
 
+The ``avocado_qemu.LinuxTest`` base test class
+~~
+
+The ``avocado_qemu.LinuxTest`` is further specialization of the
+``avocado_qemu.Test`` class, so it contains all the characteristics of
+the later plus some extra features.
+
+First of all, this base class is intended for tests that need to
+interact with a fully booted and operational Linux guest.  At this
+time, it uses a Fedora 31 guest image.  The most basic example looks
+like this:
+
+.. code::
+
+  from avocado_qemu import LinuxTest
+
+
+  class SomeTest(LinuxTest):
+
+  def test(self):
+  self.launch_and_wait()
+  self.ssh_command('some_command_to_be_run_in_the_guest')
+
+Please refer to tests that use ``avocado_qemu.LinuxTest`` under
+``tests/acceptance`` for more examples.
+
 QEMUMachine
 ~~~
 
-- 
2.31.1




[PULL 07/44] Acceptance Tests: make username/password configurable

2021-05-27 Thread John Snow
From: Cleber Rosa 

This makes the username/password used for authentication configurable,
because some guest operating systems may have restrictions on accounts
to be used for logins, and it just makes it better documented.

Signed-off-by: Cleber Rosa 
Reviewed-by: Marc-André Lureau 
Reviewed-by: Eric Auger 
Reviewed-by: Willian Rampazzo 
Message-Id: <20210412044644.55083-7-cr...@redhat.com>
Signed-off-by: John Snow 
---
 tests/acceptance/avocado_qemu/__init__.py | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tests/acceptance/avocado_qemu/__init__.py 
b/tests/acceptance/avocado_qemu/__init__.py
index 085688f..25f871f5bc6 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -308,6 +308,8 @@ class LinuxTest(Test, LinuxSSHMixIn):
 
 timeout = 900
 chksum = None
+username = 'root'
+password = 'password'
 
 def setUp(self, ssh_pubkey=None, network_device_type='virtio-net'):
 super(LinuxTest, self).setUp()
@@ -371,8 +373,8 @@ def prepare_cloudinit(self, ssh_pubkey=None):
 with open(ssh_pubkey) as pubkey:
 pubkey_content = pubkey.read()
 cloudinit.iso(cloudinit_iso, self.name,
-  username='root',
-  password='password',
+  username=self.username,
+  password=self.password,
   # QEMU's hard coded usermode router address
   phone_home_host='10.0.2.2',
   phone_home_port=self.phone_home_port,
-- 
2.31.1




[PULL 06/44] Acceptance Tests: add port redirection for ssh by default

2021-05-27 Thread John Snow
From: Cleber Rosa 

For users of the LinuxTest class, let's set up the VM with the port
redirection for SSH, instead of requiring each test to set the same
arguments.

It also sets the network device, by default, to virtio-net.

Signed-off-by: Cleber Rosa 
Reviewed-by: Marc-André Lureau 
Reviewed-by: Eric Auger 
Reviewed-by: Willian Rampazzo 
Message-Id: <20210412044644.55083-6-cr...@redhat.com>
Signed-off-by: John Snow 
---
 tests/acceptance/avocado_qemu/__init__.py | 5 -
 tests/acceptance/virtiofs_submounts.py| 4 
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/tests/acceptance/avocado_qemu/__init__.py 
b/tests/acceptance/avocado_qemu/__init__.py
index 67f75f66e56..085688f 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -309,10 +309,13 @@ class LinuxTest(Test, LinuxSSHMixIn):
 timeout = 900
 chksum = None
 
-def setUp(self, ssh_pubkey=None):
+def setUp(self, ssh_pubkey=None, network_device_type='virtio-net'):
 super(LinuxTest, self).setUp()
 self.vm.add_args('-smp', '2')
 self.vm.add_args('-m', '1024')
+# The following network device allows for SSH connections
+self.vm.add_args('-netdev', 'user,id=vnet,hostfwd=:127.0.0.1:0-:22',
+ '-device', '%s,netdev=vnet' % network_device_type)
 self.set_up_boot()
 if ssh_pubkey is None:
 ssh_pubkey, self.ssh_key = self.set_up_existing_ssh_keys()
diff --git a/tests/acceptance/virtiofs_submounts.py 
b/tests/acceptance/virtiofs_submounts.py
index bed8ce44dfc..e10a935ac4e 100644
--- a/tests/acceptance/virtiofs_submounts.py
+++ b/tests/acceptance/virtiofs_submounts.py
@@ -207,10 +207,6 @@ def setUp(self):
 self.vm.add_args('-kernel', vmlinuz,
  '-append', 'console=ttyS0 root=/dev/sda1')
 
-# Allow us to connect to SSH
-self.vm.add_args('-netdev', 'user,id=vnet,hostfwd=:127.0.0.1:0-:22',
- '-device', 'virtio-net,netdev=vnet')
-
 self.require_accelerator("kvm")
 self.vm.add_args('-accel', 'kvm')
 
-- 
2.31.1




[PULL 00/44] Python patches

2021-05-27 Thread John Snow
The following changes since commit c8616fc7670b884de5f74d2767aade224c1c5c3a:

  Merge remote-tracking branch 'remotes/philmd/tags/gitlab-ci-20210527' into 
staging (2021-05-27 16:32:57 +0100)

are available in the Git repository at:

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

for you to fetch changes up to faa40e297406445ab8814844bd9aa532a7538cde:

  gitlab: add python linters to CI (2021-05-27 18:11:17 -0400)


Python pull request

Python packaging & CI implementation
Acceptance tests sent w/ Cleber's blessing.
New CI tests send w/ stsquad's RB.

--js



Cleber Rosa (12):
  Python: expose QEMUMachine's temporary directory
  tests/acceptance/virtiofs_submounts.py: add missing accel tag
  tests/acceptance/virtiofs_submounts.py: evaluate string not length
  Python: add utility function for retrieving port redirection
  Acceptance Tests: move useful ssh methods to base class
  Acceptance Tests: add port redirection for ssh by default
  Acceptance Tests: make username/password configurable
  Acceptance Tests: set up SSH connection by default after boot for
LinuxTest
  tests/acceptance/virtiofs_submounts.py: remove launch_vm()
  Acceptance Tests: add basic documentation on LinuxTest base class
  Acceptance Tests: introduce CPU hotplug test
  tests/acceptance/virtiofs_submounts.py: fix setup of SSH pubkey

John Snow (31):
  python/console_socket: avoid one-letter variable
  python/machine: use subprocess.DEVNULL instead of
open(os.path.devnull)
  python/machine: use subprocess.run instead of subprocess.Popen
  python/console_socket: Add a pylint ignore
  python/machine: Disable pylint warning for open() in _pre_launch
  python/machine: disable warning for Popen in _launch()
  python/machine: Trim line length to below 80 chars
  iotests/297: add --namespace-packages to mypy arguments
  python: create qemu packages
  python: add qemu package installer
  python: add VERSION file
  python: add directory structure README.rst files
  python: add MANIFEST.in
  python: Add pipenv support
  python: add pylint import exceptions
  python: move pylintrc into setup.cfg
  python: add pylint to pipenv
  python: move flake8 config to setup.cfg
  python: add excluded dirs to flake8 config
  python: Add flake8 to pipenv
  python: move mypy.ini into setup.cfg
  python: add mypy to pipenv
  python: move .isort.cfg into setup.cfg
  python/qemu: add isort to pipenv
  python/qemu: add qemu package itself to pipenv
  python: add devel package requirements to setuptools
  python: add avocado-framework and tests
  python: add Makefile for some common tasks
  python: add .gitignore
  python: add tox support
  gitlab: add python linters to CI

Willian Rampazzo (1):
  acceptance tests: bump Avocado version to 88.1

 docs/devel/testing.rst  |  26 +++
 python/PACKAGE.rst  |  43 
 python/README.rst   |  58 +
 python/qemu/README.rst  |   8 +
 python/qemu/machine/README.rst  |   9 +
 python/qemu/qmp/README.rst  |   9 +
 python/qemu/utils/README.rst|   7 +
 .gitlab-ci.d/containers.yml |   5 +
 .gitlab-ci.d/static_checks.yml  |  21 ++
 python/.gitignore   |  16 ++
 python/MANIFEST.in  |   3 +
 python/Makefile |  48 
 python/Pipfile  |  13 ++
 python/Pipfile.lock | 231 
 python/VERSION  |   1 +
 python/avocado.cfg  |  10 +
 python/mypy.ini |   4 -
 python/qemu/.flake8 |   2 -
 python/qemu/.isort.cfg  |   7 -
 python/qemu/__init__.py |  11 -
 python/qemu/machine/__init__.py |  36 +++
 python/qemu/{ => machine}/console_socket.py |  11 +-
 python/qemu/{ => machine}/machine.py|  68 --
 python/qemu/{ => machine}/qtest.py  |   9 +-
 python/qemu/pylintrc|  58 -
 python/qemu/{qmp.py => qmp/__init__.py} |  12 +-
 python/qemu/utils/__init__.py   |  45 
 python/qemu/{ => utils}/accel.py|   0
 python/setup.cfg| 102 +
 python/setup.py |  23 ++
 python/tests/flake8.sh  |   2 +
 python/tests/isort.sh   |   2 +
 python/tests/mypy.sh|   2 +
 python/tests/pylint.sh  |   2 +
 tests/acceptance/avocado_qemu/__init__.py   |  69 +-
 tests/acceptance/boot_linux.py  |  18 +-
 tests/acceptance/hotplug_cpu.py |  37 
 tests/acceptance/info_usernet.py|  29 ++

[PULL 05/44] Acceptance Tests: move useful ssh methods to base class

2021-05-27 Thread John Snow
From: Cleber Rosa 

Both the virtiofs submounts and the linux ssh mips malta tests
contains useful methods related to ssh that deserve to be made
available to other tests.  Let's move them to an auxiliary, mix-in
class that will be used on the base LinuxTest class.

The method that helps with setting up an ssh connection will now
support both key and password based authentication, defaulting to key
based.

Signed-off-by: Cleber Rosa 
Reviewed-by: Wainer dos Santos Moschetta 
Reviewed-by: Willian Rampazzo 
Reviewed-by: Eric Auger 
Signed-off-by: Cleber Rosa 
Message-Id: <20210412044644.55083-5-cr...@redhat.com>
Signed-off-by: John Snow 
---
 tests/acceptance/avocado_qemu/__init__.py | 48 ++-
 tests/acceptance/linux_ssh_mips_malta.py  | 40 ++-
 tests/acceptance/virtiofs_submounts.py| 37 -
 3 files changed, 50 insertions(+), 75 deletions(-)

diff --git a/tests/acceptance/avocado_qemu/__init__.py 
b/tests/acceptance/avocado_qemu/__init__.py
index 83b1741ec85..67f75f66e56 100644
--- a/tests/acceptance/avocado_qemu/__init__.py
+++ b/tests/acceptance/avocado_qemu/__init__.py
@@ -20,6 +20,7 @@
 from avocado.utils import cloudinit
 from avocado.utils import datadrainer
 from avocado.utils import network
+from avocado.utils import ssh
 from avocado.utils import vmimage
 from avocado.utils.path import find_command
 
@@ -43,6 +44,8 @@
 from qemu.accel import kvm_available
 from qemu.accel import tcg_available
 from qemu.machine import QEMUMachine
+from qemu.utils import get_info_usernet_hostfwd_port
+
 
 def is_readable_executable_file(path):
 return os.path.isfile(path) and os.access(path, os.R_OK | os.X_OK)
@@ -253,7 +256,50 @@ def fetch_asset(self, name,
 cancel_on_missing=cancel_on_missing)
 
 
-class LinuxTest(Test):
+class LinuxSSHMixIn:
+"""Contains utility methods for interacting with a guest via SSH."""
+
+def ssh_connect(self, username, credential, credential_is_key=True):
+self.ssh_logger = logging.getLogger('ssh')
+res = self.vm.command('human-monitor-command',
+  command_line='info usernet')
+port = get_info_usernet_hostfwd_port(res)
+self.assertIsNotNone(port)
+self.assertGreater(port, 0)
+self.log.debug('sshd listening on port: %d', port)
+if credential_is_key:
+self.ssh_session = ssh.Session('127.0.0.1', port=port,
+   user=username, key=credential)
+else:
+self.ssh_session = ssh.Session('127.0.0.1', port=port,
+   user=username, password=credential)
+for i in range(10):
+try:
+self.ssh_session.connect()
+return
+except:
+time.sleep(4)
+pass
+self.fail('ssh connection timeout')
+
+def ssh_command(self, command):
+self.ssh_logger.info(command)
+result = self.ssh_session.cmd(command)
+stdout_lines = [line.rstrip() for line
+in result.stdout_text.splitlines()]
+for line in stdout_lines:
+self.ssh_logger.info(line)
+stderr_lines = [line.rstrip() for line
+in result.stderr_text.splitlines()]
+for line in stderr_lines:
+self.ssh_logger.warning(line)
+
+self.assertEqual(result.exit_status, 0,
+ f'Guest command failed: {command}')
+return stdout_lines, stderr_lines
+
+
+class LinuxTest(Test, LinuxSSHMixIn):
 """Facilitates having a cloud-image Linux based available.
 
 For tests that indend to interact with guests, this is a better choice
diff --git a/tests/acceptance/linux_ssh_mips_malta.py 
b/tests/acceptance/linux_ssh_mips_malta.py
index 052008f02d4..61c9079d047 100644
--- a/tests/acceptance/linux_ssh_mips_malta.py
+++ b/tests/acceptance/linux_ssh_mips_malta.py
@@ -12,16 +12,14 @@
 import time
 
 from avocado import skipUnless
-from avocado_qemu import Test
+from avocado_qemu import Test, LinuxSSHMixIn
 from avocado_qemu import wait_for_console_pattern
 from avocado.utils import process
 from avocado.utils import archive
 from avocado.utils import ssh
 
-from qemu.utils import get_info_usernet_hostfwd_port
 
-
-class LinuxSSH(Test):
+class LinuxSSH(Test, LinuxSSHMixIn):
 
 timeout = 150 # Not for 'configure --enable-debug --enable-debug-tcg'
 
@@ -72,41 +70,9 @@ def get_kernel_info(self, endianess, wordsize):
 def setUp(self):
 super(LinuxSSH, self).setUp()
 
-def ssh_connect(self, username, password):
-self.ssh_logger = logging.getLogger('ssh')
-res = self.vm.command('human-monitor-command',
-  command_line='info usernet')
-port = get_info_usernet_hostfwd_port(res)
-if not port:
-self.cancel("Failed to retrieve SSH port")
-self.log.debug("sshd listening 

[PULL 03/44] tests/acceptance/virtiofs_submounts.py: evaluate string not length

2021-05-27 Thread John Snow
From: Cleber Rosa 

If the vmlinuz variable is set to anything that evaluates to True,
then the respective arguments should be set.  If the variable contains
an empty string, than it will evaluate to False, and the extra
arguments will not be set.

This keeps the same logic, but improves readability a bit.

Signed-off-by: Cleber Rosa 
Reviewed-by: Beraldo Leal 
Reviewed-by: Eric Auger 
Reviewed-by: Willian Rampazzo 
Message-Id: <20210412044644.55083-3-cr...@redhat.com>
Signed-off-by: John Snow 
---
 tests/acceptance/virtiofs_submounts.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/acceptance/virtiofs_submounts.py 
b/tests/acceptance/virtiofs_submounts.py
index 5b74ce2929b..ca64b76301f 100644
--- a/tests/acceptance/virtiofs_submounts.py
+++ b/tests/acceptance/virtiofs_submounts.py
@@ -251,7 +251,7 @@ def setUp(self):
 
 super(VirtiofsSubmountsTest, self).setUp(pubkey)
 
-if len(vmlinuz) > 0:
+if vmlinuz:
 self.vm.add_args('-kernel', vmlinuz,
  '-append', 'console=ttyS0 root=/dev/sda1')
 
-- 
2.31.1




[PULL 04/44] Python: add utility function for retrieving port redirection

2021-05-27 Thread John Snow
From: Cleber Rosa 

Slightly different versions for the same utility code are currently
present on different locations.  This unifies them all, giving
preference to the version from virtiofs_submounts.py, because of the
last tweaks added to it.

While at it, this adds a "qemu.utils" module to host the utility
function and a test.

Signed-off-by: Cleber Rosa 
Reviewed-by: Wainer dos Santos Moschetta 
Reviewed-by: Eric Auger 
Reviewed-by: Willian Rampazzo 
Message-Id: <20210412044644.55083-4-cr...@redhat.com>
Signed-off-by: John Snow 
---
 python/qemu/utils.py | 33 
 tests/acceptance/info_usernet.py | 29 +
 tests/acceptance/linux_ssh_mips_malta.py | 16 +---
 tests/acceptance/virtiofs_submounts.py   | 21 ---
 tests/vm/basevm.py   |  7 ++---
 5 files changed, 76 insertions(+), 30 deletions(-)
 create mode 100644 python/qemu/utils.py
 create mode 100644 tests/acceptance/info_usernet.py

diff --git a/python/qemu/utils.py b/python/qemu/utils.py
new file mode 100644
index 000..5ed789275ee
--- /dev/null
+++ b/python/qemu/utils.py
@@ -0,0 +1,33 @@
+"""
+QEMU utility library
+
+This offers miscellaneous utility functions, which may not be easily
+distinguishable or numerous to be in their own module.
+"""
+
+# Copyright (C) 2021 Red Hat Inc.
+#
+# Authors:
+#  Cleber Rosa 
+#
+# This work is licensed under the terms of the GNU GPL, version 2.  See
+# the COPYING file in the top-level directory.
+#
+
+import re
+from typing import Optional
+
+
+def get_info_usernet_hostfwd_port(info_usernet_output: str) -> Optional[int]:
+"""
+Returns the port given to the hostfwd parameter via info usernet
+
+:param info_usernet_output: output generated by hmp command "info usernet"
+:return: the port number allocated by the hostfwd option
+"""
+for line in info_usernet_output.split('\r\n'):
+regex = r'TCP.HOST_FORWARD.*127\.0\.0\.1\s+(\d+)\s+10\.'
+match = re.search(regex, line)
+if match is not None:
+return int(match[1])
+return None
diff --git a/tests/acceptance/info_usernet.py b/tests/acceptance/info_usernet.py
new file mode 100644
index 000..9c1fd903a0b
--- /dev/null
+++ b/tests/acceptance/info_usernet.py
@@ -0,0 +1,29 @@
+# Test for the hmp command "info usernet"
+#
+# Copyright (c) 2021 Red Hat, Inc.
+#
+# Author:
+#  Cleber Rosa 
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# later.  See the COPYING file in the top-level directory.
+
+from avocado_qemu import Test
+
+from qemu.utils import get_info_usernet_hostfwd_port
+
+
+class InfoUsernet(Test):
+
+def test_hostfwd(self):
+self.vm.add_args('-netdev', 'user,id=vnet,hostfwd=:127.0.0.1:0-:22')
+self.vm.launch()
+res = self.vm.command('human-monitor-command',
+  command_line='info usernet')
+port = get_info_usernet_hostfwd_port(res)
+self.assertIsNotNone(port,
+ ('"info usernet" output content does not seem to '
+  'contain the redirected port'))
+self.assertGreater(port, 0,
+   ('Found a redirected port that is not greater than'
+' zero'))
diff --git a/tests/acceptance/linux_ssh_mips_malta.py 
b/tests/acceptance/linux_ssh_mips_malta.py
index 6dbd02d49d5..052008f02d4 100644
--- a/tests/acceptance/linux_ssh_mips_malta.py
+++ b/tests/acceptance/linux_ssh_mips_malta.py
@@ -18,6 +18,8 @@
 from avocado.utils import archive
 from avocado.utils import ssh
 
+from qemu.utils import get_info_usernet_hostfwd_port
+
 
 class LinuxSSH(Test):
 
@@ -70,18 +72,14 @@ def get_kernel_info(self, endianess, wordsize):
 def setUp(self):
 super(LinuxSSH, self).setUp()
 
-def get_portfwd(self):
+def ssh_connect(self, username, password):
+self.ssh_logger = logging.getLogger('ssh')
 res = self.vm.command('human-monitor-command',
   command_line='info usernet')
-line = res.split('\r\n')[2]
-port = re.split(r'.*TCP.HOST_FORWARD.*127\.0\.0\.1 (\d+)\s+10\..*',
-line)[1]
+port = get_info_usernet_hostfwd_port(res)
+if not port:
+self.cancel("Failed to retrieve SSH port")
 self.log.debug("sshd listening on port:" + port)
-return port
-
-def ssh_connect(self, username, password):
-self.ssh_logger = logging.getLogger('ssh')
-port = self.get_portfwd()
 self.ssh_session = ssh.Session(self.VM_IP, port=int(port),
user=username, password=password)
 for i in range(10):
diff --git a/tests/acceptance/virtiofs_submounts.py 
b/tests/acceptance/virtiofs_submounts.py
index ca64b76301f..57a7047342f 100644
--- a/tests/acceptance/virtiofs_submounts.py
+++ b/tests/acceptance/virtiofs_submounts.py
@@ 

[PULL 01/44] Python: expose QEMUMachine's temporary directory

2021-05-27 Thread John Snow
From: Cleber Rosa 

Each instance of qemu.machine.QEMUMachine currently has a "test
directory", which may not have any relation to a "test", and it's
really a temporary directory.

Users instantiating the QEMUMachine class will be able to set the
location of the directory that will *contain* the QEMUMachine unique
temporary directory, so that parameter name has been changed from
test_dir to base_temp_dir.

A property has been added to allow users to access it without using
private attributes, and with that, the directory is created on first
use of the property.

Signed-off-by: Cleber Rosa 
Message-Id: <20210211220146.2525771-3-cr...@redhat.com>
Reviewed-by: Wainer dos Santos Moschetta 
Signed-off-by: Cleber Rosa 
Signed-off-by: John Snow 
---
 python/qemu/machine.py| 24 
 python/qemu/qtest.py  |  6 +++---
 tests/qemu-iotests/iotests.py |  2 +-
 3 files changed, 20 insertions(+), 12 deletions(-)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index 6e44bda337e..b379fcbe726 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -84,7 +84,7 @@ def __init__(self,
  args: Sequence[str] = (),
  wrapper: Sequence[str] = (),
  name: Optional[str] = None,
- test_dir: str = "/var/tmp",
+ base_temp_dir: str = "/var/tmp",
  monitor_address: Optional[SocketAddrT] = None,
  socket_scm_helper: Optional[str] = None,
  sock_dir: Optional[str] = None,
@@ -97,10 +97,10 @@ def __init__(self,
 @param args: list of extra arguments
 @param wrapper: list of arguments used as prefix to qemu binary
 @param name: prefix for socket and log file names (default: qemu-PID)
-@param test_dir: where to create socket and log file
+@param base_temp_dir: default location where temporary files are 
created
 @param monitor_address: address for QMP monitor
 @param socket_scm_helper: helper program, required for send_fd_scm()
-@param sock_dir: where to create socket (overrides test_dir for sock)
+@param sock_dir: where to create socket (defaults to base_temp_dir)
 @param drain_console: (optional) True to drain console socket to buffer
 @param console_log: (optional) path to console log file
 @note: Qemu process is not started until launch() is used.
@@ -112,8 +112,8 @@ def __init__(self,
 self._wrapper = wrapper
 
 self._name = name or "qemu-%d" % os.getpid()
-self._test_dir = test_dir
-self._sock_dir = sock_dir or self._test_dir
+self._base_temp_dir = base_temp_dir
+self._sock_dir = sock_dir or self._base_temp_dir
 self._socket_scm_helper = socket_scm_helper
 
 if monitor_address is not None:
@@ -303,9 +303,7 @@ def _base_args(self) -> List[str]:
 return args
 
 def _pre_launch(self) -> None:
-self._temp_dir = tempfile.mkdtemp(prefix="qemu-machine-",
-  dir=self._test_dir)
-self._qemu_log_path = os.path.join(self._temp_dir, self._name + ".log")
+self._qemu_log_path = os.path.join(self.temp_dir, self._name + ".log")
 self._qemu_log_file = open(self._qemu_log_path, 'wb')
 
 if self._console_set:
@@ -744,3 +742,13 @@ def console_socket(self) -> socket.socket:
 file=self._console_log_path,
 drain=self._drain_console)
 return self._console_socket
+
+@property
+def temp_dir(self) -> str:
+"""
+Returns a temporary directory to be used for this machine
+"""
+if self._temp_dir is None:
+self._temp_dir = tempfile.mkdtemp(prefix="qemu-machine-",
+  dir=self._base_temp_dir)
+return self._temp_dir
diff --git a/python/qemu/qtest.py b/python/qemu/qtest.py
index 39a0cf62fe9..78b97d13cf0 100644
--- a/python/qemu/qtest.py
+++ b/python/qemu/qtest.py
@@ -112,14 +112,14 @@ def __init__(self,
  binary: str,
  args: Sequence[str] = (),
  name: Optional[str] = None,
- test_dir: str = "/var/tmp",
+ base_temp_dir: str = "/var/tmp",
  socket_scm_helper: Optional[str] = None,
  sock_dir: Optional[str] = None):
 if name is None:
 name = "qemu-%d" % os.getpid()
 if sock_dir is None:
-sock_dir = test_dir
-super().__init__(binary, args, name=name, test_dir=test_dir,
+sock_dir = base_temp_dir
+super().__init__(binary, args, name=name, base_temp_dir=base_temp_dir,
  socket_scm_helper=socket_scm_helper,
  sock_dir=sock_dir)
 self._qtest: Optional[QEMUQtestProtocol] = None
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 

Re: [PATCH] vfio: Fix unregister SaveVMHandler in vfio_migration_finalize

2021-05-27 Thread Kunkun Jiang

Hi Philippe,

On 2021/5/27 21:44, Philippe Mathieu-Daudé wrote:

On 5/27/21 2:31 PM, Kunkun Jiang wrote:

In the vfio_migration_init(), the SaveVMHandler is registered for
VFIO device. But it lacks the operation of 'unregister'. It will
lead to 'Segmentation fault (core dumped)' in
qemu_savevm_state_setup(), if performing live migration after a
VFIO device is hot deleted.

Fixes: 7c2f5f75f94 (vfio: Register SaveVMHandlers for VFIO device)
Reported-by: Qixin Gan 
Signed-off-by: Kunkun Jiang 

Cc: qemu-sta...@nongnu.org


---
  hw/vfio/migration.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 201642d75e..ef397ebe6c 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -892,6 +892,7 @@ void vfio_migration_finalize(VFIODevice *vbasedev)
  
  remove_migration_state_change_notifier(>migration_state);

  qemu_del_vm_change_state_handler(migration->vm_state);
+unregister_savevm(VMSTATE_IF(vbasedev->dev), "vfio", vbasedev);

Hmm what about devices using "%s/vfio" id?

The unregister_savevm() needs 'VMSTATEIf *obj'. If we pass a non-null 'obj'
to unregister_svevm(), it will handle the devices using "%s/vfio" id with
the following code:

    if (obj) {
    char *oid = vmstate_if_get_id(obj);
    if (oid) {
    pstrcpy(id, sizeof(id), oid);
    pstrcat(id, sizeof(id), "/");
    g_free(oid);
    }
    }
    pstrcat(id, sizeof(id), idstr);


By the way, I'm puzzled that register_savevm_live() and unregister_savevm()
handle devices using "%s/vfio" id differently. So I learned the commit
history of register_savevm_live() and unregister_savevm().

In the beginning, both them need 'DeviceState *dev', which are replaced
with VMStateIf in 3cad405babb. Later in ce62df5378b, the 'dev' was removed,
because no caller of register_savevm_live() need to pass a non-null 'dev'
at that time.

So now the vfio devices need to handle the 'id' first and then call
register_savevm_live(). I am wondering whether we need to add
'VMSTATEIf *obj' in register_savevm_live(). What do you think of this?

Thanks,
Kunkun Jiang



  vfio_migration_exit(vbasedev);
  }
  


.






Re: [PATCH] target/riscv: hardwire bits in hideleg and hedeleg

2021-05-27 Thread LIU Zhiwei

Reviewed-by: LIU Zhiwei 

Zhiwei

On 5/22/21 11:59 PM, Jose Martins wrote:

The specification mandates for certain bits to be hardwired in the
hypervisor delegation registers. This was not being enforced.

Signed-off-by: Jose Martins 
---
  target/riscv/csr.c | 13 +++--
  1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index d2585395bf..9b74a00cc9 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -394,6 +394,7 @@ static int read_timeh(CPURISCVState *env, int csrno, 
target_ulong *val)
  
  static const target_ulong delegable_ints = S_MODE_INTERRUPTS |

 VS_MODE_INTERRUPTS;
+static const target_ulong vs_delegable_ints = VS_MODE_INTERRUPTS;
  static const target_ulong all_ints = M_MODE_INTERRUPTS | S_MODE_INTERRUPTS |
   VS_MODE_INTERRUPTS;
  static const target_ulong delegable_excps =
@@ -416,6 +417,14 @@ static const target_ulong delegable_excps =
  (1ULL << (RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT)) |
  (1ULL << (RISCV_EXCP_VIRT_INSTRUCTION_FAULT)) |
  (1ULL << (RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT));
+static const target_ulong vs_delegable_excps = delegable_excps &
+~((1ULL << (RISCV_EXCP_S_ECALL)) |
+(1ULL << (RISCV_EXCP_VS_ECALL)) |
+(1ULL << (RISCV_EXCP_M_ECALL)) |
+(1ULL << (RISCV_EXCP_INST_GUEST_PAGE_FAULT)) |
+(1ULL << (RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT)) |
+(1ULL << (RISCV_EXCP_VIRT_INSTRUCTION_FAULT)) |
+(1ULL << (RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT)));
  static const target_ulong sstatus_v1_10_mask = SSTATUS_SIE | SSTATUS_SPIE |
  SSTATUS_UIE | SSTATUS_UPIE | SSTATUS_SPP | SSTATUS_FS | SSTATUS_XS |
  SSTATUS_SUM | SSTATUS_MXR | SSTATUS_SD;
@@ -963,7 +972,7 @@ static int read_hedeleg(CPURISCVState *env, int csrno, 
target_ulong *val)
  
  static int write_hedeleg(CPURISCVState *env, int csrno, target_ulong val)

  {
-env->hedeleg = val;
+env->hedeleg = val & vs_delegable_excps;
  return 0;
  }
  
@@ -975,7 +984,7 @@ static int read_hideleg(CPURISCVState *env, int csrno, target_ulong *val)
  
  static int write_hideleg(CPURISCVState *env, int csrno, target_ulong val)

  {
-env->hideleg = val;
+env->hideleg = val & vs_delegable_ints;
  return 0;
  }
  




Re: [PATCH] target/riscv: hardwire bits in hideleg and hedeleg

2021-05-27 Thread LIU Zhiwei



On 5/26/21 1:50 AM, Jose Martins wrote:

We can use it directly if only one macro VS_MODE_INTERRUPTS.

I wrote it like this to be more coherent with what was already there
which also makes it more readable. Furthermore, the compiler will just
probably optimize the variable away, right?


Hi Jose,

Sorry I missed this mail.

Sounds good. Just keep it.




I didn't find that the RISCV_EXCP_VS_ECALL should be read-only 0 from the 
specification.

You are right. I had doubts about this also. The table that defines it
in the spec is missing this bit. I raised an issue on the spec repo
(https://github.com/riscv/riscv-isa-manual/issues/649). But in my
opinion, it wouldn't really make sense to allow this exception to be
delegated.

Agree.

What do you think? Is there any use case for this to be
allowed? Maybe we'll need a clarification from the spec to reach a
final decision.

That's will be great.



However, as hedeleg is WARL, you had better reserve the other fields like 
medeleg:

env->medeleg = (env->medeleg & ~delegable_excps) | (val & delegable_excps);

Isn't the patch's implementation of hedeleg/hideleg providing a WARL
behavior already? I don't think we need this preservation behavior
since in the case of hideleg/hedeleg there can only be 0-wired bits. I
believe this won't change. For hedeleg the spec states that  "Each bit
of hedeleg shall be either writable or hardwired to zero". For
hideleg: "Among bits 15:0 of hideleg, only bits 10, 6, and 2
(corresponding to the standard VS-level interrupts) shall be writable,
and the others shall be hardwired to zero."

Agree.



I really don't understand why medeleg codes this way. Is there anyone can give 
a better explanation?

I don't know if I fully understood your question, but I don't get why
you would need to preserve non-delegable bits in medeleg in this way,
even to keep WARL behavior.

  Again, the specification only allows
medeleg bits to be hardwired to zero: "An implementation shall not
hardwire any bits of medeleg to one, i.e., any synchronous trap that
can be delegated must support not being delegated.", so a bitwise-and
should suffice.


That's the current way to implement medeleg in QEMU. I just copy the code.

In my opinion, WARL means:

1) For writable fields with WARL, any illegal written value will be 
discarded.


2) For reserved fields with WARL,  any written value will be discarded 
and it should always keep hardwired value.


I agree with your opinion. We can just use bitwise-and for medeleg.

Best Regards,
Zhiwei


José




Re: [PATCH v2] Update libslirp to v4.5.0

2021-05-27 Thread Doug Evans
On Wed, May 19, 2021 at 8:18 AM  wrote:

> From: Marc-André Lureau 
>
> Switch from stable-4.2 branch to upstream v4.5.0 release.
>
> ## [4.5.0] - 2021-05-18
>
> ### Added
>
>  - IPv6 forwarding. !62 !75 !77
>  - slirp_neighbor_info() to dump the ARP/NDP tables. !71
>
> ### Changed
>
>  - Lazy guest address resolution for IPv6. !81
>  - Improve signal handling when spawning a child. !61
>  - Set macOS deployment target to macOS 10.4. !72
>  - slirp_add_hostfwd: Ensure all error paths set errno. !80
>  - More API documentation.
>
> ### Fixed
>
>  - Assertion failure on unspecified IPv6 address. !86
>  - Disable polling for PRI on MacOS, fixing some closing streams issues.
> !73
>  - Various memory leak fixes on fastq/batchq. !68
>  - Memory leak on IPv6 fast-send. !67
>  - Slow socket response on Windows. !64
>  - Misc build and code cleanups. !60 !63 !76 !79 !84
>
> ## [4.4.0] - 2020-12-02
>
> ### Added
>
>  - udp, udp6, icmp: handle TTL value. !48
>  - Enable forwarding ICMP errors. !49
>  - Add DNS resolving for iOS. !54
>
> ### Changed
>
>  - Improve meson subproject() support. !53
>  - Removed Makefile-based build system. !56
>
> ### Fixed
>
>  - socket: consume empty packets. !55
>  - check pkt_len before reading protocol header (CVE-2020-29129). !57
>  - ip_stripoptions use memmove (fixes undefined behaviour). !47
>  - various Coverity-related changes/fixes.
>
> ## [4.3.1] - 2020-07-08
>
> ### Changed
>
>  - A silent truncation could occur in `slirp_fmt()`, which will now print a
>critical message. See also #22.
>
> ### Fixed
>
>  - CVE-2020-10756 - Drop bogus IPv6 messages that could lead to data
> leakage.
>See !44 and !42.
>  - Fix win32 builds by using the SLIRP_PACKED definition.
>  - Various coverity scan errors fixed. !41
>  - Fix new GCC warnings. !43
>
> ## [4.3.0] - 2020-04-22
>
> ### Added
>
>  - `SLIRP_VERSION_STRING` macro, with the git sha suffix when building
> from git
>  - `SlirpConfig.disable_dns`, to disable DNS redirection #16
>
> ### Changed
>
>  - `slirp_version_string()` now has the git sha suffix when building form
> git
>  - Limit DNS redirection to port 53 #16
>
> ### Fixed
>
>  - Fix build regression with mingw & NetBSD
>  - Fix use-afte-free in `ip_reass()` (CVE-2020-1983)
>
> Signed-off-by: Marc-André Lureau 
>


Reviewed-by: Doug Evans < d...@google.com>



> ---
>  slirp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/slirp b/slirp
> index 8f43a99191..a62890e711 16
> --- a/slirp
> +++ b/slirp
> @@ -1 +1 @@
> -Subproject commit 8f43a99191afb47ca3f3c6972f6306209f367ece
> +Subproject commit a62890e71126795ca593affa747f669bed88e89c
> --
> 2.29.0
>
>


Re: [PATCH v3] target/riscv: fix VS interrupts forwarding to HS

2021-05-27 Thread LIU Zhiwei



On 5/28/21 6:34 AM, Alistair Francis wrote:

On Sun, May 23, 2021 at 1:45 AM Jose Martins  wrote:

VS interrupts (2, 6, 10) were not correctly forwarded to hs-mode when
not delegated in hideleg (which was not being taken into account). This
was mainly because hs level sie was not always considered enabled when
it should. The spec states that "Interrupts for higher-privilege modes,
y>x, are always globally enabled regardless of the setting of the global
yIE bit for the higher-privilege mode." and also "For purposes of
interrupt global enables, HS-mode is considered more privileged than
VS-mode, and VS-mode is considered more privileged than VU-mode".

These interrupts should be treated the same as any other kind of
exception. Therefore, there is no need to "force an hs exception" as the
current privilege level, the state of the global ie and of the
delegation registers should be enough to route the interrupt to the
appropriate privilege level in riscv_cpu_do_interrupt. Also, these
interrupts never target m-mode, which is  guaranteed by the hardwiring
of the corresponding bits in mideleg. The same is true for synchronous
exceptions, specifically, guest page faults which must be hardwired in
to zero hedeleg. As such the hs_force_except mechanism can be removed.

Signed-off-by: Jose Martins 

This looks right to me, but this was one of the most confusing parts
of the implementation. I also don't think the patch is too long as
it's mostly just deleting stuff.

I don't fully understand your concerns Zhiwei, do you mind stating them again?


Hi Alistair,

My main concern is the commit message is to complex.

I also have a question why force hs exception in current code.
Then we can give a brief commit message.

Best Regards,
Zhiwei



Alistair


---
This version of the patch also removes the uneeded hs_force_except
functions, variables and macro.

  target/riscv/cpu.h|  2 --
  target/riscv/cpu_bits.h   |  6 -
  target/riscv/cpu_helper.c | 54 +++
  3 files changed, 9 insertions(+), 53 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 0a33d387ba..a30a64241a 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -337,8 +337,6 @@ bool riscv_cpu_exec_interrupt(CPUState *cs, int 
interrupt_request);
  bool riscv_cpu_fp_enabled(CPURISCVState *env);
  bool riscv_cpu_virt_enabled(CPURISCVState *env);
  void riscv_cpu_set_virt_enabled(CPURISCVState *env, bool enable);
-bool riscv_cpu_force_hs_excep_enabled(CPURISCVState *env);
-void riscv_cpu_set_force_hs_excep(CPURISCVState *env, bool enable);
  bool riscv_cpu_two_stage_lookup(int mmu_idx);
  int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch);
  hwaddr riscv_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index caf4599207..7322f54157 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -462,12 +462,6 @@

  /* Virtulisation Register Fields */
  #define VIRT_ONOFF  1
-/* This is used to save state for when we take an exception. If this is set
- * that means that we want to force a HS level exception (no matter what the
- * delegation is set to). This will occur for things such as a second level
- * page table fault.
- */
-#define FORCE_HS_EXCEP  2

  /* RV32 satp CSR field masks */
  #define SATP32_MODE 0x8000
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 21c54ef561..babe3d844b 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -38,36 +38,24 @@ int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch)
  #ifndef CONFIG_USER_ONLY
  static int riscv_cpu_local_irq_pending(CPURISCVState *env)
  {
-target_ulong irqs;
+target_ulong virt_enabled = riscv_cpu_virt_enabled(env);

  target_ulong mstatus_mie = get_field(env->mstatus, MSTATUS_MIE);
  target_ulong mstatus_sie = get_field(env->mstatus, MSTATUS_SIE);
-target_ulong hs_mstatus_sie = get_field(env->mstatus_hs, MSTATUS_SIE);

-target_ulong pending = env->mip & env->mie &
-   ~(MIP_VSSIP | MIP_VSTIP | MIP_VSEIP);
-target_ulong vspending = (env->mip & env->mie &
-  (MIP_VSSIP | MIP_VSTIP | MIP_VSEIP));
+target_ulong pending = env->mip & env->mie;

  target_ulong mie= env->priv < PRV_M ||
(env->priv == PRV_M && mstatus_mie);
  target_ulong sie= env->priv < PRV_S ||
(env->priv == PRV_S && mstatus_sie);
-target_ulong hs_sie = env->priv < PRV_S ||
-  (env->priv == PRV_S && hs_mstatus_sie);
+target_ulong hsie   = virt_enabled || sie;
+target_ulong vsie   = virt_enabled && sie;

-if (riscv_cpu_virt_enabled(env)) {
-target_ulong pending_hs_irq = pending & -hs_sie;
-
-if (pending_hs_irq) {
-riscv_cpu_set_force_hs_excep(env, FORCE_HS_EXCEP);
-return 

Re: [PATCH 1/1] target/riscv: Fix vsip vsie CSR ops in M and HS mode

2021-05-27 Thread LIU Zhiwei



On 5/28/21 6:19 AM, Alistair Francis wrote:

On Thu, May 27, 2021 at 7:01 PM LIU Zhiwei  wrote:

When V=1, instructions that normally read or modify a supervisor CSR
shall instead access the corresponding VS CSR. And the VS CSRs can be
accessed as themselves from M-mode or HS-mode.

In M and HS mode, VSIP or VSIE should be written normally instead of
shift by 1.

Signed-off-by: LIU Zhiwei 
---
  target/riscv/csr.c | 19 ++-
  1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index fe5628fea6..0cce474d3d 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -837,16 +837,16 @@ static RISCVException read_sie(CPURISCVState *env, int 
csrno,
  static RISCVException write_vsie(CPURISCVState *env, int csrno,
   target_ulong val)
  {
-/* Shift the S bits to their VS bit location in mie */
  target_ulong newval = (env->mie & ~VS_MODE_INTERRUPTS) |
-  ((val << 1) & env->hideleg & VS_MODE_INTERRUPTS);
+  (val & env->hideleg & VS_MODE_INTERRUPTS);

Ok, so when a Hypervisor writes to vsie it should actually set SSIE
instead of VSSIE.


Hi Alistair,

Thanks for your reply.
I think when HS or M mode writes to vsie, it should set VSSIE instead of 
SSIE.


Best Regards,
Zhiwei


That looks right.

The problem here now is that you are still using the VS mask, so this
won't set anything.


  return write_mie(env, CSR_MIE, newval);
  }

  static int write_sie(CPURISCVState *env, int csrno, target_ulong val)
  {
  if (riscv_cpu_virt_enabled(env)) {
-write_vsie(env, CSR_VSIE, val);
+/* Shift the S bits to their VS bit location in mie */
+write_vsie(env, CSR_VSIE, val << 1);

A write to SIE when virtulised actually sets VSIE, sounds good.


  } else {
  target_ulong newval = (env->mie & ~S_MODE_INTERRUPTS) |
(val & S_MODE_INTERRUPTS);
@@ -950,12 +950,9 @@ static RISCVException rmw_vsip(CPURISCVState *env, int 
csrno,
 target_ulong *ret_value,
 target_ulong new_value, target_ulong 
write_mask)
  {
-/* Shift the S bits to their VS bit location in mip */
-int ret = rmw_mip(env, 0, ret_value, new_value << 1,
-  (write_mask << 1) & vsip_writable_mask & env->hideleg);
+int ret = rmw_mip(env, 0, ret_value, new_value,
+  write_mask & vsip_writable_mask & env->hideleg);

The mask seems wrong here as well.

Alistair


  *ret_value &= VS_MODE_INTERRUPTS;
-/* Shift the VS bits to their S bit location in vsip */
-*ret_value >>= 1;
  return ret;
  }

@@ -966,7 +963,11 @@ static RISCVException rmw_sip(CPURISCVState *env, int 
csrno,
  int ret;

  if (riscv_cpu_virt_enabled(env)) {
-ret = rmw_vsip(env, CSR_VSIP, ret_value, new_value, write_mask);
+/* Shift the S bits to their VS bit location in mip */
+ret = rmw_vsip(env, CSR_VSIP, ret_value, new_value << 1,
+   write_mask << 1);
+/* Shift the VS bits to their S bit location in vsip */
+*ret_value >>= 1;
  } else {
  ret = rmw_mip(env, CSR_MSTATUS, ret_value, new_value,
write_mask & env->mideleg & sip_writable_mask);
--
2.25.1






Performance issue with qcow2/raid

2021-05-27 Thread Jose R. Ziviani
Hello team,

I'm currently investigating a performance regression detected by iozone 
filesystem benchmark (https://www.iozone.org/).

Basically, if I format a QCOW2 image with XFS filesystem in my guest and run 
iozone I'll get the following result:

$ mkfs.xfs -f /dev/xvdb1 && \
  mount -t xfs /dev/xvdb1 /mnt && \
  /opt/iozone/bin/iozone -a -e -s 16777216 -y 4 -q 8 -i 0 -i 1 -f 
/mnt/iozone.dat

kBblock len  read reread
16777216  4K 354790   348796
16777216  8K 362356   364818

However, if I revert the commit 46cd1e8a47 (qcow2: Skip copy-on-write when 
allocating a zero cluster) and run the same, I see a huge improvement:

$ mkfs.xfs -f /dev/xvdb1 && \
  mount -t xfs /dev/xvdb1 /mnt && \
  /opt/iozone/bin/iozone -a -e -s 16777216 -y 4 -q 8 -i 0 -i 1 -f 
/mnt/iozone.dat

kBblock len  read reread
16777216  4K 524067   560057
16777216  8K 538661   537004

Note that if I run iozone without re-formating the disk, I'll get results 
similar to last formatting. In other words, if I my current QEMU executable 
doesn't have commit 46cd1e8a47 and I format the disk, iozone will continue 
showing good results even if I reboot to use QEMU with that commit patched.

My system has a RAID controller[1] and runs QEMU/Xen. I'm not able to reproduce 
such behavior in other systems. 

Do you have any suggestion to help debugging this? What more info could help to 
understand it better?
My next approach is using perf, but I appreciate if you have any hints measure 
qcow efficiently.

[1]
# lspci -vv | grep -i raid
1a:00.0 RAID bus controller: Broadcom / LSI MegaRAID SAS-3 3108 [Invader] (rev 
02)
Kernel driver in use: megaraid_sas
Kernel modules: megaraid_sas

Thank you very much!

Jose R. Ziviani


signature.asc
Description: Digital signature


Re: [PATCH v3] target/riscv: fix VS interrupts forwarding to HS

2021-05-27 Thread Alistair Francis
On Sun, May 23, 2021 at 1:45 AM Jose Martins  wrote:
>
> VS interrupts (2, 6, 10) were not correctly forwarded to hs-mode when
> not delegated in hideleg (which was not being taken into account). This
> was mainly because hs level sie was not always considered enabled when
> it should. The spec states that "Interrupts for higher-privilege modes,
> y>x, are always globally enabled regardless of the setting of the global
> yIE bit for the higher-privilege mode." and also "For purposes of
> interrupt global enables, HS-mode is considered more privileged than
> VS-mode, and VS-mode is considered more privileged than VU-mode".
>
> These interrupts should be treated the same as any other kind of
> exception. Therefore, there is no need to "force an hs exception" as the
> current privilege level, the state of the global ie and of the
> delegation registers should be enough to route the interrupt to the
> appropriate privilege level in riscv_cpu_do_interrupt. Also, these
> interrupts never target m-mode, which is  guaranteed by the hardwiring
> of the corresponding bits in mideleg. The same is true for synchronous
> exceptions, specifically, guest page faults which must be hardwired in
> to zero hedeleg. As such the hs_force_except mechanism can be removed.
>
> Signed-off-by: Jose Martins 

This looks right to me, but this was one of the most confusing parts
of the implementation. I also don't think the patch is too long as
it's mostly just deleting stuff.

I don't fully understand your concerns Zhiwei, do you mind stating them again?

Alistair

> ---
> This version of the patch also removes the uneeded hs_force_except
> functions, variables and macro.
>
>  target/riscv/cpu.h|  2 --
>  target/riscv/cpu_bits.h   |  6 -
>  target/riscv/cpu_helper.c | 54 +++
>  3 files changed, 9 insertions(+), 53 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 0a33d387ba..a30a64241a 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -337,8 +337,6 @@ bool riscv_cpu_exec_interrupt(CPUState *cs, int 
> interrupt_request);
>  bool riscv_cpu_fp_enabled(CPURISCVState *env);
>  bool riscv_cpu_virt_enabled(CPURISCVState *env);
>  void riscv_cpu_set_virt_enabled(CPURISCVState *env, bool enable);
> -bool riscv_cpu_force_hs_excep_enabled(CPURISCVState *env);
> -void riscv_cpu_set_force_hs_excep(CPURISCVState *env, bool enable);
>  bool riscv_cpu_two_stage_lookup(int mmu_idx);
>  int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch);
>  hwaddr riscv_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index caf4599207..7322f54157 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -462,12 +462,6 @@
>
>  /* Virtulisation Register Fields */
>  #define VIRT_ONOFF  1
> -/* This is used to save state for when we take an exception. If this is set
> - * that means that we want to force a HS level exception (no matter what the
> - * delegation is set to). This will occur for things such as a second level
> - * page table fault.
> - */
> -#define FORCE_HS_EXCEP  2
>
>  /* RV32 satp CSR field masks */
>  #define SATP32_MODE 0x8000
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 21c54ef561..babe3d844b 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -38,36 +38,24 @@ int riscv_cpu_mmu_index(CPURISCVState *env, bool ifetch)
>  #ifndef CONFIG_USER_ONLY
>  static int riscv_cpu_local_irq_pending(CPURISCVState *env)
>  {
> -target_ulong irqs;
> +target_ulong virt_enabled = riscv_cpu_virt_enabled(env);
>
>  target_ulong mstatus_mie = get_field(env->mstatus, MSTATUS_MIE);
>  target_ulong mstatus_sie = get_field(env->mstatus, MSTATUS_SIE);
> -target_ulong hs_mstatus_sie = get_field(env->mstatus_hs, MSTATUS_SIE);
>
> -target_ulong pending = env->mip & env->mie &
> -   ~(MIP_VSSIP | MIP_VSTIP | MIP_VSEIP);
> -target_ulong vspending = (env->mip & env->mie &
> -  (MIP_VSSIP | MIP_VSTIP | MIP_VSEIP));
> +target_ulong pending = env->mip & env->mie;
>
>  target_ulong mie= env->priv < PRV_M ||
>(env->priv == PRV_M && mstatus_mie);
>  target_ulong sie= env->priv < PRV_S ||
>(env->priv == PRV_S && mstatus_sie);
> -target_ulong hs_sie = env->priv < PRV_S ||
> -  (env->priv == PRV_S && hs_mstatus_sie);
> +target_ulong hsie   = virt_enabled || sie;
> +target_ulong vsie   = virt_enabled && sie;
>
> -if (riscv_cpu_virt_enabled(env)) {
> -target_ulong pending_hs_irq = pending & -hs_sie;
> -
> -if (pending_hs_irq) {
> -riscv_cpu_set_force_hs_excep(env, FORCE_HS_EXCEP);
> -return ctz64(pending_hs_irq);
> -}
> -
> -pending = vspending;
> -}
> -
> -irqs = 

Re: [PATCH 1/1] target/riscv: Fix vsip vsie CSR ops in M and HS mode

2021-05-27 Thread Alistair Francis
On Thu, May 27, 2021 at 7:01 PM LIU Zhiwei  wrote:
>
> When V=1, instructions that normally read or modify a supervisor CSR
> shall instead access the corresponding VS CSR. And the VS CSRs can be
> accessed as themselves from M-mode or HS-mode.
>
> In M and HS mode, VSIP or VSIE should be written normally instead of
> shift by 1.
>
> Signed-off-by: LIU Zhiwei 
> ---
>  target/riscv/csr.c | 19 ++-
>  1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index fe5628fea6..0cce474d3d 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -837,16 +837,16 @@ static RISCVException read_sie(CPURISCVState *env, int 
> csrno,
>  static RISCVException write_vsie(CPURISCVState *env, int csrno,
>   target_ulong val)
>  {
> -/* Shift the S bits to their VS bit location in mie */
>  target_ulong newval = (env->mie & ~VS_MODE_INTERRUPTS) |
> -  ((val << 1) & env->hideleg & VS_MODE_INTERRUPTS);
> +  (val & env->hideleg & VS_MODE_INTERRUPTS);

Ok, so when a Hypervisor writes to vsie it should actually set SSIE
instead of VSSIE. That looks right.

The problem here now is that you are still using the VS mask, so this
won't set anything.

>  return write_mie(env, CSR_MIE, newval);
>  }
>
>  static int write_sie(CPURISCVState *env, int csrno, target_ulong val)
>  {
>  if (riscv_cpu_virt_enabled(env)) {
> -write_vsie(env, CSR_VSIE, val);
> +/* Shift the S bits to their VS bit location in mie */
> +write_vsie(env, CSR_VSIE, val << 1);

A write to SIE when virtulised actually sets VSIE, sounds good.

>  } else {
>  target_ulong newval = (env->mie & ~S_MODE_INTERRUPTS) |
>(val & S_MODE_INTERRUPTS);
> @@ -950,12 +950,9 @@ static RISCVException rmw_vsip(CPURISCVState *env, int 
> csrno,
> target_ulong *ret_value,
> target_ulong new_value, target_ulong 
> write_mask)
>  {
> -/* Shift the S bits to their VS bit location in mip */
> -int ret = rmw_mip(env, 0, ret_value, new_value << 1,
> -  (write_mask << 1) & vsip_writable_mask & env->hideleg);
> +int ret = rmw_mip(env, 0, ret_value, new_value,
> +  write_mask & vsip_writable_mask & env->hideleg);

The mask seems wrong here as well.

Alistair

>  *ret_value &= VS_MODE_INTERRUPTS;
> -/* Shift the VS bits to their S bit location in vsip */
> -*ret_value >>= 1;
>  return ret;
>  }
>
> @@ -966,7 +963,11 @@ static RISCVException rmw_sip(CPURISCVState *env, int 
> csrno,
>  int ret;
>
>  if (riscv_cpu_virt_enabled(env)) {
> -ret = rmw_vsip(env, CSR_VSIP, ret_value, new_value, write_mask);
> +/* Shift the S bits to their VS bit location in mip */
> +ret = rmw_vsip(env, CSR_VSIP, ret_value, new_value << 1,
> +   write_mask << 1);
> +/* Shift the VS bits to their S bit location in vsip */
> +*ret_value >>= 1;
>  } else {
>  ret = rmw_mip(env, CSR_MSTATUS, ret_value, new_value,
>write_mask & env->mideleg & sip_writable_mask);
> --
> 2.25.1
>
>



Re: [PATCH v6 00/17] support subsets of bitmanip extension

2021-05-27 Thread Alistair Francis
On Thu, May 6, 2021 at 2:11 AM  wrote:
>
> From: Frank Chang 
>
> This patchset implements RISC-V B-extension v0.93 version Zba, Zbb and
> Zbs subset instructions. Some Zbp instructions are also implemented as
> they have similar behavior with their Zba-, Zbb- and Zbs-family
> instructions or for Zbb pseudo instructions (e.g. rev8, orc.b).
>
> Specification:
> https://github.com/riscv/riscv-bitmanip/blob/master/bitmanip-0.93.pdf
>
> The port is available here:
> https://github.com/sifive/qemu/tree/rvb-upstream-v6
>
> To test rvb implementation, specify cpu argument with 'x-b=true' or
> 'x-b=true,bext_spec=v0.93'to enable B-extension support.
>
> Changelog:
>
> v6:
>  * rebase riscv-to-apply.next branch.
>  * remove all #ifdef TARGET_RISCV64 macros.
>
> v5:
>  * add bext_spec cpu option, default set to v0.93.
>  * rebase master branch.
>
> v4:
>  * Remove 'rd != 0' checks from immediate shift instructions.
>
> v3:
>  * Convert existing immediate shift instructions to use gen_shifti()
>and gen_shiftiw() interfaces.
>  * Rename *u.w instructions to *.uw.
>  * Rename sb* instructions to b*.
>  * Rename pcnt* instructions to cpop*.
>
> v2:
>  * Add gen_shifti(), gen_shiftw(), gen_shiftiw() helper functions.
>  * Remove addwu, subwu and addiwu instructions as they are not longer
>exist in latest draft.
>  * Optimize implementation with cleaner tcg ops.
>
> Frank Chang (6):
>   target/riscv: rvb: count bits set
>   target/riscv: add gen_shifti() and gen_shiftiw() helper functions
>   target/riscv: rvb: single-bit instructions
>   target/riscv: rvb: generalized reverse
>   target/riscv: rvb: generalized or-combine
>   target/riscv: rvb: add b-ext version cpu option
>
> Kito Cheng (11):
>   target/riscv: reformat @sh format encoding for B-extension
>   target/riscv: rvb: count leading/trailing zeros
>   target/riscv: rvb: logic-with-negate
>   target/riscv: rvb: pack two words into one register
>   target/riscv: rvb: min/max instructions
>   target/riscv: rvb: sign-extend instructions
>   target/riscv: rvb: shift ones
>   target/riscv: rvb: rotate (left/right)
>   target/riscv: rvb: address calculation
>   target/riscv: rvb: add/shift with prefix zero-extend
>   target/riscv: rvb: support and turn on B-extension from command line

Thanks!

Applied to riscv-to-apply.next

Alistair

>
>  target/riscv/bitmanip_helper.c  |  90 +
>  target/riscv/cpu.c  |  27 ++
>  target/riscv/cpu.h  |   5 +
>  target/riscv/helper.h   |   6 +
>  target/riscv/insn32.decode  |  87 -
>  target/riscv/insn_trans/trans_rvb.c.inc | 438 
>  target/riscv/insn_trans/trans_rvi.c.inc |  54 +--
>  target/riscv/meson.build|   1 +
>  target/riscv/translate.c| 306 +
>  9 files changed, 958 insertions(+), 56 deletions(-)
>  create mode 100644 target/riscv/bitmanip_helper.c
>  create mode 100644 target/riscv/insn_trans/trans_rvb.c.inc
>
> --
> 2.17.1
>
>



Re: [PATCH v6 17/17] target/riscv: rvb: add b-ext version cpu option

2021-05-27 Thread Alistair Francis
On Thu, May 6, 2021 at 2:27 AM  wrote:
>
> From: Frank Chang 
>
> Default b-ext version is v0.93.
>
> Signed-off-by: Frank Chang 

Reviewed-by: Alistair Francis 

Alistair

> ---
>  target/riscv/cpu.c | 23 +++
>  target/riscv/cpu.h |  3 +++
>  2 files changed, 26 insertions(+)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 1b3c5ba1480..32469f7c891 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -127,6 +127,11 @@ static void set_priv_version(CPURISCVState *env, int 
> priv_ver)
>  env->priv_ver = priv_ver;
>  }
>
> +static void set_bext_version(CPURISCVState *env, int bext_ver)
> +{
> +env->bext_ver = bext_ver;
> +}
> +
>  static void set_vext_version(CPURISCVState *env, int vext_ver)
>  {
>  env->vext_ver = vext_ver;
> @@ -385,6 +390,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
> **errp)
>  CPURISCVState *env = >env;
>  RISCVCPUClass *mcc = RISCV_CPU_GET_CLASS(dev);
>  int priv_version = PRIV_VERSION_1_11_0;
> +int bext_version = BEXT_VERSION_0_93_0;
>  int vext_version = VEXT_VERSION_0_07_1;
>  target_ulong target_misa = env->misa;
>  Error *local_err = NULL;
> @@ -409,6 +415,7 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
> **errp)
>  }
>
>  set_priv_version(env, priv_version);
> +set_bext_version(env, bext_version);
>  set_vext_version(env, vext_version);
>
>  if (cpu->cfg.mmu) {
> @@ -488,6 +495,21 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
> **errp)
>  }
>  if (cpu->cfg.ext_b) {
>  target_misa |= RVB;
> +
> +if (cpu->cfg.bext_spec) {
> +if (!g_strcmp0(cpu->cfg.bext_spec, "v0.93")) {
> +bext_version = BEXT_VERSION_0_93_0;
> +} else {
> +error_setg(errp,
> +   "Unsupported bitmanip spec version '%s'",
> +   cpu->cfg.bext_spec);
> +return;
> +}
> +} else {
> +qemu_log("bitmanip version is not specified, "
> + "use the default value v0.93\n");
> +}
> +set_bext_version(env, bext_version);
>  }
>  if (cpu->cfg.ext_v) {
>  target_misa |= RVV;
> @@ -566,6 +588,7 @@ static Property riscv_cpu_properties[] = {
>  DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true),
>  DEFINE_PROP_BOOL("Zicsr", RISCVCPU, cfg.ext_icsr, true),
>  DEFINE_PROP_STRING("priv_spec", RISCVCPU, cfg.priv_spec),
> +DEFINE_PROP_STRING("bext_spec", RISCVCPU, cfg.bext_spec),
>  DEFINE_PROP_STRING("vext_spec", RISCVCPU, cfg.vext_spec),
>  DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
>  DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 3cea62cd4c4..b2cca778526 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -83,6 +83,7 @@ enum {
>  #define PRIV_VERSION_1_10_0 0x00011000
>  #define PRIV_VERSION_1_11_0 0x00011100
>
> +#define BEXT_VERSION_0_93_0 0x9300
>  #define VEXT_VERSION_0_07_1 0x0701
>
>  enum {
> @@ -130,6 +131,7 @@ struct CPURISCVState {
>  target_ulong guest_phys_fault_addr;
>
>  target_ulong priv_ver;
> +target_ulong bext_ver;
>  target_ulong vext_ver;
>  target_ulong misa;
>  target_ulong misa_mask;
> @@ -295,6 +297,7 @@ struct RISCVCPU {
>
>  char *priv_spec;
>  char *user_spec;
> +char *bext_spec;
>  char *vext_spec;
>  uint16_t vlen;
>  uint16_t elen;
> --
> 2.17.1
>
>



Re: [PATCH 0/2] [RESEND] SEV firmware error list touchups

2021-05-27 Thread Philippe Mathieu-Daudé
ping^3...

On Tue, May 11, 2021 at 9:35 AM Philippe Mathieu-Daudé
 wrote:
>
> Cc'ing qemu-trivial@
>
> On 4/30/21 3:48 PM, Connor Kuehl wrote:
> > Connor Kuehl (2):
> >   sev: use explicit indices for mapping firmware error codes to strings
> >   sev: add missing firmware error conditions
> >
> >  target/i386/sev.c | 48 ---
> >  1 file changed, 25 insertions(+), 23 deletions(-)
> >
>




[PATCH v8 31/31] gitlab: add python linters to CI

2021-05-27 Thread John Snow
Add a Python container that has just enough juice for us to run the
Python code quality analysis tools. Base this container on Fedora,
because Fedora has very convenient packaging for testing multiple Python
versions.

We need python3, pip (for pulling packages), pipenv and virtualenv for
creating virtual environments, and tox for running tests. make is needed
for running 'make check-tox' and 'make venv-check' targets. Python3.10
is needed explicitly because the tox package only pulls in 3.6-3.9, but
we wish to test the forthcoming release of Python as well to help
predict any problems. Lastly, we need gcc to compile PyPI packages that
may not have a binary distribution available.


Add two tests:

check-python-pipenv uses pipenv to test a frozen, very explicit set of
packages against our minimum supported python version, Python 3.6. This
test is not allowed to fail. The dependencies this test uses do not
change unless python/Pipfile.lock is changed.

check-python-tox uses tox to install the latest versions of required
python dependencies against a wide array of Python versions from 3.6 to
3.9, even including the yet-to-be-released Python 3.10. This test is
allowed to fail with a warning.

Signed-off-by: John Snow 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Alex Bennée 
Reviewed-by: Cleber Rosa 
---
 .gitlab-ci.d/containers.yml|  5 +
 .gitlab-ci.yml | 24 
 tests/docker/dockerfiles/python.docker | 18 ++
 3 files changed, 47 insertions(+)
 create mode 100644 tests/docker/dockerfiles/python.docker

diff --git a/.gitlab-ci.d/containers.yml b/.gitlab-ci.d/containers.yml
index 7b7ca3790df..9247613030d 100644
--- a/.gitlab-ci.d/containers.yml
+++ b/.gitlab-ci.d/containers.yml
@@ -269,3 +269,8 @@ amd64-opensuse-leap-container:
   extends: .container_job_template
   variables:
 NAME: opensuse-leap
+
+python-container:
+  extends: .container_job_template
+  variables:
+NAME: python
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 6a0d311cf40..181a55c84ed 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -792,6 +792,30 @@ check-patch:
 GIT_DEPTH: 1000
   allow_failure: true
 
+
+check-python-pipenv:
+  stage: test
+  image: $CI_REGISTRY_IMAGE/qemu/python:latest
+  script:
+- make -C python venv-check
+  variables:
+GIT_DEPTH: 1
+  needs:
+job: python-container
+
+
+check-python-tox:
+  stage: test
+  image: $CI_REGISTRY_IMAGE/qemu/python:latest
+  script:
+- make -C python check-tox
+  variables:
+GIT_DEPTH: 1
+  needs:
+job: python-container
+  allow_failure: true
+
+
 check-dco:
   stage: build
   image: $CI_REGISTRY_IMAGE/qemu/centos8:latest
diff --git a/tests/docker/dockerfiles/python.docker 
b/tests/docker/dockerfiles/python.docker
new file mode 100644
index 000..56d88417df4
--- /dev/null
+++ b/tests/docker/dockerfiles/python.docker
@@ -0,0 +1,18 @@
+# Python library testing environment
+
+FROM fedora:latest
+MAINTAINER John Snow 
+
+# Please keep this list sorted alphabetically
+ENV PACKAGES \
+gcc \
+make \
+pipenv \
+python3 \
+python3-pip \
+python3-tox \
+python3-virtualenv \
+python3.10
+
+RUN dnf install -y $PACKAGES
+RUN rpm -q $PACKAGES | sort > /packages.txt
-- 
2.31.1




[PATCH v8 26/31] python: add devel package requirements to setuptools

2021-05-27 Thread John Snow
setuptools doesn't have a formal understanding of development requires,
but it has an optional feataures section. Fine; add a "devel" feature
and add the requirements to it.

To avoid duplication, we can modify pipenv to install qemu[devel]
instead. This enables us to run invocations like "pip install -e
.[devel]" and test the package on bleeding-edge packages beyond those
specified in Pipfile.lock.

Importantly, this also allows us to install the qemu development
packages in a non-networked mode: `pip3 install --no-index -e .[devel]`
will now fail if the proper development dependencies are not already
met. This can be useful for automated build scripts where fetching
network packages may be undesirable.

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
---
 python/PACKAGE.rst  |  4 
 python/README.rst   |  4 
 python/Pipfile  |  5 +
 python/Pipfile.lock | 14 +-
 python/setup.cfg|  9 +
 5 files changed, 27 insertions(+), 9 deletions(-)

diff --git a/python/PACKAGE.rst b/python/PACKAGE.rst
index 1bbfe1b58e2..05ea7789fc1 100644
--- a/python/PACKAGE.rst
+++ b/python/PACKAGE.rst
@@ -31,3 +31,7 @@ official `GitLab mirror 
`_.
 Please report bugs on the `QEMU issue tracker
 `_ and tag ``@jsnow`` in
 the report.
+
+Optional packages necessary for running code quality analysis for this
+package can be installed with the optional dependency group "devel":
+``pip install qemu[devel]``.
diff --git a/python/README.rst b/python/README.rst
index bf9bbca979a..954870973d0 100644
--- a/python/README.rst
+++ b/python/README.rst
@@ -24,6 +24,10 @@ which installs a version of the package that installs a 
forwarder
 pointing to these files, such that the package always reflects the
 latest version in your git tree.
 
+Installing ".[devel]" instead of "." will additionally pull in required
+packages for testing this package. They are not runtime requirements,
+and are not needed to simply use these libraries.
+
 See `Installing packages using pip and virtual environments
 
`_
 for more information.
diff --git a/python/Pipfile b/python/Pipfile
index dbe96f71c48..e7acb8cefa4 100644
--- a/python/Pipfile
+++ b/python/Pipfile
@@ -4,10 +4,7 @@ url = "https://pypi.org/simple;
 verify_ssl = true
 
 [dev-packages]
-flake8 = ">=3.6.0"
-isort = ">=5.1.2"
-mypy = ">=0.770"
-pylint = ">=2.8.0"
+qemu = {editable = true, extras = ["devel"], path = "."}
 
 [packages]
 qemu = {editable = true,path = "."}
diff --git a/python/Pipfile.lock b/python/Pipfile.lock
index f0bf576c31e..a2cdc1c50ea 100644
--- a/python/Pipfile.lock
+++ b/python/Pipfile.lock
@@ -1,7 +1,7 @@
 {
 "_meta": {
 "hash": {
-"sha256": 
"7c74cc4c2db3a75c954a6686411cda6fd60e464620bb6d5f1ed9a54be61db4cc"
+"sha256": 
"eff562a688ebc6f3ffe67494dbb804b883e2159ad81c4d55d96da9f7aec13e91"
 },
 "pipfile-spec": 6,
 "requires": {
@@ -35,7 +35,7 @@
 
"sha256:07528381786f2a6237b061f6e96610a4167b226cb926e2aa2b6b1d78057c576b",
 
"sha256:bf8fd46d844f616e8d47905ef3a3384edae6b4e9beb0c5101e25e3110907"
 ],
-"index": "pypi",
+"markers": "python_version >= '2.7' and python_version not in 
'3.0, 3.1, 3.2, 3.3, 3.4'",
 "version": "==3.9.2"
 },
 "importlib-metadata": {
@@ -51,7 +51,7 @@
 
"sha256:0a943902919f65c5684ac4e0154b1ad4fac6dcaa5d9f3426b732f1c8b5419be6",
 
"sha256:2bb1680aad211e3c9944dbce1d4ba09a989f04e238296c87fe2139faa26d655d"
 ],
-"index": "pypi",
+"markers": "python_version >= '3.6' and python_version < '4.0'",
 "version": "==5.8.0"
 },
 "lazy-object-proxy": {
@@ -114,7 +114,7 @@
 
"sha256:d65cc1df038ef55a99e617431f0553cd77763869eebdf9042403e16089fe746c",
 
"sha256:d7da2e1d5f558c37d6e8c1246f1aec1e7349e4913d8fb3cb289a35de573fe2eb"
 ],
-"index": "pypi",
+"markers": "python_version >= '3.5'",
 "version": "==0.812"
 },
 "mypy-extensions": {
@@ -145,9 +145,13 @@
 
"sha256:586d8fa9b1891f4b725f587ef267abe2a1bad89d6b184520c7f07a253dd6e217",
 
"sha256:f7e2072654a6b6afdf5e2fb38147d3e2d2d43c89f648637baab63e026481279b"
 ],
-"index": "pypi",
+"markers": "python_version ~= '3.6'",
 "version": "==2.8.2"
 },
+"qemu": {
+"editable": true,
+"path": "."
+},
 "toml": {
 "hashes": [
 
"sha256:806143ae5bfb6a3c6e736a764057db0e6a0e05e338b5630894a5f779cabb4f9b",
diff --git a/python/setup.cfg b/python/setup.cfg
index 3f07bd2752d..39dc135e601 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -22,6 

[PATCH v8 29/31] python: add .gitignore

2021-05-27 Thread John Snow
Ignore *Python* build and package output (build, dist, qemu.egg-info);
these files are not created as part of a QEMU build. They are created by
running the commands 'python3 setup.py ' when preparing
tarballs to upload to e.g. PyPI.

Ignore miscellaneous cached python confetti (mypy, pylint, et al)

Ignore .idea (pycharm) .vscode, and .venv (pipenv et al).

Signed-off-by: John Snow 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Cleber Rosa 
---
 python/.gitignore | 15 +++
 1 file changed, 15 insertions(+)
 create mode 100644 python/.gitignore

diff --git a/python/.gitignore b/python/.gitignore
new file mode 100644
index 000..4ed144ceac3
--- /dev/null
+++ b/python/.gitignore
@@ -0,0 +1,15 @@
+# linter/tooling cache
+.mypy_cache/
+.cache/
+
+# python packaging
+build/
+dist/
+qemu.egg-info/
+
+# editor config
+.idea/
+.vscode/
+
+# virtual environments (pipenv et al)
+.venv/
-- 
2.31.1




[PATCH v8 24/31] python/qemu: add isort to pipenv

2021-05-27 Thread John Snow
isort 5.0.0 through 5.0.4 has a bug that causes it to misinterpret
certain "from ..." clauses that are not related to imports.

isort < 5.1.1 has a bug where it does not handle comments near import
statements correctly.

Require 5.1.2 or greater.

isort can be run (in "check" mode) with 'isort -c qemu' from the python
root. isort can also be used to fix/rewrite import order automatically
by using 'isort qemu'.

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
---
 python/Pipfile  | 1 +
 python/Pipfile.lock | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/python/Pipfile b/python/Pipfile
index 796c6282e17..79c74dd8db4 100644
--- a/python/Pipfile
+++ b/python/Pipfile
@@ -5,6 +5,7 @@ verify_ssl = true
 
 [dev-packages]
 flake8 = ">=3.6.0"
+isort = ">=5.1.2"
 mypy = ">=0.770"
 pylint = ">=2.8.0"
 
diff --git a/python/Pipfile.lock b/python/Pipfile.lock
index 626e68403f7..57a5befb104 100644
--- a/python/Pipfile.lock
+++ b/python/Pipfile.lock
@@ -1,7 +1,7 @@
 {
 "_meta": {
 "hash": {
-"sha256": 
"14d171b3d86759e1fdfb9e55f66be4a696b6afa8f627d6c4778f8398c6a66b98"
+"sha256": 
"8173290ad57aab0b722c9b4f109519de4e0dd7cd1bad1e16806b78bb169bce08"
 },
 "pipfile-spec": 6,
 "requires": {
@@ -46,7 +46,7 @@
 
"sha256:0a943902919f65c5684ac4e0154b1ad4fac6dcaa5d9f3426b732f1c8b5419be6",
 
"sha256:2bb1680aad211e3c9944dbce1d4ba09a989f04e238296c87fe2139faa26d655d"
 ],
-"markers": "python_version >= '3.6' and python_version < '4.0'",
+"index": "pypi",
 "version": "==5.8.0"
 },
 "lazy-object-proxy": {
-- 
2.31.1




[PATCH v8 22/31] python: add mypy to pipenv

2021-05-27 Thread John Snow
0.730 appears to be about the oldest version that works with the
features we want, including nice human readable output (to make sure
iotest 297 passes), and type-parameterized Popen generics.

0.770, however, supports adding 'strict' to the config file, so require
at least 0.770.

Now that we are checking a namespace package, we need to tell mypy to
allow PEP420 namespaces, so modify the mypy config as part of the move.

mypy can now be run from the python root by typing 'mypy -p qemu'.

A note on mypy invocation: Running it as "mypy qemu/" changes the import
path detection mechanisms in mypy slightly, and it will fail. See
https://github.com/python/mypy/issues/8584 for a decent entry point with
more breadcrumbs on the various behaviors that contribute to this subtle
difference.

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
Tested-by: Cleber Rosa 
---
 python/Pipfile  |  1 +
 python/Pipfile.lock | 37 -
 python/setup.cfg|  1 +
 3 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/python/Pipfile b/python/Pipfile
index 053f344dcbe..796c6282e17 100644
--- a/python/Pipfile
+++ b/python/Pipfile
@@ -5,6 +5,7 @@ verify_ssl = true
 
 [dev-packages]
 flake8 = ">=3.6.0"
+mypy = ">=0.770"
 pylint = ">=2.8.0"
 
 [packages]
diff --git a/python/Pipfile.lock b/python/Pipfile.lock
index 5c34019060a..626e68403f7 100644
--- a/python/Pipfile.lock
+++ b/python/Pipfile.lock
@@ -1,7 +1,7 @@
 {
 "_meta": {
 "hash": {
-"sha256": 
"3c842ab9c72c40d24d146349aa144e00e4dec1c358c812cfa96489411f5b3f87"
+"sha256": 
"14d171b3d86759e1fdfb9e55f66be4a696b6afa8f627d6c4778f8398c6a66b98"
 },
 "pipfile-spec": 6,
 "requires": {
@@ -84,6 +84,41 @@
 ],
 "version": "==0.6.1"
 },
+"mypy": {
+"hashes": [
+
"sha256:0d0a87c0e7e3a9becdfbe936c981d32e5ee0ccda3e0f07e1ef2c3d1a817cf73e",
+
"sha256:25adde9b862f8f9aac9d2d11971f226bd4c8fbaa89fb76bdadb267ef22d10064",
+
"sha256:28fb5479c494b1bab244620685e2eb3c3f988d71fd5d64cc753195e8ed53df7c",
+
"sha256:2f9b3407c58347a452fc0736861593e105139b905cca7d097e413453a1d650b4",
+
"sha256:33f159443db0829d16f0a8d83d94df3109bb6dd801975fe86bacb9bf71628e97",
+
"sha256:3f2aca7f68580dc2508289c729bd49ee929a436208d2b2b6aab15745a70a57df",
+
"sha256:499c798053cdebcaa916eef8cd733e5584b5909f789de856b482cd7d069bdad8",
+
"sha256:4eec37370483331d13514c3f55f446fc5248d6373e7029a29ecb7b7494851e7a",
+
"sha256:552a815579aa1e995f39fd05dde6cd378e191b063f031f2acfe73ce9fb7f9e56",
+
"sha256:5873888fff1c7cf5b71efbe80e0e73153fe9212fafdf8e44adfe4c20ec9f82d7",
+
"sha256:61a3d5b97955422964be6b3baf05ff2ce7f26f52c85dd88db11d5e03e146a3a6",
+
"sha256:674e822aa665b9fd75130c6c5f5ed9564a38c6cea6a6432ce47eafb68ee578c5",
+
"sha256:7ce3175801d0ae5fdfa79b4f0cfed08807af4d075b402b7e294e6aa72af9aa2a",
+
"sha256:9743c91088d396c1a5a3c9978354b61b0382b4e3c440ce83cf77994a43e8c521",
+
"sha256:9f94aac67a2045ec719ffe6111df543bac7874cee01f41928f6969756e030564",
+
"sha256:a26f8ec704e5a7423c8824d425086705e381b4f1dfdef6e3a1edab7ba174ec49",
+
"sha256:abf7e0c3cf117c44d9285cc6128856106183938c68fd4944763003decdcfeb66",
+
"sha256:b09669bcda124e83708f34a94606e01b614fa71931d356c1f1a5297ba11f110a",
+
"sha256:cd07039aa5df222037005b08fbbfd69b3ab0b0bd7a07d7906de75ae52c4e3119",
+
"sha256:d23e0ea196702d918b60c8288561e722bf437d82cb7ef2edcd98cfa38905d506",
+
"sha256:d65cc1df038ef55a99e617431f0553cd77763869eebdf9042403e16089fe746c",
+
"sha256:d7da2e1d5f558c37d6e8c1246f1aec1e7349e4913d8fb3cb289a35de573fe2eb"
+],
+"index": "pypi",
+"version": "==0.812"
+},
+"mypy-extensions": {
+"hashes": [
+
"sha256:090fedd75945a69ae91ce1303b5824f428daf5a028d2f6ab8a299250a846f15d",
+
"sha256:2d82818f5bb3e369420cb3c4060a7970edba416647068eb4c5343488a6c604a8"
+],
+"version": "==0.4.3"
+},
 "pycodestyle": {
 "hashes": [
 
"sha256:514f76d918fcc0b55c6680472f0a37970994e07bbb80725808c17089be302068",
diff --git a/python/setup.cfg b/python/setup.cfg
index bd88b44ad80..b485d6161d5 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -31,6 +31,7 @@ exclude = __pycache__,
 strict = True
 python_version = 3.6
 warn_unused_configs = True
+namespace_packages = True
 
 [pylint.messages control]
 # Disable the message, report, category or checker with the given id(s). You
-- 
2.31.1




[PATCH v8 23/31] python: move .isort.cfg into setup.cfg

2021-05-27 Thread John Snow
Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
---
 python/.isort.cfg | 7 ---
 python/setup.cfg  | 8 
 2 files changed, 8 insertions(+), 7 deletions(-)
 delete mode 100644 python/.isort.cfg

diff --git a/python/.isort.cfg b/python/.isort.cfg
deleted file mode 100644
index 6d0fd6cc0d3..000
--- a/python/.isort.cfg
+++ /dev/null
@@ -1,7 +0,0 @@
-[settings]
-force_grid_wrap=4
-force_sort_within_sections=True
-include_trailing_comma=True
-line_length=72
-lines_after_imports=2
-multi_line_output=3
\ No newline at end of file
diff --git a/python/setup.cfg b/python/setup.cfg
index b485d6161d5..3f07bd2752d 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -61,3 +61,11 @@ good-names=i,
 [pylint.similarities]
 # Ignore imports when computing similarities.
 ignore-imports=yes
+
+[isort]
+force_grid_wrap=4
+force_sort_within_sections=True
+include_trailing_comma=True
+line_length=72
+lines_after_imports=2
+multi_line_output=3
-- 
2.31.1




[PATCH v8 30/31] python: add tox support

2021-05-27 Thread John Snow
This is intended to be a manually run, non-CI script.

Use tox to test the linters against all python versions from 3.6 to
3.10. This will only work if you actually have those versions installed
locally, but Fedora makes this easy:

> sudo dnf install python3.6 python3.7 python3.8 python3.9 python3.10

Unlike the pipenv tests (make venv-check), this pulls "whichever"
versions of the python packages, so they are unpinned and may break as
time goes on. In the case that breakages are found, setup.cfg should be
amended accordingly to avoid the bad dependant versions, or the code
should be amended to work around the issue.

With confidence that the tests pass on 3.6 through 3.10 inclusive, add
the appropriate classifiers to setup.cfg to indicate which versions we
claim to support.

Tox 3.18.0 or above is required to use the 'allowlist_externals' option.

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
Tested-by: Cleber Rosa 
---
 python/.gitignore |  1 +
 python/Makefile   |  7 ++-
 python/setup.cfg  | 23 ++-
 3 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/python/.gitignore b/python/.gitignore
index 4ed144ceac3..272ed223a84 100644
--- a/python/.gitignore
+++ b/python/.gitignore
@@ -13,3 +13,4 @@ qemu.egg-info/
 
 # virtual environments (pipenv et al)
 .venv/
+.tox/
diff --git a/python/Makefile b/python/Makefile
index a9da1689558..b5621b0d540 100644
--- a/python/Makefile
+++ b/python/Makefile
@@ -16,6 +16,8 @@ help:
@echo ""
@echo "make check:  run linters using the current environment."
@echo ""
+   @echo "make check-tox:  run linters using multiple python versions."
+   @echo ""
@echo "make clean:  remove package build output."
@echo ""
@echo "make distclean:  remove venv files, qemu package forwarder,"
@@ -36,8 +38,11 @@ develop:
 check:
@avocado --config avocado.cfg run tests/
 
+check-tox:
+   @tox
+
 clean:
python3 setup.py clean --all
 
 distclean: clean
-   rm -rf qemu.egg-info/ .venv/ dist/
+   rm -rf qemu.egg-info/ .venv/ .tox/ dist/
diff --git a/python/setup.cfg b/python/setup.cfg
index fd325194901..0fcdec6f322 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -14,6 +14,11 @@ classifiers =
 Natural Language :: English
 Operating System :: OS Independent
 Programming Language :: Python :: 3 :: Only
+Programming Language :: Python :: 3.6
+Programming Language :: Python :: 3.7
+Programming Language :: Python :: 3.8
+Programming Language :: Python :: 3.9
+Programming Language :: Python :: 3.10
 
 [options]
 python_requires = >= 3.6
@@ -30,12 +35,13 @@ devel =
 isort >= 5.1.2
 mypy >= 0.770
 pylint >= 2.8.0
-
+tox >= 3.18.0
 
 [flake8]
 extend-ignore = E722  # Prefer pylint's bare-except checks to flake8's
 exclude = __pycache__,
   .venv,
+  .tox,
 
 [mypy]
 strict = True
@@ -79,3 +85,18 @@ include_trailing_comma=True
 line_length=72
 lines_after_imports=2
 multi_line_output=3
+
+# tox (https://tox.readthedocs.io/) is a tool for running tests in
+# multiple virtualenvs. This configuration file will run the test suite
+# on all supported python versions. To use it, "pip install tox" and
+# then run "tox" from this directory. You will need all of these versions
+# of python available on your system to run this test.
+
+[tox:tox]
+envlist = py36, py37, py38, py39, py310
+
+[testenv]
+allowlist_externals = make
+deps = .[devel]
+commands =
+make check
-- 
2.31.1




[PATCH v8 20/31] python: Add flake8 to pipenv

2021-05-27 Thread John Snow
flake8 3.5.x does not support the --extend-ignore syntax used in the
.flake8 file to gracefully extend default ignores, so 3.6.x is our
minimum requirement. There is no known upper bound.

flake8 can be run from the python/ directory with no arguments.

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
Tested-by: Cleber Rosa 
---
 python/Pipfile  |  1 +
 python/Pipfile.lock | 51 -
 2 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/python/Pipfile b/python/Pipfile
index 285e2c8e671..053f344dcbe 100644
--- a/python/Pipfile
+++ b/python/Pipfile
@@ -4,6 +4,7 @@ url = "https://pypi.org/simple;
 verify_ssl = true
 
 [dev-packages]
+flake8 = ">=3.6.0"
 pylint = ">=2.8.0"
 
 [packages]
diff --git a/python/Pipfile.lock b/python/Pipfile.lock
index c9debd09503..5c34019060a 100644
--- a/python/Pipfile.lock
+++ b/python/Pipfile.lock
@@ -1,7 +1,7 @@
 {
 "_meta": {
 "hash": {
-"sha256": 
"bd4fb76fcdd145bbf23c3a9dd7ad966113c5ce43ca51cc2d828aa7e73d572901"
+"sha256": 
"3c842ab9c72c40d24d146349aa144e00e4dec1c358c812cfa96489411f5b3f87"
 },
 "pipfile-spec": 6,
 "requires": {
@@ -25,6 +25,22 @@
 "markers": "python_version ~= '3.6'",
 "version": "==2.5.6"
 },
+"flake8": {
+"hashes": [
+
"sha256:07528381786f2a6237b061f6e96610a4167b226cb926e2aa2b6b1d78057c576b",
+
"sha256:bf8fd46d844f616e8d47905ef3a3384edae6b4e9beb0c5101e25e3110907"
+],
+"index": "pypi",
+"version": "==3.9.2"
+},
+"importlib-metadata": {
+"hashes": [
+
"sha256:8c501196e49fb9df5df43833bdb1e4328f64847763ec8a50703148b73784d581",
+
"sha256:d7eb1dea6d6a6086f8be21784cc9e3bcfa55872b52309bc5fad53a8ea65d"
+],
+"markers": "python_version < '3.8'",
+"version": "==4.0.1"
+},
 "isort": {
 "hashes": [
 
"sha256:0a943902919f65c5684ac4e0154b1ad4fac6dcaa5d9f3426b732f1c8b5419be6",
@@ -68,6 +84,22 @@
 ],
 "version": "==0.6.1"
 },
+"pycodestyle": {
+"hashes": [
+
"sha256:514f76d918fcc0b55c6680472f0a37970994e07bbb80725808c17089be302068",
+
"sha256:c389c1d06bf7904078ca03399a4816f974a1d590090fecea0c63ec26ebaf1cef"
+],
+"markers": "python_version >= '2.7' and python_version not in 
'3.0, 3.1, 3.2, 3.3'",
+"version": "==2.7.0"
+},
+"pyflakes": {
+"hashes": [
+
"sha256:7893783d01b8a89811dd72d7dfd4d84ff098e5eed95cfa8905b22bbffe52efc3",
+
"sha256:f5bc8ecabc05bb9d291eb5203d6810b49040f6ff446a756326104746cc00c1db"
+],
+"markers": "python_version >= '2.7' and python_version not in 
'3.0, 3.1, 3.2, 3.3'",
+"version": "==2.3.1"
+},
 "pylint": {
 "hashes": [
 
"sha256:586d8fa9b1891f4b725f587ef267abe2a1bad89d6b184520c7f07a253dd6e217",
@@ -120,11 +152,28 @@
 "markers": "implementation_name == 'cpython' and python_version < 
'3.8'",
 "version": "==1.4.3"
 },
+"typing-extensions": {
+"hashes": [
+
"sha256:0ac0f89795dd19de6b97debb0c6af1c70987fd80a2d62d1958f7e56fcc31b497",
+
"sha256:50b6f157849174217d0656f99dc82fe932884fb250826c18350e159ec6cdf342",
+
"sha256:779383f6086d90c99ae41cf0ff39aac8a7937a9283ce0a414e5dd782f4c94a84"
+],
+"markers": "python_version < '3.8'",
+"version": "==3.10.0.0"
+},
 "wrapt": {
 "hashes": [
 
"sha256:b62ffa81fb85f4332a4f609cab4ac40709470da05643a082ec1eb88e6d9b97d7"
 ],
 "version": "==1.12.1"
+},
+"zipp": {
+"hashes": [
+
"sha256:3607921face881ba3e026887d8150cca609d517579abe052ac81fc5aeffdbd76",
+
"sha256:51cb66cc54621609dd593d1787f286ee42a5c0adbb4b29abea5a63edc3e03098"
+],
+"markers": "python_version >= '3.6'",
+"version": "==3.4.1"
 }
 }
 }
-- 
2.31.1




[PATCH v8 25/31] python/qemu: add qemu package itself to pipenv

2021-05-27 Thread John Snow
This adds the python qemu packages themselves to the pipenv manifest.
'pipenv sync' will create a virtual environment sufficient to use the SDK.
'pipenv sync --dev' will create a virtual environment sufficient to use
and test the SDK (with pylint, mypy, isort, flake8, etc.)

The qemu packages are installed in 'editable' mode; all changes made to
the python package inside the git tree will be reflected in the
installed package without reinstallation. This includes changes made
via git pull and so on.

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
Tested-by: Cleber Rosa 
---
 python/Pipfile  | 1 +
 python/Pipfile.lock | 9 +++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/python/Pipfile b/python/Pipfile
index 79c74dd8db4..dbe96f71c48 100644
--- a/python/Pipfile
+++ b/python/Pipfile
@@ -10,6 +10,7 @@ mypy = ">=0.770"
 pylint = ">=2.8.0"
 
 [packages]
+qemu = {editable = true,path = "."}
 
 [requires]
 python_version = "3.6"
diff --git a/python/Pipfile.lock b/python/Pipfile.lock
index 57a5befb104..f0bf576c31e 100644
--- a/python/Pipfile.lock
+++ b/python/Pipfile.lock
@@ -1,7 +1,7 @@
 {
 "_meta": {
 "hash": {
-"sha256": 
"8173290ad57aab0b722c9b4f109519de4e0dd7cd1bad1e16806b78bb169bce08"
+"sha256": 
"7c74cc4c2db3a75c954a6686411cda6fd60e464620bb6d5f1ed9a54be61db4cc"
 },
 "pipfile-spec": 6,
 "requires": {
@@ -15,7 +15,12 @@
 }
 ]
 },
-"default": {},
+"default": {
+"qemu": {
+"editable": true,
+"path": "."
+}
+},
 "develop": {
 "astroid": {
 "hashes": [
-- 
2.31.1




[PATCH v8 21/31] python: move mypy.ini into setup.cfg

2021-05-27 Thread John Snow
mypy supports reading its configuration values from a central project
configuration file; do so.

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
---
 python/mypy.ini  | 4 
 python/setup.cfg | 5 +
 2 files changed, 5 insertions(+), 4 deletions(-)
 delete mode 100644 python/mypy.ini

diff --git a/python/mypy.ini b/python/mypy.ini
deleted file mode 100644
index 1a581c5f1ea..000
--- a/python/mypy.ini
+++ /dev/null
@@ -1,4 +0,0 @@
-[mypy]
-strict = True
-python_version = 3.6
-warn_unused_configs = True
diff --git a/python/setup.cfg b/python/setup.cfg
index 9aeab2bb0d3..bd88b44ad80 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -27,6 +27,11 @@ extend-ignore = E722  # Prefer pylint's bare-except checks 
to flake8's
 exclude = __pycache__,
   .venv,
 
+[mypy]
+strict = True
+python_version = 3.6
+warn_unused_configs = True
+
 [pylint.messages control]
 # Disable the message, report, category or checker with the given id(s). You
 # can either give multiple identifiers separated by comma (,) or put this
-- 
2.31.1




[PATCH v8 28/31] python: add Makefile for some common tasks

2021-05-27 Thread John Snow
Add "make venv" to create the pipenv-managed virtual environment that
contains our explicitly pinned dependencies.

Add "make check" to run the python linters [in the host execution
environment].

Add "make venv-check" which combines the above two: create/update the
venv, then run the linters in that explicitly managed environment.

Add "make develop" which canonizes the runes needed to get both the
linting pre-requisites (the "[devel]" part), and the editable
live-install (the "-e" part) of these python libraries.

make clean: delete miscellaneous python packaging output possibly
created by pipenv, pip, or other python packaging utilities

make distclean: delete the above, the .venv, and the editable "qemu"
package forwarder (qemu.egg-info) if there is one.

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
Tested-by: Cleber Rosa 
---
 python/PACKAGE.rst |  6 ++
 python/README.rst  |  6 ++
 python/Makefile| 43 +++
 3 files changed, 55 insertions(+)
 create mode 100644 python/Makefile

diff --git a/python/PACKAGE.rst b/python/PACKAGE.rst
index 05ea7789fc1..b0b86cc4c31 100644
--- a/python/PACKAGE.rst
+++ b/python/PACKAGE.rst
@@ -35,3 +35,9 @@ the report.
 Optional packages necessary for running code quality analysis for this
 package can be installed with the optional dependency group "devel":
 ``pip install qemu[devel]``.
+
+``make develop`` can be used to install this package in editable mode
+(to the current environment) *and* bring in testing dependencies in one
+command.
+
+``make check`` can be used to run the available tests.
diff --git a/python/README.rst b/python/README.rst
index 6bd2c6b3547..dcf993819db 100644
--- a/python/README.rst
+++ b/python/README.rst
@@ -28,6 +28,9 @@ Installing ".[devel]" instead of "." will additionally pull 
in required
 packages for testing this package. They are not runtime requirements,
 and are not needed to simply use these libraries.
 
+Running ``make develop`` will pull in all testing dependencies and
+install QEMU in editable mode to the current environment.
+
 See `Installing packages using pip and virtual environments
 
`_
 for more information.
@@ -39,6 +42,9 @@ Files in this directory
 - ``qemu/`` Python package source directory.
 - ``tests/`` Python package tests directory.
 - ``avocado.cfg`` Configuration for the Avocado test-runner.
+  Used by ``make check`` et al.
+- ``Makefile`` provides some common testing/installation invocations.
+  Try ``make help`` to see available targets.
 - ``MANIFEST.in`` is read by python setuptools, it specifies additional files
   that should be included by a source distribution.
 - ``PACKAGE.rst`` is used as the README file that is visible on PyPI.org.
diff --git a/python/Makefile b/python/Makefile
new file mode 100644
index 000..a9da1689558
--- /dev/null
+++ b/python/Makefile
@@ -0,0 +1,43 @@
+.PHONY: help venv venv-check check clean distclean develop
+
+help:
+   @echo "python packaging help:"
+   @echo ""
+   @echo "make venv:   Create pipenv's virtual environment."
+   @echo "NOTE: Requires Python 3.6 and pipenv."
+   @echo "  Will download packages from PyPI."
+   @echo "Hint: (On Fedora): 'sudo dnf install python36 pipenv'"
+   @echo ""
+   @echo "make venv-check: run linters using pipenv's virtual environment."
+   @echo "Hint: If you don't know which test to run, run this one!"
+   @echo ""
+   @echo "make develop:Install deps for 'make check', and"
+   @echo " the qemu libs in editable/development mode."
+   @echo ""
+   @echo "make check:  run linters using the current environment."
+   @echo ""
+   @echo "make clean:  remove package build output."
+   @echo ""
+   @echo "make distclean:  remove venv files, qemu package forwarder,"
+   @echo " built distribution files, and everything"
+   @echo " from 'make clean'."
+
+venv: .venv
+.venv: Pipfile.lock
+   @PIPENV_VENV_IN_PROJECT=1 pipenv sync --dev --keep-outdated
+   @touch .venv
+
+venv-check: venv
+   @pipenv run make check
+
+develop:
+   pip3 install -e .[devel]
+
+check:
+   @avocado --config avocado.cfg run tests/
+
+clean:
+   python3 setup.py clean --all
+
+distclean: clean
+   rm -rf qemu.egg-info/ .venv/ dist/
-- 
2.31.1




[PATCH v8 17/31] python: add pylint to pipenv

2021-05-27 Thread John Snow
We are specifying >= pylint 2.8.x for several reasons:

1. For setup.cfg support, added in pylint 2.5.x
2. To specify a version that has incompatibly dropped
   bad-whitespace checks (2.6.x)
3. 2.7.x fixes "unsubscriptable" warnings in Python 3.9
4. 2.8.x adds a new, incompatible 'consider-using-with'
   warning that must be disabled in some cases.
   These pragmas cause warnings themselves in 2.7.x.

Signed-off-by: John Snow 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Cleber Rosa 
Tested-by: Cleber Rosa 
---
 python/Pipfile  |   1 +
 python/Pipfile.lock | 130 
 2 files changed, 131 insertions(+)
 create mode 100644 python/Pipfile.lock

diff --git a/python/Pipfile b/python/Pipfile
index 9534830b5eb..285e2c8e671 100644
--- a/python/Pipfile
+++ b/python/Pipfile
@@ -4,6 +4,7 @@ url = "https://pypi.org/simple;
 verify_ssl = true
 
 [dev-packages]
+pylint = ">=2.8.0"
 
 [packages]
 
diff --git a/python/Pipfile.lock b/python/Pipfile.lock
new file mode 100644
index 000..c9debd09503
--- /dev/null
+++ b/python/Pipfile.lock
@@ -0,0 +1,130 @@
+{
+"_meta": {
+"hash": {
+"sha256": 
"bd4fb76fcdd145bbf23c3a9dd7ad966113c5ce43ca51cc2d828aa7e73d572901"
+},
+"pipfile-spec": 6,
+"requires": {
+"python_version": "3.6"
+},
+"sources": [
+{
+"name": "pypi",
+"url": "https://pypi.org/simple;,
+"verify_ssl": true
+}
+]
+},
+"default": {},
+"develop": {
+"astroid": {
+"hashes": [
+
"sha256:4db03ab5fc3340cf619dbc25e42c2cc3755154ce6009469766d7143d1fc2ee4e",
+
"sha256:8a398dfce302c13f14bab13e2b14fe385d32b73f4e4853b9bdfb64598baa1975"
+],
+"markers": "python_version ~= '3.6'",
+"version": "==2.5.6"
+},
+"isort": {
+"hashes": [
+
"sha256:0a943902919f65c5684ac4e0154b1ad4fac6dcaa5d9f3426b732f1c8b5419be6",
+
"sha256:2bb1680aad211e3c9944dbce1d4ba09a989f04e238296c87fe2139faa26d655d"
+],
+"markers": "python_version >= '3.6' and python_version < '4.0'",
+"version": "==5.8.0"
+},
+"lazy-object-proxy": {
+"hashes": [
+
"sha256:17e0967ba374fc24141738c69736da90e94419338fd4c7c7bef01ee26b339653",
+
"sha256:1fee665d2638491f4d6e55bd483e15ef21f6c8c2095f235fef72601021e64f61",
+
"sha256:22ddd618cefe54305df49e4c069fa65715be4ad0e78e8d252a33debf00f6ede2",
+
"sha256:24a5045889cc2729033b3e604d496c2b6f588c754f7a62027ad4437a7ecc4837",
+
"sha256:410283732af311b51b837894fa2f24f2c0039aa7f220135192b38fcc42bd43d3",
+
"sha256:4732c765372bd78a2d6b2150a6e99d00a78ec963375f236979c0626b97ed8e43",
+
"sha256:489000d368377571c6f982fba6497f2aa13c6d1facc40660963da62f5c379726",
+
"sha256:4f60460e9f1eb632584c9685bccea152f4ac2130e299784dbaf9fae9f49891b3",
+
"sha256:5743a5ab42ae40caa8421b320ebf3a998f89c85cdc8376d6b2e00bd12bd1b587",
+
"sha256:85fb7608121fd5621cc4377a8961d0b32ccf84a7285b4f1d21988b2eae2868e8",
+
"sha256:9698110e36e2df951c7c36b6729e96429c9c32b3331989ef19976592c5f3c77a",
+
"sha256:9d397bf41caad3f489e10774667310d73cb9c4258e9aed94b9ec734b34b495fd",
+
"sha256:b579f8acbf2bdd9ea200b1d5dea36abd93cabf56cf626ab9c744a432e15c815f",
+
"sha256:b865b01a2e7f96db0c5d12cfea590f98d8c5ba64ad222300d93ce6ff9138bcad",
+
"sha256:bf34e368e8dd976423396555078def5cfc3039ebc6fc06d1ae2c5a65eebbcde4",
+
"sha256:c6938967f8528b3668622a9ed3b31d145fab161a32f5891ea7b84f6b790be05b",
+
"sha256:d1c2676e3d840852a2de7c7d5d76407c772927addff8d742b9808fe0afccebdf",
+
"sha256:d7124f52f3bd259f510651450e18e0fd081ed82f3c08541dffc7b94b883aa981",
+
"sha256:d900d949b707778696fdf01036f58c9876a0d8bfe116e8d220cfd4b15f14e741",
+
"sha256:ebfd274dcd5133e0afae738e6d9da4323c3eb021b3e13052d8cbd0e457b1256e",
+
"sha256:ed361bb83436f117f9917d282a456f9e5009ea12fd6de8742d1a4752c3017e93",
+
"sha256:f5144c75445ae3ca2057faac03fda5a902eff196702b0a24daf1d6ce0650514b"
+],
+"markers": "python_version >= '2.7' and python_version not in 
'3.0, 3.1, 3.2, 3.3, 3.4, 3.5'",
+"version": "==1.6.0"
+},
+"mccabe": {
+"hashes": [
+
"sha256:ab8a6258860da4b6677da4bd2fe5dc2c659cff31b3ee4f7f5d64e79735b80d42",
+
"sha256:dd8d182285a0fe56bace7f45b5e7d1a6ebcbf524e8f3bd87eb0f125271b8831f"
+],
+"version": "==0.6.1"
+},
+"pylint": {
+"hashes": [
+

Re: [PATCH v4] i386: Add ratelimit for bus locks acquired in guest

2021-05-27 Thread Eduardo Habkost
On Fri, May 21, 2021 at 12:38:20PM +0800, Chenyi Qiang wrote:
[...]
> @@ -4222,6 +4247,15 @@ void kvm_arch_pre_run(CPUState *cpu, struct kvm_run 
> *run)
>  }
>  }
>  
> +static void kvm_rate_limit_on_bus_lock(void)
> +{
> +uint64_t delay_ns = ratelimit_calculate_delay(_lock_ratelimit_ctrl, 
> 1);
> +
> +if (delay_ns) {
> +g_usleep(delay_ns / SCALE_US);
> +}
> +}
> +
>  MemTxAttrs kvm_arch_post_run(CPUState *cpu, struct kvm_run *run)
>  {
>  X86CPU *x86_cpu = X86_CPU(cpu);
> @@ -4237,6 +4271,9 @@ MemTxAttrs kvm_arch_post_run(CPUState *cpu, struct 
> kvm_run *run)
>  } else {
>  env->eflags &= ~IF_MASK;
>  }
> +if (run->flags & KVM_RUN_X86_BUS_LOCK) {

Does the KVM API guarantee that KVM_RUN_X86_BUS_LOCK will never
be set if KVM_BUS_LOCK_DETECTION_EXIT isn't enabled?  (Otherwise
we risk crashing in ratelimit_calculate_delay() above if rate
limiting is disabled).

If that's guaranteed, the patch looks good to me now.

> +kvm_rate_limit_on_bus_lock();
> +}
>  
>  /* We need to protect the apic state against concurrent accesses from
>   * different threads in case the userspace irqchip is used. */
> @@ -4595,6 +4632,10 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run 
> *run)
>  ioapic_eoi_broadcast(run->eoi.vector);
>  ret = 0;
>  break;
> +case KVM_EXIT_X86_BUS_LOCK:
> +/* already handled in kvm_arch_post_run */
> +ret = 0;
> +break;
>  default:
>  fprintf(stderr, "KVM: unknown exit reason %d\n", run->exit_reason);
>  ret = -1;
> -- 
> 2.17.1
> 

-- 
Eduardo




[PATCH v8 19/31] python: add excluded dirs to flake8 config

2021-05-27 Thread John Snow
Instruct flake8 to avoid certain well-known directories created by
python tooling that it ought not check.

Note that at-present, nothing actually creates a ".venv" directory; but
it is in such widespread usage as a de-facto location for a developer's
virtual environment that it should be excluded anyway. A forthcoming
commit canonizes this with a "make venv" command.

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
---
 python/setup.cfg | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/python/setup.cfg b/python/setup.cfg
index 52a89a0a290..9aeab2bb0d3 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -24,6 +24,8 @@ packages =
 
 [flake8]
 extend-ignore = E722  # Prefer pylint's bare-except checks to flake8's
+exclude = __pycache__,
+  .venv,
 
 [pylint.messages control]
 # Disable the message, report, category or checker with the given id(s). You
-- 
2.31.1




[PATCH v8 27/31] python: add avocado-framework and tests

2021-05-27 Thread John Snow
Try using avocado to manage our various tests; even though right now
they're only invoking shell scripts and not really running any
python-native code.

Create tests/, and add shell scripts which call out to mypy, flake8,
pylint and isort to enforce the standards in this directory.

Add avocado-framework to the setup.cfg development dependencies, and add
avocado.cfg to store some preferences for how we'd like the test output
to look.

Finally, add avocado-framework to the Pipfile environment and lock the
new dependencies. We are using avocado >= 87.0 here to take advantage of
some features that Cleber has helpfully added to make the test output
here *very* friendly and easy to read for developers that might chance
upon the output in Gitlab CI.

[Note: ALL of the dependencies get updated to the most modern versions
that exist at the time of this writing. No way around it that I have
seen. Not ideal, but so it goes.]

Provided you have the right development dependencies (mypy, flake8,
isort, pylint, and now avocado-framework) You should be able to run
"avocado --config avocado.cfg run tests/" from the python folder to run
all of these linters with the correct arguments.

(A forthcoming commit adds the much easier 'make check'.)

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
Tested-by: Cleber Rosa 
---
 python/README.rst  |  2 ++
 python/Pipfile.lock|  8 
 python/avocado.cfg | 10 ++
 python/setup.cfg   |  1 +
 python/tests/flake8.sh |  2 ++
 python/tests/isort.sh  |  2 ++
 python/tests/mypy.sh   |  2 ++
 python/tests/pylint.sh |  2 ++
 8 files changed, 29 insertions(+)
 create mode 100644 python/avocado.cfg
 create mode 100755 python/tests/flake8.sh
 create mode 100755 python/tests/isort.sh
 create mode 100755 python/tests/mypy.sh
 create mode 100755 python/tests/pylint.sh

diff --git a/python/README.rst b/python/README.rst
index 954870973d0..6bd2c6b3547 100644
--- a/python/README.rst
+++ b/python/README.rst
@@ -37,6 +37,8 @@ Files in this directory
 ---
 
 - ``qemu/`` Python package source directory.
+- ``tests/`` Python package tests directory.
+- ``avocado.cfg`` Configuration for the Avocado test-runner.
 - ``MANIFEST.in`` is read by python setuptools, it specifies additional files
   that should be included by a source distribution.
 - ``PACKAGE.rst`` is used as the README file that is visible on PyPI.org.
diff --git a/python/Pipfile.lock b/python/Pipfile.lock
index a2cdc1c50ea..6e344f5fadf 100644
--- a/python/Pipfile.lock
+++ b/python/Pipfile.lock
@@ -30,6 +30,14 @@
 "markers": "python_version ~= '3.6'",
 "version": "==2.5.6"
 },
+"avocado-framework": {
+"hashes": [
+
"sha256:42aa7962df98d6b78d4efd9afa2177226dc630f3d83a2a7d5baf7a0a7da7fa1b",
+
"sha256:d96ae343abf890e1ef3b3a6af5ce49e35f6bded0715770c4acb325bca555c515"
+],
+"markers": "python_version >= '3.6'",
+"version": "==88.1"
+},
 "flake8": {
 "hashes": [
 
"sha256:07528381786f2a6237b061f6e96610a4167b226cb926e2aa2b6b1d78057c576b",
diff --git a/python/avocado.cfg b/python/avocado.cfg
new file mode 100644
index 000..10dc6fb6054
--- /dev/null
+++ b/python/avocado.cfg
@@ -0,0 +1,10 @@
+[simpletests]
+# Don't show stdout/stderr in the test *summary*
+status.failure_fields = ['status']
+
+[job]
+# Don't show the full debug.log output; only select stdout/stderr.
+output.testlogs.logfiles = ['stdout', 'stderr']
+
+# Show full stdout/stderr only on tests that FAIL
+output.testlogs.statuses = ['FAIL']
diff --git a/python/setup.cfg b/python/setup.cfg
index 39dc135e601..fd325194901 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -25,6 +25,7 @@ packages =
 [options.extras_require]
 # Run `pipenv lock --dev` when changing these requirements.
 devel =
+avocado-framework >= 87.0
 flake8 >= 3.6.0
 isort >= 5.1.2
 mypy >= 0.770
diff --git a/python/tests/flake8.sh b/python/tests/flake8.sh
new file mode 100755
index 000..51e0788462b
--- /dev/null
+++ b/python/tests/flake8.sh
@@ -0,0 +1,2 @@
+#!/bin/sh -e
+python3 -m flake8
diff --git a/python/tests/isort.sh b/python/tests/isort.sh
new file mode 100755
index 000..4480405bfb0
--- /dev/null
+++ b/python/tests/isort.sh
@@ -0,0 +1,2 @@
+#!/bin/sh -e
+python3 -m isort -c qemu/
diff --git a/python/tests/mypy.sh b/python/tests/mypy.sh
new file mode 100755
index 000..5f980f563bb
--- /dev/null
+++ b/python/tests/mypy.sh
@@ -0,0 +1,2 @@
+#!/bin/sh -e
+python3 -m mypy -p qemu
diff --git a/python/tests/pylint.sh b/python/tests/pylint.sh
new file mode 100755
index 000..4b10b34db7c
--- /dev/null
+++ b/python/tests/pylint.sh
@@ -0,0 +1,2 @@
+#!/bin/sh -e
+python3 -m pylint qemu/
-- 
2.31.1




[PATCH v8 14/31] python: Add pipenv support

2021-05-27 Thread John Snow
pipenv is a tool used for managing virtual environments with pinned,
explicit dependencies. It is used for precisely recreating python
virtual environments.

pipenv uses two files to do this:

(1) Pipfile, which is similar in purpose and scope to what setup.cfg
lists. It specifies the requisite minimum to get a functional
environment for using this package.

(2) Pipfile.lock, which is similar in purpose to `pip freeze >
requirements.txt`. It specifies a canonical virtual environment used for
deployment or testing. This ensures that all users have repeatable
results.

The primary benefit of using this tool is to ensure *rock solid*
repeatable CI results with a known set of packages. Although I endeavor
to support as many versions as I can, the fluid nature of the Python
toolchain often means tailoring code for fairly specific versions.

Note that pipenv is *not* required to install or use this module; this is
purely for the sake of repeatable testing by CI or developers.

Here, a "blank" pipfile is added with no dependencies, but specifies
Python 3.6 for the virtual environment.

Pipfile will specify our version minimums, while Pipfile.lock specifies
an exact loadout of packages that were known to operate correctly. This
latter file provides the real value for easy setup of container images
and CI environments.

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
---
 python/README.rst |  3 +++
 python/Pipfile| 11 +++
 2 files changed, 14 insertions(+)
 create mode 100644 python/Pipfile

diff --git a/python/README.rst b/python/README.rst
index 0099646ae2f..bf9bbca979a 100644
--- a/python/README.rst
+++ b/python/README.rst
@@ -36,6 +36,9 @@ Files in this directory
 - ``MANIFEST.in`` is read by python setuptools, it specifies additional files
   that should be included by a source distribution.
 - ``PACKAGE.rst`` is used as the README file that is visible on PyPI.org.
+- ``Pipfile`` is used by Pipenv to generate ``Pipfile.lock``.
+- ``Pipfile.lock`` is a set of pinned package dependencies that this package
+  is tested under in our CI suite. It is used by ``make venv-check``.
 - ``README.rst`` you are here!
 - ``VERSION`` contains the PEP-440 compliant version used to describe
   this package; it is referenced by ``setup.cfg``.
diff --git a/python/Pipfile b/python/Pipfile
new file mode 100644
index 000..9534830b5eb
--- /dev/null
+++ b/python/Pipfile
@@ -0,0 +1,11 @@
+[[source]]
+name = "pypi"
+url = "https://pypi.org/simple;
+verify_ssl = true
+
+[dev-packages]
+
+[packages]
+
+[requires]
+python_version = "3.6"
-- 
2.31.1




[PATCH v8 13/31] python: add MANIFEST.in

2021-05-27 Thread John Snow
When creating a source or binary distribution via 'python3 setup.py
', the VERSION and PACKAGE.rst files aren't bundled by
default. Create a MANIFEST.in file that instructs the build tools to
include these so that installation from these files won't fail.

This is required by 'tox', as well as by the tooling needed to upload
packages to PyPI.

Exclude the 'README.rst' file -- that's intended as a guidebook to our
source tree, not a file that needs to be distributed.

Signed-off-by: John Snow 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Cleber Rosa 
---
 python/README.rst  | 2 ++
 python/MANIFEST.in | 3 +++
 2 files changed, 5 insertions(+)
 create mode 100644 python/MANIFEST.in

diff --git a/python/README.rst b/python/README.rst
index 38b0c83f321..0099646ae2f 100644
--- a/python/README.rst
+++ b/python/README.rst
@@ -33,6 +33,8 @@ Files in this directory
 ---
 
 - ``qemu/`` Python package source directory.
+- ``MANIFEST.in`` is read by python setuptools, it specifies additional files
+  that should be included by a source distribution.
 - ``PACKAGE.rst`` is used as the README file that is visible on PyPI.org.
 - ``README.rst`` you are here!
 - ``VERSION`` contains the PEP-440 compliant version used to describe
diff --git a/python/MANIFEST.in b/python/MANIFEST.in
new file mode 100644
index 000..7059ad28221
--- /dev/null
+++ b/python/MANIFEST.in
@@ -0,0 +1,3 @@
+include VERSION
+include PACKAGE.rst
+exclude README.rst
-- 
2.31.1




[PATCH v8 18/31] python: move flake8 config to setup.cfg

2021-05-27 Thread John Snow
Update the comment concerning the flake8 exception to match commit
42c0dd12, whose commit message stated:

A note on the flake8 exception: flake8 will warn on *any* bare except,
but pylint's is context-aware and will suppress the warning if you
re-raise the exception.

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
---
 python/qemu/machine/.flake8 | 2 --
 python/setup.cfg| 3 +++
 2 files changed, 3 insertions(+), 2 deletions(-)
 delete mode 100644 python/qemu/machine/.flake8

diff --git a/python/qemu/machine/.flake8 b/python/qemu/machine/.flake8
deleted file mode 100644
index 45d8146f3f5..000
--- a/python/qemu/machine/.flake8
+++ /dev/null
@@ -1,2 +0,0 @@
-[flake8]
-extend-ignore = E722  # Pylint handles this, but smarter.
\ No newline at end of file
diff --git a/python/setup.cfg b/python/setup.cfg
index 36b4253e939..52a89a0a290 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -22,6 +22,9 @@ packages =
 qemu.machine
 qemu.utils
 
+[flake8]
+extend-ignore = E722  # Prefer pylint's bare-except checks to flake8's
+
 [pylint.messages control]
 # Disable the message, report, category or checker with the given id(s). You
 # can either give multiple identifiers separated by comma (,) or put this
-- 
2.31.1




[PATCH v8 16/31] python: move pylintrc into setup.cfg

2021-05-27 Thread John Snow
Delete the empty settings now that it's sharing a home with settings for
other tools.

pylint can now be run from this folder as "pylint qemu".

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
Tested-by: Cleber Rosa 
---
 python/qemu/machine/pylintrc | 58 
 python/setup.cfg | 29 ++
 2 files changed, 29 insertions(+), 58 deletions(-)
 delete mode 100644 python/qemu/machine/pylintrc

diff --git a/python/qemu/machine/pylintrc b/python/qemu/machine/pylintrc
deleted file mode 100644
index 3f69205000d..000
--- a/python/qemu/machine/pylintrc
+++ /dev/null
@@ -1,58 +0,0 @@
-[MASTER]
-
-[MESSAGES CONTROL]
-
-# Disable the message, report, category or checker with the given id(s). You
-# can either give multiple identifiers separated by comma (,) or put this
-# option multiple times (only on the command line, not in the configuration
-# file where it should appear only once). You can also use "--disable=all" to
-# disable everything first and then reenable specific checks. For example, if
-# you want to run only the similarities checker, you can use "--disable=all
-# --enable=similarities". If you want to run only the classes checker, but have
-# no Warning level messages displayed, use "--disable=all --enable=classes
-# --disable=W".
-disable=too-many-arguments,
-too-many-instance-attributes,
-too-many-public-methods,
-
-[REPORTS]
-
-[REFACTORING]
-
-[MISCELLANEOUS]
-
-[LOGGING]
-
-[BASIC]
-
-# Good variable names which should always be accepted, separated by a comma.
-good-names=i,
-   j,
-   k,
-   ex,
-   Run,
-   _,
-   fd,
-   c,
-[VARIABLES]
-
-[STRING]
-
-[SPELLING]
-
-[FORMAT]
-
-[SIMILARITIES]
-
-# Ignore imports when computing similarities.
-ignore-imports=yes
-
-[TYPECHECK]
-
-[CLASSES]
-
-[IMPORTS]
-
-[DESIGN]
-
-[EXCEPTIONS]
diff --git a/python/setup.cfg b/python/setup.cfg
index b0010e0188f..36b4253e939 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -21,3 +21,32 @@ packages =
 qemu.qmp
 qemu.machine
 qemu.utils
+
+[pylint.messages control]
+# Disable the message, report, category or checker with the given id(s). You
+# can either give multiple identifiers separated by comma (,) or put this
+# option multiple times (only on the command line, not in the configuration
+# file where it should appear only once). You can also use "--disable=all" to
+# disable everything first and then reenable specific checks. For example, if
+# you want to run only the similarities checker, you can use "--disable=all
+# --enable=similarities". If you want to run only the classes checker, but have
+# no Warning level messages displayed, use "--disable=all --enable=classes
+# --disable=W".
+disable=too-many-arguments,
+too-many-instance-attributes,
+too-many-public-methods,
+
+[pylint.basic]
+# Good variable names which should always be accepted, separated by a comma.
+good-names=i,
+   j,
+   k,
+   ex,
+   Run,
+   _,
+   fd,
+   c,
+
+[pylint.similarities]
+# Ignore imports when computing similarities.
+ignore-imports=yes
-- 
2.31.1




[PATCH v8 09/31] python: create qemu packages

2021-05-27 Thread John Snow
move python/qemu/*.py to python/qemu/[machine, qmp, utils]/*.py and
update import directives across the tree.

This is done to create a PEP420 namespace package, in which we may
create subpackages. To do this, the namespace directory ("qemu") should
not have any modules in it. Those files will go into new 'machine',
'qmp' and 'utils' subpackages instead.

Implement machine/__init__.py making the top-level classes and functions
from its various modules available directly inside the package. Change
qmp.py to qmp/__init__.py similarly, such that all of the useful QMP
library classes are available directly from "qemu.qmp" instead of
"qemu.qmp.qmp".

Signed-off-by: John Snow 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Cleber Rosa 
---
 python/{qemu => }/.isort.cfg|  0
 python/qemu/__init__.py | 11 ---
 python/qemu/{ => machine}/.flake8   |  0
 python/qemu/machine/__init__.py | 33 +
 python/qemu/{ => machine}/console_socket.py |  0
 python/qemu/{ => machine}/machine.py| 16 ++
 python/qemu/{ => machine}/pylintrc  |  0
 python/qemu/{ => machine}/qtest.py  |  3 +-
 python/qemu/{qmp.py => qmp/__init__.py} | 12 +++-
 python/qemu/{utils.py => utils/__init__.py} | 18 +--
 python/qemu/{ => utils}/accel.py|  0
 tests/acceptance/avocado_qemu/__init__.py   |  9 +++---
 tests/acceptance/virtio-gpu.py  |  2 +-
 tests/qemu-iotests/300  |  4 +--
 tests/qemu-iotests/iotests.py   |  2 +-
 tests/vm/aarch64vm.py   |  2 +-
 tests/vm/basevm.py  |  3 +-
 17 files changed, 83 insertions(+), 32 deletions(-)
 rename python/{qemu => }/.isort.cfg (100%)
 delete mode 100644 python/qemu/__init__.py
 rename python/qemu/{ => machine}/.flake8 (100%)
 create mode 100644 python/qemu/machine/__init__.py
 rename python/qemu/{ => machine}/console_socket.py (100%)
 rename python/qemu/{ => machine}/machine.py (98%)
 rename python/qemu/{ => machine}/pylintrc (100%)
 rename python/qemu/{ => machine}/qtest.py (99%)
 rename python/qemu/{qmp.py => qmp/__init__.py} (96%)
 rename python/qemu/{utils.py => utils/__init__.py} (66%)
 rename python/qemu/{ => utils}/accel.py (100%)

diff --git a/python/qemu/.isort.cfg b/python/.isort.cfg
similarity index 100%
rename from python/qemu/.isort.cfg
rename to python/.isort.cfg
diff --git a/python/qemu/__init__.py b/python/qemu/__init__.py
deleted file mode 100644
index 4ca06c34a41..000
--- a/python/qemu/__init__.py
+++ /dev/null
@@ -1,11 +0,0 @@
-# QEMU library
-#
-# Copyright (C) 2015-2016 Red Hat Inc.
-# Copyright (C) 2012 IBM Corp.
-#
-# Authors:
-#  Fam Zheng 
-#
-# This work is licensed under the terms of the GNU GPL, version 2.  See
-# the COPYING file in the top-level directory.
-#
diff --git a/python/qemu/.flake8 b/python/qemu/machine/.flake8
similarity index 100%
rename from python/qemu/.flake8
rename to python/qemu/machine/.flake8
diff --git a/python/qemu/machine/__init__.py b/python/qemu/machine/__init__.py
new file mode 100644
index 000..98302ea31e7
--- /dev/null
+++ b/python/qemu/machine/__init__.py
@@ -0,0 +1,33 @@
+"""
+QEMU development and testing library.
+
+This library provides a few high-level classes for driving QEMU from a
+test suite, not intended for production use.
+
+- QEMUMachine: Configure and Boot a QEMU VM
+ - QEMUQtestMachine: VM class, with a qtest socket.
+
+- QEMUQtestProtocol: Connect to, send/receive qtest messages.
+"""
+
+# Copyright (C) 2020-2021 John Snow for Red Hat Inc.
+# Copyright (C) 2015-2016 Red Hat Inc.
+# Copyright (C) 2012 IBM Corp.
+#
+# Authors:
+#  John Snow 
+#  Fam Zheng 
+#
+# This work is licensed under the terms of the GNU GPL, version 2.  See
+# the COPYING file in the top-level directory.
+#
+
+from .machine import QEMUMachine
+from .qtest import QEMUQtestMachine, QEMUQtestProtocol
+
+
+__all__ = (
+'QEMUMachine',
+'QEMUQtestProtocol',
+'QEMUQtestMachine',
+)
diff --git a/python/qemu/console_socket.py 
b/python/qemu/machine/console_socket.py
similarity index 100%
rename from python/qemu/console_socket.py
rename to python/qemu/machine/console_socket.py
diff --git a/python/qemu/machine.py b/python/qemu/machine/machine.py
similarity index 98%
rename from python/qemu/machine.py
rename to python/qemu/machine/machine.py
index a8837b36e47..d33b02d2ce6 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine/machine.py
@@ -38,8 +38,14 @@
 Type,
 )
 
-from . import console_socket, qmp
-from .qmp import QMPMessage, QMPReturnValue, SocketAddrT
+from qemu.qmp import (
+QEMUMonitorProtocol,
+QMPMessage,
+QMPReturnValue,
+SocketAddrT,
+)
+
+from . import console_socket
 
 
 LOG = logging.getLogger(__name__)
@@ -139,7 +145,7 @@ def __init__(self,
 self._events: List[QMPMessage] = []
 self._iolog: Optional[str] = None
 self._qmp_set = True   # Enable QMP monitor 

[PATCH v8 05/31] python/machine: Disable pylint warning for open() in _pre_launch

2021-05-27 Thread John Snow
Shift the open() call later so that the pylint pragma applies *only* to
that one open() call. Add a note that suggests why this is safe: the
resource is unconditionally cleaned up in _post_shutdown().

_post_shutdown is called after failed launches (see launch()), and
unconditionally after every call to shutdown(), and therefore also on
__exit__.

Signed-off-by: John Snow 
Reviewed-by: Wainer dos Santos Moschetta 
Reviewed-by: Cleber Rosa 
Message-id: 20210517184808.3562549-6-js...@redhat.com
Signed-off-by: John Snow 
---
 python/qemu/machine.py | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index 04e005f3811..c66bc6a9c69 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -306,7 +306,6 @@ def _base_args(self) -> List[str]:
 
 def _pre_launch(self) -> None:
 self._qemu_log_path = os.path.join(self.temp_dir, self._name + ".log")
-self._qemu_log_file = open(self._qemu_log_path, 'wb')
 
 if self._console_set:
 self._remove_files.append(self._console_address)
@@ -321,6 +320,11 @@ def _pre_launch(self) -> None:
 nickname=self._name
 )
 
+# NOTE: Make sure any opened resources are *definitely* freed in
+# _post_shutdown()!
+# pylint: disable=consider-using-with
+self._qemu_log_file = open(self._qemu_log_path, 'wb')
+
 def _post_launch(self) -> None:
 if self._qmp_connection:
 self._qmp.accept()
-- 
2.31.1




[PATCH v8 15/31] python: add pylint import exceptions

2021-05-27 Thread John Snow
Pylint 2.5.x - 2.7.x have regressions that make import checking
inconsistent, see:

https://github.com/PyCQA/pylint/issues/3609
https://github.com/PyCQA/pylint/issues/3624
https://github.com/PyCQA/pylint/issues/3651

Pinning to 2.4.4 is worse, because it mandates versions of shared
dependencies that are too old for features we want in isort and mypy.
Oh well.

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
---
 python/qemu/machine/__init__.py | 3 +++
 python/qemu/machine/machine.py  | 2 +-
 python/qemu/machine/qtest.py| 2 +-
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/python/qemu/machine/__init__.py b/python/qemu/machine/__init__.py
index 98302ea31e7..728f27adbed 100644
--- a/python/qemu/machine/__init__.py
+++ b/python/qemu/machine/__init__.py
@@ -22,6 +22,9 @@
 # the COPYING file in the top-level directory.
 #
 
+# pylint: disable=import-error
+# see: https://github.com/PyCQA/pylint/issues/3624
+# see: https://github.com/PyCQA/pylint/issues/3651
 from .machine import QEMUMachine
 from .qtest import QEMUQtestMachine, QEMUQtestProtocol
 
diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
index d33b02d2ce6..b62435528e2 100644
--- a/python/qemu/machine/machine.py
+++ b/python/qemu/machine/machine.py
@@ -38,7 +38,7 @@
 Type,
 )
 
-from qemu.qmp import (
+from qemu.qmp import (  # pylint: disable=import-error
 QEMUMonitorProtocol,
 QMPMessage,
 QMPReturnValue,
diff --git a/python/qemu/machine/qtest.py b/python/qemu/machine/qtest.py
index e893ca3697a..93700684d1c 100644
--- a/python/qemu/machine/qtest.py
+++ b/python/qemu/machine/qtest.py
@@ -26,7 +26,7 @@
 TextIO,
 )
 
-from qemu.qmp import SocketAddrT
+from qemu.qmp import SocketAddrT  # pylint: disable=import-error
 
 from .machine import QEMUMachine
 
-- 
2.31.1




[PATCH v8 12/31] python: add directory structure README.rst files

2021-05-27 Thread John Snow
Add short readmes to python/, python/qemu/, python/qemu/machine,
python/qemu/qmp, and python/qemu/utils that explain the directory
hierarchy. These readmes are visible when browsing the source on
e.g. gitlab/github and are designed to help new developers/users quickly
make sense of the source tree.

They are not designed for inclusion in a published manual.

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
---
 python/README.rst  | 41 ++
 python/qemu/README.rst |  8 +++
 python/qemu/machine/README.rst |  9 
 python/qemu/qmp/README.rst |  9 
 python/qemu/utils/README.rst   |  7 ++
 5 files changed, 74 insertions(+)
 create mode 100644 python/README.rst
 create mode 100644 python/qemu/README.rst
 create mode 100644 python/qemu/machine/README.rst
 create mode 100644 python/qemu/qmp/README.rst
 create mode 100644 python/qemu/utils/README.rst

diff --git a/python/README.rst b/python/README.rst
new file mode 100644
index 000..38b0c83f321
--- /dev/null
+++ b/python/README.rst
@@ -0,0 +1,41 @@
+QEMU Python Tooling
+===
+
+This directory houses Python tooling used by the QEMU project to build,
+configure, and test QEMU. It is organized by namespace (``qemu``), and
+then by package (e.g. ``qemu/machine``, ``qemu/qmp``, etc).
+
+``setup.py`` is used by ``pip`` to install this tooling to the current
+environment. ``setup.cfg`` provides the packaging configuration used by
+``setup.py`` in a setuptools specific format. You will generally invoke
+it by doing one of the following:
+
+1. ``pip3 install .`` will install these packages to your current
+   environment. If you are inside a virtual environment, they will
+   install there. If you are not, it will attempt to install to the
+   global environment, which is **not recommended**.
+
+2. ``pip3 install --user .`` will install these packages to your user's
+   local python packages. If you are inside of a virtual environment,
+   this will fail; you likely want the first invocation above.
+
+If you append the ``-e`` argument, pip will install in "editable" mode;
+which installs a version of the package that installs a forwarder
+pointing to these files, such that the package always reflects the
+latest version in your git tree.
+
+See `Installing packages using pip and virtual environments
+`_
+for more information.
+
+
+Files in this directory
+---
+
+- ``qemu/`` Python package source directory.
+- ``PACKAGE.rst`` is used as the README file that is visible on PyPI.org.
+- ``README.rst`` you are here!
+- ``VERSION`` contains the PEP-440 compliant version used to describe
+  this package; it is referenced by ``setup.cfg``.
+- ``setup.cfg`` houses setuptools package configuration.
+- ``setup.py`` is the setuptools installer used by pip; See above.
diff --git a/python/qemu/README.rst b/python/qemu/README.rst
new file mode 100644
index 000..d04943f526c
--- /dev/null
+++ b/python/qemu/README.rst
@@ -0,0 +1,8 @@
+QEMU Python Namespace
+=
+
+This directory serves as the root of a `Python PEP 420 implicit
+namespace package `_.
+
+Each directory below is assumed to be an installable Python package that
+is available under the ``qemu.`` namespace.
diff --git a/python/qemu/machine/README.rst b/python/qemu/machine/README.rst
new file mode 100644
index 000..ac2b4fffb42
--- /dev/null
+++ b/python/qemu/machine/README.rst
@@ -0,0 +1,9 @@
+qemu.machine package
+
+
+This package provides core utilities used for testing and debugging
+QEMU. It is used by the iotests, vm tests, acceptance tests, and several
+other utilities in the ./scripts directory. It is not a fully-fledged
+SDK and it is subject to change at any time.
+
+See the documentation in ``__init__.py`` for more information.
diff --git a/python/qemu/qmp/README.rst b/python/qemu/qmp/README.rst
new file mode 100644
index 000..c21951491cf
--- /dev/null
+++ b/python/qemu/qmp/README.rst
@@ -0,0 +1,9 @@
+qemu.qmp package
+
+
+This package provides a library used for connecting to and communicating
+with QMP servers. It is used extensively by iotests, vm tests,
+acceptance tests, and other utilities in the ./scripts directory. It is
+not a fully-fledged SDK and is subject to change at any time.
+
+See the documentation in ``__init__.py`` for more information.
diff --git a/python/qemu/utils/README.rst b/python/qemu/utils/README.rst
new file mode 100644
index 000..975fbf4d7de
--- /dev/null
+++ b/python/qemu/utils/README.rst
@@ -0,0 +1,7 @@
+qemu.utils package
+==
+
+This package provides miscellaneous utilities used for testing and
+debugging QEMU. It is used primarily by the vm and acceptance tests.
+
+See the documentation in ``__init__.py`` for more information.
-- 
2.31.1




[PATCH v8 07/31] python/machine: Trim line length to below 80 chars

2021-05-27 Thread John Snow
One more little delinting fix that snuck in.

Signed-off-by: John Snow 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Cleber Rosa 
---
 python/qemu/machine.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index 5d72c4ca369..a8837b36e47 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -97,7 +97,7 @@ def __init__(self,
 @param args: list of extra arguments
 @param wrapper: list of arguments used as prefix to qemu binary
 @param name: prefix for socket and log file names (default: qemu-PID)
-@param base_temp_dir: default location where temporary files are 
created
+@param base_temp_dir: default location where temp files are created
 @param monitor_address: address for QMP monitor
 @param socket_scm_helper: helper program, required for send_fd_scm()
 @param sock_dir: where to create socket (defaults to base_temp_dir)
-- 
2.31.1




[PATCH v8 04/31] python/console_socket: Add a pylint ignore

2021-05-27 Thread John Snow
We manage cleaning up this resource ourselves. Pylint should shush.

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
Message-id: 20210517184808.3562549-5-js...@redhat.com
Signed-off-by: John Snow 
---
 python/qemu/console_socket.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/python/qemu/console_socket.py b/python/qemu/console_socket.py
index 87237bebef7..8c4ff598ad7 100644
--- a/python/qemu/console_socket.py
+++ b/python/qemu/console_socket.py
@@ -39,6 +39,7 @@ def __init__(self, address: str, file: Optional[str] = None,
 self.connect(address)
 self._logfile = None
 if file:
+# pylint: disable=consider-using-with
 self._logfile = open(file, "bw")
 self._open = True
 self._drain_thread = None
-- 
2.31.1




[PATCH v8 11/31] python: add VERSION file

2021-05-27 Thread John Snow
Python infrastructure as it exists today is not capable reliably of
single-sourcing a package version from a parent directory. The authors
of pip are working to correct this, but as of today this is not possible.

The problem is that when using pip to build and install a python
package, it copies files over to a temporary directory and performs its
build there. This loses access to any information in the parent
directory, including git itself.

Further, Python versions have a standard (PEP 440) that may or may not
follow QEMU's versioning. In general, it does; but naturally QEMU does
not follow PEP 440. To avoid any automatically-generated conflict, a
manual version file is preferred.


I am proposing:

- Python tooling follows the QEMU version, indirectly, but with a major
  version of 0 to indicate that the API is not expected to be
  stable. This would mean version 0.5.2.0, 0.5.1.1, 0.5.3.0, etc.

- In the event that a Python package needs to be updated independently
  of the QEMU version, a pre-release alpha version should be preferred,
  but *only* after inclusion to the qemu development or stable branches.

  e.g. 0.5.2.0a1, 0.5.2.0a2, and so on should be preferred prior to
  5.2.0's release.

- The Python core tooling makes absolutely no version compatibility
  checks or constraints. It *may* work with releases of QEMU from the
  past or future, but it is not required to.

  i.e., "qemu.machine" will, for now, remain in lock-step with QEMU.

- We reserve the right to split the qemu package into independently
  versioned subpackages at a later date. This might allow for us to
  begin versioning QMP independently from QEMU at a later date, if
  we so choose.


Implement this versioning scheme by adding a VERSION file and setting it
to 0.6.0.0a1.

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
---
 python/VERSION   | 1 +
 python/setup.cfg | 1 +
 2 files changed, 2 insertions(+)
 create mode 100644 python/VERSION

diff --git a/python/VERSION b/python/VERSION
new file mode 100644
index 000..c19f3b832b7
--- /dev/null
+++ b/python/VERSION
@@ -0,0 +1 @@
+0.6.1.0a1
diff --git a/python/setup.cfg b/python/setup.cfg
index 3fa92a2e73f..b0010e0188f 100644
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -1,5 +1,6 @@
 [metadata]
 name = qemu
+version = file:VERSION
 maintainer = QEMU Developer Team
 maintainer_email = qemu-devel@nongnu.org
 url = https://www.qemu.org/
-- 
2.31.1




[PATCH v8 06/31] python/machine: disable warning for Popen in _launch()

2021-05-27 Thread John Snow
We handle this resource rather meticulously in
shutdown/kill/wait/__exit__ et al, through the laborious mechanisms in
_do_shutdown().

Quiet this pylint warning here.

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
Message-id: 20210517184808.3562549-7-js...@redhat.com
Signed-off-by: John Snow 
---
 python/qemu/machine.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index c66bc6a9c69..5d72c4ca369 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -405,6 +405,9 @@ def _launch(self) -> None:
   self._args)
 )
 LOG.debug('VM launch command: %r', ' '.join(self._qemu_full_args))
+
+# Cleaning up of this subprocess is guaranteed by _do_shutdown.
+# pylint: disable=consider-using-with
 self._popen = subprocess.Popen(self._qemu_full_args,
stdin=subprocess.DEVNULL,
stdout=self._qemu_log_file,
-- 
2.31.1




[PATCH v8 08/31] iotests/297: add --namespace-packages to mypy arguments

2021-05-27 Thread John Snow
mypy is kind of weird about how it handles imports. For legacy reasons,
it won't load PEP 420 namespaces, because of logic implemented prior to
that becoming a standard.

So, if you plan on using any, you have to pass
--namespace-packages. Alright, fine.

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
---
 tests/qemu-iotests/297 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index a37910b42d9..433b7323368 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -95,6 +95,7 @@ def run_linters():
 '--warn-redundant-casts',
 '--warn-unused-ignores',
 '--no-implicit-reexport',
+'--namespace-packages',
 filename),
env=env,
check=False,
-- 
2.31.1




[PATCH v8 01/31] python/console_socket: avoid one-letter variable

2021-05-27 Thread John Snow
Fixes pylint warnings.

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
Reviewed-by: Philippe Mathieu-Daudé 
Message-id: 20210517184808.3562549-2-js...@redhat.com
Signed-off-by: John Snow 
---
 python/qemu/console_socket.py | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/python/qemu/console_socket.py b/python/qemu/console_socket.py
index ac21130e446..87237bebef7 100644
--- a/python/qemu/console_socket.py
+++ b/python/qemu/console_socket.py
@@ -46,11 +46,11 @@ def __init__(self, address: str, file: Optional[str] = None,
 self._drain_thread = self._thread_start()
 
 def __repr__(self) -> str:
-s = super().__repr__()
-s = s.rstrip(">")
-s = "%s,  logfile=%s, drain_thread=%s>" % (s, self._logfile,
-   self._drain_thread)
-return s
+tmp = super().__repr__()
+tmp = tmp.rstrip(">")
+tmp = "%s,  logfile=%s, drain_thread=%s>" % (tmp, self._logfile,
+ self._drain_thread)
+return tmp
 
 def _drain_fn(self) -> None:
 """Drains the socket and runs while the socket is open."""
-- 
2.31.1




[PATCH v8 10/31] python: add qemu package installer

2021-05-27 Thread John Snow
Add setup.cfg and setup.py, necessary for installing a package via
pip. Add a ReST document (PACKAGE.rst) explaining the basics of what
this package is for and who to contact for more information. This
document will be used as the landing page for the package on PyPI.

List the subpackages we intend to package by name instead of using
find_namespace because find_namespace will naively also packages tests,
things it finds in the dist/ folder, etc. I could not figure out how to
modify this behavior; adding allow/deny lists to setuptools kept
changing the packaged hierarchy. This works, roll with it.

I am not yet using a pyproject.toml style package manifest, because
"editable" installs are not defined (yet?) by PEP-517/518.

I consider editable installs crucial for development, though they have
(apparently) always been somewhat poorly defined.

Pip now (19.2 and later) now supports editable installs for projects
using pyproject.toml manifests, but might require the use of the
--no-use-pep517 flag, which somewhat defeats the point. Full support for
setup.py-less editable installs was not introduced until pip 21.1.1:
https://github.com/pypa/pip/pull/9547/commits/7a95720e796a5e56481c1cc20b6ce6249c50f357

For now, while the dust settles, stick with the de-facto
setup.py/setup.cfg combination supported by setuptools. It will be worth
re-evaluating this point again in the future when our supported build
platforms all ship a fairly modern pip.

Additional reading on this matter:

https://github.com/pypa/packaging-problems/issues/256
https://github.com/pypa/pip/issues/6334
https://github.com/pypa/pip/issues/6375
https://github.com/pypa/pip/issues/6434
https://github.com/pypa/pip/issues/6438

Signed-off-by: John Snow 
Reviewed-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Cleber Rosa 
---
 python/PACKAGE.rst | 33 +
 python/setup.cfg   | 22 ++
 python/setup.py| 23 +++
 3 files changed, 78 insertions(+)
 create mode 100644 python/PACKAGE.rst
 create mode 100644 python/setup.cfg
 create mode 100755 python/setup.py

diff --git a/python/PACKAGE.rst b/python/PACKAGE.rst
new file mode 100644
index 000..1bbfe1b58e2
--- /dev/null
+++ b/python/PACKAGE.rst
@@ -0,0 +1,33 @@
+QEMU Python Tooling
+===
+
+This package provides QEMU tooling used by the QEMU project to build,
+configure, and test QEMU. It is not a fully-fledged SDK and it is subject
+to change at any time.
+
+Usage
+-
+
+The ``qemu.qmp`` subpackage provides a library for communicating with
+QMP servers. The ``qemu.machine`` subpackage offers rudimentary
+facilities for launching and managing QEMU processes. Refer to each
+package's documentation
+(``>>> help(qemu.qmp)``, ``>>> help(qemu.machine)``)
+for more information.
+
+Contributing
+
+
+This package is maintained by John Snow  as part of
+the QEMU source tree. Contributions are welcome and follow the `QEMU
+patch submission process
+`_, which involves
+sending patches to the QEMU development mailing list.
+
+John maintains a `GitLab staging branch
+`_, and there is an
+official `GitLab mirror `_.
+
+Please report bugs on the `QEMU issue tracker
+`_ and tag ``@jsnow`` in
+the report.
diff --git a/python/setup.cfg b/python/setup.cfg
new file mode 100644
index 000..3fa92a2e73f
--- /dev/null
+++ b/python/setup.cfg
@@ -0,0 +1,22 @@
+[metadata]
+name = qemu
+maintainer = QEMU Developer Team
+maintainer_email = qemu-devel@nongnu.org
+url = https://www.qemu.org/
+download_url = https://www.qemu.org/download/
+description = QEMU Python Build, Debug and SDK tooling.
+long_description = file:PACKAGE.rst
+long_description_content_type = text/x-rst
+classifiers =
+Development Status :: 3 - Alpha
+License :: OSI Approved :: GNU General Public License v2 (GPLv2)
+Natural Language :: English
+Operating System :: OS Independent
+Programming Language :: Python :: 3 :: Only
+
+[options]
+python_requires = >= 3.6
+packages =
+qemu.qmp
+qemu.machine
+qemu.utils
diff --git a/python/setup.py b/python/setup.py
new file mode 100755
index 000..2014f81b757
--- /dev/null
+++ b/python/setup.py
@@ -0,0 +1,23 @@
+#!/usr/bin/env python3
+"""
+QEMU tooling installer script
+Copyright (c) 2020-2021 John Snow for Red Hat, Inc.
+"""
+
+import setuptools
+import pkg_resources
+
+
+def main():
+"""
+QEMU tooling installer
+"""
+
+# 
https://medium.com/@daveshawley/safely-using-setup-cfg-for-metadata-1babbe54c108
+pkg_resources.require('setuptools>=39.2')
+
+setuptools.setup()
+
+
+if __name__ == '__main__':
+main()
-- 
2.31.1




[PATCH v8 03/31] python/machine: use subprocess.run instead of subprocess.Popen

2021-05-27 Thread John Snow
use run() instead of Popen() -- to assert to pylint that we are not
forgetting to close a long-running program.

Signed-off-by: John Snow 
Reviewed-by: Cleber Rosa 
Tested-by: Cleber Rosa 
Message-id: 20210517184808.3562549-4-js...@redhat.com
Signed-off-by: John Snow 
---
 python/qemu/machine.py | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index 5b87e9ce024..04e005f3811 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -223,13 +223,16 @@ def send_fd_scm(self, fd: Optional[int] = None,
 assert fd is not None
 fd_param.append(str(fd))
 
-proc = subprocess.Popen(
-fd_param, stdin=subprocess.DEVNULL, stdout=subprocess.PIPE,
-stderr=subprocess.STDOUT, close_fds=False
+proc = subprocess.run(
+fd_param,
+stdin=subprocess.DEVNULL,
+stdout=subprocess.PIPE,
+stderr=subprocess.STDOUT,
+check=False,
+close_fds=False,
 )
-output = proc.communicate()[0]
-if output:
-LOG.debug(output)
+if proc.stdout:
+LOG.debug(proc.stdout)
 
 return proc.returncode
 
-- 
2.31.1




[PATCH v8 02/31] python/machine: use subprocess.DEVNULL instead of open(os.path.devnull)

2021-05-27 Thread John Snow
One less file resource to manage, and it helps quiet some pylint >=
2.8.0 warnings about not using a with-context manager for the open call.

Signed-off-by: John Snow 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Cleber Rosa 
Message-id: 20210517184808.3562549-3-js...@redhat.com
Signed-off-by: John Snow 
---
 python/qemu/machine.py | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/python/qemu/machine.py b/python/qemu/machine.py
index b379fcbe726..5b87e9ce024 100644
--- a/python/qemu/machine.py
+++ b/python/qemu/machine.py
@@ -223,9 +223,8 @@ def send_fd_scm(self, fd: Optional[int] = None,
 assert fd is not None
 fd_param.append(str(fd))
 
-devnull = open(os.path.devnull, 'rb')
 proc = subprocess.Popen(
-fd_param, stdin=devnull, stdout=subprocess.PIPE,
+fd_param, stdin=subprocess.DEVNULL, stdout=subprocess.PIPE,
 stderr=subprocess.STDOUT, close_fds=False
 )
 output = proc.communicate()[0]
@@ -391,7 +390,6 @@ def _launch(self) -> None:
 """
 Launch the VM and establish a QMP connection
 """
-devnull = open(os.path.devnull, 'rb')
 self._pre_launch()
 self._qemu_full_args = tuple(
 chain(self._wrapper,
@@ -401,7 +399,7 @@ def _launch(self) -> None:
 )
 LOG.debug('VM launch command: %r', ' '.join(self._qemu_full_args))
 self._popen = subprocess.Popen(self._qemu_full_args,
-   stdin=devnull,
+   stdin=subprocess.DEVNULL,
stdout=self._qemu_log_file,
stderr=subprocess.STDOUT,
shell=False,
-- 
2.31.1




[PATCH v8 00/31] python: create installable package

2021-05-27 Thread John Snow
Based-on: https://gitlab.com/cleber.gnu/qemu/-/commits/python-next
CI: https://gitlab.com/jsnow/qemu/-/pipelines/310845984
GitLab: https://gitlab.com/jsnow/qemu/-/tree/python-package-mk4
MR: https://gitlab.com/jsnow/qemu/-/merge_requests/7

(Note: v8 is just being sent to collate final RB/TB tags and get final
Message-IDs.)

ABOUT
=

This series factors the python/qemu directory as an installable
package. It does not yet actually change the mechanics of how any other
python source in the tree actually consumes it (yet), beyond the import
path -- some import statements change in a few places.

RATIONALE
=

The primary motivation of this series is primarily to formalize our
dependencies on mypy, flake8, isort, and pylint alongside versions that
are known to work. It does this using the setup.cfg and setup.py
files. It also adds explicitly pinned versions (using Pipfile.lock) of
these dependencies that should behave in a repeatable and known way for
developers and CI environments both. Lastly, it enables those CI checks
such that we can enforce Python coding quality checks via the CI tests.

An auxiliary motivation is that this package is formatted in such a way
that it COULD be uploaded to https://pypi.org/project/qemu and installed
independently of qemu.git with `pip install qemu`, but that button
remains *unpushed* and this series *will not* cause any such
releases. We have time to debate finer points like API guarantees and
versioning even after this series is merged.

Other bits of interest
--

With the python tooling as a proper package, you can install this
package in editable or production mode to a virtual environment, your
local user environment, or your system packages. The primary benefit of
this is to gain access to QMP tooling regardless of CWD, without needing
to battle sys.path (and confounding other python analysis tools).

For example: when developing, you may go to qemu/python/ and run `make
venv` followed by `pipenv shell` to activate a virtual environment that
contains the qemu python packages. These packages will always reflect
the current version of the source files in the tree. When you are
finished, you can simply exit the shell (^d) to remove these packages
from your python environment.

When not developing, you could install a version of this package to your
environment outright to gain access to the QMP and QEMUMachine classes
for lightweight scripting and testing by using pip: "pip install
[--user] ."

TESTING THIS SERIES
===

First of all, nothing should change. Without any intervention,
everything should behave exactly as it did before. The only new
information here comes from how to interact with and run the linters
that will be enforcing code quality standards in this subdirectory.

There are various invocations available that will test subtly different
combinations using subtly different environments. I am assuming some
light knowledge of Python environments and installing Python packages
here. If you have questions, I would be delighted to answer them.

To test the new tests, CD to ./python/ first, and then:

0. Try "make" or "make help" to get a sense of this series.

1. Try "make venv && pipenv shell" to get a venv with the package
   installed to it in editable mode. Ctrl+d exits this venv shell. While
   in this shell, any python script that uses "from qemu.[qmp|machine]
   import ..." should work correctly regardless of where the script is,
   or what your CWD is.

   This will pull some packages from PyPI and install them into the
   virtual environment, leaving your normal environment untouched.

   You will need Python 3.6 and pipenv installed on your system to do
   this step. For Fedora: "dnf install python36 pipenv" will do the
   trick. If you don't have this, skip down to \#4 and onwards.

2. Try "make check" while still in the shell to run the Python linters
using the venv built in the previous step. This will run avocado, which
will in turn execute mypy, flake8, isort and pylint with the correct
arguments.

3. Having exited the shell from above, try "make venv-check". This will
create and update the venv if needed, then run 'make check' within the
context of that shell. It should pass as long as the above did. You
should be able to run "make distclean" prior to running "make
venv-check" and have the entire process work start to finish.

4. Still outside of the venv, you may try running "make check". This
will not install anything, but unless you have the right Python
dependencies installed, these tests may fail for you. You might try
using "pip install --user .[devel]" to install the development packages
needed to run the tests successfully to your local user's python
environment. Once done, you will probably want to "pip uninstall qemu"
to remove the qemu packages to avoid them interfering with other things.

5. "make distclean" will delete the venv and any temporary files that
may have been created by 

Re: [PATCH v4 1/2] target/i386: Trivial code motion

2021-05-27 Thread Eduardo Habkost
On Tue, May 18, 2021 at 10:53:51AM +0800, Ziqiao Kong wrote:
> On Tue, May 18, 2021 at 4:16 AM Eduardo Habkost  wrote:
> >
> > On Fri, May 07, 2021 at 04:00:56PM +0800, Ziqiao Kong wrote:
> > > Move the float translation case to a new block by a new pair of braces.
> >
> > If you are just adding braces around the code, do you really need
> > to reindent all the code?  I don't see any mention of `switch`
> > statements on style.rst, but I see 235 existing cases where the
> > brackets are aligned below the `c` in `case`.
> 
> The indention style is from the translate.c itself like here:
> 
> https://github.com/qemu/qemu/blob/master/target/i386/tcg/translate.c#L5634
> 
> There are also many similar cases in translate.c.

Oh, right.  Makes sense to me (and sorry for not noticing this
before).

> 
> >
> > In either case, I'm looking for a description of "why", not
> > "what", but I couldn't find it.  Why are the braces necessary or
> > useful here?
> 
> I have to declare some variables in this case scope, like last_addr, last_seg,
> update_fdp and update_fip according to the previous review. Without the braces
> here, they have to be declared at the beginning of the function like
> the v2 patch.
> Shall I state that in the commit message?

Yes, please.  Ideally every commit message should state the
reason for the change.

-- 
Eduardo




Re: [PATCH v4 2/2] target/i386: Correct implementation for FCS, FIP, FDS and FDP

2021-05-27 Thread Eduardo Habkost
On Tue, May 18, 2021 at 11:06:56AM +0800, Ziqiao Kong wrote:
[...]
> > > +/*
> > > + * If CR0.PE = 1, each instruction saves FCS and FDS into memory. If
> > > + * CPUID.(EAX=07H,ECX=0H):EBX[bit 13] = 1, the processor deprecates
> > > + * FCS and FDS; it saves each as H.
> > > + */
> > > +if (!(env->features[FEAT_7_0_EBX] & CPUID_7_0_EBX_FCS_FDS)
> > > +&& (env->cr[0] & CR0_PE_MASK)) {
> > > +fpcs = env->fpcs;
> > > +fpds = env->fpds;
> >
> >
> > If you want to start supporting this feature flag, I suggest
> > moving this to a separate patch.  The description of this patch
> > seems to imply it is just a bug fix, not the implementation of a
> > new feature flag.
> >
> > When adding support for a new feature flag in TCG code, you need
> > to extend TCG_*_FEATURES in target/i386/cpu.c, otherwise the
> > feature flag will never be enabled by the CPU configuration code.
> 
> Yes, currently this feature flag is never enabled for all CPU types.
> How about removing this
> feature flag in this patch and leaving a TODO in the comment? Thus I
> can issue another patch
> to complete the feature later.

Sounds better to me.  Otherwise you'll be just adding dead code
(because the flag will is impossible to be enabled if it's not
present in TCG_*_FEATURES).

-- 
Eduardo




Re: [PATCH 5/5] hw/core/cpu: removed cpu_dump_statistics function

2021-05-27 Thread Eduardo Habkost
On Wed, May 26, 2021 at 05:21:04PM -0300, Bruno Larsen (billionai) wrote:
> No more architectures set the pointer to dump_statistics, so there's no
> point in keeping it, or the related cpu_dump_statistics function.
> 
> Suggested-by: Richard Henderson 
> Signed-off-by: Bruno Larsen (billionai) 

Acked-by: Eduardo Habkost 

-- 
Eduardo




Re: [PATCH v3] target/i386/sev: add support to query the attestation report

2021-05-27 Thread Eduardo Habkost
On Thu, Apr 29, 2021 at 12:07:28PM -0500, Brijesh Singh wrote:
> The SEV FW >= 0.23 added a new command that can be used to query the
> attestation report containing the SHA-256 digest of the guest memory
> and VMSA encrypted with the LAUNCH_UPDATE and sign it with the PEK.
> 
> Note, we already have a command (LAUNCH_MEASURE) that can be used to
> query the SHA-256 digest of the guest memory encrypted through the
> LAUNCH_UPDATE. The main difference between previous and this command
> is that the report is signed with the PEK and unlike the LAUNCH_MEASURE
> command the ATTESATION_REPORT command can be called while the guest
> is running.
> 
> Add a QMP interface "query-sev-attestation-report" that can be used
> to get the report encoded in base64.
> 
> Cc: James Bottomley 
> Cc: Tom Lendacky 
> Cc: Eric Blake 
> Cc: Paolo Bonzini 
> Cc: k...@vger.kernel.org
> Reviewed-by: James Bottomley 
> Tested-by: James Bottomley 
> Signed-off-by: Brijesh Singh 

Queued, thanks!

-- 
Eduardo




Re: [PATCH 2/2] tests/acceptance: Add tests for the Pegasos2 machine

2021-05-27 Thread Wainer dos Santos Moschetta

Hi,

On 5/15/21 10:45 AM, Philippe Mathieu-Daudé wrote:

Add a pair of tests for the Pegasos2 machine following the steps from:
https://lists.nongnu.org/archive/html/qemu-devel/2021-01/msg01553.html

   $ PEGASOS2_ROM_PATH=/tmp/pegasos2.rom AVOCADO_ALLOW_UNTRUSTED_CODE=1 \
 avocado --show=app,console,tesseract \
   run -t machine:pegasos2 tests/acceptance/
(1/2) 
tests/acceptance/machine_ppc_pegasos.py:PPCPegasos2.test_rom_serial_console:
   console: PegasosII Boot Strap (c) 2002-2003 bplan GmbH
   console: Running on CPU PVR:000C0209
   console: Enable L1 ICache... 
   Done.
   console: Reading W83194 :
   FAILED.
   console: Setting Front Side Bus to 133MHz... 
   FAILED.
   console: Configuring DDR...  
   Done.
   console: Configuring PCI0... 
   Done.
   console: Configuring PCI1... 
   Done.
   console: Configuring ETH...  
   Done.
   console: Releasing IDE reset ... 
   Done.
   console: Configuring Legacy Devices
   console: Initializing KBD... 
   Done.
   console: Testing 1000 Bytes, Pass:  Failed: 
   console: RAM TEST (fill linear)...   
   Done.
   console: 
   console: SmartFirmware:
   console: cpu0: PowerPC,G4 CPUClock 599 Mhz BUSClock 133 Mhz (Version 
0x000C,0x0209)
   console: no/bad nvramrc - performing default startup script
   console: channel 1 unit 0 : atapi | QEMU DVD-ROM 
| 2.5+
   console: ATA device not present or not responding
   console: Welcome to SmartFirmware(tm) for bplan Pegasos2 version 1.1 
(20040405172512)
   PASS (5.23 s)
(2/2) 
tests/acceptance/machine_ppc_pegasos.py:PPCPegasos2.test_morphos_cdrom_vga:
   ...
   console: Welcome to SmartFirmware(tm) for bplan Pegasos2 version 1.1 
(20040405172512)
   console: SmartFirmware(tm) Copyright 1996-2001 by CodeGen, Inc.
   console: All Rights Reserved.
   console: Pegasos BIOS Extensions Copyright 2001-2003 by bplan GmbH.
   console: All Rights Reserved.
   console: entering main read/eval loop...
   console: ok boot cd boot.img
   console: ISO-9660 filesystem:  System-ID: "MORPHOS"  Volume-ID: "MorphOSBoot"
   console: " flags=0x2 extent=0x20 size=0x1800
   console: Memory used before SYS_Init: 9MB
   console: PCI ATA/ATAPI Driver@2: PIO Mode 4
   console: PCI ATA/ATAPI Driver@2: UDMA Mode 5
   console: ide.device@2: QEMU QEMU DVD-ROM 
   console: ide.device@2:  CDRom , found, bootable
   tesseract: Ambient Screen 4: Saturday, 15 May 2021, 13:36:06 &
   tesseract: keymap
   tesseract: Albanian keyboard with 101/104 keys
   tesseract: ‘American keyboard with Greek input extension, 105 keys
   tesseract: Belarusian keyboard with 105 keys
   tesseract: Belgian keyboard with 105 keys J
   tesseract: British Apple keyboard
   tesseract: British keyboard with 105 keys
   tesseract: Bulgarian keyboard with 104 keys
   tesseract: Canadian keyboard with 105 keys
   tesseract: Colemak layout for keyboards with 101/104 keys
   tesseract: Croatian keyboard with 101/108 keys
   tesseract: Czech keyboard (QWERTY) with 101/104 keys
   tesseract: Czech keyboard (QWERTZ) with 101/104 keys
   tesseract: Danish keyboard with 105 keys
   PASS (28.56 s)
   RESULTS: PASS 2 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | 
CANCEL 0
   JOB TIME   : 34.42 s

Signed-off-by: Philippe Mathieu-Daudé 
---
  tests/acceptance/machine_ppc_pegasos.py | 98 +
  1 file changed, 98 insertions(+)
  create mode 100644 tests/acceptance/machine_ppc_pegasos.py

diff --git a/tests/acceptance/machine_ppc_pegasos.py 
b/tests/acceptance/machine_ppc_pegasos.py
new file mode 100644
index 000..d36e920ebde
--- /dev/null
+++ b/tests/acceptance/machine_ppc_pegasos.py
@@ -0,0 +1,98 @@
+# Functional tests for the Pegasos2 machine.
+#
+# Copyright (c) 2021 Philippe Mathieu-Daudé 
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or later.
+# See the COPYING file in the top-level directory.
+#
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+import os
+import time
+
+from avocado import skipUnless
+from avocado_qemu import Test
+from avocado_qemu import exec_command_and_wait_for_pattern
+from avocado_qemu import wait_for_console_pattern
+from tesseract_utils import tesseract_available, tesseract_ocr
+
+PIL_AVAILABLE = True
+try:
+from PIL import Image
+except ImportError:
+PIL_AVAILABLE = False
+
+
+@skipUnless(os.getenv('AVOCADO_ALLOW_UNTRUSTED_CODE'), 'untrusted code')
+@skipUnless(os.getenv('PEGASOS2_ROM_PATH'), 'PEGASOS2_ROM_PATH not available')
+class PPCPegasos2(Test):
+

Re: [PATCH] tcg/ppc: Fix building with Clang

2021-05-27 Thread Brad Smith

On 5/2/2021 12:02 AM, Brad Smith wrote:


On 4/22/2021 11:39 AM, Richard Henderson wrote:

On 4/22/21 2:20 AM, Peter Maydell wrote:

On Thu, 22 Apr 2021 at 06:18, Richard Henderson

I'm thinking something like

#if !defined(_CALL_SYSV) && \
  !defined(_CALL_DARWIN) && \
  !defined(_CALL_AIX) && \
  !defined(_CALL_ELF)
# if defined(__APPLE__)
#  define _CALL_DARWIN
# elif defined(__ELF__) && TCG_TARGET_REG_BITS == 32
#  define _CALL_SYSV
# else
#  error "Unknown ABI"
# endif
#endif


Doesn't this also need some case that handles "64bit ppc clang which 
doesn't

define _CALL_anything" ?


Clang does define _CALL_ELF for ppc64:

 // ABI options.
 if (ABI == "elfv1")
   Builder.defineMacro("_CALL_ELF", "1");
 if (ABI == "elfv2")
   Builder.defineMacro("_CALL_ELF", "2");



Able to spin up a patch that you think is appropriate?


ping?



Re: [PATCH] arm: Consistently use "Cortex-Axx", not "Cortex Axx"

2021-05-27 Thread Richard Henderson

On 5/27/21 2:51 AM, Peter Maydell wrote:

The official punctuation for Arm CPU names uses a hyphen, like
"Cortex-A9". We mostly follow this, but in a few places usage
without the hyphen has crept in. Fix those so we consistently
use the same way of writing the CPU name.

This commit was created with:
   git grep -z -l 'Cortex ' | xargs -0 sed -i 's/Cortex /Cortex-/'

Signed-off-by: Peter Maydell
---


Reviewed-by: Richard Henderson 

r~



Re: [PATCH v2 2/6] file-posix: try BLKSECTGET on block devices too, do not round to power of 2

2021-05-27 Thread Paolo Bonzini

On 27/05/21 17:51, Kevin Wolf wrote:

Am 24.05.2021 um 18:36 hat Paolo Bonzini geschrieben:

bs->sg is only true for character devices, but block devices can also
be used with scsi-block and scsi-generic.  Unfortunately BLKSECTGET
returns bytes in an int for /dev/sgN devices, and sectors in a short
for block devices, so account for that in the code.

The maximum transfer also need not be a power of 2 (for example I have
seen disks with 1280 KiB maximum transfer) so there's no need to pass
the result through pow2floor.

Signed-off-by: Paolo Bonzini 


Looks like this is more or less a revert of Maxim's commit 867eccfe. If
this is what we want, should this old commit be mentioned in one way or
another in the commit message?


It is (but it is not intentional).


Apparently the motivation for Maxim's patch was, if I'm reading the
description correctly, that it affected non-sg cases by imposing
unnecessary restrictions. I see that patch 1 changed the max_iov part so
that it won't affect non-sg cases any more, but max_transfer could still
be more restricted than necessary, no?


Indeed the kernel puts no limit at all, but especially with O_DIRECT we 
probably benefit from avoiding the moral equivalent of "bufferbloat".



For convenience, the bug report fixed with that patch is here:
https://bugzilla.redhat.com/show_bug.cgi?id=1647104

Are we really trying to describe different things (limits for SG_IO and
for normal I/O) in one value with max_transfer, even though it could be
two different numbers for the same block device?



-static int sg_get_max_transfer_length(int fd)
+static int sg_get_max_transfer_length(int fd, struct stat *st)


This is now a misnomer. Should we revert to the pre-867eccfe name
hdev_get_max_transfer_length()?


Yes.


  static void raw_refresh_limits(BlockDriverState *bs, Error **errp)
  {
  BDRVRawState *s = bs->opaque;
+struct stat st;
+
+if (fstat(s->fd, )) {
+return;


Don't we want to set errp? Or do you intentionally ignore the error?


Yes, since we ignore errors from the ioctl I figured it's the same for 
fstat (just do not do the ioctls).


However, skipping raw_probe_alignment is wrong.

Thanks for the review!  Should I wait for you to go through the other 
patches?


Paolo




  1   2   3   4   >