[PATCH v3 00/15] python: create installable package

[PATCH v3 00/15] python: create installable package

[John Snow]

Based-on: https://gitlab.com/jsnow/qemu/-/tree/python

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.

The point of this series is primarily to formalize our dependencies on
mypy, flake8, isort, and pylint alongside versions that are known to
work. It also adds explicitly pinned versions of these dependencies that
should behave in a repeatable and known way for developers and CI
environments both.

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.

For example: when developing, you may go to qemu/python/ and invoke
`pipenv shell` to activate a virtual environment that contains the qemu
packages.  This package will always reflect the current version of the
source files in the tree. When you are finished, you can simply exit the
shell 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] ."

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

TESTING THIS SERIES:

CD to qemu/python first, and then:

1. Try "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 your CWD.

2. Try "pipenv sync --dev" to create/update the venv with the
development packages without actually entering the venv. This should
install isort, mypy, flake8 and pylint to the venv.

3. After the above sync, try "pipenv shell" again, and from the python
project root, try any of the following:

  - pylint qemu
  - flake8 qemu
  - isort -c qemu
  - mypy qemu

4. Leave any venv you are in, and from the project root, try the
following commands:

  - pipenv run pylint qemu
  - pipenv run flake8 qemu
  - pipenv run isort -c qemu
  - pipenv run mypy qemu

V3:
 - Changed "qemu.core" to "qemu.qmp" and "qemu.machine",
   Partly to accommodate forthcoming work which would benefit from a separate
   qemu.qmp namespace.
 - Changed the initial version from 5.2.0a1 to 0.5.2.0a1, to allow for
   more rapid development while we smooth out the initial kinks.
 - 001: Renamed patch title; differences implement the new names.
 - 002: Readme changes for above.
 - 003: Version change.
 - 004: New readme for the new qmp directory.
 - 006: A few more import exceptions for pylint, hopefully temporary.
 - 009: Updated flake8 config comment to match qapi's
 - 012: Added namespace_package configuration value

001/15:[down] 'python: create qemu packages'
002/15:[0009] [FC] 'python: add qemu package installer'
003/15:[0002] [FC] 'python: add VERSION file'
004/15:[0015] [FC] 'python: add directory structure README.rst files'
005/15:[----] [--] 'python: Add pipenv support'
006/15:[down] 'python: add pylint import exceptions'
007/15:[----] [--] 'python: move pylintrc into setup.cfg'
008/15:[----] [--] 'python: add pylint to pipenv'
009/15:[0002] [FC] 'python: move flake8 config to setup.cfg'
010/15:[----] [--] 'python: Add flake8 to pipenv'
011/15:[----] [-C] 'python: move mypy.ini into setup.cfg'
012/15:[0001] [FC] 'python: add mypy to pipenv'
013/15:[----] [--] 'python: move .isort.cfg into setup.cfg'
014/15:[----] [--] 'python/qemu: add isort to pipenv'
015/15:[----] [--] 'python/qemu: add qemu package itself to pipenv'

John Snow (15):
  python: create qemu packages
  python: add qemu package installer
  python: add VERSION file
  python: add directory structure README.rst files
  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 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/PACKAGE.rst                          |  26 +++
 python/README.rst                           |  27 +++
 python/qemu/README.rst                      |   8 +
 python/qemu/machine/README.rst              |   9 +
 python/qemu/qmp/README.rst                  |   9 +
 python/Pipfile                              |  16 ++
 python/Pipfile.lock                         | 207 ++++++++++++++++++++
 python/VERSION                              |   1 +
 python/mypy.ini                             |   4 -
 python/qemu/.flake8                         |   2 -
 python/qemu/.isort.cfg                      |   7 -
 python/qemu/__init__.py                     |  11 --
 python/qemu/machine/__init__.py             |  44 +++++
 python/qemu/{ => machine}/accel.py          |   0
 python/qemu/{ => machine}/console_socket.py |   0
 python/qemu/{ => machine}/machine.py        |  16 +-
 python/qemu/{ => machine}/qtest.py          |   3 +-
 python/qemu/{qmp.py => qmp/__init__.py}     |  12 +-
 python/{qemu/pylintrc => setup.cfg}         |  67 ++++---
 python/setup.py                             |  23 +++
 tests/acceptance/boot_linux.py              |   3 +-
 tests/qemu-iotests/297                      |   2 +-
 tests/qemu-iotests/300                      |   4 +-
 tests/qemu-iotests/iotests.py               |   2 +-
 tests/vm/basevm.py                          |   3 +-
 25 files changed, 437 insertions(+), 69 deletions(-)
 create mode 100644 python/PACKAGE.rst
 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/Pipfile
 create mode 100644 python/Pipfile.lock
 create mode 100644 python/VERSION
 delete mode 100644 python/mypy.ini
 delete mode 100644 python/qemu/.flake8
 delete mode 100644 python/qemu/.isort.cfg
 delete mode 100644 python/qemu/__init__.py
 create mode 100644 python/qemu/machine/__init__.py
 rename python/qemu/{ => machine}/accel.py (100%)
 rename python/qemu/{ => machine}/console_socket.py (100%)
 rename python/qemu/{ => machine}/machine.py (98%)
 rename python/qemu/{ => machine}/qtest.py (98%)
 rename python/qemu/{qmp.py => qmp/__init__.py} (96%)
 rename python/{qemu/pylintrc => setup.cfg} (50%)
 mode change 100644 => 100755
 create mode 100755 python/setup.py

-- 
2.26.2

[PATCH v3 01/15] python: create qemu packages

[John Snow]

