Re: [PATCH 00/20] python: introduce Asynchronous QMP package

[Stefan Hajnoczi <stefanha@redhat.com>]

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

On Thu, Jul 01, 2021 at 12:12:53AM -0400, John Snow wrote:
> GitLab: https://gitlab.com/jsnow/qemu/-/commits/python-async-qmp-aqmp
> CI: https://gitlab.com/jsnow/qemu/-/pipelines/330003554
> Docs: https://people.redhat.com/~jsnow/sphinx/html/qemu.aqmp.html
> Based-on: <20210701020921.1679468-1-jsnow@redhat.com>
>           [PULL 00/15] Python patches
> 
> Hi!
> 
> This patch series adds an Asynchronous QMP package to the Python
> library. It offers a few improvements over the previous library:
> 
> - out-of-band support
> - true asynchronous event support
> - avoids undocumented interfaces abusing non-blocking sockets
> 
> This library serves as the basis for a new qmp-shell program that will
> offer improved reconnection support, true asynchronous display of
> events, VM and job status update notifiers, and so on.
> 
> My intent is to eventually publish this library directly to PyPI as a
> standalone package. I would like to phase out our usage of the old QMP
> library over time; eventually replacing it entirely with this one.
> 
> This series looks big by line count, but it's *mostly*
> docstrings. Seriously!
> 
> This package has *no* external dependencies whatsoever.
> 
> Notes & Design
> ==============
> 
> Here are some notes on the design of how the library works, to serve as
> a primer for review; however I also **highly recommend** browsing the
> generated Sphinx documentation for this series.
> 
> Here's that link again:
> https://people.redhat.com/~jsnow/sphinx/html/qemu.aqmp.html
> 
> The core machinery is split between the AsyncProtocol and QMP
> classes. AsyncProtocol provides the generic machinery, while QMP
> provides the QMP-specific details.
> 
> The design uses two independent coroutines that act as the "bottom
> half", a writer task and a reader task. These tasks run for the duration
> of the connection and independently send and receive messages,
> respectively.
> 
> A third task, disconnect, is scheduled asynchronously whenever an
> unrecoverable error occurs and facilitates coalescing of the other two
> tasks.
> 
> This diagram for how execute() operates may be helpful for understanding
> how AsyncProtocol is laid out. The arrows indicate the direction of a
> QMP message; the long horizontal dash indicates the separation between
> the upper and lower half of the event loop. The queue mechanisms between
> both dashes serve as the intermediaries between the upper and lower
> half.
> 
>                        +---------+
>                        | caller  |
>                        +---------+
>                            ^ |
>                            | v
>                        +---------+
>      +---------------> |execute()| -----------+
>      |                 +---------+            |
>      |                                        |
> [-----------------------------------------------------------]
>      |                                        |
>      |                                        v
> +----+------+    +----------------+    +------+-------+
> | ExecQueue |    | EventListeners |    |Outbound Queue|
> +----+------+    +----+-----------+    +------+-------+
>      ^                ^                       |
>      |                |                       |
> [-----------------------------------------------------------]
>      |                |                       |
>      |                |                       v
>   +--+----------------+---+       +-----------+-----------+
>   | Reader Task/Coroutine |       | Writer Task/Coroutine |
>   +-----------+-----------+       +-----------+-----------+
>               ^                               |
>               |                               v
>         +-----+------+                  +-----+------+
>         |StreamReader|                  |StreamWriter|
>         +------------+                  +------------+
> 
> The caller will invoke execute(), which in turn will deposit a message
> in the outbound send queue. This will wake up the writer task, which
> well send the message over the wire.
> 
> The execute() method will then yield to wait for a reply delivered to an
> execution queue created solely for that execute statement.
> 
> When a message arrives, the Reader task will unblock and route the
> message either to the EventListener subsystem, or place it in the
> appropriate pending execution queue.
> 
> Once a message is placed in the pending execution queue, execute() will
> unblock and the execution will conclude, returning the result of the RPC
> call to the caller.
> 
> Ugly Bits
> =========
> 
> - MultiException is ... wonky. I am still working out how to avoid needing it.
>   See patch 04/20 for details here, or see
>   https://people.redhat.com/~jsnow/sphinx/html/qemu.aqmp.error.html
> 
>   Patch 06/20 also goes into details of the ugliness; see
>   AsyncProtocol._results or view the same information here:
>   https://people.redhat.com/~jsnow/sphinx/html/_modules/qemu/aqmp/protocol.html#AsyncProtocol._results
> 
> - There are quite a few lingering questions I have over the design of the
>   EventListener subsystem; I wrote about those ugly bits in excruciating detail
>   in patch 14/20.
> 
>   You can view them formatted nicely here:
>   https://people.redhat.com/~jsnow/sphinx/html/qemu.aqmp.events.html#experimental-interfaces-design-issues
> 
> Patch Layout
> ============
> 
> Patches 1-2 are tiny pylint configuration changes.
> Patches 3-5 begin to check in Async QMP components, they are small.
> Patches 6-11 add a generic async message-based protocol class,
>              AsyncProto. They are split as small as I could
>              reasonably split them.
> Patches 12-14 check in more QMP-specific components.
> Patches 15-18 add qmp_protocol.py, the new 'QMP' class. They're split as
>               far down as I could, I hope they're easy to review.
> Patches 19-20 add a few finishing touches, they are small patches.
> 
> Future Work
> ===========
> 
> These items are in progress:
> 
> - A Synchronous QMP wrapper that allows this library to be easily used
>   from non-async code; this will also allow me to prove it works well by
>   demoing its replacement throughout iotests.
> 
> - A QMP server class; to facilitate writing of unit tests.
> 
> - Unit tests. Real unit tests.
> 
> If you made it this far in the cover letter, congrats :)
> 
> John Snow (20):
>   python/pylint: Add exception for TypeVar names ('T')
>   python/pylint: disable too-many-function-args
>   python/aqmp: add asynchronous QMP (AQMP) subpackage
>   python/aqmp: add error classes
>   python/aqmp: add asyncio compatibility wrappers
>   python/aqmp: add generic async message-based protocol support
>   python/aqmp: add runstate state machine to AsyncProtocol
>   python/aqmp: add logging to AsyncProtocol
>   python/aqmp: add AsyncProtocol.accept() method
>   python/aqmp: add _cb_inbound and _cb_inbound logging hooks
>   python/aqmp: add AsyncProtocol._readline() method
>   python/aqmp: add QMP Message format
>   python/aqmp: add well-known QMP object models
>   python/aqmp: add QMP event support
>   python/aqmp: add QMP protocol support
>   python/aqmp: Add message routing to QMP protocol
>   python/aqmp: add execute() interfaces
>   python/aqmp: add _raw() execution interface
>   python/aqmp: add asyncio_run compatibility wrapper
>   python/aqmp: add scary message
> 
>  python/qemu/aqmp/__init__.py     |  61 +++
>  python/qemu/aqmp/error.py        |  97 ++++
>  python/qemu/aqmp/events.py       | 878 +++++++++++++++++++++++++++++++
>  python/qemu/aqmp/message.py      | 207 ++++++++
>  python/qemu/aqmp/models.py       | 133 +++++
>  python/qemu/aqmp/protocol.py     | 851 ++++++++++++++++++++++++++++++
>  python/qemu/aqmp/py.typed        |   0
>  python/qemu/aqmp/qmp_protocol.py | 565 ++++++++++++++++++++
>  python/qemu/aqmp/util.py         | 183 +++++++
>  python/setup.cfg                 |   4 +-
>  10 files changed, 2978 insertions(+), 1 deletion(-)
>  create mode 100644 python/qemu/aqmp/__init__.py
>  create mode 100644 python/qemu/aqmp/error.py
>  create mode 100644 python/qemu/aqmp/events.py
>  create mode 100644 python/qemu/aqmp/message.py
>  create mode 100644 python/qemu/aqmp/models.py
>  create mode 100644 python/qemu/aqmp/protocol.py
>  create mode 100644 python/qemu/aqmp/py.typed
>  create mode 100644 python/qemu/aqmp/qmp_protocol.py
>  create mode 100644 python/qemu/aqmp/util.py

Acked-by: Stefan Hajnoczi <stefanha@redhat.com>

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