Re: [PATCH 00/10] Python: Fix qmp race condition on accept()

[John Snow]

Ping - Dan, Kevin: Any thoughts on the new API here? I only ask
because there was a bit of feedback in response to the last patch and
I didn't want to stage this without giving you a fair chance to look.

I'm not expecting any real review on the Python, just wanted to see if
you felt like this design adequately addressed your feedback from
before.

Here is the super high level:

1. what was accept() is renamed start_server_and_accept(). It's a
one-shot command to do (effectively) bind(2) listen(2) accept(2).
2. start_server() method is added. It calls bind() and listen(), and
new connections will be accept(2)'d asynchronously.
3. accept() method is added. It does not *actually* call accept(2),
it's more like "wait for accept(2) to be called in the bottom half".
Similar enough.
4. start_server_and_accept() is, by the end of the series, literally
just two calls to start_server() and accept().

Otherwise, I'll just assume no news is good news and I'll send a
pullreq soon so that Peter's NetBSD tests stop being flaky.

--js


On Fri, Feb 25, 2022 at 4:00 PM John Snow <jsnow@redhat.com> wrote:
>
> GitLab: https://gitlab.com/jsnow/qemu/-/commits/python-aqmp-accept-changes
> CI: https://gitlab.com/jsnow/qemu/-/pipelines/479795153
>
> This redesigns the async QMP interface to allow for race-free
> connections from the synchronous interface. It should hopefully address
> the race conditions Peter has been seeing on the NetBSD vm tests.
>
> John Snow (10):
>   python/aqmp: add _session_guard()
>   python/aqmp: rename 'accept()' to 'start_server_and_accept()'
>   python/aqmp: remove _new_session and _establish_connection
>   python/aqmp: split _client_connected_cb() out as _incoming()
>   python/aqmp: squelch pylint warning for too many lines
>   python/aqmp: refactor _do_accept() into two distinct steps
>   python/aqmp: stop the server during disconnect()
>   python/aqmp: add start_server() and accept() methods
>   python/aqmp: fix race condition in legacy.py
>   python/aqmp: drop _bind_hack()
>
>  python/qemu/aqmp/legacy.py   |   7 +-
>  python/qemu/aqmp/protocol.py | 393 +++++++++++++++++++++--------------
>  python/tests/protocol.py     |  45 ++--
>  3 files changed, 273 insertions(+), 172 deletions(-)
>
> --
> 2.34.1
>
>