Re: [PATCH v2 15/17] python/aqmp: Create sync QMP wrapper for iotests

2021-10-06 Thread John Snow
On Wed, Oct 6, 2021 at 10:32 AM Paolo Bonzini  wrote:

> On 06/10/21 16:24, John Snow wrote:
> >
> > I had plans at one point to make a sync.py, but with an interface that
> > matched async QMP itself more closely. I spent some time trying to
> > research how to make a "magic" sync wrapper around async QMP, and hit a
> > few trouble spots. I've still got the patch, but I felt some pressure to
> > try and switch iotests over as fast as possible to get more
> > trial-by-fire time this release cycle. I named them "sync.py" and
> > "legacy.py" in my branch accordingly. Of course, I made a beeline
> > straight for the iotests version, so now it looks odd. I may yet try to
> > clean up the other version, possibly converting legacy.py to work in
> > terms of sync.py, and then converting users in iotests so that I can
> > drop legacy.py.
>
> Got it.  So maybe aqmp.qmp_next or aqmp.qmp_experimental?  What I mean
> is, it's all but legacy. :)
>
>
Mmm, yeah I guess I meant "legacy interface" and not "legacy
implementation" ;)

I could do 'compat.py' for the iotests-compatible interface and 'sync.py'
for the "modern" one?

--js


Re: [PATCH v2 15/17] python/aqmp: Create sync QMP wrapper for iotests

2021-10-06 Thread Paolo Bonzini

On 06/10/21 16:24, John Snow wrote:


I had plans at one point to make a sync.py, but with an interface that 
matched async QMP itself more closely. I spent some time trying to 
research how to make a "magic" sync wrapper around async QMP, and hit a 
few trouble spots. I've still got the patch, but I felt some pressure to 
try and switch iotests over as fast as possible to get more 
trial-by-fire time this release cycle. I named them "sync.py" and 
"legacy.py" in my branch accordingly. Of course, I made a beeline 
straight for the iotests version, so now it looks odd. I may yet try to 
clean up the other version, possibly converting legacy.py to work in 
terms of sync.py, and then converting users in iotests so that I can 
drop legacy.py.


Got it.  So maybe aqmp.qmp_next or aqmp.qmp_experimental?  What I mean 
is, it's all but legacy. :)


Paolo

(Mabe. I am not heavily committed to any one particular approach 
here, other than being very motivated to get AQMP tested widely a good 
bit before rc0 to have a chance to smooth over any lingering problems 
that might exist.)





Re: [PATCH v2 15/17] python/aqmp: Create sync QMP wrapper for iotests

2021-10-06 Thread John Snow
On Wed, Oct 6, 2021 at 6:13 AM Paolo Bonzini  wrote:

> On 23/09/21 02:49, John Snow wrote:
> > This is a wrapper around the async QMPClient that mimics the old,
> > synchronous QEMUMonitorProtocol class. It is designed to be
> > interchangeable with the old implementation.
> >
> > It does not, however, attempt to mimic Exception compatibility.
> >
> > Signed-off-by: John Snow 
> > Acked-by: Hanna Reitz 
>
> I don't like the name (of course).  qemu-iotests shows that there is a
> use for sync wrappers so, with similar reasoning to patch 16, there's no
> need to scare people away.  Why not just qemu.aqmp.sync?
>
> Paolo
>
>
I had plans at one point to make a sync.py, but with an interface that
matched async QMP itself more closely. I spent some time trying to research
how to make a "magic" sync wrapper around async QMP, and hit a few trouble
spots. I've still got the patch, but I felt some pressure to try and switch
iotests over as fast as possible to get more trial-by-fire time this
release cycle. I named them "sync.py" and "legacy.py" in my branch
accordingly. Of course, I made a beeline straight for the iotests version,
so now it looks odd. I may yet try to clean up the other version, possibly
converting legacy.py to work in terms of sync.py, and then converting users
in iotests so that I can drop legacy.py.

(Mabe. I am not heavily committed to any one particular approach here,
other than being very motivated to get AQMP tested widely a good bit before
rc0 to have a chance to smooth over any lingering problems that might
exist.)

Thanks for taking a look! I am more than happy to stage 1-9 myself, but I
will wait for Hanna's approval on 10-14 in particular.

--js


Re: [PATCH v2 15/17] python/aqmp: Create sync QMP wrapper for iotests

2021-10-06 Thread Paolo Bonzini

On 23/09/21 02:49, John Snow wrote:

This is a wrapper around the async QMPClient that mimics the old,
synchronous QEMUMonitorProtocol class. It is designed to be
interchangeable with the old implementation.

