From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.3 required=3.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE,INCLUDES_PATCH,MAILING_LIST_MULTI, MENTIONS_GIT_HOSTING,NUMERIC_HTTP_ADDR,SPF_HELO_NONE,SPF_PASS,WEIRD_PORT autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 87EE2C6377B for ; Wed, 21 Jul 2021 20:18:13 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 2439861222 for ; Wed, 21 Jul 2021 20:18:13 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2439861222 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:40680 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1m6IfU-0001g2-4d for qemu-devel@archiver.kernel.org; Wed, 21 Jul 2021 16:18:12 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:37306) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1m6IQv-0000GK-JW for qemu-devel@nongnu.org; Wed, 21 Jul 2021 16:03:09 -0400 Received: from mail-io1-xd36.google.com ([2607:f8b0:4864:20::d36]:34321) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1m6IQs-0007ZV-Vo for qemu-devel@nongnu.org; Wed, 21 Jul 2021 16:03:09 -0400 Received: by mail-io1-xd36.google.com with SMTP id g22so3769385iom.1 for ; Wed, 21 Jul 2021 13:03:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=BrWdDIUarzMp9Hd6Ja31J2qAUr16jMaBIlsrMGDODDk=; b=dZ5brRjQod2xdt56GkR7koGK6/WgoGRh8z0VyrjyvR5gEhY24sFbxLrHY3T3tLjiA9 l7qTyT/GqPQ/nZQKm47XGh91tsD9T82IrwHMcKdv9LcdoeIs/LhwPVNiqSYPnkcqyqP0 aON0J8e+XKEx/YNQH/lhXKSM6wDgHMKgbghk9ghN6p9rzHa/qyn/Pf4VYRSCjd0TFwbp YeG61robjyc68B0QnLf59raYF7cH0Z4F/BxEGZtZ4lDQYO6FqrCPr5Fdjm/O4bYbgTYt FTaqVIDbNRPBXHMAjcPQZsXgXG69Hw1BSkUbsjN7RFXT/s07Mt1rx43mrImQQXAAhNFi Yu+Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=BrWdDIUarzMp9Hd6Ja31J2qAUr16jMaBIlsrMGDODDk=; b=RLXW1rrK6KnggpPcSv4MeEwErfeKjt1jqdLo9IA8T45C90tQpqv2bZ7JHegDr9unvN 1LwTyNiOAbW+hBZPw0/UJp1g7+oSw0Uy6aqzr3Ms88l+N/IIQZFtEWiytZ6FegJMse6m InGKnfMzvwidHEUnLpU8DCMnvU7FacZDeZj0pEYUmcwHrrEsKUcYcSZ5cGF80/LH/C5X +u4AlesSzrVXZNcTYGJGqrpina6V88NIdvAJtKKHj0iT2sqzufqDOD3JcbiqgcfD5Ctg WwJziyAQC4ztEGy52p4ePHSgrIxYnIkWHqwawqdxUrIIwzSB3PjP42BZCVEq3E7hA+i6 T1vQ== X-Gm-Message-State: AOAM531XJ0LFt7398RlGTr93tqbrP02zqN8HJYxh/blKEQ6vbczJFmP2 ScsRFTfMDzy1YbgGRtDkESB0Zm75+3zEJc9Y1Yg= X-Google-Smtp-Source: ABdhPJzcrlgjvnW5ketoaeZC3EH7+aKPgebfRV6XynjbLJiLdXwnlp4YXwJ1uHCPNnijAGg0q3xi0JXlV9U5FZHVT2o= X-Received: by 2002:a5d:818d:: with SMTP id u13mr8242175ion.92.1626897785762; Wed, 21 Jul 2021 13:03:05 -0700 (PDT) MIME-Version: 1.0 References: <20210717003253.457418-1-jsnow@redhat.com> In-Reply-To: From: "Niteesh G. S." Date: Thu, 22 Jul 2021 01:32:39 +0530 Message-ID: Subject: Re: [PATCH v2 00/24] python: introduce Asynchronous QMP package To: John Snow Content-Type: multipart/alternative; boundary="0000000000009b393405c7a7a808" Received-SPF: pass client-ip=2607:f8b0:4864:20::d36; envelope-from=niteesh.gs@gmail.com; helo=mail-io1-xd36.google.com X-Spam_score_int: -8 X-Spam_score: -0.9 X-Spam_bar: / X-Spam_report: (-0.9 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, NUMERIC_HTTP_ADDR=1.242, RCVD_IN_DNSWL_NONE=-0.0001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, WEIRD_PORT=0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Eduardo Habkost , Stefan Hajnoczi , qemu-devel , Wainer dos Santos Moschetta , Markus Armbruster , Willian Rampazzo , Cleber Rosa , Eric Blake Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" --0000000000009b393405c7a7a808 Content-Type: text/plain; charset="UTF-8" On Thu, Jul 22, 2021 at 1:25 AM John Snow 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. > wrote: > >> >> >> On Wed, Jul 21, 2021 at 11:03 PM John Snow wrote: >> >>> >>> >>> On Wed, Jul 21, 2021 at 1:04 PM Niteesh G. S. >>> 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. > > > --0000000000009b393405c7a7a808 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


On Thu, Jul 22, 2021 at 1:25 AM John Snow <jsnow@redhat.com> wrote:
Loopin= g 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, 2= 021 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-protot= ype-v3
I decided to rebase incrementally so that I can address some = of the comments posted
in my patch seri= es. While testing out, the initial=C2=A0draft of TUI which=C2=A0worked fine= in the V1
version of AQMP failed in th= is version.

Disconnecting from a fully connected state doesn't exit c= leanly.
-------------------------------= --------------------------------------------------
To reproduce the issue:
1) = Initiate a QMP server

