Re: [PATCH v2 2/3] iotests: Include QMP input in .out files
On 10/17/19 7:59 AM, Max Reitz wrote: 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 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. I'm glad you like how it turned out. Now to fix the problems ;) +++ 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. Yes. That, or "${cmd#'{'}" should also work. 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. And I should therefore fix the filter to display it as something more stable (perhaps nbd:HOST:PORT). But I also agree that the hard-coded value is pre-existing broken, so a separate patch here to improve it is warranted. [...] 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' Hm, 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. Agreed. I will rebase a v3 on top of your pending series. +++ 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.) Yep, I'll have to improve the filtering. I'll make sure I run -qed tests before posting v3. [...] 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':
Re: [PATCH v2 2/3] iotests: Include QMP input in .out files
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 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 > --- > 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' Hm, 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": {}} >
[PATCH v2 2/3] iotests: Include QMP input in .out files
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 Signed-off-by: Eric Blake --- 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 +echo "${cmd}" | _filter_testdir +fi while [ ${count} -gt 0 ] do echo "${cmd}" >&${QEMU_IN[${h}]} diff --git a/tests/qemu-iotests/085.out b/tests/qemu-iotests/085.out index 2a5f256cd3ec..e92f125b63c4 100644 --- a/tests/qemu-iotests/085.out +++ b/tests/qemu-iotests/085.out @@ -7,48 +7,61 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 === Sending capabilities === +{ 'execute': 'qmp_capabilities' } {"return": {}} === Create a single snapshot on virtio0 === +{ 'execute': 'blockdev-snapshot-sync', 'arguments': { 'device': 'virtio0', 'snapshot-file':'TEST_DIR/1-snapshot-v0.qcow2', 'format': 'qcow2' } } Formatting 'TEST_DIR/1-snapshot-v0.qcow2', fmt=qcow2 size=134217728 backing_file=TEST_DIR/t.qcow2.1 backing_fmt=qcow2 cluster_size=65536 lazy_refcounts=off refcount_bits=16 {"return": {}} === Invalid command - missing device and nodename === +{ 'execute': 'blockdev-snapshot-sync', 'arguments': { 'snapshot-file':'TEST_DIR/1-snapshot-v0.qcow2', 'format': 'qcow2' } } {"error": {"class": "GenericError", "desc": "Cannot find device= nor node_name="}} === Invalid command - missing snapshot-file === +{ 'execute': 'blockdev-snapshot-sync', 'arguments': { 'device': 'virtio0', 'format': 'qcow2' } } {"error": {"class": "GenericError", "desc": "Parameter 'snapshot-file' is missing"}} === Create several transactional group snapshots === +{ 'execute': 'transaction', 'arguments': {'actions': [ { 'type': 'blockdev-snapshot-sync', 'data' : { 'device': 'virtio0', 'snapshot-file': 'TEST_DIR/2-snapshot-v0.qcow2' } }, { 'type': 'blockdev-snapshot-sync', 'data' : { 'device': 'virtio1', 'snapshot-file': 'TEST_DIR/2-snapshot-v1.qcow2' } } ] } } Formatting 'TEST_DIR/2-snapshot-v0.qcow2', fmt=qcow2 size=134217728 backing_file=TEST_DIR/1-snapshot-v0.qcow2 backing_fmt=qcow2 cluster_size=65536 lazy_refcounts=off refcount_bits=16 Formatting 'TEST_DIR/2-snapshot-v1.qcow2', fmt=qcow2 size=134217728 backing_file=TEST_DIR/t.qcow2.2 backing_fmt=qcow2 cluster_size=65536 lazy_refcounts=off refcount_bits=16 {"return": {}} +{ 'execute': 'transaction', 'arguments': {'actions': [ { 'type': 'blockdev-snapshot-sync', 'data' : { 'device': 'virtio0', 'snapshot-file': 'TEST_DIR/3-snapshot-v0.qcow2' } }, { 'type': 'blockdev-snapshot-sync', 'data' : { 'device': 'virtio1', 'snapshot-file': 'TEST_DIR/3-snapshot-v1.qcow2' } } ] } } Formatting 'TEST_DIR/3-snapshot-v0.qcow2', fmt=qcow2 size=134217728 backing_file=TEST_DIR/2-snapshot-v0.qcow2 backing_fmt=qcow2 cluster_size=65536 lazy_refcounts=off refcount_bits=16 Formatting 'TEST_DIR/3-snapshot-v1.qcow2', fmt=qcow2