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

[John Snow <jsnow@redhat.com>]

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

On Tue, Oct 12, 2021 at 12:06 PM Hanna Reitz <hreitz@redhat.com> 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 <jsnow@redhat.com>
> > ---
> >   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',
>
>

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