move python/qemu/*.py to python/qemu/[machine, qmp]/*.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' and 'qmp'
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 <jsnow@redhat.com>
---
 python/{qemu => }/.isort.cfg                |  0
 python/qemu/__init__.py                     | 11 ------
 python/qemu/{ => machine}/.flake8           |  0
 python/qemu/machine/__init__.py             | 41 +++++++++++++++++++++
 python/qemu/{ => machine}/accel.py          |  0
 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 +++++-
 tests/acceptance/boot_linux.py              |  3 +-
 tests/qemu-iotests/297                      |  2 +-
 tests/qemu-iotests/300                      |  4 +-
 tests/qemu-iotests/iotests.py               |  2 +-
 tests/vm/basevm.py                          |  3 +-
 15 files changed, 71 insertions(+), 26 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}/accel.py (100%)
 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%)

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 4ca06c34a4..0000000000
--- 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 <famz@redhat.com>
-#
-# 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 0000000000..27b0b19abd
--- /dev/null
+++ b/python/qemu/machine/__init__.py
@@ -0,0 +1,41 @@
+"""
+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.
+
+- list_accel: List available accelerators
+- kvm_available: Probe for KVM support
+- tcg_available: Probe for TCG support
+"""
+
+# Copyright (C) 2020 John Snow for Red Hat Inc.
+# Copyright (C) 2015-2016 Red Hat Inc.
+# Copyright (C) 2012 IBM Corp.
+#
+# Authors:
+#  John Snow <jsnow@redhat.com>
+#  Fam Zheng <fam@euphon.net>
+#
+# This work is licensed under the terms of the GNU GPL, version 2.  See
+# the COPYING file in the top-level directory.
+#
+
+from .accel import kvm_available, list_accel, tcg_available
+from .machine import QEMUMachine
+from .qtest import QEMUQtestMachine, QEMUQtestProtocol
+
+
+__all__ = (
+    'list_accel',
+    'kvm_available',
+    'tcg_available',
+    'QEMUMachine',
+    'QEMUQtestProtocol',
+    'QEMUQtestMachine',
+)
diff --git a/python/qemu/accel.py b/python/qemu/machine/accel.py
similarity index 100%
rename from python/qemu/accel.py
rename to python/qemu/machine/accel.py
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 6420f01bed..a5dc305539 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 by default.
-        self._qmp_connection: Optional[qmp.QEMUMonitorProtocol] = None
+        self._qmp_connection: Optional[QEMUMonitorProtocol] = None
         self._qemu_full_args: Tuple[str, ...] = ()
         self._temp_dir: Optional[str] = None
         self._launched = False
@@ -314,7 +320,7 @@ def _pre_launch(self) -> None:
             if self._remove_monitor_sockfile:
                 assert isinstance(self._monitor_address, str)
                 self._remove_files.append(self._monitor_address)
-            self._qmp_connection = qmp.QEMUMonitorProtocol(
+            self._qmp_connection = QEMUMonitorProtocol(
                 self._monitor_address,
                 server=True,
                 nickname=self._name
@@ -534,7 +540,7 @@ def set_qmp_monitor(self, enabled: bool = True) -> None:
         self._qmp_set = enabled
 
     @property
-    def _qmp(self) -> qmp.QEMUMonitorProtocol:
+    def _qmp(self) -> QEMUMonitorProtocol:
         if self._qmp_connection is None:
             raise QEMUMachineError("Attempt to access QMP with no connection")
         return self._qmp_connection
diff --git a/python/qemu/pylintrc b/python/qemu/machine/pylintrc
similarity index 100%
rename from python/qemu/pylintrc
rename to python/qemu/machine/pylintrc
diff --git a/python/qemu/qtest.py b/python/qemu/machine/qtest.py
similarity index 99%
rename from python/qemu/qtest.py
rename to python/qemu/machine/qtest.py
index 39a0cf62fe..53926e434a 100644
--- a/python/qemu/qtest.py
+++ b/python/qemu/machine/qtest.py
@@ -26,8 +26,9 @@
     TextIO,
 )
 
+from qemu.qmp import SocketAddrT
+
 from .machine import QEMUMachine
-from .qmp import SocketAddrT
 
 
 class QEMUQtestProtocol:
diff --git a/python/qemu/qmp.py b/python/qemu/qmp/__init__.py
similarity index 96%
rename from python/qemu/qmp.py
rename to python/qemu/qmp/__init__.py
index 2cd4d43036..9606248a3d 100644
--- a/python/qemu/qmp.py
+++ b/python/qemu/qmp/__init__.py
@@ -1,4 +1,14 @@
-""" QEMU Monitor Protocol Python class """
+"""
+QEMU Monitor Protocol (QMP) development library & tooling.
+
+This package provides a fairly low-level class for communicating to QMP
+protocol servers, as implemented by QEMU, the QEMU Guest Agent, and the
+QEMU Storage Daemon. This library is not intended for production use.
+
+`QEMUMonitorProtocol` is the primary class of interest, and all errors
+raised derive from `QMPError`.
+"""
+
 # Copyright (C) 2009, 2010 Red Hat Inc.
 #
 # Authors:
diff --git a/tests/acceptance/boot_linux.py b/tests/acceptance/boot_linux.py
index 0055dc7cee..54f999e647 100644
--- a/tests/acceptance/boot_linux.py
+++ b/tests/acceptance/boot_linux.py
@@ -12,8 +12,7 @@
 
 from avocado_qemu import Test, BUILD_DIR
 
-from qemu.accel import kvm_available
-from qemu.accel import tcg_available
+from qemu.machine import kvm_available, tcg_available
 
 from avocado.utils import cloudinit
 from avocado.utils import network
diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
index 5c5420712b..8236875222 100755
--- a/tests/qemu-iotests/297
+++ b/tests/qemu-iotests/297
@@ -36,7 +36,7 @@ MYPYPATH=../../python/ mypy --warn-unused-configs --disallow-subclassing-any \
     --disallow-any-generics --disallow-incomplete-defs \
     --disallow-untyped-decorators --no-implicit-optional \
     --warn-redundant-casts --warn-unused-ignores \
-    --no-implicit-reexport iotests.py
+    --no-implicit-reexport --namespace-packages iotests.py
 
 # success, all done
 echo "*** done"
diff --git a/tests/qemu-iotests/300 b/tests/qemu-iotests/300
index 5b75121b84..a41fac4e1d 100755
--- a/tests/qemu-iotests/300
+++ b/tests/qemu-iotests/300
@@ -23,7 +23,7 @@ import random
 import re
 from typing import Dict, List, Optional, Union
 import iotests
-import qemu
+from qemu.machine import machine
 
 BlockBitmapMapping = List[Dict[str, Union[str, List[Dict[str, str]]]]]
 
@@ -454,7 +454,7 @@ class TestBlockBitmapMappingErrors(TestDirtyBitmapMigration):
         # the failed migration
         try:
             self.vm_b.shutdown()
-        except qemu.machine.AbnormalShutdown:
+        except machine.AbnormalShutdown:
             pass
 
     def test_aliased_bitmap_name_too_long(self) -> None:
diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 63d2ace93c..6574afd735 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -37,7 +37,7 @@
 
 # pylint: disable=import-error, wrong-import-position
 sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
-from qemu import qtest
+from qemu.machine import qtest
 from qemu.qmp import QMPMessage
 
 # Use this logger for logging messages directly from the iotests module
diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
index 3fac20e929..b38969f61f 100644
--- a/tests/vm/basevm.py
+++ b/tests/vm/basevm.py
@@ -19,8 +19,7 @@
 import time
 import datetime
 sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
-from qemu.accel import kvm_available
-from qemu.machine import QEMUMachine
+from qemu.machine import kvm_available, QEMUMachine
 import subprocess
 import hashlib
 import argparse
-- 
2.26.2

[PATCH v3 02/15] python: add qemu package installer

[John Snow]

Add setup.cfg and setup.py, necessary for installing a package via
pip. Add a rst document 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.

I am not yet using a pyproject.toml style package manifest, because
using pyproject.toml (and PEP-517) style packages means that pip is not
able to install in "editable" or "develop" mode, which I consider
necessary for the development of this package.

Use a light-weight setup.py instead.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/PACKAGE.rst | 26 ++++++++++++++++++++++++++
 python/setup.cfg   | 18 ++++++++++++++++++
 python/setup.py    | 23 +++++++++++++++++++++++
 3 files changed, 67 insertions(+)
 create mode 100644 python/PACKAGE.rst
 create mode 100755 python/setup.cfg
 create mode 100755 python/setup.py

diff --git a/python/PACKAGE.rst b/python/PACKAGE.rst
new file mode 100644
index 0000000000..b82f32f489
--- /dev/null
+++ b/python/PACKAGE.rst
@@ -0,0 +1,26 @@
+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
+pacakge's documentation
+(``>>> help(qemu.qmp)``, ``>>> help(qemu.machine)``)
+for more information.
+
+Contributing
+------------
+
+This package is maintained by John Snow <jsnow@redhat.com> as part of
+the QEMU source tree. Contributions are welcome and follow the `QEMU
+patch submission process
+<https://wiki.qemu.org/Contribute/SubmitAPatch>`_. There is a `Gitlab
+mirror <https://gitlab.com/jsnow/qemu/-/tree/python>`_, but
+contributions must be sent to the list for inclusion.
diff --git a/python/setup.cfg b/python/setup.cfg
new file mode 100755
index 0000000000..12b99a796e
--- /dev/null
+++ b/python/setup.cfg
@@ -0,0 +1,18 @@
+[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
+
+[options]
+python_requires = >= 3.6
+packages = find_namespace:
diff --git a/python/setup.py b/python/setup.py
new file mode 100755
index 0000000000..e93d0075d1
--- /dev/null
+++ b/python/setup.py
@@ -0,0 +1,23 @@
+#!/usr/bin/env python3
+"""
+QEMU tooling installer script
+Copyright (c) 2020 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.26.2

[PATCH v3 03/15] python: add VERSION file

[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
to my knowledge.

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 always 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.5.2.0a1.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 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 0000000000..ceef7e1ad0
--- /dev/null
+++ b/python/VERSION
@@ -0,0 +1 @@
+0.5.2.0a1
diff --git a/python/setup.cfg b/python/setup.cfg
index 12b99a796e..260f7f4e94 100755
--- 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.26.2

[PATCH v3 04/15] python: add directory structure README.rst files

[John Snow]

Add short readmes to python/, python/qemu/, python/qemu/machine, and
python/qemu/machine 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 <jsnow@redhat.com>
---
 python/README.rst              | 27 +++++++++++++++++++++++++++
 python/qemu/README.rst         |  8 ++++++++
 python/qemu/machine/README.rst |  9 +++++++++
 python/qemu/qmp/README.rst     |  9 +++++++++
 4 files changed, 53 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

diff --git a/python/README.rst b/python/README.rst
new file mode 100644
index 0000000000..ff40e4c931
--- /dev/null
+++ b/python/README.rst
@@ -0,0 +1,27 @@
+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 (``qemu/machine``, ``qemu/qmp``).
+
+``setup.py`` is used by ``pip`` to install this tooling to the current
+environment. 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.
+
+If you amend the ``-e`` argument, pip will install in "editable" mode;
+which installs a version of the package that uses symlinks to these
+files, such that the package always reflects the latest version in your
+git tree.
+
+See `Installing packages using pip and virtual environments
+<https://packaging.python.org/guides/installing-using-pip-and-virtual-environments/>`_
+for more information.
diff --git a/python/qemu/README.rst b/python/qemu/README.rst
new file mode 100644
index 0000000000..31209c80a5
--- /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 <<https://www.python.org/dev/peps/pep-0420/>`_.
+
+Each directory below is assumed to be an installable Python package that
+is available under the ``qemu.<package>`` namespace.
diff --git a/python/qemu/machine/README.rst b/python/qemu/machine/README.rst
new file mode 100644
index 0000000000..73ad23c501
--- /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, 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 0000000000..f35dffe582
--- /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, 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.
-- 
2.26.2

[PATCH v3 05/15] python: Add pipenv support

[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.py
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 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 loudout 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 <jsnow@redhat.com>
---
 python/Pipfile | 11 +++++++++++
 1 file changed, 11 insertions(+)
 create mode 100644 python/Pipfile

diff --git a/python/Pipfile b/python/Pipfile
new file mode 100644
index 0000000000..9534830b5e
--- /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.26.2

[PATCH v3 06/15] python: add pylint import exceptions

[John Snow]

Pylint 2.5.x and 2.6.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 <jsnow@redhat.com>
---
 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 27b0b19abd..15e466a8d3 100644
--- a/python/qemu/machine/__init__.py
+++ b/python/qemu/machine/__init__.py
@@ -26,6 +26,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 .accel import kvm_available, list_accel, tcg_available
 from .machine import QEMUMachine
 from .qtest import QEMUQtestMachine, QEMUQtestProtocol
diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
index a5dc305539..88c51f5052 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 53926e434a..c3adf4e301 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.26.2

[PATCH v3 07/15] python: move pylintrc into setup.cfg

[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 <jsnow@redhat.com>
---
 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 3f69205000..0000000000
--- 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 260f7f4e94..a65802d85e 100755
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -17,3 +17,32 @@ classifiers =
 [options]
 python_requires = >= 3.6
 packages = find_namespace:
+
+[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.26.2

[PATCH v3 08/15] python: add pylint to pipenv

[John Snow]

We are specifying >= pylint 2.6.x for two reasons:

1. For setup.cfg support, added in pylint 2.5.x
2. To clarify that we are using a version that has incompatibly dropped
bad-whitespace checks.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 python/Pipfile      |   1 +
 python/Pipfile.lock | 127 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 128 insertions(+)
 create mode 100644 python/Pipfile.lock

diff --git a/python/Pipfile b/python/Pipfile
index 9534830b5e..1e58986c89 100644
--- a/python/Pipfile
+++ b/python/Pipfile
@@ -4,6 +4,7 @@ url = "https://pypi.org/simple"
 verify_ssl = true
 
 [dev-packages]
+pylint = ">=2.6.0"
 
 [packages]
 
diff --git a/python/Pipfile.lock b/python/Pipfile.lock
new file mode 100644
index 0000000000..71e0d40ffb
--- /dev/null
+++ b/python/Pipfile.lock
@@ -0,0 +1,127 @@
+{
+    "_meta": {
+        "hash": {
+            "sha256": "b7ac1f2ad73bc166244c0378298afba64951a16bb749b81a9668dc41f40f941c"
+        },
+        "pipfile-spec": 6,
+        "requires": {
+            "python_version": "3.6"
+        },
+        "sources": [
+            {
+                "name": "pypi",
+                "url": "https://pypi.org/simple",
+                "verify_ssl": true
+            }
+        ]
+    },
+    "default": {},
+    "develop": {
+        "astroid": {
+            "hashes": [
+                "sha256:2f4078c2a41bf377eea06d71c9d2ba4eb8f6b1af2135bec27bbbb7d8f12bb703",
+                "sha256:bc58d83eb610252fd8de6363e39d4f1d0619c894b0ed24603b881c02e64c7386"
+            ],
+            "markers": "python_version >= '3.5'",
+            "version": "==2.4.2"
+        },
+        "isort": {
+            "hashes": [
+                "sha256:dcab1d98b469a12a1a624ead220584391648790275560e1a43e54c5dceae65e7",
+                "sha256:dcaeec1b5f0eca77faea2a35ab790b4f3680ff75590bfcb7145986905aab2f58"
+            ],
+            "markers": "python_version >= '3.6' and python_version < '4.0'",
+            "version": "==5.6.4"
+        },
+        "lazy-object-proxy": {
+            "hashes": [
+                "sha256:0c4b206227a8097f05c4dbdd323c50edf81f15db3b8dc064d08c62d37e1a504d",
+                "sha256:194d092e6f246b906e8f70884e620e459fc54db3259e60cf69a4d66c3fda3449",
+                "sha256:1be7e4c9f96948003609aa6c974ae59830a6baecc5376c25c92d7d697e684c08",
+                "sha256:4677f594e474c91da97f489fea5b7daa17b5517190899cf213697e48d3902f5a",
+                "sha256:48dab84ebd4831077b150572aec802f303117c8cc5c871e182447281ebf3ac50",
+                "sha256:5541cada25cd173702dbd99f8e22434105456314462326f06dba3e180f203dfd",
+                "sha256:59f79fef100b09564bc2df42ea2d8d21a64fdcda64979c0fa3db7bdaabaf6239",
+                "sha256:8d859b89baf8ef7f8bc6b00aa20316483d67f0b1cbf422f5b4dc56701c8f2ffb",
+                "sha256:9254f4358b9b541e3441b007a0ea0764b9d056afdeafc1a5569eee1cc6c1b9ea",
+                "sha256:9651375199045a358eb6741df3e02a651e0330be090b3bc79f6d0de31a80ec3e",
+                "sha256:97bb5884f6f1cdce0099f86b907aa41c970c3c672ac8b9c8352789e103cf3156",
+                "sha256:9b15f3f4c0f35727d3a0fba4b770b3c4ebbb1fa907dbcc046a1d2799f3edd142",
+                "sha256:a2238e9d1bb71a56cd710611a1614d1194dc10a175c1e08d75e1a7bcc250d442",
+                "sha256:a6ae12d08c0bf9909ce12385803a543bfe99b95fe01e752536a60af2b7797c62",
+                "sha256:ca0a928a3ddbc5725be2dd1cf895ec0a254798915fb3a36af0964a0a4149e3db",
+                "sha256:cb2c7c57005a6804ab66f106ceb8482da55f5314b7fcb06551db1edae4ad1531",
+                "sha256:d74bb8693bf9cf75ac3b47a54d716bbb1a92648d5f781fc799347cfc95952383",
+                "sha256:d945239a5639b3ff35b70a88c5f2f491913eb94871780ebfabb2568bd58afc5a",
+                "sha256:eba7011090323c1dadf18b3b689845fd96a61ba0a1dfbd7f24b921398affc357",
+                "sha256:efa1909120ce98bbb3777e8b6f92237f5d5c8ea6758efea36a473e1d38f7d3e4",
+                "sha256:f3900e8a5de27447acbf900b4750b0ddfd7ec1ea7fbaf11dfa911141bc522af0"
+            ],
+            "markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3'",
+            "version": "==1.4.3"
+        },
+        "mccabe": {
+            "hashes": [
+                "sha256:ab8a6258860da4b6677da4bd2fe5dc2c659cff31b3ee4f7f5d64e79735b80d42",
+                "sha256:dd8d182285a0fe56bace7f45b5e7d1a6ebcbf524e8f3bd87eb0f125271b8831f"
+            ],
+            "version": "==0.6.1"
+        },
+        "pylint": {
+            "hashes": [
+                "sha256:bb4a908c9dadbc3aac18860550e870f58e1a02c9f2c204fdf5693d73be061210",
+                "sha256:bfe68f020f8a0fece830a22dd4d5dddb4ecc6137db04face4c3420a46a52239f"
+            ],
+            "index": "pypi",
+            "version": "==2.6.0"
+        },
+        "six": {
+            "hashes": [
+                "sha256:30639c035cdb23534cd4aa2dd52c3bf48f06e5f4a941509c8bafd8ce11080259",
+                "sha256:8b74bedcbbbaca38ff6d7491d76f2b06b3592611af620f8426e82dddb04a5ced"
+            ],
+            "markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2'",
+            "version": "==1.15.0"
+        },
+        "toml": {
+            "hashes": [
+                "sha256:926b612be1e5ce0634a2ca03470f95169cf16f939018233a670519cb4ac58b0f",
+                "sha256:bda89d5935c2eac546d648028b9901107a595863cb36bae0c73ac804a9b4ce88"
+            ],
+            "version": "==0.10.1"
+        },
+        "typed-ast": {
+            "hashes": [
+                "sha256:0666aa36131496aed8f7be0410ff974562ab7eeac11ef351def9ea6fa28f6355",
+                "sha256:0c2c07682d61a629b68433afb159376e24e5b2fd4641d35424e462169c0a7919",
+                "sha256:249862707802d40f7f29f6e1aad8d84b5aa9e44552d2cc17384b209f091276aa",
+                "sha256:24995c843eb0ad11a4527b026b4dde3da70e1f2d8806c99b7b4a7cf491612652",
+                "sha256:269151951236b0f9a6f04015a9004084a5ab0d5f19b57de779f908621e7d8b75",
+                "sha256:4083861b0aa07990b619bd7ddc365eb7fa4b817e99cf5f8d9cf21a42780f6e01",
+                "sha256:498b0f36cc7054c1fead3d7fc59d2150f4d5c6c56ba7fb150c013fbc683a8d2d",
+                "sha256:4e3e5da80ccbebfff202a67bf900d081906c358ccc3d5e3c8aea42fdfdfd51c1",
+                "sha256:6daac9731f172c2a22ade6ed0c00197ee7cc1221aa84cfdf9c31defeb059a907",
+                "sha256:715ff2f2df46121071622063fc7543d9b1fd19ebfc4f5c8895af64a77a8c852c",
+                "sha256:73d785a950fc82dd2a25897d525d003f6378d1cb23ab305578394694202a58c3",
+                "sha256:8c8aaad94455178e3187ab22c8b01a3837f8ee50e09cf31f1ba129eb293ec30b",
+                "sha256:8ce678dbaf790dbdb3eba24056d5364fb45944f33553dd5869b7580cdbb83614",
+                "sha256:aaee9905aee35ba5905cfb3c62f3e83b3bec7b39413f0a7f19be4e547ea01ebb",
+                "sha256:bcd3b13b56ea479b3650b82cabd6b5343a625b0ced5429e4ccad28a8973f301b",
+                "sha256:c9e348e02e4d2b4a8b2eedb48210430658df6951fa484e59de33ff773fbd4b41",
+                "sha256:d205b1b46085271b4e15f670058ce182bd1199e56b317bf2ec004b6a44f911f6",
+                "sha256:d43943ef777f9a1c42bf4e552ba23ac77a6351de620aa9acf64ad54933ad4d34",
+                "sha256:d5d33e9e7af3b34a40dc05f498939f0ebf187f07c385fd58d591c533ad8562fe",
+                "sha256:fc0fea399acb12edbf8a628ba8d2312f583bdbdb3335635db062fa98cf71fca4",
+                "sha256:fe460b922ec15dd205595c9b5b99e2f056fd98ae8f9f56b888e7a17dc2b757e7"
+            ],
+            "markers": "implementation_name == 'cpython' and python_version < '3.8'",
+            "version": "==1.4.1"
+        },
+        "wrapt": {
+            "hashes": [
+                "sha256:b62ffa81fb85f4332a4f609cab4ac40709470da05643a082ec1eb88e6d9b97d7"
+            ],
+            "version": "==1.12.1"
+        }
+    }
+}
-- 
2.26.2

[PATCH v3 09/15] python: move flake8 config to setup.cfg

[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 <jsnow@redhat.com>
---
 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 45d8146f3f..0000000000
--- 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 a65802d85e..87506a4f10 100755
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -18,6 +18,9 @@ classifiers =
 python_requires = >= 3.6
 packages = find_namespace:
 
+[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.26.2

[PATCH v3 10/15] python: Add flake8 to pipenv

[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 <jsnow@redhat.com>
---
 python/Pipfile      |  1 +
 python/Pipfile.lock | 42 +++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/python/Pipfile b/python/Pipfile
index 1e58986c89..d1f7045f68 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.6.0"
 
 [packages]
diff --git a/python/Pipfile.lock b/python/Pipfile.lock
index 71e0d40ffb..eb5ae75a0c 100644
--- a/python/Pipfile.lock
+++ b/python/Pipfile.lock
@@ -1,7 +1,7 @@
 {
     "_meta": {
         "hash": {
-            "sha256": "b7ac1f2ad73bc166244c0378298afba64951a16bb749b81a9668dc41f40f941c"
+            "sha256": "9f6d4857a6c72ad40fc1ec1e58cdb76f187a2986ac4156f0027e5eb798ec69a9"
         },
         "pipfile-spec": 6,
         "requires": {
@@ -25,6 +25,22 @@
             "markers": "python_version >= '3.5'",
             "version": "==2.4.2"
         },
+        "flake8": {
+            "hashes": [
+                "sha256:749dbbd6bfd0cf1318af27bf97a14e28e5ff548ef8e5b1566ccfb25a11e7c839",
+                "sha256:aadae8761ec651813c24be05c6f7b4680857ef6afaae4651a4eccaef97ce6c3b"
+            ],
+            "index": "pypi",
+            "version": "==3.8.4"
+        },
+        "importlib-metadata": {
+            "hashes": [
+                "sha256:77a540690e24b0305878c37ffd421785a6f7e53c8b5720d211b211de8d0e95da",
+                "sha256:cefa1a2f919b866c5beb7c9f7b0ebb4061f30a8a9bf16d609b000e2dfaceb9c3"
+            ],
+            "markers": "python_version < '3.8'",
+            "version": "==2.0.0"
+        },
         "isort": {
             "hashes": [
                 "sha256:dcab1d98b469a12a1a624ead220584391648790275560e1a43e54c5dceae65e7",
@@ -67,6 +83,22 @@
             ],
             "version": "==0.6.1"
         },
+        "pycodestyle": {
+            "hashes": [
+                "sha256:2295e7b2f6b5bd100585ebcb1f616591b652db8a741695b3d8f5d28bdc934367",
+                "sha256:c58a7d2815e0e8d7972bf1803331fb0152f867bd89adf8a01dfd55085434192e"
+            ],
+            "markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3'",
+            "version": "==2.6.0"
+        },
+        "pyflakes": {
+            "hashes": [
+                "sha256:0d94e0e05a19e57a99444b6ddcf9a6eb2e5c68d3ca1e98e90707af8152c90a92",
+                "sha256:35b2d75ee967ea93b55750aa9edbbf72813e06a66ba54438df2cfac9e3c27fc8"
+            ],
+            "markers": "python_version >= '2.7' and python_version not in '3.0, 3.1, 3.2, 3.3'",
+            "version": "==2.2.0"
+        },
         "pylint": {
             "hashes": [
                 "sha256:bb4a908c9dadbc3aac18860550e870f58e1a02c9f2c204fdf5693d73be061210",
@@ -122,6 +154,14 @@
                 "sha256:b62ffa81fb85f4332a4f609cab4ac40709470da05643a082ec1eb88e6d9b97d7"
             ],
             "version": "==1.12.1"
+        },
+        "zipp": {
+            "hashes": [
+                "sha256:64ad89efee774d1897a58607895d80789c59778ea02185dd846ac38394a8642b",
+                "sha256:eed8ec0b8d1416b2ca33516a37a08892442f3954dee131e92cfd92d8fe3e7066"
+            ],
+            "markers": "python_version >= '3.6'",
+            "version": "==3.3.0"
         }
     }
 }
-- 
2.26.2

[PATCH v3 11/15] python: move mypy.ini into setup.cfg

[John Snow]

mypy supports reading its configuration values from a central project
configuration file; do so.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 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 1a581c5f1e..0000000000
--- 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 87506a4f10..206e5a91dc 100755
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -21,6 +21,11 @@ packages = find_namespace:
 [flake8]
 extend-ignore = E722  # Prefer pylint's bare-except checks to flake8's
 
+[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.26.2

[PATCH v3 12/15] python: add mypy to pipenv

[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 qemu'.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 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 d1f7045f68..51c537b0d1 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.6.0"
 
 [packages]
diff --git a/python/Pipfile.lock b/python/Pipfile.lock
index eb5ae75a0c..376f95a6a8 100644
--- a/python/Pipfile.lock
+++ b/python/Pipfile.lock
@@ -1,7 +1,7 @@
 {
     "_meta": {
         "hash": {
-            "sha256": "9f6d4857a6c72ad40fc1ec1e58cdb76f187a2986ac4156f0027e5eb798ec69a9"
+            "sha256": "65f010a8f2e55e9870d2b7e0d8af129516097d23abf2504f396552748b067ade"
         },
         "pipfile-spec": 6,
         "requires": {
@@ -83,6 +83,33 @@
             ],
             "version": "==0.6.1"
         },
+        "mypy": {
+            "hashes": [
+                "sha256:0a0d102247c16ce93c97066443d11e2d36e6cc2a32d8ccc1f705268970479324",
+                "sha256:0d34d6b122597d48a36d6c59e35341f410d4abfa771d96d04ae2c468dd201abc",
+                "sha256:2170492030f6faa537647d29945786d297e4862765f0b4ac5930ff62e300d802",
+                "sha256:2842d4fbd1b12ab422346376aad03ff5d0805b706102e475e962370f874a5122",
+                "sha256:2b21ba45ad9ef2e2eb88ce4aeadd0112d0f5026418324176fd494a6824b74975",
+                "sha256:72060bf64f290fb629bd4a67c707a66fd88ca26e413a91384b18db3876e57ed7",
+                "sha256:af4e9ff1834e565f1baa74ccf7ae2564ae38c8df2a85b057af1dbbc958eb6666",
+                "sha256:bd03b3cf666bff8d710d633d1c56ab7facbdc204d567715cb3b9f85c6e94f669",
+                "sha256:c614194e01c85bb2e551c421397e49afb2872c88b5830e3554f0519f9fb1c178",
+                "sha256:cf4e7bf7f1214826cf7333627cb2547c0db7e3078723227820d0a2490f117a01",
+                "sha256:da56dedcd7cd502ccd3c5dddc656cb36113dd793ad466e894574125945653cea",
+                "sha256:e86bdace26c5fe9cf8cb735e7cedfe7850ad92b327ac5d797c656717d2ca66de",
+                "sha256:e97e9c13d67fbe524be17e4d8025d51a7dca38f90de2e462243ab8ed8a9178d1",
+                "sha256:eea260feb1830a627fb526d22fbb426b750d9f5a47b624e8d5e7e004359b219c"
+            ],
+            "index": "pypi",
+            "version": "==0.790"
+        },
+        "mypy-extensions": {
+            "hashes": [
+                "sha256:090fedd75945a69ae91ce1303b5824f428daf5a028d2f6ab8a299250a846f15d",
+                "sha256:2d82818f5bb3e369420cb3c4060a7970edba416647068eb4c5343488a6c604a8"
+            ],
+            "version": "==0.4.3"
+        },
         "pycodestyle": {
             "hashes": [
                 "sha256:2295e7b2f6b5bd100585ebcb1f616591b652db8a741695b3d8f5d28bdc934367",
@@ -149,6 +176,14 @@
             "markers": "implementation_name == 'cpython' and python_version < '3.8'",
             "version": "==1.4.1"
         },
+        "typing-extensions": {
+            "hashes": [
+                "sha256:7cb407020f00f7bfc3cb3e7881628838e69d8f3fcab2f64742a5e76b2f841918",
+                "sha256:99d4073b617d30288f569d3f13d2bd7548c3a7e4c8de87db09a9d29bb3a4a60c",
+                "sha256:dafc7639cde7f1b6e1acc0f457842a83e722ccca8eef5270af2d74792619a89f"
+            ],
+            "version": "==3.7.4.3"
+        },
         "wrapt": {
             "hashes": [
                 "sha256:b62ffa81fb85f4332a4f609cab4ac40709470da05643a082ec1eb88e6d9b97d7"
diff --git a/python/setup.cfg b/python/setup.cfg
index 206e5a91dc..308805b43f 100755
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -25,6 +25,7 @@ extend-ignore = E722  # Prefer pylint's bare-except checks to flake8's
 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.26.2

[PATCH v3 13/15] python: move .isort.cfg into setup.cfg

[John Snow]

Signed-off-by: John Snow <jsnow@redhat.com>
---
 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 6d0fd6cc0d..0000000000
--- 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 308805b43f..08def52372 100755
--- a/python/setup.cfg
+++ b/python/setup.cfg
@@ -55,3 +55,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.26.2

[PATCH v3 14/15] python/qemu: add isort to pipenv

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

Require 5.0.5 or greater.

isort can be run with 'isort -c qemu' from the python root.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 python/Pipfile      | 1 +
 python/Pipfile.lock | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/python/Pipfile b/python/Pipfile
index 51c537b0d1..75b96f29d8 100644
--- a/python/Pipfile
+++ b/python/Pipfile
@@ -5,6 +5,7 @@ verify_ssl = true
 
 [dev-packages]
 flake8 = ">=3.6.0"
+isort = ">=5.0.5"
 mypy = ">=0.770"
 pylint = ">=2.6.0"
 
diff --git a/python/Pipfile.lock b/python/Pipfile.lock
index 376f95a6a8..74563b444c 100644
--- a/python/Pipfile.lock
+++ b/python/Pipfile.lock
@@ -1,7 +1,7 @@
 {
     "_meta": {
         "hash": {
-            "sha256": "65f010a8f2e55e9870d2b7e0d8af129516097d23abf2504f396552748b067ade"
+            "sha256": "b89c7a1b8a414f2a4cd708964123fb427d55419ee0b39e088bf2e7d4fbc11979"
         },
         "pipfile-spec": 6,
         "requires": {
@@ -46,7 +46,7 @@
                 "sha256:dcab1d98b469a12a1a624ead220584391648790275560e1a43e54c5dceae65e7",
                 "sha256:dcaeec1b5f0eca77faea2a35ab790b4f3680ff75590bfcb7145986905aab2f58"
             ],
-            "markers": "python_version >= '3.6' and python_version < '4.0'",
+            "index": "pypi",
             "version": "==5.6.4"
         },
         "lazy-object-proxy": {
-- 
2.26.2

[PATCH v3 15/15] python/qemu: add qemu package itself to pipenv

[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 <jsnow@redhat.com>
---
 python/Pipfile      | 1 +
 python/Pipfile.lock | 9 +++++++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/python/Pipfile b/python/Pipfile
index 75b96f29d8..214fb175e7 100644
--- a/python/Pipfile
+++ b/python/Pipfile
@@ -10,6 +10,7 @@ mypy = ">=0.770"
 pylint = ">=2.6.0"
 
 [packages]
+qemu = {editable = true,path = "."}
 
 [requires]
 python_version = "3.6"
diff --git a/python/Pipfile.lock b/python/Pipfile.lock
index 74563b444c..386ca54ea6 100644
--- a/python/Pipfile.lock
+++ b/python/Pipfile.lock
@@ -1,7 +1,7 @@
 {
     "_meta": {
         "hash": {
-            "sha256": "b89c7a1b8a414f2a4cd708964123fb427d55419ee0b39e088bf2e7d4fbc11979"
+            "sha256": "e38d142c3fadc2f2ed849e86f7ebd14e25974dc12228751490aef5a9ee074f2f"
         },
         "pipfile-spec": 6,
         "requires": {
@@ -15,7 +15,12 @@
             }
         ]
     },
-    "default": {},
+    "default": {
+        "qemu": {
+            "editable": true,
+            "path": "."
+        }
+    },
     "develop": {
         "astroid": {
             "hashes": [
-- 
2.26.2

Re: [PATCH v3 00/15] python: create installable package

[John Snow]

Ping O:-)

Looking for feedback from at least Cleber and Eduardo before I barge 
ahead and send a PR to include this on master. Additional packaging and 
versioning feedback from Dan would be nice.

(I know we have a very busy two weeks here; I will continue pinging, but 
I have every intention of merging this prior to 5.2.)

--js

On 10/20/20 3:35 PM, John Snow wrote:
> Based-on: https://gitlab.com/jsnow/qemu/-/tree/python
> 
> 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.
>  > The point of this series is primarily to formalize our dependencies on
> mypy, flake8, isort, and pylint alongside versions that are known to
> work. It also adds explicitly pinned versions of these dependencies that
> should behave in a repeatable and known way for developers and CI
> environments both.
> 
> 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.
> 
> For example: when developing, you may go to qemu/python/ and invoke
> `pipenv shell` to activate a virtual environment that contains the qemu
> packages.  This package will always reflect the current version of the
> source files in the tree. When you are finished, you can simply exit the
> shell 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] ."
> 
> Finally, 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.
> 
> TESTING THIS SERIES:
> 
> CD to qemu/python first, and then:
> 
> 1. Try "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 your CWD.
> 
> 2. Try "pipenv sync --dev" to create/update the venv with the
> development packages without actually entering the venv. This should
> install isort, mypy, flake8 and pylint to the venv.
> 
> 3. After the above sync, try "pipenv shell" again, and from the python
> project root, try any of the following:
> 
>    - pylint qemu
>    - flake8 qemu
>    - isort -c qemu
>    - mypy qemu
> 
> 4. Leave any venv you are in, and from the project root, try the
> following commands:
> 
>    - pipenv run pylint qemu
>    - pipenv run flake8 qemu
>    - pipenv run isort -c qemu
>    - pipenv run mypy qemu
> 
> V3:
>   - Changed "qemu.core" to "qemu.qmp" and "qemu.machine",
>     Partly to accommodate forthcoming work which would benefit from a separate
>     qemu.qmp namespace.
>   - Changed the initial version from 5.2.0a1 to 0.5.2.0a1, to allow for
>     more rapid development while we smooth out the initial kinks.
>   - 001: Renamed patch title; differences implement the new names.
>   - 002: Readme changes for above.
>   - 003: Version change.
>   - 004: New readme for the new qmp directory.
>   - 006: A few more import exceptions for pylint, hopefully temporary.
>   - 009: Updated flake8 config comment to match qapi's
>   - 012: Added namespace_package configuration value
> 
> 001/15:[down] 'python: create qemu packages'
> 002/15:[0009] [FC] 'python: add qemu package installer'
> 003/15:[0002] [FC] 'python: add VERSION file'
> 004/15:[0015] [FC] 'python: add directory structure README.rst files'
> 005/15:[----] [--] 'python: Add pipenv support'
> 006/15:[down] 'python: add pylint import exceptions'
> 007/15:[----] [--] 'python: move pylintrc into setup.cfg'
> 008/15:[----] [--] 'python: add pylint to pipenv'
> 009/15:[0002] [FC] 'python: move flake8 config to setup.cfg'
> 010/15:[----] [--] 'python: Add flake8 to pipenv'
> 011/15:[----] [-C] 'python: move mypy.ini into setup.cfg'
> 012/15:[0001] [FC] 'python: add mypy to pipenv'
> 013/15:[----] [--] 'python: move .isort.cfg into setup.cfg'
> 014/15:[----] [--] 'python/qemu: add isort to pipenv'
> 015/15:[----] [--] 'python/qemu: add qemu package itself to pipenv'
> 
> John Snow (15):
>    python: create qemu packages
>    python: add qemu package installer
>    python: add VERSION file
>    python: add directory structure README.rst files
>    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 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/PACKAGE.rst                          |  26 +++
>   python/README.rst                           |  27 +++
>   python/qemu/README.rst                      |   8 +
>   python/qemu/machine/README.rst              |   9 +
>   python/qemu/qmp/README.rst                  |   9 +
>   python/Pipfile                              |  16 ++
>   python/Pipfile.lock                         | 207 ++++++++++++++++++++
>   python/VERSION                              |   1 +
>   python/mypy.ini                             |   4 -
>   python/qemu/.flake8                         |   2 -
>   python/qemu/.isort.cfg                      |   7 -
>   python/qemu/__init__.py                     |  11 --
>   python/qemu/machine/__init__.py             |  44 +++++
>   python/qemu/{ => machine}/accel.py          |   0
>   python/qemu/{ => machine}/console_socket.py |   0
>   python/qemu/{ => machine}/machine.py        |  16 +-
>   python/qemu/{ => machine}/qtest.py          |   3 +-
>   python/qemu/{qmp.py => qmp/__init__.py}     |  12 +-
>   python/{qemu/pylintrc => setup.cfg}         |  67 ++++---
>   python/setup.py                             |  23 +++
>   tests/acceptance/boot_linux.py              |   3 +-
>   tests/qemu-iotests/297                      |   2 +-
>   tests/qemu-iotests/300                      |   4 +-
>   tests/qemu-iotests/iotests.py               |   2 +-
>   tests/vm/basevm.py                          |   3 +-
>   25 files changed, 437 insertions(+), 69 deletions(-)
>   create mode 100644 python/PACKAGE.rst
>   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/Pipfile
>   create mode 100644 python/Pipfile.lock
>   create mode 100644 python/VERSION
>   delete mode 100644 python/mypy.ini
>   delete mode 100644 python/qemu/.flake8
>   delete mode 100644 python/qemu/.isort.cfg
>   delete mode 100644 python/qemu/__init__.py
>   create mode 100644 python/qemu/machine/__init__.py
>   rename python/qemu/{ => machine}/accel.py (100%)
>   rename python/qemu/{ => machine}/console_socket.py (100%)
>   rename python/qemu/{ => machine}/machine.py (98%)
>   rename python/qemu/{ => machine}/qtest.py (98%)
>   rename python/qemu/{qmp.py => qmp/__init__.py} (96%)
>   rename python/{qemu/pylintrc => setup.cfg} (50%)
>   mode change 100644 => 100755
>   create mode 100755 python/setup.py
> 

Re: [PATCH v3 00/15] python: create installable package

[Philippe Mathieu-Daudé]

On 10/27/20 11:08 PM, John Snow wrote:
> Ping O:-)
> 
> Looking for feedback from at least Cleber and Eduardo before I barge
> ahead and send a PR to include this on master. Additional packaging and
> versioning feedback from Dan would be nice.
> 
> (I know we have a very busy two weeks here; I will continue pinging, but
> I have every intention of merging this prior to 5.2.)

Too pythonic for me, so I'll defer that to Python experts.

It would be nice to have this in 5.2 indeed...

Regards,

Phil.

Re: [PATCH v3 01/15] python: create qemu packages

[Cleber Rosa]

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

On Tue, Oct 20, 2020 at 03:35:41PM -0400, John Snow wrote:
> move python/qemu/*.py to python/qemu/[machine, qmp]/*.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' and 'qmp'
> 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 <jsnow@redhat.com>
> ---
>  python/{qemu => }/.isort.cfg                |  0
>  python/qemu/__init__.py                     | 11 ------
>  python/qemu/{ => machine}/.flake8           |  0
>  python/qemu/machine/__init__.py             | 41 +++++++++++++++++++++
>  python/qemu/{ => machine}/accel.py          |  0
>  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 +++++-
>  tests/acceptance/boot_linux.py              |  3 +-
>  tests/qemu-iotests/297                      |  2 +-
>  tests/qemu-iotests/300                      |  4 +-
>  tests/qemu-iotests/iotests.py               |  2 +-
>  tests/vm/basevm.py                          |  3 +-
>  15 files changed, 71 insertions(+), 26 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}/accel.py (100%)
>  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%)
> 
> 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 4ca06c34a4..0000000000
> --- 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 <famz@redhat.com>
> -#
> -# 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 0000000000..27b0b19abd
> --- /dev/null
> +++ b/python/qemu/machine/__init__.py
> @@ -0,0 +1,41 @@
> +"""
> +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.
> +
> +- list_accel: List available accelerators
> +- kvm_available: Probe for KVM support
> +- tcg_available: Probe for TCG support
> +"""
> +
> +# Copyright (C) 2020 John Snow for Red Hat Inc.
> +# Copyright (C) 2015-2016 Red Hat Inc.
> +# Copyright (C) 2012 IBM Corp.
> +#
> +# Authors:
> +#  John Snow <jsnow@redhat.com>
> +#  Fam Zheng <fam@euphon.net>
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2.  See
> +# the COPYING file in the top-level directory.
> +#
> +
> +from .accel import kvm_available, list_accel, tcg_available
> +from .machine import QEMUMachine
> +from .qtest import QEMUQtestMachine, QEMUQtestProtocol
> +
> +
> +__all__ = (
> +    'list_accel',
> +    'kvm_available',
> +    'tcg_available',
> +    'QEMUMachine',
> +    'QEMUQtestProtocol',
> +    'QEMUQtestMachine',
> +)

How far would this approach go?  I mean, should all future APIs be developed
inside module under "qemu/machine", say "qemu/machine/foo.py" and their main
functionality imported here?  Or do you see this as a temporary state?

IMO, this is hard to maintain, and is a very easy path to future
inconsistencies.

> diff --git a/python/qemu/accel.py b/python/qemu/machine/accel.py
> similarity index 100%
> rename from python/qemu/accel.py
> rename to python/qemu/machine/accel.py
> 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 6420f01bed..a5dc305539 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 by default.
> -        self._qmp_connection: Optional[qmp.QEMUMonitorProtocol] = None
> +        self._qmp_connection: Optional[QEMUMonitorProtocol] = None
>          self._qemu_full_args: Tuple[str, ...] = ()
>          self._temp_dir: Optional[str] = None
>          self._launched = False
> @@ -314,7 +320,7 @@ def _pre_launch(self) -> None:
>              if self._remove_monitor_sockfile:
>                  assert isinstance(self._monitor_address, str)
>                  self._remove_files.append(self._monitor_address)
> -            self._qmp_connection = qmp.QEMUMonitorProtocol(
> +            self._qmp_connection = QEMUMonitorProtocol(
>                  self._monitor_address,
>                  server=True,
>                  nickname=self._name
> @@ -534,7 +540,7 @@ def set_qmp_monitor(self, enabled: bool = True) -> None:
>          self._qmp_set = enabled
>  
>      @property
> -    def _qmp(self) -> qmp.QEMUMonitorProtocol:
> +    def _qmp(self) -> QEMUMonitorProtocol:
>          if self._qmp_connection is None:
>              raise QEMUMachineError("Attempt to access QMP with no connection")
>          return self._qmp_connection
> diff --git a/python/qemu/pylintrc b/python/qemu/machine/pylintrc
> similarity index 100%
> rename from python/qemu/pylintrc
> rename to python/qemu/machine/pylintrc
> diff --git a/python/qemu/qtest.py b/python/qemu/machine/qtest.py
> similarity index 99%
> rename from python/qemu/qtest.py
> rename to python/qemu/machine/qtest.py
> index 39a0cf62fe..53926e434a 100644
> --- a/python/qemu/qtest.py
> +++ b/python/qemu/machine/qtest.py
> @@ -26,8 +26,9 @@
>      TextIO,
>  )
>  
> +from qemu.qmp import SocketAddrT
> +
>  from .machine import QEMUMachine
> -from .qmp import SocketAddrT
>  
>  
>  class QEMUQtestProtocol:
> diff --git a/python/qemu/qmp.py b/python/qemu/qmp/__init__.py
> similarity index 96%
> rename from python/qemu/qmp.py
> rename to python/qemu/qmp/__init__.py
> index 2cd4d43036..9606248a3d 100644
> --- a/python/qemu/qmp.py
> +++ b/python/qemu/qmp/__init__.py
> @@ -1,4 +1,14 @@
> -""" QEMU Monitor Protocol Python class """
> +"""
> +QEMU Monitor Protocol (QMP) development library & tooling.
> +
> +This package provides a fairly low-level class for communicating to QMP
> +protocol servers, as implemented by QEMU, the QEMU Guest Agent, and the
> +QEMU Storage Daemon. This library is not intended for production use.
> +
> +`QEMUMonitorProtocol` is the primary class of interest, and all errors
> +raised derive from `QMPError`.
> +"""
> +
>  # Copyright (C) 2009, 2010 Red Hat Inc.
>  #
>  # Authors:
> diff --git a/tests/acceptance/boot_linux.py b/tests/acceptance/boot_linux.py
> index 0055dc7cee..54f999e647 100644
> --- a/tests/acceptance/boot_linux.py
> +++ b/tests/acceptance/boot_linux.py
> @@ -12,8 +12,7 @@
>  
>  from avocado_qemu import Test, BUILD_DIR
>  
> -from qemu.accel import kvm_available
> -from qemu.accel import tcg_available
> +from qemu.machine import kvm_available, tcg_available
>

I think this reveals a slight downgrade of the API caused by the usage
of the symbols from "qemu.machine" namespace.

My point is that there are valid use cases for using "kvm_available"
without using a "QEMUMachine".  This presuposes that "qemu.machine" is
named like that because "QEMUMachine" are the main actors of this
namespace.

Naming is hard, (so don't assume I believe "utils" below is the best
or even a good name), but I'd be more confortable using an API along
the lines:

   from qemu.utils.accel import kvm_available
   kvm_available()

OR

   from qemu.utils import accel
   accel.kvm_available()

This would also remove the need to keep importing symbols into the
current "qemu.machine" namespace.

>  from avocado.utils import cloudinit
>  from avocado.utils import network
> diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
> index 5c5420712b..8236875222 100755
> --- a/tests/qemu-iotests/297
> +++ b/tests/qemu-iotests/297
> @@ -36,7 +36,7 @@ MYPYPATH=../../python/ mypy --warn-unused-configs --disallow-subclassing-any \
>      --disallow-any-generics --disallow-incomplete-defs \
>      --disallow-untyped-decorators --no-implicit-optional \
>      --warn-redundant-casts --warn-unused-ignores \
> -    --no-implicit-reexport iotests.py
> +    --no-implicit-reexport --namespace-packages iotests.py
>  
>  # success, all done
>  echo "*** done"
> diff --git a/tests/qemu-iotests/300 b/tests/qemu-iotests/300
> index 5b75121b84..a41fac4e1d 100755
> --- a/tests/qemu-iotests/300
> +++ b/tests/qemu-iotests/300
> @@ -23,7 +23,7 @@ import random
>  import re
>  from typing import Dict, List, Optional, Union
>  import iotests
> -import qemu
> +from qemu.machine import machine
>

I believe this gives my previous comment some extra strength.  I think
this shows that "qemu.machine" containing "machine" really means that
the first "machine" naming component doesn't really mean that.

"qemu.utils" is not perfect, but it's much more honest IMO.

Cheers,
- Cleber.

>  BlockBitmapMapping = List[Dict[str, Union[str, List[Dict[str, str]]]]]
>  
> @@ -454,7 +454,7 @@ class TestBlockBitmapMappingErrors(TestDirtyBitmapMigration):
>          # the failed migration
>          try:
>              self.vm_b.shutdown()
> -        except qemu.machine.AbnormalShutdown:
> +        except machine.AbnormalShutdown:
>              pass
>  
>      def test_aliased_bitmap_name_too_long(self) -> None:
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index 63d2ace93c..6574afd735 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -37,7 +37,7 @@
>  
>  # pylint: disable=import-error, wrong-import-position
>  sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
> -from qemu import qtest
> +from qemu.machine import qtest
>  from qemu.qmp import QMPMessage
>  
>  # Use this logger for logging messages directly from the iotests module
> diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
> index 3fac20e929..b38969f61f 100644
> --- a/tests/vm/basevm.py
> +++ b/tests/vm/basevm.py
> @@ -19,8 +19,7 @@
>  import time
>  import datetime
>  sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
> -from qemu.accel import kvm_available
> -from qemu.machine import QEMUMachine
> +from qemu.machine import kvm_available, QEMUMachine
>  import subprocess
>  import hashlib
>  import argparse
> -- 
> 2.26.2
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 02/15] python: add qemu package installer

[Cleber Rosa]

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

On Tue, Oct 20, 2020 at 03:35:42PM -0400, John Snow wrote:
> Add setup.cfg and setup.py, necessary for installing a package via
> pip. Add a rst document 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.
> 
> I am not yet using a pyproject.toml style package manifest, because
> using pyproject.toml (and PEP-517) style packages means that pip is not
> able to install in "editable" or "develop" mode, which I consider
> necessary for the development of this package.
> 
> Use a light-weight setup.py instead.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  python/PACKAGE.rst | 26 ++++++++++++++++++++++++++
>  python/setup.cfg   | 18 ++++++++++++++++++
>  python/setup.py    | 23 +++++++++++++++++++++++
>  3 files changed, 67 insertions(+)
>  create mode 100644 python/PACKAGE.rst
>  create mode 100755 python/setup.cfg
>  create mode 100755 python/setup.py
> 
> diff --git a/python/PACKAGE.rst b/python/PACKAGE.rst
> new file mode 100644
> index 0000000000..b82f32f489
> --- /dev/null
> +++ b/python/PACKAGE.rst
> @@ -0,0 +1,26 @@
> +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
> +pacakge's documentation

Typo here: s/pacakge/package/

> +(``>>> help(qemu.qmp)``, ``>>> help(qemu.machine)``)
> +for more information.
> +
> +Contributing
> +------------
> +
> +This package is maintained by John Snow <jsnow@redhat.com> as part of
> +the QEMU source tree. Contributions are welcome and follow the `QEMU
> +patch submission process
> +<https://wiki.qemu.org/Contribute/SubmitAPatch>`_. There is a `Gitlab

Git*L*ab.  Given that we're also under a company named with two words,
better given them that :)

> +mirror <https://gitlab.com/jsnow/qemu/-/tree/python>`_, but
> +contributions must be sent to the list for inclusion.

IMO it's not clear if this branch/mirror is your development work, a
staging area, etc.

> diff --git a/python/setup.cfg b/python/setup.cfg
> new file mode 100755
> index 0000000000..12b99a796e
> --- /dev/null
> +++ b/python/setup.cfg
> @@ -0,0 +1,18 @@
> +[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
> +

I know the sky is the limit, but I miss the Python version classifier,
at least:

  Programming Language :: Python :: 3 :: Only

And optionally those:

  Programming Language :: Python :: 3.6
  Programming Language :: Python :: 3.7
  Programming Language :: Python :: 3.8
  Programming Language :: Python :: 3.9

Although it may be a good idea to add them along test jobs on those
specific Python versions.

> +[options]
> +python_requires = >= 3.6
> +packages = find_namespace:
> diff --git a/python/setup.py b/python/setup.py
> new file mode 100755
> index 0000000000..e93d0075d1
> --- /dev/null
> +++ b/python/setup.py
> @@ -0,0 +1,23 @@
> +#!/usr/bin/env python3
> +"""
> +QEMU tooling installer script
> +Copyright (c) 2020 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')

Getting back to the "test jobs on those specific Python versions" I
was really anxious that environments with Python 3.6 will fail to
have such a "recent" setuptools version.

CentOS 8 has that specific version, while Ubuntu 18.04 has version
39.0.  Ubuntu 20.04 has a recent enough version though.  Given that
all GitLab CI moved to 20.04, this should be safe.

- Cleber.

> +
> +    setuptools.setup()
> +
> +
> +if __name__ == '__main__':
> +    main()
> -- 
> 2.26.2
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 01/15] python: create qemu packages

[John Snow]

On 10/28/20 10:46 AM, Cleber Rosa wrote:
> On Tue, Oct 20, 2020 at 03:35:41PM -0400, John Snow wrote:
>> move python/qemu/*.py to python/qemu/[machine, qmp]/*.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' and 'qmp'
>> 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 <jsnow@redhat.com>
>> ---
>>   python/{qemu => }/.isort.cfg                |  0
>>   python/qemu/__init__.py                     | 11 ------
>>   python/qemu/{ => machine}/.flake8           |  0
>>   python/qemu/machine/__init__.py             | 41 +++++++++++++++++++++
>>   python/qemu/{ => machine}/accel.py          |  0
>>   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 +++++-
>>   tests/acceptance/boot_linux.py              |  3 +-
>>   tests/qemu-iotests/297                      |  2 +-
>>   tests/qemu-iotests/300                      |  4 +-
>>   tests/qemu-iotests/iotests.py               |  2 +-
>>   tests/vm/basevm.py                          |  3 +-
>>   15 files changed, 71 insertions(+), 26 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}/accel.py (100%)
>>   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%)
>>
>> 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 4ca06c34a4..0000000000
>> --- 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 <famz@redhat.com>
>> -#
>> -# 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 0000000000..27b0b19abd
>> --- /dev/null
>> +++ b/python/qemu/machine/__init__.py
>> @@ -0,0 +1,41 @@
>> +"""
>> +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.
>> +
>> +- list_accel: List available accelerators
>> +- kvm_available: Probe for KVM support
>> +- tcg_available: Probe for TCG support
>> +"""
>> +
>> +# Copyright (C) 2020 John Snow for Red Hat Inc.
>> +# Copyright (C) 2015-2016 Red Hat Inc.
>> +# Copyright (C) 2012 IBM Corp.
>> +#
>> +# Authors:
>> +#  John Snow <jsnow@redhat.com>
>> +#  Fam Zheng <fam@euphon.net>
>> +#
>> +# This work is licensed under the terms of the GNU GPL, version 2.  See
>> +# the COPYING file in the top-level directory.
>> +#
>> +
>> +from .accel import kvm_available, list_accel, tcg_available
>> +from .machine import QEMUMachine
>> +from .qtest import QEMUQtestMachine, QEMUQtestProtocol
>> +
>> +
>> +__all__ = (
>> +    'list_accel',
>> +    'kvm_available',
>> +    'tcg_available',
>> +    'QEMUMachine',
>> +    'QEMUQtestProtocol',
>> +    'QEMUQtestMachine',
>> +)
> 
> How far would this approach go?  I mean, should all future APIs be developed
> inside module under "qemu/machine", say "qemu/machine/foo.py" and their main
> functionality imported here?  Or do you see this as a temporary state?
> 

I assume any new functionality can be directly imported. Having this 
here doesn't prevent using the fully qualified module names.

i.e. you can still do "from qemu.machine.machine import QEMUMachine".

Offering root-level aliases seems like a very prevalent paradigm in 
popular third-party packages like 'requests'. Not justification in and 
of itself, but there's precedent.

> IMO, this is hard to maintain, and is a very easy path to future
> inconsistencies.
> 

Maybe. The alternative is to do what I did to QMP and stick it back 
inside of __init__.

What I don't like is:

from qemu.machine.machine import QEMUMachine

I prefer:

from qemu.machine import QEMUMachine


Of course, I also like help(qemu.machine) to show something useful, 
which is going to generally involve updating the docstring to explain 
what the major APIs here are anyway, so I think actually you cannot 
escape your fate of having to keep a list of APIs updated in a 
multi-module package.

Maybe you could make an argument that keeping both the docstring and the 
__all__ attribute in sync is an undue maintenance burden, but I'd need 
some more convincing.

>> diff --git a/python/qemu/accel.py b/python/qemu/machine/accel.py
>> similarity index 100%
>> rename from python/qemu/accel.py
>> rename to python/qemu/machine/accel.py
>> 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 6420f01bed..a5dc305539 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 by default.
>> -        self._qmp_connection: Optional[qmp.QEMUMonitorProtocol] = None
>> +        self._qmp_connection: Optional[QEMUMonitorProtocol] = None
>>           self._qemu_full_args: Tuple[str, ...] = ()
>>           self._temp_dir: Optional[str] = None
>>           self._launched = False
>> @@ -314,7 +320,7 @@ def _pre_launch(self) -> None:
>>               if self._remove_monitor_sockfile:
>>                   assert isinstance(self._monitor_address, str)
>>                   self._remove_files.append(self._monitor_address)
>> -            self._qmp_connection = qmp.QEMUMonitorProtocol(
>> +            self._qmp_connection = QEMUMonitorProtocol(
>>                   self._monitor_address,
>>                   server=True,
>>                   nickname=self._name
>> @@ -534,7 +540,7 @@ def set_qmp_monitor(self, enabled: bool = True) -> None:
>>           self._qmp_set = enabled
>>   
>>       @property
>> -    def _qmp(self) -> qmp.QEMUMonitorProtocol:
>> +    def _qmp(self) -> QEMUMonitorProtocol:
>>           if self._qmp_connection is None:
>>               raise QEMUMachineError("Attempt to access QMP with no connection")
>>           return self._qmp_connection
>> diff --git a/python/qemu/pylintrc b/python/qemu/machine/pylintrc
>> similarity index 100%
>> rename from python/qemu/pylintrc
>> rename to python/qemu/machine/pylintrc
>> diff --git a/python/qemu/qtest.py b/python/qemu/machine/qtest.py
>> similarity index 99%
>> rename from python/qemu/qtest.py
>> rename to python/qemu/machine/qtest.py
>> index 39a0cf62fe..53926e434a 100644
>> --- a/python/qemu/qtest.py
>> +++ b/python/qemu/machine/qtest.py
>> @@ -26,8 +26,9 @@
>>       TextIO,
>>   )
>>   
>> +from qemu.qmp import SocketAddrT
>> +
>>   from .machine import QEMUMachine
>> -from .qmp import SocketAddrT
>>   
>>   
>>   class QEMUQtestProtocol:
>> diff --git a/python/qemu/qmp.py b/python/qemu/qmp/__init__.py
>> similarity index 96%
>> rename from python/qemu/qmp.py
>> rename to python/qemu/qmp/__init__.py
>> index 2cd4d43036..9606248a3d 100644
>> --- a/python/qemu/qmp.py
>> +++ b/python/qemu/qmp/__init__.py
>> @@ -1,4 +1,14 @@
>> -""" QEMU Monitor Protocol Python class """
>> +"""
>> +QEMU Monitor Protocol (QMP) development library & tooling.
>> +
>> +This package provides a fairly low-level class for communicating to QMP
>> +protocol servers, as implemented by QEMU, the QEMU Guest Agent, and the
>> +QEMU Storage Daemon. This library is not intended for production use.
>> +
>> +`QEMUMonitorProtocol` is the primary class of interest, and all errors
>> +raised derive from `QMPError`.
>> +"""
>> +
>>   # Copyright (C) 2009, 2010 Red Hat Inc.
>>   #
>>   # Authors:
>> diff --git a/tests/acceptance/boot_linux.py b/tests/acceptance/boot_linux.py
>> index 0055dc7cee..54f999e647 100644
>> --- a/tests/acceptance/boot_linux.py
>> +++ b/tests/acceptance/boot_linux.py
>> @@ -12,8 +12,7 @@
>>   
>>   from avocado_qemu import Test, BUILD_DIR
>>   
>> -from qemu.accel import kvm_available
>> -from qemu.accel import tcg_available
>> +from qemu.machine import kvm_available, tcg_available
>>
> 
> I think this reveals a slight downgrade of the API caused by the usage
> of the symbols from "qemu.machine" namespace.
> 
> My point is that there are valid use cases for using "kvm_available"
> without using a "QEMUMachine".  This presuposes that "qemu.machine" is
> named like that because "QEMUMachine" are the main actors of this
> namespace.
> 

No, the accel functions literally start a QEMU process and probe its 
output to determine what they support. This belongs directly alongside 
the machine code in my opinion.

(I also don't know why'd you call these functions unless you intended to 
create a machine based on what you found.


I see your point that it is not a method of the Machine class itself nor 
an extension thereof; but I think for the purpose of three small helper 
functions it's not worth creating an entirely new package to contain.

Nor is it worth watering down the "machine" name to something less 
specific to accommodate three tiny helper functions.

If I am considering "machine" the "package that knows how to start and 
interact with QEMU", then accel belongs here.

> Naming is hard, (so don't assume I believe "utils" below is the best
> or even a good name), but I'd be more confortable using an API along
> the lines:
> 
>     from qemu.utils.accel import kvm_available
>     kvm_available()
> 
> OR
> 
>     from qemu.utils import accel
>     accel.kvm_available()
> 

utils/tools I am reserving as a package for various debugging scripts 
and tools in the future that I don't intend to upload to PyPI, e.g. many 
of the "thematically loose" scripts in ./scripts.

> This would also remove the need to keep importing symbols into the
> current "qemu.machine" namespace.
> 

It would, but it's three functions. It doesn't warrant an entire package 
for a single module with three standalone functions.

That calculus may change in the future if we accumulate lots more 
miscellaneous thingies to make that more worthwhile!

>>   from avocado.utils import cloudinit
>>   from avocado.utils import network
>> diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
>> index 5c5420712b..8236875222 100755
>> --- a/tests/qemu-iotests/297
>> +++ b/tests/qemu-iotests/297
>> @@ -36,7 +36,7 @@ MYPYPATH=../../python/ mypy --warn-unused-configs --disallow-subclassing-any \
>>       --disallow-any-generics --disallow-incomplete-defs \
>>       --disallow-untyped-decorators --no-implicit-optional \
>>       --warn-redundant-casts --warn-unused-ignores \
>> -    --no-implicit-reexport iotests.py
>> +    --no-implicit-reexport --namespace-packages iotests.py
>>   
>>   # success, all done
>>   echo "*** done"
>> diff --git a/tests/qemu-iotests/300 b/tests/qemu-iotests/300
>> index 5b75121b84..a41fac4e1d 100755
>> --- a/tests/qemu-iotests/300
>> +++ b/tests/qemu-iotests/300
>> @@ -23,7 +23,7 @@ import random
>>   import re
>>   from typing import Dict, List, Optional, Union
>>   import iotests
>> -import qemu
>> +from qemu.machine import machine
>>
> 
> I believe this gives my previous comment some extra strength.  I think
> this shows that "qemu.machine" containing "machine" really means that
> the first "machine" naming component doesn't really mean that.
> 

Yeah, I don't like the duplication of 'machine' here either. I believe 
this test needs access to specific error classes which I didn't believe 
were important enough to receive an alias.

Might be that the correct thing to do is to stick machine.py in 
machine/__init__.py to avoid that duplication, just like I did with qmp.py.

> "qemu.utils" is not perfect, but it's much more honest IMO.
> 
> Cheers,
> - Cleber.
> 
>>   BlockBitmapMapping = List[Dict[str, Union[str, List[Dict[str, str]]]]]
>>   
>> @@ -454,7 +454,7 @@ class TestBlockBitmapMappingErrors(TestDirtyBitmapMigration):
>>           # the failed migration
>>           try:
>>               self.vm_b.shutdown()
>> -        except qemu.machine.AbnormalShutdown:
>> +        except machine.AbnormalShutdown:
>>               pass
>>   
>>       def test_aliased_bitmap_name_too_long(self) -> None:
>> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
>> index 63d2ace93c..6574afd735 100644
>> --- a/tests/qemu-iotests/iotests.py
>> +++ b/tests/qemu-iotests/iotests.py
>> @@ -37,7 +37,7 @@
>>   
>>   # pylint: disable=import-error, wrong-import-position
>>   sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
>> -from qemu import qtest
>> +from qemu.machine import qtest
>>   from qemu.qmp import QMPMessage
>>   
>>   # Use this logger for logging messages directly from the iotests module
>> diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py
>> index 3fac20e929..b38969f61f 100644
>> --- a/tests/vm/basevm.py
>> +++ b/tests/vm/basevm.py
>> @@ -19,8 +19,7 @@
>>   import time
>>   import datetime
>>   sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
>> -from qemu.accel import kvm_available
>> -from qemu.machine import QEMUMachine
>> +from qemu.machine import kvm_available, QEMUMachine
>>   import subprocess
>>   import hashlib
>>   import argparse
>> -- 
>> 2.26.2
>>

Re: [PATCH v3 01/15] python: create qemu packages

[Cleber Rosa]

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

On Wed, Oct 28, 2020 at 11:21:19AM -0400, John Snow wrote:
> On 10/28/20 10:46 AM, Cleber Rosa wrote:
> > On Tue, Oct 20, 2020 at 03:35:41PM -0400, John Snow wrote:
> > > move python/qemu/*.py to python/qemu/[machine, qmp]/*.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' and 'qmp'
> > > 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 <jsnow@redhat.com>
> > > ---
> > >   python/{qemu => }/.isort.cfg                |  0
> > >   python/qemu/__init__.py                     | 11 ------
> > >   python/qemu/{ => machine}/.flake8           |  0
> > >   python/qemu/machine/__init__.py             | 41 +++++++++++++++++++++
> > >   python/qemu/{ => machine}/accel.py          |  0
> > >   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 +++++-
> > >   tests/acceptance/boot_linux.py              |  3 +-
> > >   tests/qemu-iotests/297                      |  2 +-
> > >   tests/qemu-iotests/300                      |  4 +-
> > >   tests/qemu-iotests/iotests.py               |  2 +-
> > >   tests/vm/basevm.py                          |  3 +-
> > >   15 files changed, 71 insertions(+), 26 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}/accel.py (100%)
> > >   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%)
> > > 
> > > 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 4ca06c34a4..0000000000
> > > --- 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 <famz@redhat.com>
> > > -#
> > > -# 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 0000000000..27b0b19abd
> > > --- /dev/null
> > > +++ b/python/qemu/machine/__init__.py
> > > @@ -0,0 +1,41 @@
> > > +"""
> > > +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.
> > > +
> > > +- list_accel: List available accelerators
> > > +- kvm_available: Probe for KVM support
> > > +- tcg_available: Probe for TCG support
> > > +"""
> > > +
> > > +# Copyright (C) 2020 John Snow for Red Hat Inc.
> > > +# Copyright (C) 2015-2016 Red Hat Inc.
> > > +# Copyright (C) 2012 IBM Corp.
> > > +#
> > > +# Authors:
> > > +#  John Snow <jsnow@redhat.com>
> > > +#  Fam Zheng <fam@euphon.net>
> > > +#
> > > +# This work is licensed under the terms of the GNU GPL, version 2.  See
> > > +# the COPYING file in the top-level directory.
> > > +#
> > > +
> > > +from .accel import kvm_available, list_accel, tcg_available
> > > +from .machine import QEMUMachine
> > > +from .qtest import QEMUQtestMachine, QEMUQtestProtocol
> > > +
> > > +
> > > +__all__ = (
> > > +    'list_accel',
> > > +    'kvm_available',
> > > +    'tcg_available',
> > > +    'QEMUMachine',
> > > +    'QEMUQtestProtocol',
> > > +    'QEMUQtestMachine',
> > > +)
> > 
> > How far would this approach go?  I mean, should all future APIs be developed
> > inside module under "qemu/machine", say "qemu/machine/foo.py" and their main
> > functionality imported here?  Or do you see this as a temporary state?
> > 
> 
> I assume any new functionality can be directly imported. Having this here
> doesn't prevent using the fully qualified module names.
> 
> i.e. you can still do "from qemu.machine.machine import QEMUMachine".
> 
> Offering root-level aliases seems like a very prevalent paradigm in popular
> third-party packages like 'requests'. Not justification in and of itself,
> but there's precedent.
>

There's always someone doing something someway... I think the point
here is to try to deliver something that makes the most sense to most
users of the API.  Not an easy task, but it's our task.

> > IMO, this is hard to maintain, and is a very easy path to future
> > inconsistencies.
> > 
> 
> Maybe. The alternative is to do what I did to QMP and stick it back inside
> of __init__.
> 
> What I don't like is:
> 
> from qemu.machine.machine import QEMUMachine
> 
> I prefer:
> 
> from qemu.machine import QEMUMachine
> 
> 
> Of course, I also like help(qemu.machine) to show something useful, which is
> going to generally involve updating the docstring to explain what the major
> APIs here are anyway, so I think actually you cannot escape your fate of
> having to keep a list of APIs updated in a multi-module package.
> 
> Maybe you could make an argument that keeping both the docstring and the
> __all__ attribute in sync is an undue maintenance burden, but I'd need some
> more convincing.
> 
> > > diff --git a/python/qemu/accel.py b/python/qemu/machine/accel.py
> > > similarity index 100%
> > > rename from python/qemu/accel.py
> > > rename to python/qemu/machine/accel.py
> > > 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 6420f01bed..a5dc305539 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 by default.
> > > -        self._qmp_connection: Optional[qmp.QEMUMonitorProtocol] = None
> > > +        self._qmp_connection: Optional[QEMUMonitorProtocol] = None
> > >           self._qemu_full_args: Tuple[str, ...] = ()
> > >           self._temp_dir: Optional[str] = None
> > >           self._launched = False
> > > @@ -314,7 +320,7 @@ def _pre_launch(self) -> None:
> > >               if self._remove_monitor_sockfile:
> > >                   assert isinstance(self._monitor_address, str)
> > >                   self._remove_files.append(self._monitor_address)
> > > -            self._qmp_connection = qmp.QEMUMonitorProtocol(
> > > +            self._qmp_connection = QEMUMonitorProtocol(
> > >                   self._monitor_address,
> > >                   server=True,
> > >                   nickname=self._name
> > > @@ -534,7 +540,7 @@ def set_qmp_monitor(self, enabled: bool = True) -> None:
> > >           self._qmp_set = enabled
> > >       @property
> > > -    def _qmp(self) -> qmp.QEMUMonitorProtocol:
> > > +    def _qmp(self) -> QEMUMonitorProtocol:
> > >           if self._qmp_connection is None:
> > >               raise QEMUMachineError("Attempt to access QMP with no connection")
> > >           return self._qmp_connection
> > > diff --git a/python/qemu/pylintrc b/python/qemu/machine/pylintrc
> > > similarity index 100%
> > > rename from python/qemu/pylintrc
> > > rename to python/qemu/machine/pylintrc
> > > diff --git a/python/qemu/qtest.py b/python/qemu/machine/qtest.py
> > > similarity index 99%
> > > rename from python/qemu/qtest.py
> > > rename to python/qemu/machine/qtest.py
> > > index 39a0cf62fe..53926e434a 100644
> > > --- a/python/qemu/qtest.py
> > > +++ b/python/qemu/machine/qtest.py
> > > @@ -26,8 +26,9 @@
> > >       TextIO,
> > >   )
> > > +from qemu.qmp import SocketAddrT
> > > +
> > >   from .machine import QEMUMachine
> > > -from .qmp import SocketAddrT
> > >   class QEMUQtestProtocol:
> > > diff --git a/python/qemu/qmp.py b/python/qemu/qmp/__init__.py
> > > similarity index 96%
> > > rename from python/qemu/qmp.py
> > > rename to python/qemu/qmp/__init__.py
> > > index 2cd4d43036..9606248a3d 100644
> > > --- a/python/qemu/qmp.py
> > > +++ b/python/qemu/qmp/__init__.py
> > > @@ -1,4 +1,14 @@
> > > -""" QEMU Monitor Protocol Python class """
> > > +"""
> > > +QEMU Monitor Protocol (QMP) development library & tooling.
> > > +
> > > +This package provides a fairly low-level class for communicating to QMP
> > > +protocol servers, as implemented by QEMU, the QEMU Guest Agent, and the
> > > +QEMU Storage Daemon. This library is not intended for production use.
> > > +
> > > +`QEMUMonitorProtocol` is the primary class of interest, and all errors
> > > +raised derive from `QMPError`.
> > > +"""
> > > +
> > >   # Copyright (C) 2009, 2010 Red Hat Inc.
> > >   #
> > >   # Authors:
> > > diff --git a/tests/acceptance/boot_linux.py b/tests/acceptance/boot_linux.py
> > > index 0055dc7cee..54f999e647 100644
> > > --- a/tests/acceptance/boot_linux.py
> > > +++ b/tests/acceptance/boot_linux.py
> > > @@ -12,8 +12,7 @@
> > >   from avocado_qemu import Test, BUILD_DIR
> > > -from qemu.accel import kvm_available
> > > -from qemu.accel import tcg_available
> > > +from qemu.machine import kvm_available, tcg_available
> > > 
> > 
> > I think this reveals a slight downgrade of the API caused by the usage
> > of the symbols from "qemu.machine" namespace.
> > 
> > My point is that there are valid use cases for using "kvm_available"
> > without using a "QEMUMachine".  This presuposes that "qemu.machine" is
> > named like that because "QEMUMachine" are the main actors of this
> > namespace.
> > 
> 
> No, the accel functions literally start a QEMU process and probe its output
> to determine what they support. This belongs directly alongside the machine
> code in my opinion.
>

accel.kvm_available() may not even run QEMU at all... and when it
does, it runs it just like your most mundane command line application,
"grepping" its stdout.

> (I also don't know why'd you call these functions unless you intended to
> create a machine based on what you found.
>

*If* this is an API that you intend others to leverage, you should
reverse the question: "Is there any reason for not making this utility
available and as obvious and as easily available as possible to be
used by people whom I can't actually tell how they'd use it?".

For instance, I may want to write Python code that skips over a number
of tests if I detect that KVM is not available.  This code of mine,
may be completely separate from actual code creating the QEMUMachines,
or may even be compiled C code that I don't want to call because I
know that KVM is not available.

And that's not hypothetical.  I personally have a use case for that.

> 
> I see your point that it is not a method of the Machine class itself nor an
> extension thereof; but I think for the purpose of three small helper
> functions it's not worth creating an entirely new package to contain.
>

Not only that, but it's not even used by QEMUMachine itself.

> Nor is it worth watering down the "machine" name to something less specific
> to accommodate three tiny helper functions.
> 
> If I am considering "machine" the "package that knows how to start and
> interact with QEMU", then accel belongs here.
>

That's pretty broad and generic.  Do you mean that "machine" should
know how to interact with, say, "info qtree"?  Would a utility that
outputs and parse that deserve to live under "machine"?

Please understand that these are the awkward, speculative and hard
questions that we have to ask *if* the intention is to make an API
available for general use.

> > Naming is hard, (so don't assume I believe "utils" below is the best
> > or even a good name), but I'd be more confortable using an API along
> > the lines:
> > 
> >     from qemu.utils.accel import kvm_available
> >     kvm_available()
> > 
> > OR
> > 
> >     from qemu.utils import accel
> >     accel.kvm_available()
> > 
> 
> utils/tools I am reserving as a package for various debugging scripts and
> tools in the future that I don't intend to upload to PyPI, e.g. many of the
> "thematically loose" scripts in ./scripts.
>

The *actual* name, if already "taken in advance", is not the core issue.

> > This would also remove the need to keep importing symbols into the
> > current "qemu.machine" namespace.
> > 
> 
> It would, but it's three functions. It doesn't warrant an entire package for
> a single module with three standalone functions.
> 
> That calculus may change in the future if we accumulate lots more
> miscellaneous thingies to make that more worthwhile!
> 
> > >   from avocado.utils import cloudinit
> > >   from avocado.utils import network
> > > diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
> > > index 5c5420712b..8236875222 100755
> > > --- a/tests/qemu-iotests/297
> > > +++ b/tests/qemu-iotests/297
> > > @@ -36,7 +36,7 @@ MYPYPATH=../../python/ mypy --warn-unused-configs --disallow-subclassing-any \
> > >       --disallow-any-generics --disallow-incomplete-defs \
> > >       --disallow-untyped-decorators --no-implicit-optional \
> > >       --warn-redundant-casts --warn-unused-ignores \
> > > -    --no-implicit-reexport iotests.py
> > > +    --no-implicit-reexport --namespace-packages iotests.py
> > >   # success, all done
> > >   echo "*** done"
> > > diff --git a/tests/qemu-iotests/300 b/tests/qemu-iotests/300
> > > index 5b75121b84..a41fac4e1d 100755
> > > --- a/tests/qemu-iotests/300
> > > +++ b/tests/qemu-iotests/300
> > > @@ -23,7 +23,7 @@ import random
> > >   import re
> > >   from typing import Dict, List, Optional, Union
> > >   import iotests
> > > -import qemu
> > > +from qemu.machine import machine
> > > 
> > 
> > I believe this gives my previous comment some extra strength.  I think
> > this shows that "qemu.machine" containing "machine" really means that
> > the first "machine" naming component doesn't really mean that.
> > 
> 
> Yeah, I don't like the duplication of 'machine' here either. I believe this
> test needs access to specific error classes which I didn't believe were
> important enough to receive an alias.
> 
> Might be that the correct thing to do is to stick machine.py in
> machine/__init__.py to avoid that duplication, just like I did with qmp.py.
>

I agree this is a better approach wrt *machine.py* itself, from the
API perspective.

In a broad sense (excluding qmp) you're proposing that current usage of:

 "from qemu import myutility"

Becomes:

 "from qemu.machine import myutility"

And "everything" (take it with a grain of salt), will live under
"qemu.machine".  I still think, as you put it, that the future will
show that other modules not connected to "machine", but generally
useful, will need a place to live.

This may be an opportunity to *keep* a place where contributors can
put generally useful bits.  But, if you want to recognize that current
"from qemu import myutility" was an unintended naming side effect, and
that namespace is really about *machine*, and broaden the meaning of
"machine", OK.

Cheers,
- Cleber.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 02/15] python: add qemu package installer

[John Snow]

On 10/28/20 11:10 AM, Cleber Rosa wrote:
> On Tue, Oct 20, 2020 at 03:35:42PM -0400, John Snow wrote:
>> Add setup.cfg and setup.py, necessary for installing a package via
>> pip. Add a rst document 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.
>>
>> I am not yet using a pyproject.toml style package manifest, because
>> using pyproject.toml (and PEP-517) style packages means that pip is not
>> able to install in "editable" or "develop" mode, which I consider
>> necessary for the development of this package.
>>
>> Use a light-weight setup.py instead.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>   python/PACKAGE.rst | 26 ++++++++++++++++++++++++++
>>   python/setup.cfg   | 18 ++++++++++++++++++
>>   python/setup.py    | 23 +++++++++++++++++++++++
>>   3 files changed, 67 insertions(+)
>>   create mode 100644 python/PACKAGE.rst
>>   create mode 100755 python/setup.cfg
>>   create mode 100755 python/setup.py
>>
>> diff --git a/python/PACKAGE.rst b/python/PACKAGE.rst
>> new file mode 100644
>> index 0000000000..b82f32f489
>> --- /dev/null
>> +++ b/python/PACKAGE.rst
>> @@ -0,0 +1,26 @@
>> +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
>> +pacakge's documentation
> 
> Typo here: s/pacakge/package/
> 

oops,

>> +(``>>> help(qemu.qmp)``, ``>>> help(qemu.machine)``)
>> +for more information.
>> +
>> +Contributing
>> +------------
>> +
>> +This package is maintained by John Snow <jsnow@redhat.com> as part of
>> +the QEMU source tree. Contributions are welcome and follow the `QEMU
>> +patch submission process
>> +<https://wiki.qemu.org/Contribute/SubmitAPatch>`_. There is a `Gitlab
> 
> Git*L*ab.  Given that we're also under a company named with two words,
> better given them that :)
> 

They can send me a paycheck if they want me to adhere to their brand 
standards!

(But, OK.)

>> +mirror <https://gitlab.com/jsnow/qemu/-/tree/python>`_, but
>> +contributions must be sent to the list for inclusion.
> 
> IMO it's not clear if this branch/mirror is your development work, a
> staging area, etc.
> 

Fair enough. jsnow/qemu/python is intended as a staging area for patches 
that have been vetted on-list.

jsnow/qemu/master is a lazily-updated mirror. (I tend to update it every 
day as part of my development process, but there are days I don't write 
code.)

jsnow/qemu/python-* is development work; review branches, etc.

I'll try to rephrase this a bit. What I want to communicate:

- This package exists as a subfolder of a larger project
- I am responsible for maintaining this package, but not for the larger 
project
- Please contact *me* for problems with this package
- Contributions should go through qemu-devel (I will gently redirect 
contributors who may send pull requests to the qemu devel list.)

>> diff --git a/python/setup.cfg b/python/setup.cfg
>> new file mode 100755
>> index 0000000000..12b99a796e
>> --- /dev/null
>> +++ b/python/setup.cfg
>> @@ -0,0 +1,18 @@
>> +[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
>> +
> 

Also ... licensing question, do we need *L*GPLv2, or does Python not 
have a "linking exception" problem?

I guess we can't really re-license these files anyway, so nevermind.

(I immediately regret asking this.)

> I know the sky is the limit, but I miss the Python version classifier,
> at least:
> 
>    Programming Language :: Python :: 3 :: Only
> 

Sure.

Wait, why can you specify Python as a language? Is it possible to have 
non-Python packages on PyPI?

*CONCERNED*

> And optionally those:
> 
>    Programming Language :: Python :: 3.6
>    Programming Language :: Python :: 3.7
>    Programming Language :: Python :: 3.8
>    Programming Language :: Python :: 3.9
> 
> Although it may be a good idea to add them along test jobs on those
> specific Python versions.
> 

Are these worth adding? I've got python_requires >= 3.6 down below. From 
my test of a blank package upload to PyPI, it seems to display prominently:

https://pypi.org/project/qemu/

Is there a tangible benefit that you are aware of?

>> +[options]
>> +python_requires = >= 3.6
>> +packages = find_namespace:
>> diff --git a/python/setup.py b/python/setup.py
>> new file mode 100755
>> index 0000000000..e93d0075d1
>> --- /dev/null
>> +++ b/python/setup.py
>> @@ -0,0 +1,23 @@
>> +#!/usr/bin/env python3
>> +"""
>> +QEMU tooling installer script
>> +Copyright (c) 2020 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')
> 
> Getting back to the "test jobs on those specific Python versions" I
> was really anxious that environments with Python 3.6 will fail to
> have such a "recent" setuptools version.
> 

Reasonable doubt. However, this isn't *required* to use the library (the 
QEMU code can continue to just set PYTHONPATH or sys.path as necessary) 
and bypasses the installer entirely.

That gives us some leeway apart from the usual version constraints; in 
order to independently use this library outside of the QEMU tree we may 
impose more modern setups -- as long as the minimum requirements for 
QEMU itself don't break.

Having a modern setuptools in order to install seems like less of a 
problem barrier; and it seemed like a good idea to make it explicitly 
fail instead of silently doing something weird if it didn't 
see/understand setup.cfg.

(And it seems like good practice to update pip in your venv, so I think 
we'll be OK except for the stodgiest of users, but sometimes you can't 
have new things on old systems without learning some new tricks!)

> CentOS 8 has that specific version, while Ubuntu 18.04 has version
> 39.0.  Ubuntu 20.04 has a recent enough version though.  Given that
> all GitLab CI moved to 20.04, this should be safe.
> 
> - Cleber.
> 

FWIW, for the purposes of running the linters, I am using Fedora32 and 
the python36 package.

>> +
>> +    setuptools.setup()
>> +
>> +
>> +if __name__ == '__main__':
>> +    main()
>> -- 
>> 2.26.2
>>

Re: [PATCH v3 02/15] python: add qemu package installer

[Cleber Rosa]

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

On Wed, Oct 28, 2020 at 01:02:52PM -0400, John Snow wrote:
> On 10/28/20 11:10 AM, Cleber Rosa wrote:
> > > +mirror <https://gitlab.com/jsnow/qemu/-/tree/python>`_, but
> > > +contributions must be sent to the list for inclusion.
> > 
> > IMO it's not clear if this branch/mirror is your development work, a
> > staging area, etc.
> > 
> 
> Fair enough. jsnow/qemu/python is intended as a staging area for patches
> that have been vetted on-list.
> 
> jsnow/qemu/master is a lazily-updated mirror. (I tend to update it every day
> as part of my development process, but there are days I don't write code.)
> 
> jsnow/qemu/python-* is development work; review branches, etc.
> 
> I'll try to rephrase this a bit. What I want to communicate:
> 
> - This package exists as a subfolder of a larger project
> - I am responsible for maintaining this package, but not for the larger
> project
> - Please contact *me* for problems with this package
> - Contributions should go through qemu-devel (I will gently redirect
> contributors who may send pull requests to the qemu devel list.)
>

OK, sounds good.  I'll look at the exact rewording on v+1.

> > > diff --git a/python/setup.cfg b/python/setup.cfg
> > > new file mode 100755
> > > index 0000000000..12b99a796e
> > > --- /dev/null
> > > +++ b/python/setup.cfg
> > > @@ -0,0 +1,18 @@
> > > +[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
> > > +
> > 
> 
> Also ... licensing question, do we need *L*GPLv2, or does Python not have a
> "linking exception" problem?
> 
> I guess we can't really re-license these files anyway, so nevermind.
> 
> (I immediately regret asking this.)
>

I'll just pretend you never did.

> > I know the sky is the limit, but I miss the Python version classifier,
> > at least:
> > 
> >    Programming Language :: Python :: 3 :: Only
> > 
> 
> Sure.
> 
> Wait, why can you specify Python as a language? Is it possible to have
> non-Python packages on PyPI?
> 
> *CONCERNED*
>

I guess it has to do with packages that can interact or serve other
languages.  Or, that are (partially) written in another language?

> > And optionally those:
> > 
> >    Programming Language :: Python :: 3.6
> >    Programming Language :: Python :: 3.7
> >    Programming Language :: Python :: 3.8
> >    Programming Language :: Python :: 3.9
> > 
> > Although it may be a good idea to add them along test jobs on those
> > specific Python versions.
> > 
> 
> Are these worth adding? I've got python_requires >= 3.6 down below. From my
> test of a blank package upload to PyPI, it seems to display prominently:
> 
> https://pypi.org/project/qemu/
> 
> Is there a tangible benefit that you are aware of?
>

AFAICT, the classifiers are about letting people search for packages
that match a given criteria.  It's all metadata, so the benefits are
not super tangible.  I've used those to keep track / document the
Python versions that I know the project has been actively tested on,
and that's the reason of my comment about (CI) test jobs.

> > > +[options]
> > > +python_requires = >= 3.6
> > > +packages = find_namespace:
> > > diff --git a/python/setup.py b/python/setup.py
> > > new file mode 100755
> > > index 0000000000..e93d0075d1
> > > --- /dev/null
> > > +++ b/python/setup.py
> > > @@ -0,0 +1,23 @@
> > > +#!/usr/bin/env python3
> > > +"""
> > > +QEMU tooling installer script
> > > +Copyright (c) 2020 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')
> > 
> > Getting back to the "test jobs on those specific Python versions" I
> > was really anxious that environments with Python 3.6 will fail to
> > have such a "recent" setuptools version.
> > 
> 
> Reasonable doubt. However, this isn't *required* to use the library (the
> QEMU code can continue to just set PYTHONPATH or sys.path as necessary) and
> bypasses the installer entirely.
>

Right, but I had the impression that activating it in develop mode (at
least) was the intention down the line.

> That gives us some leeway apart from the usual version constraints; in order
> to independently use this library outside of the QEMU tree we may impose
> more modern setups -- as long as the minimum requirements for QEMU itself
> don't break.
>

OK.

> Having a modern setuptools in order to install seems like less of a problem
> barrier; and it seemed like a good idea to make it explicitly fail instead
> of silently doing something weird if it didn't see/understand setup.cfg.
>

Agreed.

> (And it seems like good practice to update pip in your venv, so I think
> we'll be OK except for the stodgiest of users, but sometimes you can't have
> new things on old systems without learning some new tricks!)
> 
> > CentOS 8 has that specific version, while Ubuntu 18.04 has version
> > 39.0.  Ubuntu 20.04 has a recent enough version though.  Given that
> > all GitLab CI moved to 20.04, this should be safe.
> > 
> > - Cleber.
> > 
> 
> FWIW, for the purposes of running the linters, I am using Fedora32 and the
> python36 package.
>

This is a minor suggestion: use CentOS 8 stock Python 3.6 packages,
and then Fedora 33 with also stock Python 3.9.  Even though all
tools are pinned, it's still a good idea to test at least min/max
(if not all) Python versions.

- Cleber.

> > > +
> > > +    setuptools.setup()
> > > +
> > > +
> > > +if __name__ == '__main__':
> > > +    main()
> > > -- 
> > > 2.26.2
> > > 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 02/15] python: add qemu package installer

[Cleber Rosa]

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

On Tue, Oct 20, 2020 at 03:35:42PM -0400, John Snow wrote:
> Add setup.cfg and setup.py, necessary for installing a package via
> pip. Add a rst document 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.
> 
> I am not yet using a pyproject.toml style package manifest, because
> using pyproject.toml (and PEP-517) style packages means that pip is not
> able to install in "editable" or "develop" mode, which I consider
> necessary for the development of this package.
> 
> Use a light-weight setup.py instead.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  python/PACKAGE.rst | 26 ++++++++++++++++++++++++++
>  python/setup.cfg   | 18 ++++++++++++++++++
>  python/setup.py    | 23 +++++++++++++++++++++++
>  3 files changed, 67 insertions(+)
>  create mode 100644 python/PACKAGE.rst
>  create mode 100755 python/setup.cfg

I missed this earlier: setup.cfg does not need to have the execute bit
on.

- Cleber.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 03/15] python: add VERSION file

[Cleber Rosa]

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

On Tue, Oct 20, 2020 at 03:35:43PM -0400, John Snow wrote:
> 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
> to my knowledge.
> 
> 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 always 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.5.2.0a1.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>

I'd rather have the version to be sync'd with QEMU, but, I understand
this is a more conservative approach that can maybe evolve into that.

Reviewed-by: Cleber Rosa <crosa@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 01/15] python: create qemu packages

[John Snow]

On 10/28/20 12:39 PM, Cleber Rosa wrote:
> On Wed, Oct 28, 2020 at 11:21:19AM -0400, John Snow wrote:
>> On 10/28/20 10:46 AM, Cleber Rosa wrote:
>>> On Tue, Oct 20, 2020 at 03:35:41PM -0400, John Snow wrote:
>>>> move python/qemu/*.py to python/qemu/[machine, qmp]/*.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' and 'qmp'
>>>> 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 <jsnow@redhat.com>
>>>> ---
>>>>    python/{qemu => }/.isort.cfg                |  0
>>>>    python/qemu/__init__.py                     | 11 ------
>>>>    python/qemu/{ => machine}/.flake8           |  0
>>>>    python/qemu/machine/__init__.py             | 41 +++++++++++++++++++++
>>>>    python/qemu/{ => machine}/accel.py          |  0
>>>>    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 +++++-
>>>>    tests/acceptance/boot_linux.py              |  3 +-
>>>>    tests/qemu-iotests/297                      |  2 +-
>>>>    tests/qemu-iotests/300                      |  4 +-
>>>>    tests/qemu-iotests/iotests.py               |  2 +-
>>>>    tests/vm/basevm.py                          |  3 +-
>>>>    15 files changed, 71 insertions(+), 26 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}/accel.py (100%)
>>>>    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%)
>>>>
>>>> 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 4ca06c34a4..0000000000
>>>> --- 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 <famz@redhat.com>
>>>> -#
>>>> -# 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 0000000000..27b0b19abd
>>>> --- /dev/null
>>>> +++ b/python/qemu/machine/__init__.py
>>>> @@ -0,0 +1,41 @@
>>>> +"""
>>>> +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.
>>>> +
>>>> +- list_accel: List available accelerators
>>>> +- kvm_available: Probe for KVM support
>>>> +- tcg_available: Probe for TCG support
>>>> +"""
>>>> +
>>>> +# Copyright (C) 2020 John Snow for Red Hat Inc.
>>>> +# Copyright (C) 2015-2016 Red Hat Inc.
>>>> +# Copyright (C) 2012 IBM Corp.
>>>> +#
>>>> +# Authors:
>>>> +#  John Snow <jsnow@redhat.com>
>>>> +#  Fam Zheng <fam@euphon.net>
>>>> +#
>>>> +# This work is licensed under the terms of the GNU GPL, version 2.  See
>>>> +# the COPYING file in the top-level directory.
>>>> +#
>>>> +
>>>> +from .accel import kvm_available, list_accel, tcg_available
>>>> +from .machine import QEMUMachine
>>>> +from .qtest import QEMUQtestMachine, QEMUQtestProtocol
>>>> +
>>>> +
>>>> +__all__ = (
>>>> +    'list_accel',
>>>> +    'kvm_available',
>>>> +    'tcg_available',
>>>> +    'QEMUMachine',
>>>> +    'QEMUQtestProtocol',
>>>> +    'QEMUQtestMachine',
>>>> +)
>>>
>>> How far would this approach go?  I mean, should all future APIs be developed
>>> inside module under "qemu/machine", say "qemu/machine/foo.py" and their main
>>> functionality imported here?  Or do you see this as a temporary state?
>>>
>>
>> I assume any new functionality can be directly imported. Having this here
>> doesn't prevent using the fully qualified module names.
>>
>> i.e. you can still do "from qemu.machine.machine import QEMUMachine".
>>
>> Offering root-level aliases seems like a very prevalent paradigm in popular
>> third-party packages like 'requests'. Not justification in and of itself,
>> but there's precedent.
>>
> 
> There's always someone doing something someway... I think the point
> here is to try to deliver something that makes the most sense to most
> users of the API.  Not an easy task, but it's our task.
> 

Just gotta follow the patterns I see in prominent examples that are 
often held up as good citizens. In my experience, commonly used aliases 
for the purposes of ease-of-use is a common pattern.

I don't think this *hurts* anything, but I suppose there's deeper 
questions about what we want the imports to look like. OK, but I think 
this discussion about the use of __all__ winds up being unrelated to the 
actual matter at hand.

>>> IMO, this is hard to maintain, and is a very easy path to future
>>> inconsistencies.
>>>
>>
>> Maybe. The alternative is to do what I did to QMP and stick it back inside
>> of __init__.
>>
>> What I don't like is:
>>
>> from qemu.machine.machine import QEMUMachine
>>
>> I prefer:
>>
>> from qemu.machine import QEMUMachine
>>
>>
>> Of course, I also like help(qemu.machine) to show something useful, which is
>> going to generally involve updating the docstring to explain what the major
>> APIs here are anyway, so I think actually you cannot escape your fate of
>> having to keep a list of APIs updated in a multi-module package.
>>
>> Maybe you could make an argument that keeping both the docstring and the
>> __all__ attribute in sync is an undue maintenance burden, but I'd need some
>> more convincing.
>>
>>>> diff --git a/python/qemu/accel.py b/python/qemu/machine/accel.py
>>>> similarity index 100%
>>>> rename from python/qemu/accel.py
>>>> rename to python/qemu/machine/accel.py
>>>> 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 6420f01bed..a5dc305539 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 by default.
>>>> -        self._qmp_connection: Optional[qmp.QEMUMonitorProtocol] = None
>>>> +        self._qmp_connection: Optional[QEMUMonitorProtocol] = None
>>>>            self._qemu_full_args: Tuple[str, ...] = ()
>>>>            self._temp_dir: Optional[str] = None
>>>>            self._launched = False
>>>> @@ -314,7 +320,7 @@ def _pre_launch(self) -> None:
>>>>                if self._remove_monitor_sockfile:
>>>>                    assert isinstance(self._monitor_address, str)
>>>>                    self._remove_files.append(self._monitor_address)
>>>> -            self._qmp_connection = qmp.QEMUMonitorProtocol(
>>>> +            self._qmp_connection = QEMUMonitorProtocol(
>>>>                    self._monitor_address,
>>>>                    server=True,
>>>>                    nickname=self._name
>>>> @@ -534,7 +540,7 @@ def set_qmp_monitor(self, enabled: bool = True) -> None:
>>>>            self._qmp_set = enabled
>>>>        @property
>>>> -    def _qmp(self) -> qmp.QEMUMonitorProtocol:
>>>> +    def _qmp(self) -> QEMUMonitorProtocol:
>>>>            if self._qmp_connection is None:
>>>>                raise QEMUMachineError("Attempt to access QMP with no connection")
>>>>            return self._qmp_connection
>>>> diff --git a/python/qemu/pylintrc b/python/qemu/machine/pylintrc
>>>> similarity index 100%
>>>> rename from python/qemu/pylintrc
>>>> rename to python/qemu/machine/pylintrc
>>>> diff --git a/python/qemu/qtest.py b/python/qemu/machine/qtest.py
>>>> similarity index 99%
>>>> rename from python/qemu/qtest.py
>>>> rename to python/qemu/machine/qtest.py
>>>> index 39a0cf62fe..53926e434a 100644
>>>> --- a/python/qemu/qtest.py
>>>> +++ b/python/qemu/machine/qtest.py
>>>> @@ -26,8 +26,9 @@
>>>>        TextIO,
>>>>    )
>>>> +from qemu.qmp import SocketAddrT
>>>> +
>>>>    from .machine import QEMUMachine
>>>> -from .qmp import SocketAddrT
>>>>    class QEMUQtestProtocol:
>>>> diff --git a/python/qemu/qmp.py b/python/qemu/qmp/__init__.py
>>>> similarity index 96%
>>>> rename from python/qemu/qmp.py
>>>> rename to python/qemu/qmp/__init__.py
>>>> index 2cd4d43036..9606248a3d 100644
>>>> --- a/python/qemu/qmp.py
>>>> +++ b/python/qemu/qmp/__init__.py
>>>> @@ -1,4 +1,14 @@
>>>> -""" QEMU Monitor Protocol Python class """
>>>> +"""
>>>> +QEMU Monitor Protocol (QMP) development library & tooling.
>>>> +
>>>> +This package provides a fairly low-level class for communicating to QMP
>>>> +protocol servers, as implemented by QEMU, the QEMU Guest Agent, and the
>>>> +QEMU Storage Daemon. This library is not intended for production use.
>>>> +
>>>> +`QEMUMonitorProtocol` is the primary class of interest, and all errors
>>>> +raised derive from `QMPError`.
>>>> +"""
>>>> +
>>>>    # Copyright (C) 2009, 2010 Red Hat Inc.
>>>>    #
>>>>    # Authors:
>>>> diff --git a/tests/acceptance/boot_linux.py b/tests/acceptance/boot_linux.py
>>>> index 0055dc7cee..54f999e647 100644
>>>> --- a/tests/acceptance/boot_linux.py
>>>> +++ b/tests/acceptance/boot_linux.py
>>>> @@ -12,8 +12,7 @@
>>>>    from avocado_qemu import Test, BUILD_DIR
>>>> -from qemu.accel import kvm_available
>>>> -from qemu.accel import tcg_available
>>>> +from qemu.machine import kvm_available, tcg_available
>>>>
>>>
>>> I think this reveals a slight downgrade of the API caused by the usage
>>> of the symbols from "qemu.machine" namespace.
>>>
>>> My point is that there are valid use cases for using "kvm_available"
>>> without using a "QEMUMachine".  This presuposes that "qemu.machine" is
>>> named like that because "QEMUMachine" are the main actors of this
>>> namespace.
>>>
>>
>> No, the accel functions literally start a QEMU process and probe its output
>> to determine what they support. This belongs directly alongside the machine
>> code in my opinion.
>>
> 
> accel.kvm_available() may not even run QEMU at all... and when it
> does, it runs it just like your most mundane command line application,
> "grepping" its stdout.
> 
>> (I also don't know why'd you call these functions unless you intended to
>> create a machine based on what you found.
>>
> 
> *If* this is an API that you intend others to leverage, you should
> reverse the question: "Is there any reason for not making this utility
> available and as obvious and as easily available as possible to be
> used by people whom I can't actually tell how they'd use it?".
> 
> For instance, I may want to write Python code that skips over a number
> of tests if I detect that KVM is not available.  This code of mine,
> may be completely separate from actual code creating the QEMUMachines,
> or may even be compiled C code that I don't want to call because I
> know that KVM is not available.
> 
> And that's not hypothetical.  I personally have a use case for that.
> 

All of the usages in tests/acceptance pass a binary in. The vm/ tests 
are the ones that don't.

With tcg_available necessitating providing a binary, it's weird that 
kvm_available has an overloaded invocation where it avoids doing so. The 
generic detection for host KVM support likely ought to be in a different 
function.

(And that one, you are right, would have only the most tenuous 
relationship to a qemu.machine package.)

>>
>> I see your point that it is not a method of the Machine class itself nor an
>> extension thereof; but I think for the purpose of three small helper
>> functions it's not worth creating an entirely new package to contain.
>>
> 
> Not only that, but it's not even used by QEMUMachine itself.
> 

But it's not wholly unrelated: I maintain the most obvious use for these 
is to probe a binary and adjust flags accordingly. I would go so far as 
to say that shipping qemu.machine without such a feature would create a 
deficit in that package!

>> Nor is it worth watering down the "machine" name to something less specific
>> to accommodate three tiny helper functions.
>>
>> If I am considering "machine" the "package that knows how to start and
>> interact with QEMU", then accel belongs here.
>>
> 
> That's pretty broad and generic.  Do you mean that "machine" should
> know how to interact with, say, "info qtree"?  Would a utility that
> outputs and parse that deserve to live under "machine"?
> 

It is broad and generic. I am arguing explicitly against creating an 
entire package to house a single module which contains three utility 
functions. What I am really pressing on you to answer is: "What are they 
utility functions *for*?"

Answering that might give us good names. Without names, I can't respin 
the patches to your liking. :)

But, yes, that means I am considering "machine" as something very broad 
and generic. I believe that's correct until such time as we have 
justification to create something more targeted and focused. If you have 
a better name I'm open to it, but I did intent to intuit that the 
primary purpose of this package was to provide QEMUMachine and friends, 
because it is.

That's also why I have adopted Andrea's suggestion to start with a 0.x 
version: I am treating this domain as an incubation space, and not 
stable APIs -- precisely because this type of argument is not one I am 
sure needed to be solved up front. We can graduate packages out of this 
incubator.


What I had initially:

qemu.core, containing *everything*


What I have now:

qemu.qmp     - QMP library and tools that rely on the QMP library
qemu.machine - machine.py, console_socket.py, qtest.py, accel helpers


What you want, I believe:

qemu.qmp     - QMP library and misc QMP-using tools
qemu.machine - machine.py, console_socket.py, qtest.py
qemu.util    - accel helpers


What I'm protesting against is creating a "utilities" package at all, 
because with no central thesis to its existence, I do not know how to 
version or maintain such a package.

It's my belief that keeping the subpackages tightly focused (qmp, 
machine) it will help guide what kind of API we want to develop for each.

Making a "util" package to house random bits and pieces seems like the 
sort of thing that will be very hard to maintain in a meaningful way; 
and it's something I'd like to avoid supporting for external usage.

> Please understand that these are the awkward, speculative and hard
> questions that we have to ask *if* the intention is to make an API
> available for general use.
> 

I did invite bikeshedding! Now's the time to do it, if ever there was one.

>>> Naming is hard, (so don't assume I believe "utils" below is the best
>>> or even a good name), but I'd be more confortable using an API along
>>> the lines:
>>>
>>>      from qemu.utils.accel import kvm_available
>>>      kvm_available()
>>>
>>> OR
>>>
>>>      from qemu.utils import accel
>>>      accel.kvm_available()
>>>
>>
>> utils/tools I am reserving as a package for various debugging scripts and
>> tools in the future that I don't intend to upload to PyPI, e.g. many of the
>> "thematically loose" scripts in ./scripts.
>>
> 
> The *actual* name, if already "taken in advance", is not the core issue.
> 

Didn't mean to imply the name was an issue, it was the semantics. I do 
intend to create a type of "misc" subpackage, but it's one I won't 
support; functionally it's something for internal dev usage on the QEMU 
tree was my intent.

If you want accel functions to be supported and available outside the 
QEMU source tree, I am saying that I don't want to support a "util" 
package to do that.

>>> This would also remove the need to keep importing symbols into the
>>> current "qemu.machine" namespace.
>>>
>>
>> It would, but it's three functions. It doesn't warrant an entire package for
>> a single module with three standalone functions.
>>
>> That calculus may change in the future if we accumulate lots more
>> miscellaneous thingies to make that more worthwhile!
>>
>>>>    from avocado.utils import cloudinit
>>>>    from avocado.utils import network
>>>> diff --git a/tests/qemu-iotests/297 b/tests/qemu-iotests/297
>>>> index 5c5420712b..8236875222 100755
>>>> --- a/tests/qemu-iotests/297
>>>> +++ b/tests/qemu-iotests/297
>>>> @@ -36,7 +36,7 @@ MYPYPATH=../../python/ mypy --warn-unused-configs --disallow-subclassing-any \
>>>>        --disallow-any-generics --disallow-incomplete-defs \
>>>>        --disallow-untyped-decorators --no-implicit-optional \
>>>>        --warn-redundant-casts --warn-unused-ignores \
>>>> -    --no-implicit-reexport iotests.py
>>>> +    --no-implicit-reexport --namespace-packages iotests.py
>>>>    # success, all done
>>>>    echo "*** done"
>>>> diff --git a/tests/qemu-iotests/300 b/tests/qemu-iotests/300
>>>> index 5b75121b84..a41fac4e1d 100755
>>>> --- a/tests/qemu-iotests/300
>>>> +++ b/tests/qemu-iotests/300
>>>> @@ -23,7 +23,7 @@ import random
>>>>    import re
>>>>    from typing import Dict, List, Optional, Union
>>>>    import iotests
>>>> -import qemu
>>>> +from qemu.machine import machine
>>>>
>>>
>>> I believe this gives my previous comment some extra strength.  I think
>>> this shows that "qemu.machine" containing "machine" really means that
>>> the first "machine" naming component doesn't really mean that.
>>>
>>
>> Yeah, I don't like the duplication of 'machine' here either. I believe this
>> test needs access to specific error classes which I didn't believe were
>> important enough to receive an alias.
>>
>> Might be that the correct thing to do is to stick machine.py in
>> machine/__init__.py to avoid that duplication, just like I did with qmp.py.
>>
> 
> I agree this is a better approach wrt *machine.py* itself, from the
> API perspective.
> 
> In a broad sense (excluding qmp) you're proposing that current usage of:
> 
>   "from qemu import myutility"
> 
> Becomes:
> 
>   "from qemu.machine import myutility"
> 
> And "everything" (take it with a grain of salt), will live under
> "qemu.machine".  I still think, as you put it, that the future will
> show that other modules not connected to "machine", but generally
> useful, will need a place to live.
> 

I mean, the only things we *have* that aren't directly connected to 
Machine are these three accel functions, and even those launch QEMU 
binaries to probe them for features for the purpose of .. passing flags 
to QEMUMachine.

> This may be an opportunity to *keep* a place where contributors can
> put generally useful bits.  But, if you want to recognize that current
> "from qemu import myutility" was an unintended naming side effect, and
> that namespace is really about *machine*, and broaden the meaning of
> "machine", OK.
> 

You're right, it is the "removal" of a place where we can put random 
bits and pieces. That's a conscious decision, yes.

We can create that space (and I intend to, as intuited above), but in so 
doing I also intend to preclude it from any kind of API stability 
guarantees, visibility in our sphinx manuals, that sort of thing. I 
think you actually do want to keep using accel and want to support 
people using it, so it needs a place to live.

Reintroduction of a "core" package that we intend to support externally 
could work; but without a strong idea of what will go in there, I'd 
rather not make one pre-emptively.

I guess put another way:

I'd prefer accel helpers to be in the machine package, since they 
provide an obvious utility for helping to launch machines (though they 
may provide additional utility in further contexts), instead of creating 
a completely un-themed miscellaneous package to house just a few things, 
especially for callers who are *almost guaranteed* to be using the other 
subpackages already.

> Cheers,
> - Cleber.
> 

All that said, I'm not truly married to the exact names I chose. Feel 
free to recommend some, but this is the best I could come up with. I 
think "qemu.machine" in particular could have a better name, but what to 
name it? I don't know.

--js

Re: [PATCH v3 03/15] python: add VERSION file

[John Snow]

On 10/28/20 3:51 PM, Cleber Rosa wrote:
> On Tue, Oct 20, 2020 at 03:35:43PM -0400, John Snow wrote:
>> 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
>> to my knowledge.
>>
>> 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 always 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.5.2.0a1.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
> 
> I'd rather have the version to be sync'd with QEMU, but, I understand
> this is a more conservative approach that can maybe evolve into that.
> 
> Reviewed-by: Cleber Rosa <crosa@redhat.com>
> 

It's definitely me taking the cowardly way out. I did use the exact QEMU 
version in the last spin, because that seemed like the dumbest thing :)

I think qemu.machine would eventually evolve to track the QEMU version 
directly, whereas qemu.qmp would evolve to keep its own independent 
versioning system.

This is just, I guess, one more semantic nudge towards the idea that 
this is really an independent little component. I just want to give it 
some more time in the oven before I declare it truly and genuinely 
supported as its own project. Once it's on PyPI, I am beholden to more 
than the other QEMU contributors. Satisfying both crowds simultaneously 
seems like it will be tough.

--js

Re: [PATCH v3 02/15] python: add qemu package installer

[John Snow]

On 10/28/20 3:46 PM, Cleber Rosa wrote:
> On Wed, Oct 28, 2020 at 01:02:52PM -0400, John Snow wrote:
>> On 10/28/20 11:10 AM, Cleber Rosa wrote:
>>>> +mirror <https://gitlab.com/jsnow/qemu/-/tree/python>`_, but
>>>> +contributions must be sent to the list for inclusion.
>>>
>>> IMO it's not clear if this branch/mirror is your development work, a
>>> staging area, etc.
>>>
>>
>> Fair enough. jsnow/qemu/python is intended as a staging area for patches
>> that have been vetted on-list.
>>
>> jsnow/qemu/master is a lazily-updated mirror. (I tend to update it every day
>> as part of my development process, but there are days I don't write code.)
>>
>> jsnow/qemu/python-* is development work; review branches, etc.
>>
>> I'll try to rephrase this a bit. What I want to communicate:
>>
>> - This package exists as a subfolder of a larger project
>> - I am responsible for maintaining this package, but not for the larger
>> project
>> - Please contact *me* for problems with this package
>> - Contributions should go through qemu-devel (I will gently redirect
>> contributors who may send pull requests to the qemu devel list.)
>>
> 
> OK, sounds good.  I'll look at the exact rewording on v+1.
> 
>>>> diff --git a/python/setup.cfg b/python/setup.cfg
>>>> new file mode 100755
>>>> index 0000000000..12b99a796e
>>>> --- /dev/null
>>>> +++ b/python/setup.cfg
>>>> @@ -0,0 +1,18 @@
>>>> +[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
>>>> +
>>>
>>
>> Also ... licensing question, do we need *L*GPLv2, or does Python not have a
>> "linking exception" problem?
>>
>> I guess we can't really re-license these files anyway, so nevermind.
>>
>> (I immediately regret asking this.)
>>
> 
> I'll just pretend you never did.
> 
>>> I know the sky is the limit, but I miss the Python version classifier,
>>> at least:
>>>
>>>     Programming Language :: Python :: 3 :: Only
>>>
>>
>> Sure.
>>
>> Wait, why can you specify Python as a language? Is it possible to have
>> non-Python packages on PyPI?
>>
>> *CONCERNED*
>>
> 
> I guess it has to do with packages that can interact or serve other
> languages.  Or, that are (partially) written in another language?
> 
>>> And optionally those:
>>>
>>>     Programming Language :: Python :: 3.6
>>>     Programming Language :: Python :: 3.7
>>>     Programming Language :: Python :: 3.8
>>>     Programming Language :: Python :: 3.9
>>>
>>> Although it may be a good idea to add them along test jobs on those
>>> specific Python versions.
>>>
>>
>> Are these worth adding? I've got python_requires >= 3.6 down below. From my
>> test of a blank package upload to PyPI, it seems to display prominently:
>>
>> https://pypi.org/project/qemu/
>>
>> Is there a tangible benefit that you are aware of?
>>
> 
> AFAICT, the classifiers are about letting people search for packages
> that match a given criteria.  It's all metadata, so the benefits are
> not super tangible.  I've used those to keep track / document the
> Python versions that I know the project has been actively tested on,
> and that's the reason of my comment about (CI) test jobs.
> 

OK, let's add them alongside a tox/pytest configuration or something in 
the future when we add those versions as being supported.

I guess I can add the 3.6 version for starters, since it's explicitly 
supported?

>>>> +[options]
>>>> +python_requires = >= 3.6
>>>> +packages = find_namespace:
>>>> diff --git a/python/setup.py b/python/setup.py
>>>> new file mode 100755
>>>> index 0000000000..e93d0075d1
>>>> --- /dev/null
>>>> +++ b/python/setup.py
>>>> @@ -0,0 +1,23 @@
>>>> +#!/usr/bin/env python3
>>>> +"""
>>>> +QEMU tooling installer script
>>>> +Copyright (c) 2020 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')
>>>
>>> Getting back to the "test jobs on those specific Python versions" I
>>> was really anxious that environments with Python 3.6 will fail to
>>> have such a "recent" setuptools version.
>>>
>>
>> Reasonable doubt. However, this isn't *required* to use the library (the
>> QEMU code can continue to just set PYTHONPATH or sys.path as necessary) and
>> bypasses the installer entirely.
>>
> 
> Right, but I had the impression that activating it in develop mode (at
> least) was the intention down the line.
> 

For builds at all?

I guess we could, yeah, if we wanted to start making a "build venv" and 
install it there. I think there's a lot of questions to work out there 
first, though. I am not really there yet, myself.

Right now, *right now*, all of this code is used only for testing and 
CI, so we've skirted around build requirements.

I did want to start picking up some other packages though, like 'qapi', 
for the purposes of applying the linter paradigm though ... and I 
figured I'd cross that bridge when I got there.

Right now, having a forwarder script with a sys.path hack works.

(Probably by the time we figure that out, setuptools 39 will be standard 
on all of our supported build platforms...)

>> That gives us some leeway apart from the usual version constraints; in order
>> to independently use this library outside of the QEMU tree we may impose
>> more modern setups -- as long as the minimum requirements for QEMU itself
>> don't break.
>>
> 
> OK.
> 
>> Having a modern setuptools in order to install seems like less of a problem
>> barrier; and it seemed like a good idea to make it explicitly fail instead
>> of silently doing something weird if it didn't see/understand setup.cfg.
>>
> 
> Agreed.
> 
>> (And it seems like good practice to update pip in your venv, so I think
>> we'll be OK except for the stodgiest of users, but sometimes you can't have
>> new things on old systems without learning some new tricks!)
>>
>>> CentOS 8 has that specific version, while Ubuntu 18.04 has version
>>> 39.0.  Ubuntu 20.04 has a recent enough version though.  Given that
>>> all GitLab CI moved to 20.04, this should be safe.
>>>
>>> - Cleber.
>>>
>>
>> FWIW, for the purposes of running the linters, I am using Fedora32 and the
>> python36 package.
>>
> 
> This is a minor suggestion: use CentOS 8 stock Python 3.6 packages,
> and then Fedora 33 with also stock Python 3.9.  Even though all
> tools are pinned, it's still a good idea to test at least min/max
> (if not all) Python versions.
> 

I can use CentOS, sure. I don't think it matters tremendously whose 
Python 3.6 we use. I opted for Fedora because we package old python 
interpreters on purpose, which makes it easy to say "I want Python3.6 
and not a drop older or newer."

I assume the same is true on CentOS. We can talk about this on the CI 
series; though I will likely merge these two series into one for future 
revisions.

I don't have a framework in mind for doing python version matrix testing 
yet. I guess tox is the canonical tool for that. We can cross that 
bridge when we get there, I guess. (We currently have: 0 tests for the 
qemu python library. oops!)

For the meantime, though, I think it's OK to only run the linter on 
Python 3.6: it's not a test of the package itself, it's just a specific 
environment that we use to enforce code quality. It so happens to be a 
Python 3.6 environment. Pinning it to a specific version of python there 
is useful because the linters *do* sometimes have different behavior 
depending on version; due largely in part to changes in the stdlib 
typing library.

> - Cleber.
> 
>>>> +
>>>> +    setuptools.setup()
>>>> +
>>>> +
>>>> +if __name__ == '__main__':
>>>> +    main()
>>>> -- 
>>>> 2.26.2
>>>>
>>

Re: [PATCH v3 02/15] python: add qemu package installer

[John Snow]

On 10/28/20 3:49 PM, Cleber Rosa wrote:
> On Tue, Oct 20, 2020 at 03:35:42PM -0400, John Snow wrote:
>> Add setup.cfg and setup.py, necessary for installing a package via
>> pip. Add a rst document 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.
>>
>> I am not yet using a pyproject.toml style package manifest, because
>> using pyproject.toml (and PEP-517) style packages means that pip is not
>> able to install in "editable" or "develop" mode, which I consider
>> necessary for the development of this package.
>>
>> Use a light-weight setup.py instead.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
>>   python/PACKAGE.rst | 26 ++++++++++++++++++++++++++
>>   python/setup.cfg   | 18 ++++++++++++++++++
>>   python/setup.py    | 23 +++++++++++++++++++++++
>>   3 files changed, 67 insertions(+)
>>   create mode 100644 python/PACKAGE.rst
>>   create mode 100755 python/setup.cfg
> 
> I missed this earlier: setup.cfg does not need to have the execute bit
> on.
> 
> - Cleber.
> 

Fixed this locally already; it came along when I ran 'cp setup.py 
setup.cfg', obviously!

Re: [PATCH v3 04/15] python: add directory structure README.rst files

[Cleber Rosa]

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

On Tue, Oct 20, 2020 at 03:35:44PM -0400, John Snow wrote:
> Add short readmes to python/, python/qemu/, python/qemu/machine, and
> python/qemu/machine 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 <jsnow@redhat.com>
> ---
>  python/README.rst              | 27 +++++++++++++++++++++++++++
>  python/qemu/README.rst         |  8 ++++++++
>  python/qemu/machine/README.rst |  9 +++++++++
>  python/qemu/qmp/README.rst     |  9 +++++++++
>  4 files changed, 53 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
> 
> diff --git a/python/README.rst b/python/README.rst
> new file mode 100644
> index 0000000000..ff40e4c931
> --- /dev/null
> +++ b/python/README.rst
> @@ -0,0 +1,27 @@
> +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 (``qemu/machine``, ``qemu/qmp``).
> +
> +``setup.py`` is used by ``pip`` to install this tooling to the current
> +environment. 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.
> +
> +If you amend the ``-e`` argument, pip will install in "editable" mode;
> +which installs a version of the package that uses symlinks to these
> +files, such that the package always reflects the latest version in your
> +git tree.
> +

It actually uses *egg-link* files, which are not what people will
understand by "symlinks".

> +See `Installing packages using pip and virtual environments
> +<https://packaging.python.org/guides/installing-using-pip-and-virtual-environments/>`_
> +for more information.
> diff --git a/python/qemu/README.rst b/python/qemu/README.rst
> new file mode 100644
> index 0000000000..31209c80a5
> --- /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 <<https://www.python.org/dev/peps/pep-0420/>`_.
> +
> +Each directory below is assumed to be an installable Python package that
> +is available under the ``qemu.<package>`` namespace.
> diff --git a/python/qemu/machine/README.rst b/python/qemu/machine/README.rst
> new file mode 100644
> index 0000000000..73ad23c501
> --- /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, and several other utilities
> +in the ./scripts directory. It is not a fully-fledged SDK and it is
> +subject to change at any time.
> +

I'm not sure if you intend to list all test types that use this, but
the acceptance tests also do use them.

- Cleber.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 05/15] python: Add pipenv support

[Cleber Rosa]

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

On Tue, Oct 20, 2020 at 03:35:45PM -0400, John Snow wrote:
> 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.py
> 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 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 loudout 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 <jsnow@redhat.com>

Reviewed-by: Cleber Rosa <crosa@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 06/15] python: add pylint import exceptions

[Cleber Rosa]

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

On Tue, Oct 20, 2020 at 03:35:46PM -0400, John Snow wrote:
> Pylint 2.5.x and 2.6.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
>

Are these whitespaces on purpose?

> 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 <jsnow@redhat.com>

Other than that,

Reviewed-by: Cleber Rosa <crosa@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 07/15] python: move pylintrc into setup.cfg

[Cleber Rosa]

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

On Tue, Oct 20, 2020 at 03:35:47PM -0400, John Snow wrote:
> 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 <jsnow@redhat.com>
> ---

Reviewed-by: Cleber Rosa <crosa@redhat.com>
Tested-by: Cleber Rosa <crosa@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 08/15] python: add pylint to pipenv

[Cleber Rosa]

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

On Tue, Oct 20, 2020 at 03:35:48PM -0400, John Snow wrote:
> We are specifying >= pylint 2.6.x for two reasons:
> 
> 1. For setup.cfg support, added in pylint 2.5.x
> 2. To clarify that we are using a version that has incompatibly dropped
> bad-whitespace checks.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---

I'm not a huge fan of this level of verbosity that pipenv generates,
but at the same time, I've been bitten too many times by not providing
the entire dep tree in a "requirements.txt"-like style.  And it is
what pipenv uses, so there's no way around that.

Reviewed-by: Cleber Rosa <crosa@redhat.com>
Tested-by: Cleber Rosa <crosa@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 09/15] python: move flake8 config to setup.cfg

[Cleber Rosa]

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

On Tue, Oct 20, 2020 at 03:35:49PM -0400, John Snow wrote:
> 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 <jsnow@redhat.com>
> ---

Reviewed-by: Cleber Rosa <crosa@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 10/15] python: Add flake8 to pipenv

[Cleber Rosa]

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

On Tue, Oct 20, 2020 at 03:35:50PM -0400, John Snow wrote:
> 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 <jsnow@redhat.com>
> ---

Reviewed-by: Cleber Rosa <crosa@redhat.com>
Tested-by: Cleber Rosa <crosa@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 11/15] python: move mypy.ini into setup.cfg

[Cleber Rosa]

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

On Tue, Oct 20, 2020 at 03:35:51PM -0400, John Snow wrote:
> mypy supports reading its configuration values from a central project
> configuration file; do so.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---

Reviewed-by: Cleber Rosa <crosa@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 12/15] python: add mypy to pipenv

[Cleber Rosa]

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

On Tue, Oct 20, 2020 at 03:35:52PM -0400, John Snow wrote:
> 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 qemu'.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---

Reviewed-by: Cleber Rosa <crosa@redhat.com>
Tested-by: Cleber Rosa <crosa@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 13/15] python: move .isort.cfg into setup.cfg

[Cleber Rosa]

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

On Tue, Oct 20, 2020 at 03:35:53PM -0400, John Snow wrote:
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---

Reviewed-by: Cleber Rosa <crosa@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 14/15] python/qemu: add isort to pipenv

[Cleber Rosa]

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

On Tue, Oct 20, 2020 at 03:35:54PM -0400, John Snow wrote:
> 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.
> 
> Require 5.0.5 or greater.
> 
> isort can be run with 'isort -c qemu' from the python root.
> 
> Signed-off-by: John Snow <jsnow@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---

Reviewed-by: Cleber Rosa <crosa@redhat.com>
Tested-by: Cleber Rosa <crosa@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 15/15] python/qemu: add qemu package itself to pipenv

[Cleber Rosa]

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

On Tue, Oct 20, 2020 at 03:35:55PM -0400, John Snow wrote:
> 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 <jsnow@redhat.com>
> ---

Reviewed-by: Cleber Rosa <crosa@redhat.com>
Tested-by: Cleber Rosa <crosa@redhat.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 04/15] python: add directory structure README.rst files

[John Snow]

On 10/28/20 6:05 PM, Cleber Rosa wrote:
> On Tue, Oct 20, 2020 at 03:35:44PM -0400, John Snow wrote:
>> Add short readmes to python/, python/qemu/, python/qemu/machine, and
>> python/qemu/machine 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 <jsnow@redhat.com>
>> ---
>>   python/README.rst              | 27 +++++++++++++++++++++++++++
>>   python/qemu/README.rst         |  8 ++++++++
>>   python/qemu/machine/README.rst |  9 +++++++++
>>   python/qemu/qmp/README.rst     |  9 +++++++++
>>   4 files changed, 53 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
>>
>> diff --git a/python/README.rst b/python/README.rst
>> new file mode 100644
>> index 0000000000..ff40e4c931
>> --- /dev/null
>> +++ b/python/README.rst
>> @@ -0,0 +1,27 @@
>> +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 (``qemu/machine``, ``qemu/qmp``).
>> +
>> +``setup.py`` is used by ``pip`` to install this tooling to the current
>> +environment. 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.
>> +
>> +If you amend the ``-e`` argument, pip will install in "editable" mode;
>> +which installs a version of the package that uses symlinks to these
>> +files, such that the package always reflects the latest version in your
>> +git tree.
>> +
> 
> It actually uses *egg-link* files, which are not what people will
> understand by "symlinks".
> 

Ehm, good point; I don't want to *lie* here. I think I do want to avoid 
the jargon though.

"that installs a forwarder pointing to these files" -- better?

>> +See `Installing packages using pip and virtual environments
>> +<https://packaging.python.org/guides/installing-using-pip-and-virtual-environments/>`_
>> +for more information.
>> diff --git a/python/qemu/README.rst b/python/qemu/README.rst
>> new file mode 100644
>> index 0000000000..31209c80a5
>> --- /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 <<https://www.python.org/dev/peps/pep-0420/>`_.
>> +
>> +Each directory below is assumed to be an installable Python package that
>> +is available under the ``qemu.<package>`` namespace.
>> diff --git a/python/qemu/machine/README.rst b/python/qemu/machine/README.rst
>> new file mode 100644
>> index 0000000000..73ad23c501
>> --- /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, and several other utilities
>> +in the ./scripts directory. It is not a fully-fledged SDK and it is
>> +subject to change at any time.
>> +
> 
> I'm not sure if you intend to list all test types that use this, but
> the acceptance tests also do use them.
> 

Wasn't a conscious omission, but didn't necessarily mean to make it 
comprehensive; I can add it while fixing the above feedback item too.

> - Cleber.
> 

Thanks!

Re: [PATCH v3 06/15] python: add pylint import exceptions

[John Snow]

On 10/28/20 6:24 PM, Cleber Rosa wrote:
> On Tue, Oct 20, 2020 at 03:35:46PM -0400, John Snow wrote:
>> Pylint 2.5.x and 2.6.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
>>
> 
> Are these whitespaces on purpose?
> 

Uh, nope. How'd that happen?

>> 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 <jsnow@redhat.com>
> 
> Other than that,
> 
> Reviewed-by: Cleber Rosa <crosa@redhat.com>
> 

Re: [PATCH v3 08/15] python: add pylint to pipenv

[John Snow]

On 10/28/20 6:38 PM, Cleber Rosa wrote:
> On Tue, Oct 20, 2020 at 03:35:48PM -0400, John Snow wrote:
>> We are specifying >= pylint 2.6.x for two reasons:
>>
>> 1. For setup.cfg support, added in pylint 2.5.x
>> 2. To clarify that we are using a version that has incompatibly dropped
>> bad-whitespace checks.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> ---
> 
> I'm not a huge fan of this level of verbosity that pipenv generates,
> but at the same time, I've been bitten too many times by not providing
> the entire dep tree in a "requirements.txt"-like style.  And it is
> what pipenv uses, so there's no way around that.
> 

Unless we don't use Pipenv :)

> Reviewed-by: Cleber Rosa <crosa@redhat.com>
> Tested-by: Cleber Rosa <crosa@redhat.com>
> 

I will say I'm open to using tools that aren't Pipenv -- this is just 
something I knew how to use in order to provide a fairly robust 
venv-recreation mechanism, so I am using it. Don't know what I don't 
know, otherwise.

I hear Poetry is nice, but I haven't looked into it yet.

Tox I believe also does venv-management to some extent, though my 
impression of it was that it was less specific about the environments 
than Pipenv was and allowed more wiggle room. Could be wrong.

Pipenv does not manage multiple environments either, unlike Tox, which 
can multiplex your environments for different versions of things. That's 
one point in favor, perhaps.

If you'd like to propose using something else, feel free to fork this 
branch and implement something else; it's probably quicker that way than 
to have me learn tox/poetry/etc.

What I was using Pipenv to solve here is this:

"Create a package list pinned to explicit versions that is not for the 
purpose of installing or using the package, but is for the purpose of 
recreating a precise testing environment. Use this environment to run 
the linters with precisely known tooling versions."

My initial read was that using pip and requirements.txt alone was 
insufficient for this purpose; so I have been using Pipenv for the purpose.

Suggestions/patches welcome.

--js

Re: [PATCH v3 14/15] python/qemu: add isort to pipenv

[John Snow]

On 10/28/20 6:46 PM, Cleber Rosa wrote:
> On Tue, Oct 20, 2020 at 03:35:54PM -0400, John Snow wrote:
>> 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.
>>
>> Require 5.0.5 or greater.
>>
>> isort can be run with 'isort -c qemu' from the python root.
>>
>> Signed-off-by: John Snow <jsnow@redhat.com>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
> 
> Reviewed-by: Cleber Rosa <crosa@redhat.com>
> Tested-by: Cleber Rosa <crosa@redhat.com>
> 

Good suggestion on isort, btw. I was afraid at first it was going to be 
annoying to integrate a fourth tool, but it wound up sliding in very 
nicely as you can see. :)

I like the effect it has already had on these and the QAPI patches.

--js

Re: [PATCH v3 15/15] python/qemu: add qemu package itself to pipenv

[John Snow]

On 10/28/20 6:59 PM, Cleber Rosa wrote:
> On Tue, Oct 20, 2020 at 03:35:55PM -0400, John Snow wrote:
>> 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 <jsnow@redhat.com>
>> ---
> 
> Reviewed-by: Cleber Rosa <crosa@redhat.com>
> Tested-by: Cleber Rosa <crosa@redhat.com>
> 

Thanks! It looks like the major questions remaining are:

- Seriously, how should we lay the files out, and what should the 
package names be?
- Do we want to use something besides pipenv?

And from the CI series, a whole heap of other questions ;)

Thanks for taking a look and testing. It took a non-trivial amount of 
time to get all of this corralled together in precisely the right way to 
make it work in a variety of environments, so I am hoping it's "close" 
to something that's going to work for everyone.

--js