Re: [PATCH v2 13/17] iotests: Accommodate async QMP Exception classes

2021-10-12 Thread John Snow 
On Tue, Oct 12, 2021 at 12:06 PM Hanna Reitz  wrote:

> On 23.09.21 02:49, John Snow wrote:
> > (But continue to support the old ones for now, too.)
> >
> > There are very few cases of any user of QEMUMachine or a subclass
> > thereof relying on a QMP Exception type. If you'd like to check for
> > yourself, you want to grep for all of the derivatives of QMPError,
> > excluding 'AQMPError' and its derivatives. That'd be these:
> >
> > - QMPError
> > - QMPConnectError
> > - QMPCapabilitiesError
> > - QMPTimeoutError
> > - QMPProtocolError
> > - QMPResponseError
> > - QMPBadPortError
> >
> >
> > Signed-off-by: John Snow 
> > ---
> >   scripts/simplebench/bench_block_job.py| 3 ++-
> >   tests/qemu-iotests/tests/mirror-top-perms | 3 ++-
> >   2 files changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/scripts/simplebench/bench_block_job.py
> b/scripts/simplebench/bench_block_job.py
> > index 4f03c121697..a403c35b08f 100755
> > --- a/scripts/simplebench/bench_block_job.py
> > +++ b/scripts/simplebench/bench_block_job.py
> > @@ -28,6 +28,7 @@
> >   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
> >
> >
> >   def bench_block_job(cmd, cmd_args, qemu_args):
> > @@ -49,7 +50,7 @@ def bench_block_job(cmd, cmd_args, qemu_args):
> >   vm.launch()
> >   except OSError as e:
> >   return {'error': 'popen failed: ' + str(e)}
> > -except (QMPConnectError, socket.timeout):
> > +except (QMPConnectError, ConnectError, socket.timeout):
> >   return {'error': 'qemu failed: ' + str(vm.get_log())}
> >
> >   try:
> > diff --git a/tests/qemu-iotests/tests/mirror-top-perms
> b/tests/qemu-iotests/tests/mirror-top-perms
> > index 2fc8dd66e0a..9fe315e3b01 100755
> > --- a/tests/qemu-iotests/tests/mirror-top-perms
> > +++ b/tests/qemu-iotests/tests/mirror-top-perms
> > @@ -26,6 +26,7 @@ from iotests import qemu_img
> >   # Import qemu after iotests.py has amended sys.path
> >   # pylint: disable=wrong-import-order
> >   import qemu
> > +from qemu.aqmp import ConnectError
>
> With this change, the test emits the “AQMP is in development” warning,
> breaking the test.  Do we want to pull patch 16 before this patch?
>
>
Oh, good spot, I hadn't considered this.

Uh, yeah, I'll have to front-load the other patch.


> (I also wonder whether we want to import QMPConnectError, too, because
> the `except (qemu.qmp.*, *)` below looks so... heterogeneous.)
>

Will do.

(I don't think I've ever had code critiqued as "heterogeneous" before !)

