Re: [PATCH v2 00/24] python: introduce Asynchronous QMP package

["Niteesh G. S." <niteesh.gs@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 7434 bytes --]

On Thu, Jul 22, 2021 at 1:25 AM John Snow <jsnow@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.gs@gmail.com>
> wrote:
>
>>
>>
>> On Wed, Jul 21, 2021 at 11:03 PM John Snow <jsnow@redhat.com> wrote:
>>
>>>
>>>
>>> On Wed, Jul 21, 2021 at 1:04 PM Niteesh G. S. <niteesh.gs@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.

>
>
>

[-- Attachment #2: Type: text/html, Size: 11530 bytes --]