On Thu, Jul 22, 2021 at 1:25 AM John Snow <js...@redhat.com> wrote: > Looping qemu-devel back in: I removed them by accident by not hitting > reply-all :( > > On Wed, Jul 21, 2021 at 2:06 PM Niteesh G. S. <niteesh...@gmail.com> > wrote: > >> >> >> On Wed, Jul 21, 2021 at 11:03 PM John Snow <js...@redhat.com> wrote: >> >>> >>> >>> On Wed, Jul 21, 2021 at 1:04 PM Niteesh G. S. <niteesh...@gmail.com> >>> wrote: >>> >>>> Hello all, >>>> >>>> I recently rebased(incrementally) my TUI on this V2 patch and faced an >>>> issue. >>>> https://gitlab.com/niteesh.gs/qemu/-/commits/aqmp-tui-prototype-v3 >>>> I decided to rebase incrementally so that I can address some of the >>>> comments posted >>>> in my patch series. While testing out, the initial draft of TUI >>>> which worked fine in the V1 >>>> version of AQMP failed in this version. >>>> >>>> Disconnecting from a fully connected state doesn't exit cleanly. >>>> >>>> --------------------------------------------------------------------------------- >>>> To reproduce the issue: >>>> 1) Initiate a QMP server >>>> >>> >>> Please provide the command line. >>> >> qemu-system-x86_64 -qmp tcp:localhost:1234,server,wait=on >> >>> >>> >>>> 2) Connect the TUI to the server using aqmp-tui localhost:1234 >>>> --log-file log.txt >>>> >>> >>> The entry point isn't defined yet in your series, so I will assume >>> "python3 -m qemu.aqmp.aqmp_tui localhost:1234" should work here. >>> >> Yup, sorry about that. I realized this later when recreated the venv. >> >>> >>> >>>> 3) Once the TUI is connected and running, press 'Esc' to exit the app. >>>> This should result >>>> in the following exception. >>>> >>>> -------------------------------------------------------------------------------------------------------------------------------------------- >>>> Transitioning from 'Runstate.IDLE' to 'Runstate.CONNECTING'. >>>> Connecting to ('localhost', 1234) ... >>>> Connected. >>>> Awaiting greeting ... >>>> Response: { >>>> "QMP": { >>>> .......... Skipping >>>> } >>>> } >>>> Negotiating capabilities ... >>>> Request: { >>>> "execute": "qmp_capabilities", >>>> .......... Skipping >>>> } >>>> } >>>> Response: { >>>> "return": {} >>>> } >>>> Transitioning from 'Runstate.CONNECTING' to 'Runstate.RUNNING'. >>>> Transitioning from 'Runstate.RUNNING' to 'Runstate.DISCONNECTING'. >>>> Scheduling disconnect. >>>> Draining the outbound queue ... >>>> Flushing the StreamWriter ... >>>> Cancelling writer task ... >>>> Task.Writer: cancelled. >>>> Task.Writer: exiting. >>>> Cancelling reader task ... >>>> Task.Reader: cancelled. >>>> Task.Reader: exiting. >>>> Closing StreamWriter. >>>> Waiting for StreamWriter to close ... >>>> QMP Disconnected. >>>> Transitioning from 'Runstate.DISCONNECTING' to 'Runstate.IDLE'. >>>> _kill_app: Connection lost >>>> Connection lost >>>> | Traceback (most recent call last): >>>> | File >>>> "/home/niteesh/development/qemu/python/qemu/aqmp/aqmp_tui.py", line 246, in >>>> run >>>> | main_loop.run() >>>> | File >>>> "/home/niteesh/development/qemu/python/.venv/lib/python3.6/site-packages/urwid/main_loop.py", >>>> line 287, in run >>>> | self._run() >>>> | File >>>> "/home/niteesh/development/qemu/python/.venv/lib/python3.6/site-packages/urwid/main_loop.py", >>>> line 385, in _run >>>> | self.event_loop.run() >>>> | File >>>> "/home/niteesh/development/qemu/python/.venv/lib/python3.6/site-packages/urwid/main_loop.py", >>>> line 1494, in run >>>> | reraise(*exc_info) >>>> | File >>>> "/home/niteesh/development/qemu/python/.venv/lib/python3.6/site-packages/urwid/compat.py", >>>> line 58, in reraise >>>> | raise value >>>> | File >>>> "/home/niteesh/development/qemu/python/qemu/aqmp/aqmp_tui.py", line 206, in >>>> _kill_app >>>> | raise err >>>> | File >>>> "/home/niteesh/development/qemu/python/qemu/aqmp/aqmp_tui.py", line 201, in >>>> _kill_app >>>> | await self.disconnect() >>>> | File >>>> "/home/niteesh/development/qemu/python/qemu/aqmp/protocol.py", line 303, in >>>> disconnect >>>> | await self._wait_disconnect() >>>> | File >>>> "/home/niteesh/development/qemu/python/qemu/aqmp/protocol.py", line 573, in >>>> _wait_disconnect >>>> | await self._dc_task >>>> | File >>>> "/home/niteesh/development/qemu/python/qemu/aqmp/qmp_client.py", line 316, >>>> in _bh_disconnect >>>> | await super()._bh_disconnect() >>>> | File >>>> "/home/niteesh/development/qemu/python/qemu/aqmp/protocol.py", line 644, in >>>> _bh_disconnect >>>> | await wait_closed(self._writer) >>>> | File "/home/niteesh/development/qemu/python/qemu/aqmp/util.py", >>>> line 137, in wait_closed >>>> | await flush(writer) >>>> | File "/home/niteesh/development/qemu/python/qemu/aqmp/util.py", >>>> line 49, in flush >>>> | await writer.drain() >>>> | File "/usr/lib/python3.6/asyncio/streams.py", line 339, in drain >>>> | yield from self._protocol._drain_helper() >>>> | File "/usr/lib/python3.6/asyncio/streams.py", line 210, in >>>> _drain_helper >>>> | raise ConnectionResetError('Connection lost') >>>> | ConnectionResetError: Connection lost >>>> >>>> -------------------------------------------------------------------------------------------------------------------------------------------- >>>> >>>> >>> I can't reproduce in Python 3.9, but I *can* reproduce in python 3.6 >>> using the pipenv environment; i.e. >>> >>> > make check-pipenv >>> > pipenv shell >>> > python3 -m qemu.aqmp.aqmp_tui 127.0.0.1:1234 >>> >>> What python version are you using to see this failure? Is it 3.6 ? >>> >> Yes, I was using python 3.6. I just tried it on 3.8 and I don't face this >> issue. >> >>> >>> It seems like the wait_closed() wrapper I wrote isn't quite compatible >>> with Python 3.6, it looks like it's not really safe to try and flush a >>> closing socket. I was doing so in an attempt to tell when the socket had >>> finished closing out its buffer (expecting it to normally be a no-op) but >>> in this case even a no-op drain in 3.6 seems to raise an error if we >>> attempt it after we've asked for the socket to close. >>> >> >> >>> wait_closed() was added in Python 3.7 and we just don't have access to >>> it here ... I'm not sure if there's something else we can do here to serve >>> as a workaround for not having this function. >>> >>> --js >>> >>> > I can't find a *nice* workaround, but I found one that should probably > work in most universes. We can remove this ugly code when we support 3.7 as > a minimum. However, please try this patch as a fixup: > > diff --git a/python/qemu/aqmp/util.py b/python/qemu/aqmp/util.py > index de0df44cbd7..eaa5fc7d5f9 100644 > --- a/python/qemu/aqmp/util.py > +++ b/python/qemu/aqmp/util.py > @@ -134,7 +134,17 @@ async def wait_closed(writer: asyncio.StreamWriter) > -> None: > > while not transport.is_closing(): > await asyncio.sleep(0) > - await flush(writer) > + > + # This is an ugly workaround, but it's the best I can come up with. > + sock = transport.get_extra_info('socket') > + > + if sock is None: > + # Our transport doesn't have a socket? ... > + # Nothing we can reasonably do. > + return > + > + while sock.fileno() != -1: > + await asyncio.sleep(0) > Thanks for the patch. I am now able to disconnect/quit without any exceptions.
Thanks, Niteesh. > > >
