* [Qemu-devel] [PATCH 0/2] qemu-iotests: fix 067 on s390x
@ 2016-02-09 13:23 Sascha Silbe
2016-02-09 13:23 ` [Qemu-devel] [PATCH 1/2] qemu-iotests: add _filter_qmp_events() for filtering out QMP events Sascha Silbe
2016-02-09 13:23 ` [Qemu-devel] [PATCH 2/2] qemu-iotests: 067: ignore " Sascha Silbe
0 siblings, 2 replies; 7+ messages in thread
From: Sascha Silbe @ 2016-02-09 13:23 UTC (permalink / raw)
To: qemu-devel, Kevin Wolf, qemu-block; +Cc: Tu Bo
qemu-iotests test case 067 fails on s390x because the ordering of its
output is architecture-dependent (asynchronous vs. synchronous hotplug
handling). This series fixes the failure by filtering events from QMP
output for 067. There are two possible alternative approaches; see my
notes on the second patch.
Sascha Silbe (2):
qemu-iotests: add _filter_qmp_events() for filtering out QMP events
qemu-iotests: 067: ignore QMP events
tests/qemu-iotests/067 | 4 +-
tests/qemu-iotests/067.out | 527 +++------------------------------------
tests/qemu-iotests/common.filter | 6 +
3 files changed, 41 insertions(+), 496 deletions(-)
--
2.1.4
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH 1/2] qemu-iotests: add _filter_qmp_events() for filtering out QMP events
2016-02-09 13:23 [Qemu-devel] [PATCH 0/2] qemu-iotests: fix 067 on s390x Sascha Silbe
@ 2016-02-09 13:23 ` Sascha Silbe
2016-02-10 16:43 ` Max Reitz
2016-02-09 13:23 ` [Qemu-devel] [PATCH 2/2] qemu-iotests: 067: ignore " Sascha Silbe
1 sibling, 1 reply; 7+ messages in thread
From: Sascha Silbe @ 2016-02-09 13:23 UTC (permalink / raw)
To: qemu-devel, Kevin Wolf, qemu-block; +Cc: Tu Bo
The order of some QMP events may depend on the architecture being
tested. Add support for filtering out QMP events so we can use a
single reference output for all architecture when the test doesn't
care about the events.
Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
---
tests/qemu-iotests/common.filter | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
index 84b7434..b908aa2 100644
--- a/tests/qemu-iotests/common.filter
+++ b/tests/qemu-iotests/common.filter
@@ -178,6 +178,12 @@ _filter_qmp()
-e ' QMP_VERSION'
}
+# remove QMP events from output
+_filter_qmp_events()
+{
+ sed -e '/^{\(.*, \)"event": ".*}$/ d'
+}
+
# replace driver-specific options in the "Formatting..." line
_filter_img_create()
{
--
2.1.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH 2/2] qemu-iotests: 067: ignore QMP events
2016-02-09 13:23 [Qemu-devel] [PATCH 0/2] qemu-iotests: fix 067 on s390x Sascha Silbe
2016-02-09 13:23 ` [Qemu-devel] [PATCH 1/2] qemu-iotests: add _filter_qmp_events() for filtering out QMP events Sascha Silbe
@ 2016-02-09 13:23 ` Sascha Silbe
1 sibling, 0 replies; 7+ messages in thread
From: Sascha Silbe @ 2016-02-09 13:23 UTC (permalink / raw)
To: qemu-devel, Kevin Wolf, qemu-block; +Cc: Tu Bo
The relative ordering of the "device_del" return value and the
"DEVICE_DELETED" QMP event depends on the architecture being
tested. On x86 unplugging virtio disks is asynchronous
(=qdev_unplug()= → =hotplug_handler_unplug_request()=) while on s390x
it is synchronous (=qdev_unplug()= → =hotplug_handler_unplug()=). This
leads to the actual output on s390x consistently differing from the
reference output (that was probably produced on x86).
The easiest way to address this is to filter out QMP events in
067. The DEVICE_DELETED event is already getting explicitly tested by
the Python-based test case 139, so the test coverage should be
unaffected. Make use of _filter_qmp_events() introduced by the
previous patch to remove QMP events from the test case output and
adjust the reference output accordingly.
Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
---
This patch is a lot easier to read using the following command after
applying it locally (will hide all whitespace-only changes):
git log -p -1 --word-diff-regex='[^[:space:]]'
There are two alternative solutions:
1. Add per-architecture reference output for architectures with
synchronous hotplug handling.
Contributors often forget to update the reference output files
after changing the qemu output. With per-architecture files, this
gets a lot worse as contributors would need to run the tests for
all supported architectures, possibly *on* all supported
architectures, to notice the changed output and verify their
changes to the reference output.
2. Reimplement 067 in Python.
067 currently covers a lot of QMP output for (virtio) block
devices, even if just as a side effect. If we don't want to risk
reducing the test coverage, we'd need to either ensure some other
test is covering this output (and will continue to do so in the
future) or add a lot of explicit checking in the Python test. This
isn't quite as bad as it sounds as we can check the entire result
against a dictionary literal each time.
Both alternatives have the advantage that we don't need very long
lines (currently up to 753 characters) in the reference output.
---
tests/qemu-iotests/067 | 4 +-
tests/qemu-iotests/067.out | 527 +++------------------------------------------
2 files changed, 35 insertions(+), 496 deletions(-)
diff --git a/tests/qemu-iotests/067 b/tests/qemu-iotests/067
index 3788534..8fd19f5 100755
--- a/tests/qemu-iotests/067
+++ b/tests/qemu-iotests/067
@@ -41,7 +41,7 @@ _unsupported_imgopts 'refcount_bits=\([^1]\|.\([^6]\|$\)\)'
function do_run_qemu()
{
echo Testing: "$@"
- $QEMU -nographic -qmp-pretty stdio -serial none "$@"
+ $QEMU -nographic -qmp stdio -serial none "$@"
echo
}
@@ -49,7 +49,7 @@ function run_qemu()
{
do_run_qemu "$@" 2>&1 | _filter_testdir | _filter_qmp | _filter_qemu \
| sed -e 's/\("actual-size":\s*\)[0-9]\+/\1SIZE/g' \
- | _filter_generated_node_ids
+ | _filter_generated_node_ids | _filter_qmp_events
}
size=128M
diff --git a/tests/qemu-iotests/067.out b/tests/qemu-iotests/067.out
index ae3fccb..1877271 100644
--- a/tests/qemu-iotests/067.out
+++ b/tests/qemu-iotests/067.out
@@ -4,514 +4,53 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728
=== -drive/-device and device_del ===
Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,if=none,id=disk -device virtio-blk,drive=disk,id=virtio0
-{
- QMP_VERSION
-}
-{
- "return": {
- }
-}
-{
- "return": [
- {
- "io-status": "ok",
- "device": "disk",
- "locked": false,
- "removable": false,
- "inserted": {
- "iops_rd": 0,
- "detect_zeroes": "off",
- "image": {
- "virtual-size": 134217728,
- "filename": "TEST_DIR/t.qcow2",
- "cluster-size": 65536,
- "format": "qcow2",
- "actual-size": SIZE,
- "format-specific": {
- "type": "qcow2",
- "data": {
- "compat": "1.1",
- "lazy-refcounts": false,
- "refcount-bits": 16,
- "corrupt": false
- }
- },
- "dirty-flag": false
- },
- "iops_wr": 0,
- "ro": false,
- "node-name": "NODE_NAME",
- "backing_file_depth": 0,
- "drv": "qcow2",
- "iops": 0,
- "bps_wr": 0,
- "write_threshold": 0,
- "encrypted": false,
- "bps": 0,
- "bps_rd": 0,
- "cache": {
- "no-flush": false,
- "direct": false,
- "writeback": true
- },
- "file": "TEST_DIR/t.qcow2",
- "encryption_key_missing": false
- },
- "type": "unknown"
- }
- ]
-}
-{
- "return": {
- }
-}
-{
- "return": {
- }
-}
-{
- "timestamp": {
- "seconds": TIMESTAMP,
- "microseconds": TIMESTAMP
- },
- "event": "DEVICE_DELETED",
- "data": {
- "path": "/machine/peripheral/virtio0/virtio-backend"
- }
-}
-{
- "timestamp": {
- "seconds": TIMESTAMP,
- "microseconds": TIMESTAMP
- },
- "event": "DEVICE_DELETED",
- "data": {
- "device": "virtio0",
- "path": "/machine/peripheral/virtio0"
- }
-}
-{
- "timestamp": {
- "seconds": TIMESTAMP,
- "microseconds": TIMESTAMP
- },
- "event": "RESET"
-}
-{
- "return": [
- ]
-}
-{
- "return": {
- }
-}
-{
- "timestamp": {
- "seconds": TIMESTAMP,
- "microseconds": TIMESTAMP
- },
- "event": "SHUTDOWN"
-}
+QMP_VERSION
+{"return": {}}
+{"return": [{"io-status": "ok", "device": "disk", "locked": false, "removable": false, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "image": {"virtual-size": 134217728, "filename": "TEST_DIR/t.qcow2", "cluster-size": 65536, "format": "qcow2", "actual-size": SIZE, "format-specific": {"type": "qcow2", "data": {"compat": "1.1", "lazy-refcounts": false, "refcount-bits": 16, "corrupt": false}}, "dirty-flag": false}, "iops_wr": 0, "ro": false, "node-name": "NODE_NAME", "backing_file_depth": 0, "drv": "qcow2", "iops": 0, "bps_wr": 0, "write_threshold": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "cache": {"no-flush": false, "direct": false, "writeback": true}, "file": "TEST_DIR/t.qcow2", "encryption_key_missing": false}, "type": "unknown"}]}
+{"return": {}}
+{"return": {}}
+{"return": []}
+{"return": {}}
=== -drive/device_add and device_del ===
Testing: -drive file=TEST_DIR/t.qcow2,format=qcow2,if=none,id=disk
-{
- QMP_VERSION
-}
-{
- "return": {
- }
-}
-{
- "return": [
- {
- "device": "disk",
- "locked": false,
- "removable": true,
- "inserted": {
- "iops_rd": 0,
- "detect_zeroes": "off",
- "image": {
- "virtual-size": 134217728,
- "filename": "TEST_DIR/t.qcow2",
- "cluster-size": 65536,
- "format": "qcow2",
- "actual-size": SIZE,
- "format-specific": {
- "type": "qcow2",
- "data": {
- "compat": "1.1",
- "lazy-refcounts": false,
- "refcount-bits": 16,
- "corrupt": false
- }
- },
- "dirty-flag": false
- },
- "iops_wr": 0,
- "ro": false,
- "node-name": "NODE_NAME",
- "backing_file_depth": 0,
- "drv": "qcow2",
- "iops": 0,
- "bps_wr": 0,
- "write_threshold": 0,
- "encrypted": false,
- "bps": 0,
- "bps_rd": 0,
- "cache": {
- "no-flush": false,
- "direct": false,
- "writeback": true
- },
- "file": "TEST_DIR/t.qcow2",
- "encryption_key_missing": false
- },
- "type": "unknown"
- }
- ]
-}
-{
- "return": {
- }
-}
-{
- "return": {
- }
-}
-{
- "return": {
- }
-}
-{
- "timestamp": {
- "seconds": TIMESTAMP,
- "microseconds": TIMESTAMP
- },
- "event": "DEVICE_DELETED",
- "data": {
- "path": "/machine/peripheral/virtio0/virtio-backend"
- }
-}
-{
- "timestamp": {
- "seconds": TIMESTAMP,
- "microseconds": TIMESTAMP
- },
- "event": "DEVICE_DELETED",
- "data": {
- "device": "virtio0",
- "path": "/machine/peripheral/virtio0"
- }
-}
-{
- "timestamp": {
- "seconds": TIMESTAMP,
- "microseconds": TIMESTAMP
- },
- "event": "RESET"
-}
-{
- "return": [
- ]
-}
-{
- "return": {
- }
-}
-{
- "timestamp": {
- "seconds": TIMESTAMP,
- "microseconds": TIMESTAMP
- },
- "event": "SHUTDOWN"
-}
+QMP_VERSION
+{"return": {}}
+{"return": [{"device": "disk", "locked": false, "removable": true, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "image": {"virtual-size": 134217728, "filename": "TEST_DIR/t.qcow2", "cluster-size": 65536, "format": "qcow2", "actual-size": SIZE, "format-specific": {"type": "qcow2", "data": {"compat": "1.1", "lazy-refcounts": false, "refcount-bits": 16, "corrupt": false}}, "dirty-flag": false}, "iops_wr": 0, "ro": false, "node-name": "NODE_NAME", "backing_file_depth": 0, "drv": "qcow2", "iops": 0, "bps_wr": 0, "write_threshold": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "cache": {"no-flush": false, "direct": false, "writeback": true}, "file": "TEST_DIR/t.qcow2", "encryption_key_missing": false}, "type": "unknown"}]}
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"return": []}
+{"return": {}}
=== drive_add/device_add and device_del ===
Testing:
-{
- QMP_VERSION
-}
-{
- "return": {
- }
-}
-{
- "return": "OK\r\n"
-}
-{
- "return": [
- {
- "device": "disk",
- "locked": false,
- "removable": true,
- "inserted": {
- "iops_rd": 0,
- "detect_zeroes": "off",
- "image": {
- "virtual-size": 134217728,
- "filename": "TEST_DIR/t.qcow2",
- "cluster-size": 65536,
- "format": "qcow2",
- "actual-size": SIZE,
- "format-specific": {
- "type": "qcow2",
- "data": {
- "compat": "1.1",
- "lazy-refcounts": false,
- "refcount-bits": 16,
- "corrupt": false
- }
- },
- "dirty-flag": false
- },
- "iops_wr": 0,
- "ro": false,
- "node-name": "NODE_NAME",
- "backing_file_depth": 0,
- "drv": "qcow2",
- "iops": 0,
- "bps_wr": 0,
- "write_threshold": 0,
- "encrypted": false,
- "bps": 0,
- "bps_rd": 0,
- "cache": {
- "no-flush": false,
- "direct": false,
- "writeback": true
- },
- "file": "TEST_DIR/t.qcow2",
- "encryption_key_missing": false
- },
- "type": "unknown"
- }
- ]
-}
-{
- "return": {
- }
-}
-{
- "return": {
- }
-}
-{
- "return": {
- }
-}
-{
- "timestamp": {
- "seconds": TIMESTAMP,
- "microseconds": TIMESTAMP
- },
- "event": "DEVICE_DELETED",
- "data": {
- "path": "/machine/peripheral/virtio0/virtio-backend"
- }
-}
-{
- "timestamp": {
- "seconds": TIMESTAMP,
- "microseconds": TIMESTAMP
- },
- "event": "DEVICE_DELETED",
- "data": {
- "device": "virtio0",
- "path": "/machine/peripheral/virtio0"
- }
-}
-{
- "timestamp": {
- "seconds": TIMESTAMP,
- "microseconds": TIMESTAMP
- },
- "event": "RESET"
-}
-{
- "return": [
- ]
-}
-{
- "return": {
- }
-}
-{
- "timestamp": {
- "seconds": TIMESTAMP,
- "microseconds": TIMESTAMP
- },
- "event": "SHUTDOWN"
-}
+QMP_VERSION
+{"return": {}}
+{"return": "OK\r\n"}
+{"return": [{"device": "disk", "locked": false, "removable": true, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "image": {"virtual-size": 134217728, "filename": "TEST_DIR/t.qcow2", "cluster-size": 65536, "format": "qcow2", "actual-size": SIZE, "format-specific": {"type": "qcow2", "data": {"compat": "1.1", "lazy-refcounts": false, "refcount-bits": 16, "corrupt": false}}, "dirty-flag": false}, "iops_wr": 0, "ro": false, "node-name": "NODE_NAME", "backing_file_depth": 0, "drv": "qcow2", "iops": 0, "bps_wr": 0, "write_threshold": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "cache": {"no-flush": false, "direct": false, "writeback": true}, "file": "TEST_DIR/t.qcow2", "encryption_key_missing": false}, "type": "unknown"}]}
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"return": []}
+{"return": {}}
=== blockdev_add/device_add and device_del ===
Testing:
-{
- QMP_VERSION
-}
-{
- "return": {
- }
-}
-{
- "return": {
- }
-}
-{
- "return": [
- {
- "device": "disk",
- "locked": false,
- "removable": true,
- "inserted": {
- "iops_rd": 0,
- "detect_zeroes": "off",
- "image": {
- "virtual-size": 134217728,
- "filename": "TEST_DIR/t.qcow2",
- "cluster-size": 65536,
- "format": "qcow2",
- "actual-size": SIZE,
- "format-specific": {
- "type": "qcow2",
- "data": {
- "compat": "1.1",
- "lazy-refcounts": false,
- "refcount-bits": 16,
- "corrupt": false
- }
- },
- "dirty-flag": false
- },
- "iops_wr": 0,
- "ro": false,
- "node-name": "NODE_NAME",
- "backing_file_depth": 0,
- "drv": "qcow2",
- "iops": 0,
- "bps_wr": 0,
- "write_threshold": 0,
- "encrypted": false,
- "bps": 0,
- "bps_rd": 0,
- "cache": {
- "no-flush": false,
- "direct": false,
- "writeback": true
- },
- "file": "TEST_DIR/t.qcow2",
- "encryption_key_missing": false
- },
- "type": "unknown"
- }
- ]
-}
-{
- "return": {
- }
-}
-{
- "return": {
- }
-}
-{
- "return": {
- }
-}
-{
- "timestamp": {
- "seconds": TIMESTAMP,
- "microseconds": TIMESTAMP
- },
- "event": "DEVICE_DELETED",
- "data": {
- "path": "/machine/peripheral/virtio0/virtio-backend"
- }
-}
-{
- "timestamp": {
- "seconds": TIMESTAMP,
- "microseconds": TIMESTAMP
- },
- "event": "DEVICE_DELETED",
- "data": {
- "device": "virtio0",
- "path": "/machine/peripheral/virtio0"
- }
-}
-{
- "timestamp": {
- "seconds": TIMESTAMP,
- "microseconds": TIMESTAMP
- },
- "event": "RESET"
-}
-{
- "return": [
- {
- "io-status": "ok",
- "device": "disk",
- "locked": false,
- "removable": true,
- "inserted": {
- "iops_rd": 0,
- "detect_zeroes": "off",
- "image": {
- "virtual-size": 134217728,
- "filename": "TEST_DIR/t.qcow2",
- "cluster-size": 65536,
- "format": "qcow2",
- "actual-size": SIZE,
- "format-specific": {
- "type": "qcow2",
- "data": {
- "compat": "1.1",
- "lazy-refcounts": false,
- "refcount-bits": 16,
- "corrupt": false
- }
- },
- "dirty-flag": false
- },
- "iops_wr": 0,
- "ro": false,
- "node-name": "NODE_NAME",
- "backing_file_depth": 0,
- "drv": "qcow2",
- "iops": 0,
- "bps_wr": 0,
- "write_threshold": 0,
- "encrypted": false,
- "bps": 0,
- "bps_rd": 0,
- "cache": {
- "no-flush": false,
- "direct": false,
- "writeback": true
- },
- "file": "TEST_DIR/t.qcow2",
- "encryption_key_missing": false
- },
- "type": "unknown"
- }
- ]
-}
-{
- "return": {
- }
-}
-{
- "timestamp": {
- "seconds": TIMESTAMP,
- "microseconds": TIMESTAMP
- },
- "event": "SHUTDOWN"
-}
+QMP_VERSION
+{"return": {}}
+{"return": {}}
+{"return": [{"device": "disk", "locked": false, "removable": true, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "image": {"virtual-size": 134217728, "filename": "TEST_DIR/t.qcow2", "cluster-size": 65536, "format": "qcow2", "actual-size": SIZE, "format-specific": {"type": "qcow2", "data": {"compat": "1.1", "lazy-refcounts": false, "refcount-bits": 16, "corrupt": false}}, "dirty-flag": false}, "iops_wr": 0, "ro": false, "node-name": "NODE_NAME", "backing_file_depth": 0, "drv": "qcow2", "iops": 0, "bps_wr": 0, "write_threshold": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "cache": {"no-flush": false, "direct": false, "writeback": true}, "file": "TEST_DIR/t.qcow2", "encryption_key_missing": false}, "type": "unknown"}]}
+{"return": {}}
+{"return": {}}
+{"return": {}}
+{"return": [{"io-status": "ok", "device": "disk", "locked": false, "removable": true, "inserted": {"iops_rd": 0, "detect_zeroes": "off", "image": {"virtual-size": 134217728, "filename": "TEST_DIR/t.qcow2", "cluster-size": 65536, "format": "qcow2", "actual-size": SIZE, "format-specific": {"type": "qcow2", "data": {"compat": "1.1", "lazy-refcounts": false, "refcount-bits": 16, "corrupt": false}}, "dirty-flag": false}, "iops_wr": 0, "ro": false, "node-name": "NODE_NAME", "backing_file_depth": 0, "drv": "qcow2", "iops": 0, "bps_wr": 0, "write_threshold": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "cache": {"no-flush": false, "direct": false, "writeback": true}, "file": "TEST_DIR/t.qcow2", "encryption_key_missing": false}, "type": "unknown"}]}
+{"return": {}}
*** done
--
2.1.4
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] qemu-iotests: add _filter_qmp_events() for filtering out QMP events
2016-02-09 13:23 ` [Qemu-devel] [PATCH 1/2] qemu-iotests: add _filter_qmp_events() for filtering out QMP events Sascha Silbe
@ 2016-02-10 16:43 ` Max Reitz
2016-02-10 18:52 ` Sascha Silbe
0 siblings, 1 reply; 7+ messages in thread
From: Max Reitz @ 2016-02-10 16:43 UTC (permalink / raw)
To: Sascha Silbe, qemu-devel, Kevin Wolf, qemu-block; +Cc: Tu Bo
[-- Attachment #1: Type: text/plain, Size: 1786 bytes --]
On 09.02.2016 14:23, Sascha Silbe wrote:
> The order of some QMP events may depend on the architecture being
> tested. Add support for filtering out QMP events so we can use a
> single reference output for all architecture when the test doesn't
> care about the events.
>
> Signed-off-by: Sascha Silbe <silbe@linux.vnet.ibm.com>
> ---
> tests/qemu-iotests/common.filter | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter
> index 84b7434..b908aa2 100644
> --- a/tests/qemu-iotests/common.filter
> +++ b/tests/qemu-iotests/common.filter
> @@ -178,6 +178,12 @@ _filter_qmp()
> -e ' QMP_VERSION'
> }
>
> +# remove QMP events from output
> +_filter_qmp_events()
> +{
> + sed -e '/^{\(.*, \)"event": ".*}$/ d'
> +}
There is a pretty good reason test 067 uses -qmp-pretty (as you yourself
say, the lines get pretty long otherwise, and if we have any change
within, the whole line needs to be changed). Using the following ugly
piece of code here instead, we would still be able to use it:
tr '\n' '\t' \
| sed -e
's/{\s*"timestamp":\s*{[^}]*},\s*"event":[^,}]*\(,\s*"data":\s*{[^}]*}\)\?\s*}\s*//g'
\
| tr '\t' '\n'
(I'm too lazy for multi-line sed, obviously; and this will break if the
data contains any JSON objects in turn.)
The correct way would be to actually parse the JSON (using perl, python
or whatever) and remove all the top-level objects containing an "event"
key, obviously... But that's probably too much.
I'm not strictly against just dropping -qmp-pretty in 067, but there is
a good reason it's there.
Max
> +
> # replace driver-specific options in the "Formatting..." line
> _filter_img_create()
> {
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] qemu-iotests: add _filter_qmp_events() for filtering out QMP events
2016-02-10 16:43 ` Max Reitz
@ 2016-02-10 18:52 ` Sascha Silbe
2016-02-10 22:14 ` Eric Blake
0 siblings, 1 reply; 7+ messages in thread
From: Sascha Silbe @ 2016-02-10 18:52 UTC (permalink / raw)
To: Max Reitz, qemu-devel, Kevin Wolf, qemu-block; +Cc: Tu Bo
Dear Max,
Max Reitz <mreitz@redhat.com> writes:
>> +# remove QMP events from output
>> +_filter_qmp_events()
>> +{
>> + sed -e '/^{\(.*, \)"event": ".*}$/ d'
>> +}
>
> There is a pretty good reason test 067 uses -qmp-pretty (as you yourself
> say, the lines get pretty long otherwise, and if we have any change
> within, the whole line needs to be changed).
Additionally, it's a lot easier to read when indented properly,
especially with the block info containing nested dicts.
> Using the following ugly
> piece of code here instead, we would still be able to use it:
>
> tr '\n' '\t' \
> | sed -e
> 's/{\s*"timestamp":\s*{[^}]*},\s*"event":[^,}]*\(,\s*"data":\s*{[^}]*}\)\?\s*}\s*//g'
> \
> | tr '\t' '\n'
Nice trick. Why didn't I come up with it? ;)
It definitely is a bit ugly, though. We can't just drop the entire line
(using "d") as the entire stream now is a single line. Matching
parenthesis pairs is context sensitive, so we can't just use regular
expressions to aggregate results into lines. And before I start
implementing a JSON indenter in awk, I'd rather rewrite the whole test
in Python. So if we stay with the shell test for now, we need something
like your incantation above. It's not perfect, but good enough for now
and I can't think of anything significantly simpler right now either.
Will test your version and send a v2. Thanks for the suggestion!
Sascha
--
Softwareentwicklung Sascha Silbe, Niederhofenstraße 5/1, 71229 Leonberg
https://se-silbe.de/
USt-IdNr. DE281696641
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] qemu-iotests: add _filter_qmp_events() for filtering out QMP events
2016-02-10 18:52 ` Sascha Silbe
@ 2016-02-10 22:14 ` Eric Blake
2016-02-11 16:54 ` Sascha Silbe
0 siblings, 1 reply; 7+ messages in thread
From: Eric Blake @ 2016-02-10 22:14 UTC (permalink / raw)
To: Sascha Silbe, Max Reitz, qemu-devel, Kevin Wolf, qemu-block; +Cc: Tu Bo
[-- Attachment #1: Type: text/plain, Size: 2031 bytes --]
On 02/10/2016 11:52 AM, Sascha Silbe wrote:
> Dear Max,
>
> Max Reitz <mreitz@redhat.com> writes:
>
>>> +# remove QMP events from output
>>> +_filter_qmp_events()
>>> +{
>>> + sed -e '/^{\(.*, \)"event": ".*}$/ d'
>>> +}
>>
>> There is a pretty good reason test 067 uses -qmp-pretty (as you yourself
>> say, the lines get pretty long otherwise, and if we have any change
>> within, the whole line needs to be changed).
>
> Additionally, it's a lot easier to read when indented properly,
> especially with the block info containing nested dicts.
>
>> Using the following ugly
>> piece of code here instead, we would still be able to use it:
>>
>> tr '\n' '\t' \
>> | sed -e
>> 's/{\s*"timestamp":\s*{[^}]*},\s*"event":[^,}]*\(,\s*"data":\s*{[^}]*}\)\?\s*}\s*//g'
>> \
>> | tr '\t' '\n'
>
> Nice trick. Why didn't I come up with it? ;)
Mishandles any event whose data includes nested dicts. But a little bit
more creativity can do it in two passes (the first modifies the stream
to mark the start of an event, the second then does multiline matching
to nuke the entire event):
tr '\n' '\t' |
sed 's/^{\(\s*"timestamp":\s*{[^}]*},\s*"event":\)/{MARK\1/g' |
tr '\t' '\n' |
sed '/^{MARK/,/^}/d'
> It definitely is a bit ugly, though. We can't just drop the entire line
> (using "d") as the entire stream now is a single line. Matching
> parenthesis pairs is context sensitive, so we can't just use regular
> expressions to aggregate results into lines. And before I start
> implementing a JSON indenter in awk, I'd rather rewrite the whole test
> in Python. So if we stay with the shell test for now, we need something
> like your incantation above. It's not perfect, but good enough for now
> and I can't think of anything significantly simpler right now either.
>
> Will test your version and send a v2. Thanks for the suggestion!
>
> Sascha
>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] qemu-iotests: add _filter_qmp_events() for filtering out QMP events
2016-02-10 22:14 ` Eric Blake
@ 2016-02-11 16:54 ` Sascha Silbe
0 siblings, 0 replies; 7+ messages in thread
From: Sascha Silbe @ 2016-02-11 16:54 UTC (permalink / raw)
To: Eric Blake, Max Reitz, qemu-devel, Kevin Wolf, qemu-block; +Cc: Tu Bo
Dear Eric,
Eric Blake <eblake@redhat.com> writes:
>>> tr '\n' '\t' \
>>> | sed -e
>>> 's/{\s*"timestamp":\s*{[^}]*},\s*"event":[^,}]*\(,\s*"data":\s*{[^}]*}\)\?\s*}\s*//g'
>>> \
>>> | tr '\t' '\n'
>>
>> Nice trick. Why didn't I come up with it? ;)
>
> Mishandles any event whose data includes nested dicts. But a little bit
> more creativity can do it in two passes (the first modifies the stream
> to mark the start of an event, the second then does multiline matching
> to nuke the entire event):
>
> tr '\n' '\t' |
> sed 's/^{\(\s*"timestamp":\s*{[^}]*},\s*"event":\)/{MARK\1/g' |
> tr '\t' '\n' |
> sed '/^{MARK/,/^}/d'
This is starting to get too complex for my taste. If
_filter_qmp_events() isn't generic enough to be used by other test
cases, I can make it internal to 067. Alternatively I can port 067 to
Python, which can handle JSON data types natively with no need for
post-processing hacks.
Sascha
--
Softwareentwicklung Sascha Silbe, Niederhofenstraße 5/1, 71229 Leonberg
https://se-silbe.de/
USt-IdNr. DE281696641
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2016-02-11 16:54 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-09 13:23 [Qemu-devel] [PATCH 0/2] qemu-iotests: fix 067 on s390x Sascha Silbe
2016-02-09 13:23 ` [Qemu-devel] [PATCH 1/2] qemu-iotests: add _filter_qmp_events() for filtering out QMP events Sascha Silbe
2016-02-10 16:43 ` Max Reitz
2016-02-10 18:52 ` Sascha Silbe
2016-02-10 22:14 ` Eric Blake
2016-02-11 16:54 ` Sascha Silbe
2016-02-09 13:23 ` [Qemu-devel] [PATCH 2/2] qemu-iotests: 067: ignore " Sascha Silbe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).