>
> Hanna
>
> >   image_size = 1 * 1024 * 1024
> > @@ -102,7 +103,7 @@ class TestMirrorTopPerms(iotests.QMPTestCase):
> >   self.vm_b.launch()
> >   print('ERROR: VM B launched successfully, this should not
> have '
> > 'happened')
> > -except qemu.qmp.QMPConnectError:
> > +except (qemu.qmp.QMPConnectError, ConnectError):
> >   assert 'Is another process using the image' in
> self.vm_b.get_log()
> >
> >   result = self.vm.qmp('block-job-cancel',
>
>

Re: [PATCH v2 13/17] iotests: Accommodate async QMP Exception classes

2021-10-12 Thread Hanna Reitz 

On 23.09.21 02:49, John Snow wrote:

(But continue to support the old ones for now, too.)

There are very few cases of any user of QEMUMachine or a subclass
thereof relying on a QMP Exception type. If you'd like to check for
yourself, you want to grep for all of the derivatives of QMPError,
excluding 'AQMPError' and its derivatives. That'd be these:

- QMPError
- QMPConnectError
- QMPCapabilitiesError
- QMPTimeoutError
- QMPProtocolError
- QMPResponseError
- QMPBadPortError


Signed-off-by: John Snow 
---
  scripts/simplebench/bench_block_job.py| 3 ++-
  tests/qemu-iotests/tests/mirror-top-perms | 3 ++-
  2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/scripts/simplebench/bench_block_job.py 
b/scripts/simplebench/bench_block_job.py
index 4f03c121697..a403c35b08f 100755
--- a/scripts/simplebench/bench_block_job.py
+++ b/scripts/simplebench/bench_block_job.py
@@ -28,6 +28,7 @@
  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
  
  
  def bench_block_job(cmd, cmd_args, qemu_args):

@@ -49,7 +50,7 @@ def bench_block_job(cmd, cmd_args, qemu_args):
  vm.launch()
  except OSError as e:
  return {'error': 'popen failed: ' + str(e)}
-except (QMPConnectError, socket.timeout):
+except (QMPConnectError, ConnectError, socket.timeout):
  return {'error': 'qemu failed: ' + str(vm.get_log())}
  
  try:

diff --git a/tests/qemu-iotests/tests/mirror-top-perms 
b/tests/qemu-iotests/tests/mirror-top-perms
index 2fc8dd66e0a..9fe315e3b01 100755
--- a/tests/qemu-iotests/tests/mirror-top-perms
+++ b/tests/qemu-iotests/tests/mirror-top-perms
@@ -26,6 +26,7 @@ from iotests import qemu_img
  # Import qemu after iotests.py has amended sys.path
  # pylint: disable=wrong-import-order
  import qemu
+from qemu.aqmp import ConnectError


With this change, the test emits the “AQMP is in development” warning, 
breaking the test.  Do we want to pull patch 16 before this patch?


(I also wonder whether we want to import QMPConnectError, too, because 
the `except (qemu.qmp.*, *)` below looks so... heterogeneous.)


Hanna


  image_size = 1 * 1024 * 1024
@@ -102,7 +103,7 @@ class TestMirrorTopPerms(iotests.QMPTestCase):
  self.vm_b.launch()
  print('ERROR: VM B launched successfully, this should not have '
'happened')
-except qemu.qmp.QMPConnectError:
+except (qemu.qmp.QMPConnectError, ConnectError):
  assert 'Is another process using the image' in self.vm_b.get_log()
  
  result = self.vm.qmp('block-job-cancel',

[PATCH v2 13/17] iotests: Accommodate async QMP Exception classes

2021-09-22 Thread John Snow 
(But continue to support the old ones for now, too.)

There are very few cases of any user of QEMUMachine or a subclass
thereof relying on a QMP Exception type. If you'd like to check for
yourself, you want to grep for all of the derivatives of QMPError,
excluding 'AQMPError' and its derivatives. That'd be these:

- QMPError
- QMPConnectError
- QMPCapabilitiesError
- QMPTimeoutError
- QMPProtocolError
- QMPResponseError
- QMPBadPortError


Signed-off-by: John Snow 
---
 scripts/simplebench/bench_block_job.py| 3 ++-
 tests/qemu-iotests/tests/mirror-top-perms | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/scripts/simplebench/bench_block_job.py 
b/scripts/simplebench/bench_block_job.py
index 4f03c121697..a403c35b08f 100755
--- a/scripts/simplebench/bench_block_job.py
+++ b/scripts/simplebench/bench_block_job.py
@@ -28,6 +28,7 @@
 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
 
 
 def bench_block_job(cmd, cmd_args, qemu_args):
@@ -49,7 +50,7 @@ def bench_block_job(cmd, cmd_args, qemu_args):
 vm.launch()
 except OSError as e:
 return {'error': 'popen failed: ' + str(e)}
-except (QMPConnectError, socket.timeout):
+except (QMPConnectError, ConnectError, socket.timeout):
 return {'error': 'qemu failed: ' + str(vm.get_log())}
 
 try:
diff --git a/tests/qemu-iotests/tests/mirror-top-perms 
b/tests/qemu-iotests/tests/mirror-top-perms
index 2fc8dd66e0a..9fe315e3b01 100755
--- a/tests/qemu-iotests/tests/mirror-top-perms
+++ b/tests/qemu-iotests/tests/mirror-top-perms
@@ -26,6 +26,7 @@ from iotests import qemu_img
 # Import qemu after iotests.py has amended sys.path
 # pylint: disable=wrong-import-order
 import qemu
+from qemu.aqmp import ConnectError
 
 
 image_size = 1 * 1024 * 1024
@@ -102,7 +103,7 @@ class TestMirrorTopPerms(iotests.QMPTestCase):
 self.vm_b.launch()
 print('ERROR: VM B launched successfully, this should not have '
   'happened')
-except qemu.qmp.QMPConnectError:
+except (qemu.qmp.QMPConnectError, ConnectError):
 assert 'Is another process using the image' in self.vm_b.get_log()
 
 result = self.vm.qmp('block-job-cancel',
-- 
2.31.1