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

[Hanna Reitz <hreitz@redhat.com>]

On 17.09.21 07:40, 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 | 6 +++++-
>   2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/simplebench/bench_block_job.py b/scripts/simplebench/bench_block_job.py
> index 4f03c12169..a403c35b08 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 451a0666f8..7d448f4d23 100755
> --- a/tests/qemu-iotests/tests/mirror-top-perms
> +++ b/tests/qemu-iotests/tests/mirror-top-perms
> @@ -103,7 +103,11 @@ class TestMirrorTopPerms(iotests.QMPTestCase):
>               print('ERROR: VM B launched successfully, this should not have '
>                     'happened')
>           except qemu.qmp.QMPConnectError:
> -            assert 'Is another process using the image' in self.vm_b.get_log()
> +            pass
> +        except qemu.aqmp.ConnectError:
> +            pass
> +
> +        assert 'Is another process using the image' in self.vm_b.get_log()

But this assertion will fail if there was no exception, and so we won’t 
get to see the real problem, which is the original VM aborting (see the 
doc string).

It doesn’t really matter that much that VM B can start (hence it being a 
logged error message, not a fatal error), and when it can start, of 
course it won’t print an error – but what’s important is that the 
original VM will then abort.

I mean, not an absolute showstopper by any means, but still, the 
assertion was deliberately placed into the `except` block.

Hanna