Message ID | 20220225205948.3693480-1-jsnow@redhat.com |
---|---|
Headers | show |
Series | Python: Fix qmp race condition on accept() | expand |
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 > >
Am 25.02.2022 um 21:59 hat John Snow geschrieben: > 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. Acked-by: Kevin Wolf <kwolf@redhat.com>
On Fri, Mar 4, 2022 at 12:49 PM Kevin Wolf <kwolf@redhat.com> wrote: > > Am 25.02.2022 um 21:59 hat John Snow geschrieben: > > 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. > > Acked-by: Kevin Wolf <kwolf@redhat.com> > Thanks so much to the both of you! I was dreading this fix, but it wound up being *pretty* clean in the end, I think. I really appreciate the double-check here. --js