Re: [PATCH v2 2/3] iotests: Include QMP input in .out files

[Max Reitz]

On 15.10.19 21:35, Eric Blake wrote:
> We generally include relevant HMP input in .out files, by virtue of
> the fact that HMP echoes its input.  But QMP does not, so we have to
> explicitly inject it in the output stream, in order to make it easier
> to read .out files to see what behavior is being tested (especially
> true where the output file is a sequence of {'return': {}}).
> 
> Suggested-by: Max Reitz <address@hidden>

That was actually not my intention. :-)

I was thinking of a new parameter that enables this behavior and is
disabled by default so that existing tests don’t change.

But then again I did see that you interpreted my suggestion in a
slightly different way, and thought this is probably better, actually.

> Signed-off-by: Eric Blake <address@hidden>
> ---
>  tests/qemu-iotests/common.qemu |  9 ++++
>  tests/qemu-iotests/085.out     | 26 ++++++++++
>  tests/qemu-iotests/094.out     |  4 ++
>  tests/qemu-iotests/095.out     |  2 +
>  tests/qemu-iotests/109.out     | 88 ++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/117.out     |  5 ++
>  tests/qemu-iotests/127.out     |  4 ++
>  tests/qemu-iotests/140.out     |  5 ++
>  tests/qemu-iotests/141.out     | 26 ++++++++++
>  tests/qemu-iotests/143.out     |  3 ++
>  tests/qemu-iotests/144.out     |  5 ++
>  tests/qemu-iotests/153.out     | 11 +++++
>  tests/qemu-iotests/156.out     | 11 +++++
>  tests/qemu-iotests/161.out     |  8 ++++
>  tests/qemu-iotests/173.out     |  4 ++
>  tests/qemu-iotests/182.out     |  8 ++++
>  tests/qemu-iotests/183.out     | 11 +++++
>  tests/qemu-iotests/185.out     | 18 +++++++
>  tests/qemu-iotests/191.out     |  8 ++++
>  tests/qemu-iotests/200.out     |  1 +
>  tests/qemu-iotests/223.out     | 19 ++++++++
>  tests/qemu-iotests/229.out     |  3 ++
>  tests/qemu-iotests/249.out     |  6 +++
>  23 files changed, 285 insertions(+)
> diff --git a/tests/qemu-iotests/common.qemu b/tests/qemu-iotests/common.qemu
> index 8d2021a7eb0c..abc231743e82 100644
> --- a/tests/qemu-iotests/common.qemu
> +++ b/tests/qemu-iotests/common.qemu
> @@ -123,6 +123,9 @@ _timed_wait_for()
>  # until either timeout, or a response.  If it is not set, or <=0,
>  # then the command is only sent once.
>  #
> +# If neither $silent nor $mismatch_only is set, and $cmd begins with '{',
> +# echo the command before sending it the first time.
> +#
>  # If $qemu_error_no_exit is set, then even if the expected response
>  # is not seen, we will not exit.  $QEMU_STATUS[$1] will be set it -1 in
>  # that case.
> @@ -152,6 +155,12 @@ _send_qemu_cmd()
>          shift $(($# - 2))
>      fi
> 
> +    # Display QMP being sent, but not HMP (since HMP already echoes its
> +    # input back to output); decide based on leading '{'
> +    if [ -z "$silent" ] && [ -z "$mismatch_only" ] &&
> +            [ "$cmd" != "${cmd#{}" ]; then

It’s a shame that this breaks syntax highlighting in (my) vim.  (Also I
have to admit googling to understand ${cmd#{} wasn’t trivial.)

Can I persuade you to use "${cmd#\{}" instead?  That seems to work for me.

> diff --git a/tests/qemu-iotests/094.out b/tests/qemu-iotests/094.out
> index f3b9ecf22b73..f3e1a9ecf736 100644
> --- a/tests/qemu-iotests/094.out
> +++ b/tests/qemu-iotests/094.out
> @@ -1,16 +1,20 @@
>  QA output created by 094
>  Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
>  Formatting 'TEST_DIR/source.IMGFMT', fmt=IMGFMT size=67108864
> +{'execute': 'qmp_capabilities'}
>  {"return": {}}
> +{'execute': 'drive-mirror', 'arguments': {'device': 'src', 'target': 
> 'nbd:127.0.0.1:10810', 'format': 'nbd', 'sync':'full', 'mode':'existing'}}

This reminds me that we need to fix nbd’s $TEST_IMG to not be fixed to
port 10810.  I get intermittent failures because of that.

[...]

> diff --git a/tests/qemu-iotests/140.out b/tests/qemu-iotests/140.out
> index 67fe44a3e390..3857675f7ebd 100644
> --- a/tests/qemu-iotests/140.out
> +++ b/tests/qemu-iotests/140.out
> @@ -2,14 +2,19 @@ QA output created by 140
>  Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=65536
>  wrote 65536/65536 bytes at offset 0
>  64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +{ 'execute': 'qmp_capabilities' }
>  {"return": {}}
> +{ 'execute': 'nbd-server-start', 'arguments': { 'addr': { 'type': 'unix', 
> 'data': { 'path': 'TEST_DIR/nbd' }}}}

Hmmmmm, this conflicts with my SOCK_DIR series.  common.qemu would then
also need a SOCK_DIR filter.  Well, or 140 should filter it (and the
other tests that are concerned).  I’m not 100 % sure, but a SOCK_DIR
filter in common.qemu probably can’t hurt.

>  {"return": {}}
> +{ 'execute': 'nbd-server-add', 'arguments': { 'device': 'drv' }}
>  {"return": {}}
>  read 65536/65536 bytes at offset 0
>  64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +{ 'execute': 'eject', 'arguments': { 'device': 'drv' }}
>  {"return": {}}
>  qemu-io: can't open device nbd+unix:///drv?socket=TEST_DIR/nbd: Requested 
> export not available
>  server reported: export 'drv' not present
> +{ 'execute': 'quit' }
>  {"return": {}}
>  {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": 
> "SHUTDOWN", "data": {"guest": false, "reason": "host-qmp-quit"}}
>  *** done
> diff --git a/tests/qemu-iotests/141.out b/tests/qemu-iotests/141.out
> index e3b578282da4..cb37ccd8ea42 100644
> --- a/tests/qemu-iotests/141.out
> +++ b/tests/qemu-iotests/141.out
> @@ -2,82 +2,108 @@ QA output created by 141
>  Formatting 'TEST_DIR/b.IMGFMT', fmt=IMGFMT size=1048576
>  Formatting 'TEST_DIR/m.IMGFMT', fmt=IMGFMT size=1048576 
> backing_file=TEST_DIR/b.IMGFMT
>  Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1048576 
> backing_file=TEST_DIR/m.IMGFMT
> +{'execute': 'qmp_capabilities'}
>  {"return": {}}
> 
>  === Testing drive-backup ===
> 
> +{'execute': 'blockdev-add', 'arguments': { 'node-name': 'drv0', 'driver': 
> 'qcow2', 'file': { 'driver': 'file', 'filename': 'TEST_DIR/t.qcow2' }}}

141 also supports qed, so this then results in a mismatch.  I suppose
common.qemu should filter the image format.

(Same for 156, 161, and 229.)

[...]

> diff --git a/tests/qemu-iotests/156.out b/tests/qemu-iotests/156.out
> index 4c391a760371..d1865044f81a 100644
> --- a/tests/qemu-iotests/156.out
> +++ b/tests/qemu-iotests/156.out
> @@ -5,21 +5,27 @@ wrote 262144/262144 bytes at offset 0
>  256 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
>  wrote 196608/196608 bytes at offset 65536
>  192 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +{ 'execute': 'qmp_capabilities' }
>  {"return": {}}
>  Formatting 'TEST_DIR/t.IMGFMT.overlay', fmt=IMGFMT size=1048576 
> backing_file=TEST_DIR/t.IMGFMT
> +{ 'execute': 'blockdev-snapshot-sync', 'arguments': { 'device': 'source', 
> 'snapshot-file': 'TEST_DIR/t.qcow2.overlay', 'format': 'qcow2', 'mode': 
> 'existing' } }

Same here (as said above), although there’s also the fact to consider
that 156 supports generic protocols.  I hope _filter_testdir handles
that, though.

Max

signature.asc
Description: OpenPGP digital signature