It does not, however, attempt to mimic Exception compatibility.

Signed-off-by: John Snow 
Acked-by: Hanna Reitz 


I don't like the name (of course).  qemu-iotests shows that there is a 
use for sync wrappers so, with similar reasoning to patch 16, there's no 
need to scare people away.  Why not just qemu.aqmp.sync?


Paolo


---
  python/qemu/aqmp/legacy.py | 135 +
  1 file changed, 135 insertions(+)
  create mode 100644 python/qemu/aqmp/legacy.py

diff --git a/python/qemu/aqmp/legacy.py b/python/qemu/aqmp/legacy.py
new file mode 100644
index 000..a75dbc599c0
--- /dev/null
+++ b/python/qemu/aqmp/legacy.py
@@ -0,0 +1,135 @@
+"""
+Sync QMP Wrapper
+
+This class pretends to be qemu.qmp.QEMUMonitorProtocol.
+"""
+
+import asyncio
+from typing import (
+Awaitable,
+List,
+Optional,
+TypeVar,
+Union,
+)
+
+import qemu.qmp
+from qemu.qmp import QMPMessage, QMPReturnValue, SocketAddrT
+
+from .qmp_client import QMPClient
+
+
+# pylint: disable=missing-docstring
+
+
+class QEMUMonitorProtocol(qemu.qmp.QEMUMonitorProtocol):
+def __init__(self, address: SocketAddrT,
+ server: bool = False,
+ nickname: Optional[str] = None):
+
+# pylint: disable=super-init-not-called
+self._aqmp = QMPClient(nickname)
+self._aloop = asyncio.get_event_loop()
+self._address = address
+self._timeout: Optional[float] = None
+
+_T = TypeVar('_T')
+
+def _sync(
+self, future: Awaitable[_T], timeout: Optional[float] = None
+) -> _T:
+return self._aloop.run_until_complete(
+asyncio.wait_for(future, timeout=timeout)
+)
+
+def _get_greeting(self) -> Optional[QMPMessage]:
+if self._aqmp.greeting is not None:
+# pylint: disable=protected-access
+return self._aqmp.greeting._asdict()
+return None
+
+# __enter__ and __exit__ need no changes
+# parse_address needs no changes
+
+def connect(self, negotiate: bool = True) -> Optional[QMPMessage]:
+self._aqmp.await_greeting = negotiate
+self._aqmp.negotiate = negotiate
+
+self._sync(
+self._aqmp.connect(self._address)
+)
+return self._get_greeting()
+
+def accept(self, timeout: Optional[float] = 15.0) -> QMPMessage:
+self._aqmp.await_greeting = True
+self._aqmp.negotiate = True
+
+self._sync(
+self._aqmp.accept(self._address),
+timeout
+)
+
+ret = self._get_greeting()
+assert ret is not None
+return ret
+
+def cmd_obj(self, qmp_cmd: QMPMessage) -> QMPMessage:
+return dict(
+self._sync(
+# pylint: disable=protected-access
+
+# _raw() isn't a public API, because turning off
+# automatic ID assignment is discouraged. For
+# compatibility with iotests *only*, do it anyway.
+self._aqmp._raw(qmp_cmd, assign_id=False),
+self._timeout
+)
+)
+
+# Default impl of cmd() delegates to cmd_obj
+
+def command(self, cmd: str, **kwds: object) -> QMPReturnValue:
+return self._sync(
+self._aqmp.execute(cmd, kwds),
+self._timeout
+)
+
+def pull_event(self,
+   wait: Union[bool, float] = False) -> Optional[QMPMessage]:
+if wait is False:
+# Return None if there's no event ready to go
+if self._aqmp.events.empty():
+return None
+
+timeout = None
+if isinstance(wait, float):
+timeout = wait
+
+return dict(
+self._sync(
+self._aqmp.events.get(),
+timeout
+)
+)
+
+def get_events(self, wait: Union[bool, float] = False) -> List[QMPMessage]:
+events = [dict(x) for x in self._aqmp.events.clear()]
+if events:
+return events
+
+event = self.pull_event(wait)
+return [event] if event is not None else []
+
+def clear_events(self) -> None:
+self._aqmp.events.clear()
+
+def close(self) -> None:
+self._sync(
+self._aqmp.disconnect()
+)
+
+def settimeout(self, timeout: Optional[float]) -> None:
+self._timeout = timeout
+
+def send_fd_scm(self, fd: int) -> None:
+self._aqmp.send_fd_scm(fd)






