[PATCH v2 0/4] iotests: finalize switch to async QMP

[PATCH v2 0/4] iotests: finalize switch to async QMP

[John Snow]

GitLab: https://gitlab.com/jsnow/qemu/-/commits/python-qmp-legacy-switch-pt1b
CI: https://gitlab.com/jsnow/qemu/-/pipelines/495951187

This series isolates the iotests-centric changes required to switch to
the new QMP library. It doesn't do a whole lot!

This is a re-send just being sent for the purposes of aggregating tags
before pulling it into my Python branch.

John Snow (4):
  python/machine: permanently switch to AQMP
  scripts/bench-block-job: switch to AQMP
  iotests/mirror-top-perms: switch to AQMP
  iotests: switch to AQMP

 python/qemu/machine/machine.py            | 18 +++++++-----------
 python/qemu/machine/qtest.py              |  2 +-
 scripts/simplebench/bench_block_job.py    |  3 +--
 tests/qemu-iotests/iotests.py             |  3 +--
 tests/qemu-iotests/tests/mirror-top-perms |  9 +++------
 5 files changed, 13 insertions(+), 22 deletions(-)

-- 
2.34.1

[PATCH v2 1/4] python/machine: permanently switch to AQMP

[John Snow]

Remove the QEMU_PYTHON_LEGACY_QMP environment variable, making the
switch from sync qmp to async qmp permanent. Update exceptions and
import paths as necessary.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Beraldo Leal <bleal@redhat.com>
Acked-by: Hanna Reitz <hreitz@redhat.com>
---
 python/qemu/machine/machine.py | 18 +++++++-----------
 python/qemu/machine/qtest.py   |  2 +-
 2 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/python/qemu/machine/machine.py b/python/qemu/machine/machine.py
index a5972fab4d..41be025ac7 100644
--- a/python/qemu/machine/machine.py
+++ b/python/qemu/machine/machine.py
@@ -40,21 +40,16 @@
     TypeVar,
 )
 