Please pr= ovide the command line.
qemu-system-x86_64 -qmp= tcp:localhost:1234,server,wait=3Don
=C2=A0
2) Connect the TUI to the server using= aqmp-tui=C2=A0localhost: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" shou= ld work here.
Yup, sorry about that. I realized this la= ter when recreated the venv.=C2=A0
=C2=A0
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 ('localhos= t', 1234) ...
Connected.
Awaiting greeting ...
Response: {
= =C2=A0 "QMP": {
=C2=A0 =C2=A0 .......... Skipping
=C2=A0 }<= br>}
Negotiating capabilities ...
Request: {
=C2=A0 "execute&= quot;: "qmp_capabilities",
=C2=A0 =C2=A0 .......... Skipping=C2=A0 }
}
Response: {
=C2=A0 "return": {}
}
Tra= nsitioning from 'Runstate.CONNECTING' to 'Runstate.RUNNING'= .
Transitioning from 'Runstate.RUNNING' to 'Runstate.DISCONN= ECTING'.
Scheduling disconnect.
Draining the outbound queue ...Flushing the StreamWriter ...
Cancelling writer task ...
Task.Write= r: cancelled.
Task.Writer: exiting.
Cancelling reader task ...
Tas= k.Reader: cancelled.
Task.Reader: exiting.
Closing StreamWriter.
W= aiting for StreamWriter to close ...
QMP Disconnected.
Transitioning = from 'Runstate.DISCONNECTING' to 'Runstate.IDLE'.
_kill_= app: Connection lost
Connection lost
=C2=A0 | Traceback (most recent = call last):
=C2=A0 | =C2=A0 File "/home/niteesh/development/qemu/py= thon/qemu/aqmp/aqmp_tui.py", line 246, in run
=C2=A0 | =C2=A0 =C2= =A0 main_loop.run()
=C2=A0 | =C2=A0 File "/home/niteesh/development= /qemu/python/.venv/lib/python3.6/site-packages/urwid/main_loop.py", li= ne 287, in run
=C2=A0 | =C2=A0 =C2=A0 self._run()
=C2=A0 | =C2=A0 Fil= e "/home/niteesh/development/qemu/python/.venv/lib/python3.6/site-pack= ages/urwid/main_loop.py", line 385, in _run
=C2=A0 | =C2=A0 =C2=A0 = self.event_loop.run()
=C2=A0 | =C2=A0 File "/home/niteesh/developme= nt/qemu/python/.venv/lib/python3.6/site-packages/urwid/main_loop.py", = line 1494, in run
=C2=A0 | =C2=A0 =C2=A0 reraise(*exc_info)
=C2=A0 | = =C2=A0 File "/home/niteesh/development/qemu/python/.venv/lib/python3.6= /site-packages/urwid/compat.py", line 58, in reraise
=C2=A0 | =C2= =A0 =C2=A0 raise value
=C2=A0 | =C2=A0 File "/home/niteesh/developm= ent/qemu/python/qemu/aqmp/aqmp_tui.py", line 206, in _kill_app
=C2= =A0 | =C2=A0 =C2=A0 raise err
=C2=A0 | =C2=A0 File "/home/niteesh/d= evelopment/qemu/python/qemu/aqmp/aqmp_tui.py", line 201, in _kill_app<= br>=C2=A0 | =C2=A0 =C2=A0 await self.disconnect()
=C2=A0 | =C2=A0 File &= quot;/home/niteesh/development/qemu/python/qemu/aqmp/protocol.py", lin= e 303, in disconnect
=C2=A0 | =C2=A0 =C2=A0 await self._wait_disconnect(= )
=C2=A0 | =C2=A0 File "/home/niteesh/development/qemu/python/qemu/= aqmp/protocol.py", line 573, in _wait_disconnect
=C2=A0 | =C2=A0 = =C2=A0 await self._dc_task
=C2=A0 | =C2=A0 File "/home/niteesh/deve= lopment/qemu/python/qemu/aqmp/qmp_client.py", line 316, in _bh_disconn= ect
=C2=A0 | =C2=A0 =C2=A0 await super()._bh_disconnect()
=C2=A0 | = =C2=A0 File "/home/niteesh/development/qemu/python/qemu/aqmp/protocol.= py", line 644, in _bh_disconnect
=C2=A0 | =C2=A0 =C2=A0 await wait_= closed(self._writer)
=C2=A0 | =C2=A0 File "/home/niteesh/developmen= t/qemu/python/qemu/aqmp/util.py", line 137, in wait_closed
=C2=A0 |= =C2=A0 =C2=A0 await flush(writer)
=C2=A0 | =C2=A0 File "/home/nite= esh/development/qemu/python/qemu/aqmp/util.py", line 49, in flush
= =C2=A0 | =C2=A0 =C2=A0 await writer.drain()
=C2=A0 | =C2=A0 File "/= usr/lib/python3.6/asyncio/streams.py", line 339, in drain
=C2=A0 | = =C2=A0 =C2=A0 yield from self._protocol._drain_helper()
=C2=A0 | =C2=A0 = File "/usr/lib/python3.6/asyncio/streams.py", line 210, in _drain= _helper
=C2=A0 | =C2=A0 =C2=A0 raise ConnectionResetError('Connectio= n lost')
=C2=A0 | 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 chec= k-pipenv
> pipenv shell
> python3 -m qemu.aqmp.aq= mp_tui 127.0.0.1:1234