[PATCH v2 15/17] python/aqmp: Create sync QMP wrapper for iotests

2021-09-22 Thread John Snow
This is a wrapper around the async QMPClient that mimics the old,
synchronous QEMUMonitorProtocol class. It is designed to be
interchangeable with the old implementation.

It does not, however, attempt to mimic Exception compatibility.

Signed-off-by: John Snow 
Acked-by: Hanna Reitz 
---
 python/qemu/aqmp/legacy.py | 135 +
 1 file changed, 135 insertions(+)
 create mode 100644 python/qemu/aqmp/legacy.py

diff --git a/python/qemu/aqmp/legacy.py b/python/qemu/aqmp/legacy.py
new file mode 100644
index 000..a75dbc599c0
--- /dev/null
+++ b/python/qemu/aqmp/legacy.py
@@ -0,0 +1,135 @@
+"""
+Sync QMP Wrapper
+
+This class pretends to be qemu.qmp.QEMUMonitorProtocol.
+"""
+
+import asyncio
+from typing import (
+Awaitable,
+List,
+Optional,
+TypeVar,
+Union,
+)
+
+import qemu.qmp
+from qemu.qmp import QMPMessage, QMPReturnValue, SocketAddrT
+
+from .qmp_client import QMPClient
+
+
+# pylint: disable=missing-docstring
+
+
+class QEMUMonitorProtocol(qemu.qmp.QEMUMonitorProtocol):
+def __init__(self, address: SocketAddrT,
+ server: bool = False,
+ nickname: Optional[str] = None):
+
+# pylint: disable=super-init-not-called
+self._aqmp = QMPClient(nickname)
+self._aloop = asyncio.get_event_loop()
+self._address = address
+self._timeout: Optional[float] = None
+
+_T = TypeVar('_T')
+
+def _sync(
+self, future: Awaitable[_T], timeout: Optional[float] = None
+) -> _T:
+return self._aloop.run_until_complete(
+asyncio.wait_for(future, timeout=timeout)
+)
+
+def _get_greeting(self) -> Optional[QMPMessage]:
+if self._aqmp.greeting is not None:
+# pylint: disable=protected-access
+return self._aqmp.greeting._asdict()
+return None
+
+# __enter__ and __exit__ need no changes
+# parse_address needs no changes
+
+def connect(self, negotiate: bool = True) -> Optional[QMPMessage]:
+self._aqmp.await_greeting = negotiate
+self._aqmp.negotiate = negotiate
+
+self._sync(
+self._aqmp.connect(self._address)
+)
+return self._get_greeting()
+
+def accept(self, timeout: Optional[float] = 15.0) -> QMPMessage:
+self._aqmp.await_greeting = True
+self._aqmp.negotiate = True
+
+self._sync(
+self._aqmp.accept(self._address),
+timeout
+)
+
+ret = self._get_greeting()
+assert ret is not None
+return ret
+
+def cmd_obj(self, qmp_cmd: QMPMessage) -> QMPMessage:
+return dict(
+self._sync(
+# pylint: disable=protected-access
+
+# _raw() isn't a public API, because turning off
+# automatic ID assignment is discouraged. For
+# compatibility with iotests *only*, do it anyway.
+self._aqmp._raw(qmp_cmd, assign_id=False),
+self._timeout
+)
+)
+
+# Default impl of cmd() delegates to cmd_obj
+
+def command(self, cmd: str, **kwds: object) -> QMPReturnValue:
+return self._sync(
+self._aqmp.execute(cmd, kwds),
+self._timeout
+)
+
+def pull_event(self,
+   wait: Union[bool, float] = False) -> Optional[QMPMessage]:
+if wait is False:
+# Return None if there's no event ready to go
+if self._aqmp.events.empty():
+return None
+
+timeout = None
+if isinstance(wait, float):
+timeout = wait
+
+return dict(
+self._sync(
+self._aqmp.events.get(),
+timeout
+)
+)
+
+def get_events(self, wait: Union[bool, float] = False) -> List[QMPMessage]:
+events = [dict(x) for x in self._aqmp.events.clear()]
+if events:
+return events
+
+event = self.pull_event(wait)
+return [event] if event is not None else []
+
+def clear_events(self) -> None:
+self._aqmp.events.clear()
+
+def close(self) -> None:
+self._sync(
+self._aqmp.disconnect()
+)
+
+def settimeout(self, timeout: Optional[float]) -> None:
+self._timeout = timeout
+
+def send_fd_scm(self, fd: int) -> None:
+self._aqmp.send_fd_scm(fd)
-- 
2.31.1