-from qemu.qmp import (  # pylint: disable=import-error
+from qemu.aqmp import SocketAddrT
+from qemu.aqmp.legacy import (
+    QEMUMonitorProtocol,
     QMPMessage,
     QMPReturnValue,
-    SocketAddrT,
 )
 
 from . import console_socket
 
 
-if os.environ.get('QEMU_PYTHON_LEGACY_QMP'):
-    from qemu.qmp import QEMUMonitorProtocol
-else:
-    from qemu.aqmp.legacy import QEMUMonitorProtocol
-
-
 LOG = logging.getLogger(__name__)
 
 
@@ -743,8 +738,9 @@ def events_wait(self,
         :param timeout: Optional timeout, in seconds.
                         See QEMUMonitorProtocol.pull_event.
 
-        :raise QMPTimeoutError: If timeout was non-zero and no matching events
-                                were found.
+        :raise asyncio.TimeoutError:
+            If timeout was non-zero and no matching events were found.
+
         :return: A QMP event matching the filter criteria.
                  If timeout was 0 and no event matched, None.
         """
@@ -767,7 +763,7 @@ def _match(event: QMPMessage) -> bool:
             event = self._qmp.pull_event(wait=timeout)
             if event is None:
                 # NB: None is only returned when timeout is false-ish.
-                # Timeouts raise QMPTimeoutError instead!
+                # Timeouts raise asyncio.TimeoutError instead!
                 break
             if _match(event):
                 return event
diff --git a/python/qemu/machine/qtest.py b/python/qemu/machine/qtest.py
index f2f9aaa5e5..13e0aaff84 100644
--- a/python/qemu/machine/qtest.py
+++ b/python/qemu/machine/qtest.py
@@ -26,7 +26,7 @@
     TextIO,
 )
 
-from qemu.qmp import SocketAddrT  # pylint: disable=import-error
+from qemu.aqmp import SocketAddrT
 
 from .machine import QEMUMachine
 
-- 
2.34.1

[PATCH v2 2/4] scripts/bench-block-job: switch to AQMP

[John Snow]

For this commit, we only need to remove accommodations for the
synchronous QMP library.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Beraldo Leal <bleal@redhat.com>
Acked-by: Hanna Reitz <hreitz@redhat.com>
---
 scripts/simplebench/bench_block_job.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/scripts/simplebench/bench_block_job.py b/scripts/simplebench/bench_block_job.py
index a403c35b08..af9d1646a4 100755
--- a/scripts/simplebench/bench_block_job.py
+++ b/scripts/simplebench/bench_block_job.py
@@ -27,7 +27,6 @@
 
 sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
 from qemu.machine import QEMUMachine
-from qemu.qmp import QMPConnectError
 from qemu.aqmp import ConnectError
 
 
@@ -50,7 +49,7 @@ def bench_block_job(cmd, cmd_args, qemu_args):
         vm.launch()
     except OSError as e:
         return {'error': 'popen failed: ' + str(e)}
-    except (QMPConnectError, ConnectError, socket.timeout):
+    except (ConnectError, socket.timeout):
         return {'error': 'qemu failed: ' + str(vm.get_log())}
 
     try:
-- 
2.34.1

[PATCH v2 3/4] iotests/mirror-top-perms: switch to AQMP

[John Snow]

We don't have to maintain compatibility with both QMP libraries anymore,
so we can just remove the old exception. While we're here, take
advantage of the extra fields present in the VMLaunchFailure exception
that machine.py now raises.

(Note: I'm leaving the logging suppression here unchanged. I had
suggested previously we use filters to scrub the PID out of the logging
information so it could just be diffed as part of the iotest output, but
that meant *always* scrubbing PID from logger output, which defeated the
point of even offering that information in the output to begin with.

Ultimately, I decided it's fine to just suppress the logger temporarily.)

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Acked-by: Hanna Reitz <hreitz@redhat.com>
---
 tests/qemu-iotests/tests/mirror-top-perms | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/tests/qemu-iotests/tests/mirror-top-perms b/tests/qemu-iotests/tests/mirror-top-perms
index b5849978c4..223f3c1620 100755
--- a/tests/qemu-iotests/tests/mirror-top-perms
+++ b/tests/qemu-iotests/tests/mirror-top-perms
@@ -22,7 +22,6 @@
 import os
 
 from qemu.machine import machine
-from qemu.qmp import QMPConnectError
 
 import iotests
 from iotests import change_log_level, qemu_img
@@ -99,15 +98,13 @@ class TestMirrorTopPerms(iotests.QMPTestCase):
         self.vm_b.add_blockdev(f'file,node-name=drive0,filename={source}')
         self.vm_b.add_device('virtio-blk,drive=drive0,share-rw=on')
         try:
-            # Silence AQMP errors temporarily.
-            # TODO: Remove this and just allow the errors to be logged when
-            # AQMP fully replaces QMP.
+            # Silence AQMP logging errors temporarily.
             with change_log_level('qemu.aqmp'):
                 self.vm_b.launch()
                 print('ERROR: VM B launched successfully, '
                       'this should not have happened')
-        except (QMPConnectError, machine.VMLaunchFailure):
-            assert 'Is another process using the image' in self.vm_b.get_log()
+        except machine.VMLaunchFailure as exc:
+            assert 'Is another process using the image' in exc.output
 
         result = self.vm.qmp('block-job-cancel',
                              device='mirror')
-- 
2.34.1

[PATCH v2 4/4] iotests: switch to AQMP

[John Snow]

iotests is already using async QMP, but to finalize the switchover we
only need to update any remaining import paths to rely solely on the new
library instead.

Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Reviewed-by: Beraldo Leal <bleal@redhat.com>
Acked-by: Hanna Reitz <hreitz@redhat.com>
---
 tests/qemu-iotests/iotests.py | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 508adade9e..8760e2c310 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -38,8 +38,7 @@
 from contextlib import contextmanager
 
 from qemu.machine import qtest
-from qemu.qmp import QMPMessage
-from qemu.aqmp.legacy import QEMUMonitorProtocol
+from qemu.aqmp.legacy import QMPMessage, QEMUMonitorProtocol
 
 # Use this logger for logging messages directly from the iotests module
 logger = logging.getLogger('qemu.iotests')
-- 
2.34.1

Re: [PATCH v2 0/4] iotests: finalize switch to async QMP

[John Snow]

On Mon, Mar 21, 2022 at 4:33 PM John Snow <jsnow@redhat.com> wrote:
>
> GitLab: https://gitlab.com/jsnow/qemu/-/commits/python-qmp-legacy-switch-pt1b
> CI: https://gitlab.com/jsnow/qemu/-/pipelines/495951187
>
> This series isolates the iotests-centric changes required to switch to
> the new QMP library. It doesn't do a whole lot!
>
> This is a re-send just being sent for the purposes of aggregating tags
> before pulling it into my Python branch.
>
> John Snow (4):
>   python/machine: permanently switch to AQMP
>   scripts/bench-block-job: switch to AQMP
>   iotests/mirror-top-perms: switch to AQMP
>   iotests: switch to AQMP
>
>  python/qemu/machine/machine.py            | 18 +++++++-----------
>  python/qemu/machine/qtest.py              |  2 +-
>  scripts/simplebench/bench_block_job.py    |  3 +--
>  tests/qemu-iotests/iotests.py             |  3 +--
>  tests/qemu-iotests/tests/mirror-top-perms |  9 +++------
>  5 files changed, 13 insertions(+), 22 deletions(-)
>
> --
> 2.34.1
>

Tentatively staged on my Python branch, thanks.

--js