=

I can't find a *nice* workaround, but I found one t= hat should probably work in most universes. We can remove this ugly code wh= en we support 3.7 as a minimum. However, please try this patch as a fixup:<= /div>

diff --git a/python/qemu/aqmp/util.py b/python/qem= u/aqmp/util.py
index de0df44cbd7..eaa5fc7d5f9 100644
--- a/python/qem= u/aqmp/util.py
+++ b/python/qemu/aqmp/util.py
@@ -134,7 +134,17 @@ as= ync def wait_closed(writer: asyncio.StreamWriter) -> None:
=C2=A0
= =C2=A0 =C2=A0 =C2=A0while not transport.is_closing():
=C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0await asyncio.sleep(0)
- =C2=A0 =C2=A0await flush(write= r)
+
+ =C2=A0 =C2=A0# This is an ugly workaround, but it's the be= st I can come up with.
+ =C2=A0 =C2=A0sock =3D transport.get_extra_info(= 'socket')
+
+ =C2=A0 =C2=A0if sock is None:
+ =C2=A0 =C2= =A0 =C2=A0 =C2=A0# Our transport doesn't have a socket? ...
+ =C2=A0= =C2=A0 =C2=A0 =C2=A0# Nothing we can reasonably do.
+ =C2=A0 =C2=A0 =C2= =A0 =C2=A0return
+
+ =C2=A0 =C2=A0while sock.fileno() !=3D -1:
+ = =C2=A0 =C2=A0 =C2=A0 =C2=A0await asyncio.sleep(0)
Thanks fo= r the patch. I am now able to disconnect/quit without any exceptions.

Than= ks,
Niteesh.=C2=A0

=C2=A0<= /div>
--0000000000009b393405c7a7a808--