Re: [PATCH v4 00/16] python: add mypy support to python/qemu
On 6/29/20 10:30 AM, John Snow wrote: > > > On 6/29/20 4:26 AM, Philippe Mathieu-Daudé wrote: >> +Cleber/Wainer. >> >> On 6/26/20 10:41 PM, John Snow wrote: >>> Based-on: 20200626202350.11060-1-js...@redhat.com >>> >>> This series modifies the python/qemu library to comply with mypy --strict. >>> This requires my "refactor shutdown" patch as a pre-requisite. >>> >>> v4: >>> 001/16:[] [--] 'python/qmp.py: Define common types' >>> 002/16:[] [--] 'iotests.py: use qemu.qmp type aliases' >>> 003/16:[] [--] 'python/qmp.py: re-absorb MonitorResponseError' >>> 004/16:[] [--] 'python/qmp.py: Do not return None from cmd_obj' >>> 005/16:[] [--] 'python/qmp.py: add casts to JSON deserialization' >>> 006/16:[] [--] 'python/qmp.py: add QMPProtocolError' >>> 007/16:[] [--] 'python/machine.py: Fix monitor address typing' >>> 008/16:[] [--] 'python/machine.py: reorder __init__' >>> 009/16:[] [--] 'python/machine.py: Don't modify state in _base_args()' >>> 010/16:[] [--] 'python/machine.py: Handle None events in events_wait' >>> 011/16:[] [--] 'python/machine.py: use qmp.command' >>> 012/16:[0010] [FC] 'python/machine.py: Add _qmp access shim' >>> 013/16:[] [-C] 'python/machine.py: fix _popen access' >>> 014/16:[] [--] 'python/qemu: make 'args' style arguments immutable' >>> 015/16:[] [--] 'iotests.py: Adjust HMP kwargs typing' >>> 016/16:[] [-C] 'python/qemu: Add mypy type annotations' >>> >>> - Rebased on "refactor shutdown" v4 >>> - Fixed _qmp access for scripts that disable QMP >> >> See: >> >> https://travis-ci.org/github/philmd/qemu/jobs/702507625#L5439 >> >> / # uname -a >> Linux buildroot 4.5.0-2-4kc-malta #1 Debian 4.5.5-1 (2016-05-29) mips >> GNU/Linux >> / # reboot >> / # reboot: Restarting system > {'execute': 'quit'} >> qemu received signal 9; command: "mips-softmmu/qemu-system-mips -display >> none -vga none -chardev >> socket,id=mon,path=/tmp/tmpcojsc5u3/qemu-14540-monitor.sock -mon >> chardev=mon,mode=control -machine malta -chardev >> socket,id=console,path=/tmp/tmpcojsc5u3/qemu-14540-console.sock,server,nowait >> -serial chardev:console -kernel >> /tmp/avocado_xgmck2k5/avocado_job_83wkzs2f/12-tests_acceptance_boot_linux_console.py_BootLinuxConsole.test_mips_malta_cpio/boot/vmlinux-4.5.0-2-4kc-malta >> -initrd >> /tmp/avocado_xgmck2k5/avocado_job_83wkzs2f/12-tests_acceptance_boot_linux_console.py_BootLinuxConsole.test_mips_malta_cpiorootfs.cpio >> -append printk.time=0 console=ttyS0 console=tty rdinit=/sbin/init >> noreboot -no-reboot" >> >> Reproduced traceback from: >> /home/travis/build/philmd/qemu/build/tests/venv/lib/python3.6/site-packages/avocado/core/test.py:886 >> Traceback (most recent call last): >> File >> "/home/travis/build/philmd/qemu/build/tests/acceptance/avocado_qemu/__init__.py", >> line 195, in tearDown >> vm.shutdown() >> File "/home/travis/build/philmd/qemu/python/qemu/machine.py", line >> 457, in shutdown >> self._do_shutdown(has_quit) >> File "/home/travis/build/philmd/qemu/python/qemu/machine.py", line >> 434, in _do_shutdown >> self._soft_shutdown(has_quit, timeout) >> File "/home/travis/build/philmd/qemu/python/qemu/machine.py", line >> 414, in _soft_shutdown >> self._qmp.cmd('quit') >> File "/home/travis/build/philmd/qemu/python/qemu/qmp.py", line 271, in cmd >> return self.cmd_obj(qmp_cmd) >> File "/home/travis/build/philmd/qemu/python/qemu/qmp.py", line 249, in >> cmd_obj >> self.__sock.sendall(json.dumps(qmp_cmd).encode('utf-8')) >> BrokenPipeError: [Errno 32] Broken pipe >> > > Might be racing between QMP going away and Python not being aware that > the process has closed yet. It's the only explanation I have left. > > I wish I could reproduce this, though. When I submit jobs to Travis I am > not seeing these failures. > > I'll see what I can do, but at this point I'll not re-send the patch > series until I can conclusively fix your build testing. > > --js > OK, this is a very big mea culpa. There are two problems. 1. I should catch ConnectionReset and not ConnectionResetError; one is a catch-all for socket problems and the other is a very specific errno that does not include BrokenPipeError. 2. Even if I do, it can still race, actually. QEMU might be in the middle of shutting down, but has already lost the control channel. Solving the second problem as the interface is currently designed is hard. You can wait, but then if the wait failed, you need to re-raise the control channel error instead of the wait failure. IOW, you need to wait in order to learn if the control channel error is "important" or not. So, the architecture of this is starting to look wrong; _soft_shutdown should keep clarity of purpose. Either it was able to to a nice, soft shutdown -- or it wasn't. I'm starting to think that the real problem is that machine.py shouldn't try to hide connection errors on shutdown at all if we expected to be able to issue a quit command. Changing my mind rapi
Re: [PATCH v4 00/16] python: add mypy support to python/qemu
On 6/29/20 4:26 AM, Philippe Mathieu-Daudé wrote: > +Cleber/Wainer. > > On 6/26/20 10:41 PM, John Snow wrote: >> Based-on: 20200626202350.11060-1-js...@redhat.com >> >> This series modifies the python/qemu library to comply with mypy --strict. >> This requires my "refactor shutdown" patch as a pre-requisite. >> >> v4: >> 001/16:[] [--] 'python/qmp.py: Define common types' >> 002/16:[] [--] 'iotests.py: use qemu.qmp type aliases' >> 003/16:[] [--] 'python/qmp.py: re-absorb MonitorResponseError' >> 004/16:[] [--] 'python/qmp.py: Do not return None from cmd_obj' >> 005/16:[] [--] 'python/qmp.py: add casts to JSON deserialization' >> 006/16:[] [--] 'python/qmp.py: add QMPProtocolError' >> 007/16:[] [--] 'python/machine.py: Fix monitor address typing' >> 008/16:[] [--] 'python/machine.py: reorder __init__' >> 009/16:[] [--] 'python/machine.py: Don't modify state in _base_args()' >> 010/16:[] [--] 'python/machine.py: Handle None events in events_wait' >> 011/16:[] [--] 'python/machine.py: use qmp.command' >> 012/16:[0010] [FC] 'python/machine.py: Add _qmp access shim' >> 013/16:[] [-C] 'python/machine.py: fix _popen access' >> 014/16:[] [--] 'python/qemu: make 'args' style arguments immutable' >> 015/16:[] [--] 'iotests.py: Adjust HMP kwargs typing' >> 016/16:[] [-C] 'python/qemu: Add mypy type annotations' >> >> - Rebased on "refactor shutdown" v4 >> - Fixed _qmp access for scripts that disable QMP > > See: > > https://travis-ci.org/github/philmd/qemu/jobs/702507625#L5439 > > / # uname -a > Linux buildroot 4.5.0-2-4kc-malta #1 Debian 4.5.5-1 (2016-05-29) mips > GNU/Linux > / # reboot > / # reboot: Restarting system {'execute': 'quit'} > qemu received signal 9; command: "mips-softmmu/qemu-system-mips -display > none -vga none -chardev > socket,id=mon,path=/tmp/tmpcojsc5u3/qemu-14540-monitor.sock -mon > chardev=mon,mode=control -machine malta -chardev > socket,id=console,path=/tmp/tmpcojsc5u3/qemu-14540-console.sock,server,nowait > -serial chardev:console -kernel > /tmp/avocado_xgmck2k5/avocado_job_83wkzs2f/12-tests_acceptance_boot_linux_console.py_BootLinuxConsole.test_mips_malta_cpio/boot/vmlinux-4.5.0-2-4kc-malta > -initrd > /tmp/avocado_xgmck2k5/avocado_job_83wkzs2f/12-tests_acceptance_boot_linux_console.py_BootLinuxConsole.test_mips_malta_cpiorootfs.cpio > -append printk.time=0 console=ttyS0 console=tty rdinit=/sbin/init > noreboot -no-reboot" > > Reproduced traceback from: > /home/travis/build/philmd/qemu/build/tests/venv/lib/python3.6/site-packages/avocado/core/test.py:886 > Traceback (most recent call last): > File > "/home/travis/build/philmd/qemu/build/tests/acceptance/avocado_qemu/__init__.py", > line 195, in tearDown > vm.shutdown() > File "/home/travis/build/philmd/qemu/python/qemu/machine.py", line > 457, in shutdown > self._do_shutdown(has_quit) > File "/home/travis/build/philmd/qemu/python/qemu/machine.py", line > 434, in _do_shutdown > self._soft_shutdown(has_quit, timeout) > File "/home/travis/build/philmd/qemu/python/qemu/machine.py", line > 414, in _soft_shutdown > self._qmp.cmd('quit') > File "/home/travis/build/philmd/qemu/python/qemu/qmp.py", line 271, in cmd > return self.cmd_obj(qmp_cmd) > File "/home/travis/build/philmd/qemu/python/qemu/qmp.py", line 249, in > cmd_obj > self.__sock.sendall(json.dumps(qmp_cmd).encode('utf-8')) > BrokenPipeError: [Errno 32] Broken pipe > Might be racing between QMP going away and Python not being aware that the process has closed yet. It's the only explanation I have left. I wish I could reproduce this, though. When I submit jobs to Travis I am not seeing these failures. I'll see what I can do, but at this point I'll not re-send the patch series until I can conclusively fix your build testing. --js
Re: [PATCH v4 00/16] python: add mypy support to python/qemu
+Cleber/Wainer. On 6/26/20 10:41 PM, John Snow wrote: > Based-on: 20200626202350.11060-1-js...@redhat.com > > This series modifies the python/qemu library to comply with mypy --strict. > This requires my "refactor shutdown" patch as a pre-requisite. > > v4: > 001/16:[] [--] 'python/qmp.py: Define common types' > 002/16:[] [--] 'iotests.py: use qemu.qmp type aliases' > 003/16:[] [--] 'python/qmp.py: re-absorb MonitorResponseError' > 004/16:[] [--] 'python/qmp.py: Do not return None from cmd_obj' > 005/16:[] [--] 'python/qmp.py: add casts to JSON deserialization' > 006/16:[] [--] 'python/qmp.py: add QMPProtocolError' > 007/16:[] [--] 'python/machine.py: Fix monitor address typing' > 008/16:[] [--] 'python/machine.py: reorder __init__' > 009/16:[] [--] 'python/machine.py: Don't modify state in _base_args()' > 010/16:[] [--] 'python/machine.py: Handle None events in events_wait' > 011/16:[] [--] 'python/machine.py: use qmp.command' > 012/16:[0010] [FC] 'python/machine.py: Add _qmp access shim' > 013/16:[] [-C] 'python/machine.py: fix _popen access' > 014/16:[] [--] 'python/qemu: make 'args' style arguments immutable' > 015/16:[] [--] 'iotests.py: Adjust HMP kwargs typing' > 016/16:[] [-C] 'python/qemu: Add mypy type annotations' > > - Rebased on "refactor shutdown" v4 > - Fixed _qmp access for scripts that disable QMP See: https://travis-ci.org/github/philmd/qemu/jobs/702507625#L5439 / # uname -a Linux buildroot 4.5.0-2-4kc-malta #1 Debian 4.5.5-1 (2016-05-29) mips GNU/Linux / # reboot / # reboot: Restarting system >>> {'execute': 'quit'} qemu received signal 9; command: "mips-softmmu/qemu-system-mips -display none -vga none -chardev socket,id=mon,path=/tmp/tmpcojsc5u3/qemu-14540-monitor.sock -mon chardev=mon,mode=control -machine malta -chardev socket,id=console,path=/tmp/tmpcojsc5u3/qemu-14540-console.sock,server,nowait -serial chardev:console -kernel /tmp/avocado_xgmck2k5/avocado_job_83wkzs2f/12-tests_acceptance_boot_linux_console.py_BootLinuxConsole.test_mips_malta_cpio/boot/vmlinux-4.5.0-2-4kc-malta -initrd /tmp/avocado_xgmck2k5/avocado_job_83wkzs2f/12-tests_acceptance_boot_linux_console.py_BootLinuxConsole.test_mips_malta_cpiorootfs.cpio -append printk.time=0 console=ttyS0 console=tty rdinit=/sbin/init noreboot -no-reboot" Reproduced traceback from: /home/travis/build/philmd/qemu/build/tests/venv/lib/python3.6/site-packages/avocado/core/test.py:886 Traceback (most recent call last): File "/home/travis/build/philmd/qemu/build/tests/acceptance/avocado_qemu/__init__.py", line 195, in tearDown vm.shutdown() File "/home/travis/build/philmd/qemu/python/qemu/machine.py", line 457, in shutdown self._do_shutdown(has_quit) File "/home/travis/build/philmd/qemu/python/qemu/machine.py", line 434, in _do_shutdown self._soft_shutdown(has_quit, timeout) File "/home/travis/build/philmd/qemu/python/qemu/machine.py", line 414, in _soft_shutdown self._qmp.cmd('quit') File "/home/travis/build/philmd/qemu/python/qemu/qmp.py", line 271, in cmd return self.cmd_obj(qmp_cmd) File "/home/travis/build/philmd/qemu/python/qemu/qmp.py", line 249, in cmd_obj self.__sock.sendall(json.dumps(qmp_cmd).encode('utf-8')) BrokenPipeError: [Errno 32] Broken pipe
[PATCH v4 00/16] python: add mypy support to python/qemu
Based-on: 20200626202350.11060-1-js...@redhat.com This series modifies the python/qemu library to comply with mypy --strict. This requires my "refactor shutdown" patch as a pre-requisite. v4: 001/16:[] [--] 'python/qmp.py: Define common types' 002/16:[] [--] 'iotests.py: use qemu.qmp type aliases' 003/16:[] [--] 'python/qmp.py: re-absorb MonitorResponseError' 004/16:[] [--] 'python/qmp.py: Do not return None from cmd_obj' 005/16:[] [--] 'python/qmp.py: add casts to JSON deserialization' 006/16:[] [--] 'python/qmp.py: add QMPProtocolError' 007/16:[] [--] 'python/machine.py: Fix monitor address typing' 008/16:[] [--] 'python/machine.py: reorder __init__' 009/16:[] [--] 'python/machine.py: Don't modify state in _base_args()' 010/16:[] [--] 'python/machine.py: Handle None events in events_wait' 011/16:[] [--] 'python/machine.py: use qmp.command' 012/16:[0010] [FC] 'python/machine.py: Add _qmp access shim' 013/16:[] [-C] 'python/machine.py: fix _popen access' 014/16:[] [--] 'python/qemu: make 'args' style arguments immutable' 015/16:[] [--] 'iotests.py: Adjust HMP kwargs typing' 016/16:[] [-C] 'python/qemu: Add mypy type annotations' - Rebased on "refactor shutdown" v4 - Fixed _qmp access for scripts that disable QMP v3: 005: Removed a cast, per Kevin Wolf's tip 010: Renamed with correct function name; Rewrote docstring and added comments 016: Use SocketAddrT instead of Union[Tuple[str,str],str] "v2": - This version supports iotests 297 - Many patches merged by Phil are removed - Replaces iotests.py type aliases with centralized ones (See patch 2) - Imports etc are reworked to use the non-installable package layout instead. (Mostly important for patch 3) Testing this out: - You'll need Python3.6+ - I encourage you to use a virtual environment! - You don't necessarily need these exact versions, but I didn't test the lower bounds, use older versions at your peril: - pylint==2.5.0 - mypy=0.770 - flake8=3.7.8 > cd ~/src/qemu/python/ > flake8 qemu > mypy --strict qemu > cd qemu > pylint *.py These should all 100% pass. --- Open RFC: What's the right way to hook this into make check to prevent regressions? John Snow (16): python/qmp.py: Define common types iotests.py: use qemu.qmp type aliases python/qmp.py: re-absorb MonitorResponseError python/qmp.py: Do not return None from cmd_obj python/qmp.py: add casts to JSON deserialization python/qmp.py: add QMPProtocolError python/machine.py: Fix monitor address typing python/machine.py: reorder __init__ python/machine.py: Don't modify state in _base_args() python/machine.py: Handle None events in events_wait python/machine.py: use qmp.command python/machine.py: Add _qmp access shim python/machine.py: fix _popen access python/qemu: make 'args' style arguments immutable iotests.py: Adjust HMP kwargs typing python/qemu: Add mypy type annotations python/qemu/accel.py | 8 +- python/qemu/machine.py| 294 -- python/qemu/qmp.py| 111 + python/qemu/qtest.py | 53 +++--- scripts/render_block_graph.py | 7 +- tests/qemu-iotests/iotests.py | 11 +- 6 files changed, 300 insertions(+), 184 deletions(-) -- 2.21.3