[PATCH v4 0/4] Python: Improvements for iotest 040,041

John Snow posted 4 patches 2 years, 2 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20220201041134.1237016-1-jsnow@redhat.com
Maintainers: Cleber Rosa <crosa@redhat.com>, Kevin Wolf <kwolf@redhat.com>, John Snow <jsnow@redhat.com>, Eduardo Habkost <eduardo@habkost.net>, Hanna Reitz <hreitz@redhat.com>
python/Pipfile.lock                       | 66 +++++++++++++----------
python/qemu/aqmp/legacy.py                |  3 ++
python/qemu/aqmp/protocol.py              | 41 ++++++++++++--
python/qemu/aqmp/qmp_client.py            |  4 +-
python/qemu/machine/machine.py            | 45 +++++++++++++---
python/setup.cfg                          |  2 +-
tests/qemu-iotests/tests/mirror-top-perms |  3 +-
7 files changed, 123 insertions(+), 41 deletions(-)
[PATCH v4 0/4] Python: Improvements for iotest 040,041
Posted by John Snow 2 years, 2 months ago
GitLab: https://gitlab.com/jsnow/qemu/-/commits/python-aqmp-fixes
CI: https://gitlab.com/jsnow/qemu/-/pipelines/455146881

Fixes and improvements all relating to "iotest 040,041, intermittent
failure in netbsd VM"
https://lists.gnu.org/archive/html/qemu-devel/2022-01/msg01975.html

See each patch for details.

V4:
 - Just commit message changes, and applying Hanna's RBs.

V3:
 - Retitled series
 - Dropped patch that was already merged
 - Reworded some comments, docstrings, etc.

John Snow (4):
  python/aqmp: Fix negotiation with pre-"oob" QEMU
  python/machine: raise VMLaunchFailure exception from launch()
  python: upgrade mypy to 0.780
  python/aqmp: add socket bind step to legacy.py

 python/Pipfile.lock                       | 66 +++++++++++++----------
 python/qemu/aqmp/legacy.py                |  3 ++
 python/qemu/aqmp/protocol.py              | 41 ++++++++++++--
 python/qemu/aqmp/qmp_client.py            |  4 +-
 python/qemu/machine/machine.py            | 45 +++++++++++++---
 python/setup.cfg                          |  2 +-
 tests/qemu-iotests/tests/mirror-top-perms |  3 +-
 7 files changed, 123 insertions(+), 41 deletions(-)

-- 
2.31.1



Re: [PATCH v4 0/4] Python: Improvements for iotest 040,041
Posted by Kevin Wolf 2 years, 2 months ago
Am 01.02.2022 um 05:11 hat John Snow geschrieben:
> GitLab: https://gitlab.com/jsnow/qemu/-/commits/python-aqmp-fixes
> CI: https://gitlab.com/jsnow/qemu/-/pipelines/455146881
> 
> Fixes and improvements all relating to "iotest 040,041, intermittent
> failure in netbsd VM"
> https://lists.gnu.org/archive/html/qemu-devel/2022-01/msg01975.html
> 
> See each patch for details.

Thanks, the new output when QEMU fails to start looks really useful!

The only thing we could probably still improve is detecting that the
QEMU process has exited instead of waiting for the socket connection to
time out. But since it only happens in case of failure, the additional
seconds of waiting are probably only a bit annoying for debugging, but
not a big problem.

Reviewed-by: Kevin Wolf <kwolf@redhat.com>


Re: [PATCH v4 0/4] Python: Improvements for iotest 040,041
Posted by John Snow 2 years, 2 months ago
On Tue, Feb 1, 2022 at 8:28 AM Kevin Wolf <kwolf@redhat.com> wrote:
>
> Am 01.02.2022 um 05:11 hat John Snow geschrieben:
> > GitLab: https://gitlab.com/jsnow/qemu/-/commits/python-aqmp-fixes
> > CI: https://gitlab.com/jsnow/qemu/-/pipelines/455146881
> >
> > Fixes and improvements all relating to "iotest 040,041, intermittent
> > failure in netbsd VM"
> > https://lists.gnu.org/archive/html/qemu-devel/2022-01/msg01975.html
> >
> > See each patch for details.
>
> Thanks, the new output when QEMU fails to start looks really useful!
>
> The only thing we could probably still improve is detecting that the
> QEMU process has exited instead of waiting for the socket connection to
> time out. But since it only happens in case of failure, the additional
> seconds of waiting are probably only a bit annoying for debugging, but
> not a big problem.

That's absolutely on my radar, I assure you!

It's something that is easy to solve with asyncio:

async def launch(self, ...):
    task1 = asyncio.create_task(self.qmp.accept(...))
    task2 = asyncio.create_subprocess_exec(...)
    ret = asyncio.wait_for((task1, task2))
    ...

If either task raises, then the wait_for will also end prematurely
(and cancel the other task). I'm sure it won't actually be this easy,
but that's the general idea.

Though, that's a pretty big upheaval to the Python code again, so it's
not something I can jam in quickly. I have some light sketches that
examine a "what-if" for an asyncio-native machine.py using the asyncio
QMP code, but there are some design concerns there -- if I go through
the effort of doing this, I will want to publish that python package
upstream as well, and if I do that, it needs to be carefully thought
through in terms of a supportable interface.

Well, I'm thinking about it, anyway. For right now I am interested in
getting the qemu.qmp project out the door and onto PyPI.org, and then
I'll worry about machine improvements. If in the meantime you have a
good idea for how to fix up the machine.py code we already have, I'm
happy to take patches. Otherwise, I'll try to get to it "soon".

>
> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
>

Thanks!