* [Qemu-devel] [PATCH v4 0/3] chardev: Add -qmp-pretty
@ 2014-11-17 12:31 Max Reitz
2014-11-17 12:31 ` [Qemu-devel] [PATCH v4 1/3] " Max Reitz
` (4 more replies)
0 siblings, 5 replies; 15+ messages in thread
From: Max Reitz @ 2014-11-17 12:31 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, Stefan Hajnoczi, Max Reitz
This series does not add new functionality. Adding a QMP monitor with
prettily formatted JSON output can be done as follows:
$ qemu -chardev stdio,id=mon0 -mon chardev=mon0,mode=control,pretty=on
However, this is rather cumbersome, so this series (its first patch)
adds a shortcut in the form of the new command line option -qmp-pretty.
Since the argument given to a monitor command line option (such as -qmp)
is parsed depending on its prefix and probably also depending on the
current phase of the moon, this is cleaner than trying to add a "switch"
to -qmp itself (in the form of "-qmp stdio,pretty=on").
Patch 3 makes uses of the new option in qemu-iotest 067 to greatly
increase maintainability of its reference output. Patch 2 extends the
QMP filter for qemu-iotests so it is able to filter out the QMP version
object in pretty mode.
v4:
- Patch 2: Add newline in sed script after c\ [Eric]
v3:
- Patch 2: Cull useless "discard=0"
v2:
- Patch 2: Replaced the multi-line QMP_VERSION replacement written in
bash by a nice sed script [Eric]
git-backport-diff against v3:
Key:
[----] : patches are identical
[####] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively
001/3:[----] [--] 'chardev: Add -qmp-pretty'
002/3:[0003] [FC] 'iotests: _filter_qmp for pretty JSON output'
003/3:[----] [--] 'iotests: Use -qmp-pretty in 067'
Max Reitz (3):
chardev: Add -qmp-pretty
iotests: _filter_qmp for pretty JSON output
iotests: Use -qmp-pretty in 067
qemu-options.hx | 8 +
tests/qemu-iotests/067 | 2 +-
tests/qemu-iotests/067.out | 779 ++++++++++++++++++++++++++++++++++++---
tests/qemu-iotests/common.filter | 4 +-
vl.c | 15 +-
5 files changed, 744 insertions(+), 64 deletions(-)
--
1.9.3
^ permalink raw reply [flat|nested] 15+ messages in thread* [Qemu-devel] [PATCH v4 1/3] chardev: Add -qmp-pretty 2014-11-17 12:31 [Qemu-devel] [PATCH v4 0/3] chardev: Add -qmp-pretty Max Reitz @ 2014-11-17 12:31 ` Max Reitz 2014-11-17 12:31 ` [Qemu-devel] [PATCH v4 2/3] iotests: _filter_qmp for pretty JSON output Max Reitz ` (3 subsequent siblings) 4 siblings, 0 replies; 15+ messages in thread From: Max Reitz @ 2014-11-17 12:31 UTC (permalink / raw) To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, Stefan Hajnoczi, Max Reitz Add a command line option for adding a QMP monitor using pretty JSON formatting. Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> --- qemu-options.hx | 8 ++++++++ vl.c | 15 ++++++++++----- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/qemu-options.hx b/qemu-options.hx index da9851d..bc7b4c2 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -2788,6 +2788,14 @@ STEXI @findex -qmp Like -monitor but opens in 'control' mode. ETEXI +DEF("qmp-pretty", HAS_ARG, QEMU_OPTION_qmp_pretty, \ + "-qmp-pretty dev like -qmp but uses pretty JSON formatting\n", + QEMU_ARCH_ALL) +STEXI +@item -qmp-pretty @var{dev} +@findex -qmp-pretty +Like -qmp but uses pretty JSON formatting. +ETEXI DEF("mon", HAS_ARG, QEMU_OPTION_mon, \ "-mon [chardev=]name[,mode=readline|control][,default]\n", QEMU_ARCH_ALL) diff --git a/vl.c b/vl.c index f4a6e5e..c7bebad 100644 --- a/vl.c +++ b/vl.c @@ -2284,7 +2284,7 @@ static int mon_init_func(QemuOpts *opts, void *opaque) return 0; } -static void monitor_parse(const char *optarg, const char *mode) +static void monitor_parse(const char *optarg, const char *mode, bool pretty) { static int monitor_device_index = 0; QemuOpts *opts; @@ -2314,6 +2314,7 @@ static void monitor_parse(const char *optarg, const char *mode) } qemu_opt_set(opts, "mode", mode); qemu_opt_set(opts, "chardev", label); + qemu_opt_set_bool(opts, "pretty", pretty); if (def) qemu_opt_set(opts, "default", "on"); monitor_device_index++; @@ -3292,11 +3293,15 @@ int main(int argc, char **argv, char **envp) case QEMU_OPTION_monitor: default_monitor = 0; if (strncmp(optarg, "none", 4)) { - monitor_parse(optarg, "readline"); + monitor_parse(optarg, "readline", false); } break; case QEMU_OPTION_qmp: - monitor_parse(optarg, "control"); + monitor_parse(optarg, "control", false); + default_monitor = 0; + break; + case QEMU_OPTION_qmp_pretty: + monitor_parse(optarg, "control", true); default_monitor = 0; break; case QEMU_OPTION_mon: @@ -3994,7 +3999,7 @@ int main(int argc, char **argv, char **envp) add_device_config(DEV_SCLP, "stdio"); } if (default_monitor) - monitor_parse("stdio", "readline"); + monitor_parse("stdio", "readline", false); } } else { if (default_serial) @@ -4002,7 +4007,7 @@ int main(int argc, char **argv, char **envp) if (default_parallel) add_device_config(DEV_PARALLEL, "vc:80Cx24C"); if (default_monitor) - monitor_parse("vc:80Cx24C", "readline"); + monitor_parse("vc:80Cx24C", "readline", false); if (default_virtcon) add_device_config(DEV_VIRTCON, "vc:80Cx24C"); if (default_sclp) { -- 1.9.3 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH v4 2/3] iotests: _filter_qmp for pretty JSON output 2014-11-17 12:31 [Qemu-devel] [PATCH v4 0/3] chardev: Add -qmp-pretty Max Reitz 2014-11-17 12:31 ` [Qemu-devel] [PATCH v4 1/3] " Max Reitz @ 2014-11-17 12:31 ` Max Reitz 2014-11-17 16:04 ` Eric Blake 2014-11-17 12:31 ` [Qemu-devel] [PATCH v4 3/3] iotests: Use -qmp-pretty in 067 Max Reitz ` (2 subsequent siblings) 4 siblings, 1 reply; 15+ messages in thread From: Max Reitz @ 2014-11-17 12:31 UTC (permalink / raw) To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, Stefan Hajnoczi, Max Reitz _filter_qmp should be able to correctly filter out the QMP version object for pretty JSON output. Signed-off-by: Max Reitz <mreitz@redhat.com> --- tests/qemu-iotests/common.filter | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter index 3acdb30..dfb9726 100644 --- a/tests/qemu-iotests/common.filter +++ b/tests/qemu-iotests/common.filter @@ -167,7 +167,9 @@ _filter_qmp() { _filter_win32 | \ sed -e 's#\("\(micro\)\?seconds": \)[0-9]\+#\1 TIMESTAMP#g' \ - -e 's#^{"QMP":.*}$#QMP_VERSION#' + -e 's#^{"QMP":.*}$#QMP_VERSION#' \ + -e '/^ "QMP": {\s*$/, /^ }\s*$/ c\' \ + -e ' QMP_VERSION' } # replace driver-specific options in the "Formatting..." line -- 1.9.3 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/3] iotests: _filter_qmp for pretty JSON output 2014-11-17 12:31 ` [Qemu-devel] [PATCH v4 2/3] iotests: _filter_qmp for pretty JSON output Max Reitz @ 2014-11-17 16:04 ` Eric Blake 2014-11-17 16:14 ` Max Reitz 0 siblings, 1 reply; 15+ messages in thread From: Eric Blake @ 2014-11-17 16:04 UTC (permalink / raw) To: Max Reitz, qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, Stefan Hajnoczi [-- Attachment #1: Type: text/plain, Size: 1341 bytes --] On 11/17/2014 05:31 AM, Max Reitz wrote: > _filter_qmp should be able to correctly filter out the QMP version > object for pretty JSON output. > > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > tests/qemu-iotests/common.filter | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter > index 3acdb30..dfb9726 100644 > --- a/tests/qemu-iotests/common.filter > +++ b/tests/qemu-iotests/common.filter > @@ -167,7 +167,9 @@ _filter_qmp() > { > _filter_win32 | \ > sed -e 's#\("\(micro\)\?seconds": \)[0-9]\+#\1 TIMESTAMP#g' \ > - -e 's#^{"QMP":.*}$#QMP_VERSION#' > + -e 's#^{"QMP":.*}$#QMP_VERSION#' \ > + -e '/^ "QMP": {\s*$/, /^ }\s*$/ c\' \ \s is a GNU sed extension. But we don't really need to care about whitespace to the end of the line; I think that it is sufficient to just match the following regex: -e '/^ "QMP": {/, /^ }/ c\' \ > + -e ' QMP_VERSION' Either way, it's not the first time we've used a GNU sed extension, and since other series are now depending on this one, I can live with: Reviewed-by: Eric Blake <eblake@redhat.com> -- 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: 539 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/3] iotests: _filter_qmp for pretty JSON output 2014-11-17 16:04 ` Eric Blake @ 2014-11-17 16:14 ` Max Reitz 2014-11-17 17:06 ` Eric Blake 0 siblings, 1 reply; 15+ messages in thread From: Max Reitz @ 2014-11-17 16:14 UTC (permalink / raw) To: Eric Blake, qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, Stefan Hajnoczi On 2014-11-17 at 17:04, Eric Blake wrote: > On 11/17/2014 05:31 AM, Max Reitz wrote: >> _filter_qmp should be able to correctly filter out the QMP version >> object for pretty JSON output. >> >> Signed-off-by: Max Reitz <mreitz@redhat.com> >> --- >> tests/qemu-iotests/common.filter | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter >> index 3acdb30..dfb9726 100644 >> --- a/tests/qemu-iotests/common.filter >> +++ b/tests/qemu-iotests/common.filter >> @@ -167,7 +167,9 @@ _filter_qmp() >> { >> _filter_win32 | \ >> sed -e 's#\("\(micro\)\?seconds": \)[0-9]\+#\1 TIMESTAMP#g' \ >> - -e 's#^{"QMP":.*}$#QMP_VERSION#' >> + -e 's#^{"QMP":.*}$#QMP_VERSION#' \ >> + -e '/^ "QMP": {\s*$/, /^ }\s*$/ c\' \ > \s is a GNU sed extension. But we don't really need to care about > whitespace to the end of the line; I think that it is sufficient to just > match the following regex: > > -e '/^ "QMP": {/, /^ }/ c\' \ That doesn't match, however. QEMU's JSON formatter sometimes outputs whitespace at the end of line. >> + -e ' QMP_VERSION' > Either way, it's not the first time we've used a GNU sed extension, and > since other series are now depending on this one, I can live with: Ooooh, you mean version 3 was actually fine? (which used 'c\' without a line break) ;-) > Reviewed-by: Eric Blake <eblake@redhat.com> Thank you! Max ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH v4 2/3] iotests: _filter_qmp for pretty JSON output 2014-11-17 16:14 ` Max Reitz @ 2014-11-17 17:06 ` Eric Blake 0 siblings, 0 replies; 15+ messages in thread From: Eric Blake @ 2014-11-17 17:06 UTC (permalink / raw) To: Max Reitz, qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, Stefan Hajnoczi [-- Attachment #1: Type: text/plain, Size: 1464 bytes --] On 11/17/2014 09:14 AM, Max Reitz wrote: >>> + -e '/^ "QMP": {\s*$/, /^ }\s*$/ c\' \ >> \s is a GNU sed extension. But we don't really need to care about >> whitespace to the end of the line; I think that it is sufficient to just >> match the following regex: >> >> -e '/^ "QMP": {/, /^ }/ c\' \ > > That doesn't match, however. QEMU's JSON formatter sometimes outputs > whitespace at the end of line. No, I explicitly omitted the $. The lines match because they are anchored to the beginning, regardless of trailing whitespace at the end. > >>> + -e ' QMP_VERSION' >> Either way, it's not the first time we've used a GNU sed extension, and >> since other series are now depending on this one, I can live with: > > Ooooh, you mean version 3 was actually fine? (which used 'c\' without a > line break) ;-) I'm more comfortable with v4 than v3 (\s is well-known, and can be easily converted into an alternative if someone proves their system doesn't support it. But c\ is not well-known; and reading 'info sed' to learn about c\ only documents its use with a newline; so using c\ without a newline is relying on undocumented GNU behavior, which is very risky). Of course, a v5 that avoids \s would avoid any confusion, but now we're bikeshedding, and I'm not sure it's worth the time. -- 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: 539 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH v4 3/3] iotests: Use -qmp-pretty in 067 2014-11-17 12:31 [Qemu-devel] [PATCH v4 0/3] chardev: Add -qmp-pretty Max Reitz 2014-11-17 12:31 ` [Qemu-devel] [PATCH v4 1/3] " Max Reitz 2014-11-17 12:31 ` [Qemu-devel] [PATCH v4 2/3] iotests: _filter_qmp for pretty JSON output Max Reitz @ 2014-11-17 12:31 ` Max Reitz 2014-11-20 18:39 ` Kevin Wolf 2014-11-20 17:02 ` [Qemu-devel] [PATCH v4 0/3] chardev: Add -qmp-pretty Kevin Wolf 2014-11-28 15:55 ` Markus Armbruster 4 siblings, 1 reply; 15+ messages in thread From: Max Reitz @ 2014-11-17 12:31 UTC (permalink / raw) To: qemu-devel; +Cc: Kevin Wolf, Paolo Bonzini, Stefan Hajnoczi, Max Reitz 067 invokes query-block, resulting in a reference output with really long lines (which may pose a problem in email patches and always poses a problem when the output changes, because it is hard to see what has actually changed). Use -qmp-pretty to mitigate this issue. Signed-off-by: Max Reitz <mreitz@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> --- tests/qemu-iotests/067 | 2 +- tests/qemu-iotests/067.out | 779 +++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 723 insertions(+), 58 deletions(-) diff --git a/tests/qemu-iotests/067 b/tests/qemu-iotests/067 index d025192..29cd6b5 100755 --- a/tests/qemu-iotests/067 +++ b/tests/qemu-iotests/067 @@ -39,7 +39,7 @@ _supported_os Linux function do_run_qemu() { echo Testing: "$@" - $QEMU -nographic -qmp stdio -serial none "$@" + $QEMU -nographic -qmp-pretty stdio -serial none "$@" echo } diff --git a/tests/qemu-iotests/067.out b/tests/qemu-iotests/067.out index 0f72dcf..929dc74 100644 --- a/tests/qemu-iotests/067.out +++ b/tests/qemu-iotests/067.out @@ -4,77 +4,742 @@ 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-pci,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, "corrupt": false}}, "dirty-flag": false}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "qcow2", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_DIR/t.qcow2", "encryption_key_missing": false}, "type": "unknown"}, {"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": 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": [{"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}]} -{"return": {}} -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN"} -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "ide1-cd0", "tray-open": true}} -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "floppy0", "tray-open": true}} +{ + 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, + "corrupt": false + } + }, + "dirty-flag": false + }, + "iops_wr": 0, + "ro": false, + "backing_file_depth": 0, + "drv": "qcow2", + "iops": 0, + "bps_wr": 0, + "encrypted": false, + "bps": 0, + "bps_rd": 0, + "file": "TEST_DIR/t.qcow2", + "encryption_key_missing": false + }, + "type": "unknown" + }, + { + "io-status": "ok", + "device": "ide1-cd0", + "locked": false, + "removable": true, + "tray_open": false, + "type": "unknown" + }, + { + "device": "floppy0", + "locked": false, + "removable": true, + "tray_open": false, + "type": "unknown" + }, + { + "device": "sd0", + "locked": false, + "removable": true, + "tray_open": 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": [ + { + "io-status": "ok", + "device": "ide1-cd0", + "locked": false, + "removable": true, + "tray_open": false, + "type": "unknown" + }, + { + "device": "floppy0", + "locked": false, + "removable": true, + "tray_open": false, + "type": "unknown" + }, + { + "device": "sd0", + "locked": false, + "removable": true, + "tray_open": false, + "type": "unknown" + } + ] +} +{ + "return": { + } +} +{ + "timestamp": { + "seconds": TIMESTAMP, + "microseconds": TIMESTAMP + }, + "event": "SHUTDOWN" +} +{ + "timestamp": { + "seconds": TIMESTAMP, + "microseconds": TIMESTAMP + }, + "event": "DEVICE_TRAY_MOVED", + "data": { + "device": "ide1-cd0", + "tray-open": true + } +} +{ + "timestamp": { + "seconds": TIMESTAMP, + "microseconds": TIMESTAMP + }, + "event": "DEVICE_TRAY_MOVED", + "data": { + "device": "floppy0", + "tray-open": true + } +} === -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, "corrupt": false}}, "dirty-flag": false}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "qcow2", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_DIR/t.qcow2", "encryption_key_missing": false}, "tray_open": false, "type": "unknown"}, {"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": 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": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}]} -{"return": {}} -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN"} -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "ide1-cd0", "tray-open": true}} -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "floppy0", "tray-open": true}} +{ + 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, + "corrupt": false + } + }, + "dirty-flag": false + }, + "iops_wr": 0, + "ro": false, + "backing_file_depth": 0, + "drv": "qcow2", + "iops": 0, + "bps_wr": 0, + "encrypted": false, + "bps": 0, + "bps_rd": 0, + "file": "TEST_DIR/t.qcow2", + "encryption_key_missing": false + }, + "tray_open": false, + "type": "unknown" + }, + { + "io-status": "ok", + "device": "ide1-cd0", + "locked": false, + "removable": true, + "tray_open": false, + "type": "unknown" + }, + { + "device": "floppy0", + "locked": false, + "removable": true, + "tray_open": false, + "type": "unknown" + }, + { + "device": "sd0", + "locked": false, + "removable": true, + "tray_open": 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": "ide1-cd0", + "locked": false, + "removable": true, + "tray_open": false, + "type": "unknown" + }, + { + "device": "floppy0", + "locked": false, + "removable": true, + "tray_open": false, + "type": "unknown" + }, + { + "device": "sd0", + "locked": false, + "removable": true, + "tray_open": false, + "type": "unknown" + } + ] +} +{ + "return": { + } +} +{ + "timestamp": { + "seconds": TIMESTAMP, + "microseconds": TIMESTAMP + }, + "event": "SHUTDOWN" +} +{ + "timestamp": { + "seconds": TIMESTAMP, + "microseconds": TIMESTAMP + }, + "event": "DEVICE_TRAY_MOVED", + "data": { + "device": "ide1-cd0", + "tray-open": true + } +} +{ + "timestamp": { + "seconds": TIMESTAMP, + "microseconds": TIMESTAMP + }, + "event": "DEVICE_TRAY_MOVED", + "data": { + "device": "floppy0", + "tray-open": true + } +} === drive_add/device_add and device_del === Testing: -QMP_VERSION -{"return": {}} -{"return": "OK\r\n"} -{"return": [{"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"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, "corrupt": false}}, "dirty-flag": false}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "qcow2", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_DIR/t.qcow2", "encryption_key_missing": false}, "tray_open": 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": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}]} -{"return": {}} -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN"} -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "ide1-cd0", "tray-open": true}} -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "floppy0", "tray-open": true}} +{ + QMP_VERSION +} +{ + "return": { + } +} +{ + "return": "OK\r\n" +} +{ + "return": [ + { + "io-status": "ok", + "device": "ide1-cd0", + "locked": false, + "removable": true, + "tray_open": false, + "type": "unknown" + }, + { + "device": "floppy0", + "locked": false, + "removable": true, + "tray_open": false, + "type": "unknown" + }, + { + "device": "sd0", + "locked": false, + "removable": true, + "tray_open": false, + "type": "unknown" + }, + { + "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, + "corrupt": false + } + }, + "dirty-flag": false + }, + "iops_wr": 0, + "ro": false, + "backing_file_depth": 0, + "drv": "qcow2", + "iops": 0, + "bps_wr": 0, + "encrypted": false, + "bps": 0, + "bps_rd": 0, + "file": "TEST_DIR/t.qcow2", + "encryption_key_missing": false + }, + "tray_open": 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": "ide1-cd0", + "locked": false, + "removable": true, + "tray_open": false, + "type": "unknown" + }, + { + "device": "floppy0", + "locked": false, + "removable": true, + "tray_open": false, + "type": "unknown" + }, + { + "device": "sd0", + "locked": false, + "removable": true, + "tray_open": false, + "type": "unknown" + } + ] +} +{ + "return": { + } +} +{ + "timestamp": { + "seconds": TIMESTAMP, + "microseconds": TIMESTAMP + }, + "event": "SHUTDOWN" +} +{ + "timestamp": { + "seconds": TIMESTAMP, + "microseconds": TIMESTAMP + }, + "event": "DEVICE_TRAY_MOVED", + "data": { + "device": "ide1-cd0", + "tray-open": true + } +} +{ + "timestamp": { + "seconds": TIMESTAMP, + "microseconds": TIMESTAMP + }, + "event": "DEVICE_TRAY_MOVED", + "data": { + "device": "floppy0", + "tray-open": true + } +} === blockdev_add/device_add and device_del === Testing: -QMP_VERSION -{"return": {}} -{"return": {}} -{"return": [{"io-status": "ok", "device": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"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, "corrupt": false}}, "dirty-flag": false}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "qcow2", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_DIR/t.qcow2", "encryption_key_missing": false}, "tray_open": 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": "ide1-cd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "floppy0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"device": "sd0", "locked": false, "removable": true, "tray_open": false, "type": "unknown"}, {"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, "corrupt": false}}, "dirty-flag": false}, "iops_wr": 0, "ro": false, "backing_file_depth": 0, "drv": "qcow2", "iops": 0, "bps_wr": 0, "encrypted": false, "bps": 0, "bps_rd": 0, "file": "TEST_DIR/t.qcow2", "encryption_key_missing": false}, "tray_open": false, "type": "unknown"}]} -{"return": {}} -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "SHUTDOWN"} -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "ide1-cd0", "tray-open": true}} -{"timestamp": {"seconds": TIMESTAMP, "microseconds": TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "floppy0", "tray-open": true}} +{ + QMP_VERSION +} +{ + "return": { + } +} +{ + "return": { + } +} +{ + "return": [ + { + "io-status": "ok", + "device": "ide1-cd0", + "locked": false, + "removable": true, + "tray_open": false, + "type": "unknown" + }, + { + "device": "floppy0", + "locked": false, + "removable": true, + "tray_open": false, + "type": "unknown" + }, + { + "device": "sd0", + "locked": false, + "removable": true, + "tray_open": false, + "type": "unknown" + }, + { + "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, + "corrupt": false + } + }, + "dirty-flag": false + }, + "iops_wr": 0, + "ro": false, + "backing_file_depth": 0, + "drv": "qcow2", + "iops": 0, + "bps_wr": 0, + "encrypted": false, + "bps": 0, + "bps_rd": 0, + "file": "TEST_DIR/t.qcow2", + "encryption_key_missing": false + }, + "tray_open": 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": "ide1-cd0", + "locked": false, + "removable": true, + "tray_open": false, + "type": "unknown" + }, + { + "device": "floppy0", + "locked": false, + "removable": true, + "tray_open": false, + "type": "unknown" + }, + { + "device": "sd0", + "locked": false, + "removable": true, + "tray_open": false, + "type": "unknown" + }, + { + "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, + "corrupt": false + } + }, + "dirty-flag": false + }, + "iops_wr": 0, + "ro": false, + "backing_file_depth": 0, + "drv": "qcow2", + "iops": 0, + "bps_wr": 0, + "encrypted": false, + "bps": 0, + "bps_rd": 0, + "file": "TEST_DIR/t.qcow2", + "encryption_key_missing": false + }, + "tray_open": false, + "type": "unknown" + } + ] +} +{ + "return": { + } +} +{ + "timestamp": { + "seconds": TIMESTAMP, + "microseconds": TIMESTAMP + }, + "event": "SHUTDOWN" +} +{ + "timestamp": { + "seconds": TIMESTAMP, + "microseconds": TIMESTAMP + }, + "event": "DEVICE_TRAY_MOVED", + "data": { + "device": "ide1-cd0", + "tray-open": true + } +} +{ + "timestamp": { + "seconds": TIMESTAMP, + "microseconds": TIMESTAMP + }, + "event": "DEVICE_TRAY_MOVED", + "data": { + "device": "floppy0", + "tray-open": true + } +} *** done -- 1.9.3 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH v4 3/3] iotests: Use -qmp-pretty in 067 2014-11-17 12:31 ` [Qemu-devel] [PATCH v4 3/3] iotests: Use -qmp-pretty in 067 Max Reitz @ 2014-11-20 18:39 ` Kevin Wolf 2014-11-20 18:56 ` Max Reitz 0 siblings, 1 reply; 15+ messages in thread From: Kevin Wolf @ 2014-11-20 18:39 UTC (permalink / raw) To: Max Reitz; +Cc: Paolo Bonzini, qemu-devel, Stefan Hajnoczi Am 17.11.2014 um 13:31 hat Max Reitz geschrieben: > 067 invokes query-block, resulting in a reference output with really > long lines (which may pose a problem in email patches and always poses a > problem when the output changes, because it is hard to see what has > actually changed). Use -qmp-pretty to mitigate this issue. > > Signed-off-by: Max Reitz <mreitz@redhat.com> > Reviewed-by: Eric Blake <eblake@redhat.com> > --- > tests/qemu-iotests/067 | 2 +- > tests/qemu-iotests/067.out | 779 +++++++++++++++++++++++++++++++++++++++++---- > 2 files changed, 723 insertions(+), 58 deletions(-) Hm, when I tried to rebase my query-block patch that adds cache information, I noticed that I get a trailing space on every line of the JSON dump, which isn't there in this patch. So 'cp 067.out.bad 067.out' ended up changing every single line. :-/ Did you postprocess the reference output or do really get it without trailing whitespace? Kevin ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH v4 3/3] iotests: Use -qmp-pretty in 067 2014-11-20 18:39 ` Kevin Wolf @ 2014-11-20 18:56 ` Max Reitz 2014-11-20 19:43 ` Eric Blake 0 siblings, 1 reply; 15+ messages in thread From: Max Reitz @ 2014-11-20 18:56 UTC (permalink / raw) To: Kevin Wolf; +Cc: Paolo Bonzini, qemu-devel, Stefan Hajnoczi On 20.11.2014 19:39, Kevin Wolf wrote: > Am 17.11.2014 um 13:31 hat Max Reitz geschrieben: >> 067 invokes query-block, resulting in a reference output with really >> long lines (which may pose a problem in email patches and always poses a >> problem when the output changes, because it is hard to see what has >> actually changed). Use -qmp-pretty to mitigate this issue. >> >> Signed-off-by: Max Reitz <mreitz@redhat.com> >> Reviewed-by: Eric Blake <eblake@redhat.com> >> --- >> tests/qemu-iotests/067 | 2 +- >> tests/qemu-iotests/067.out | 779 +++++++++++++++++++++++++++++++++++++++++---- >> 2 files changed, 723 insertions(+), 58 deletions(-) > Hm, when I tried to rebase my query-block patch that adds cache > information, I noticed that I get a trailing space on every line of the > JSON dump, which isn't there in this patch. So 'cp 067.out.bad 067.out' > ended up changing every single line. :-/ > > Did you postprocess the reference output or do really get it without > trailing whitespace? Yes, I post-processed it, because you once complained about me sending patches with trailing whitespace. :-P As I wrote in some reply to some reply from Eric on some version of this patch, QEMU's pretty JSON formatter emits whitespace at the end of some lines. Max ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH v4 3/3] iotests: Use -qmp-pretty in 067 2014-11-20 18:56 ` Max Reitz @ 2014-11-20 19:43 ` Eric Blake 0 siblings, 0 replies; 15+ messages in thread From: Eric Blake @ 2014-11-20 19:43 UTC (permalink / raw) To: Max Reitz, Kevin Wolf; +Cc: Paolo Bonzini, qemu-devel, Stefan Hajnoczi [-- Attachment #1: Type: text/plain, Size: 2028 bytes --] On 11/20/2014 11:56 AM, Max Reitz wrote: > On 20.11.2014 19:39, Kevin Wolf wrote: >> Am 17.11.2014 um 13:31 hat Max Reitz geschrieben: >>> 067 invokes query-block, resulting in a reference output with really >>> long lines (which may pose a problem in email patches and always poses a >>> problem when the output changes, because it is hard to see what has >>> actually changed). Use -qmp-pretty to mitigate this issue. >>> >>> Signed-off-by: Max Reitz <mreitz@redhat.com> >>> Reviewed-by: Eric Blake <eblake@redhat.com> >>> --- >>> tests/qemu-iotests/067 | 2 +- >>> tests/qemu-iotests/067.out | 779 >>> +++++++++++++++++++++++++++++++++++++++++---- >>> 2 files changed, 723 insertions(+), 58 deletions(-) >> Hm, when I tried to rebase my query-block patch that adds cache >> information, I noticed that I get a trailing space on every line of the >> JSON dump, which isn't there in this patch. So 'cp 067.out.bad 067.out' >> ended up changing every single line. :-/ >> >> Did you postprocess the reference output or do really get it without >> trailing whitespace? > > Yes, I post-processed it, because you once complained about me sending > patches with trailing whitespace. :-P > > As I wrote in some reply to some reply from Eric on some version of this > patch, QEMU's pretty JSON formatter emits whitespace at the end of some > lines. Trailing whitespace in canonical sources is annoying and should be avoided. But this file is about trailing whitespace in generated files (such as reference output) and which is therefore worth keeping, unless we fix the cause of the reference material having trailing whitespace in the first place. That way, we can regenerate the file from the same reference source, and won't have needless differences in trailing whitespace. Sounds like a bug in the JSON pretty formatter, but fixing it should be a separate series. -- 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: 539 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH v4 0/3] chardev: Add -qmp-pretty 2014-11-17 12:31 [Qemu-devel] [PATCH v4 0/3] chardev: Add -qmp-pretty Max Reitz ` (2 preceding siblings ...) 2014-11-17 12:31 ` [Qemu-devel] [PATCH v4 3/3] iotests: Use -qmp-pretty in 067 Max Reitz @ 2014-11-20 17:02 ` Kevin Wolf 2014-11-28 15:55 ` Markus Armbruster 4 siblings, 0 replies; 15+ messages in thread From: Kevin Wolf @ 2014-11-20 17:02 UTC (permalink / raw) To: Max Reitz; +Cc: Paolo Bonzini, qemu-devel, Stefan Hajnoczi Am 17.11.2014 um 13:31 hat Max Reitz geschrieben: > This series does not add new functionality. Adding a QMP monitor with > prettily formatted JSON output can be done as follows: > > $ qemu -chardev stdio,id=mon0 -mon chardev=mon0,mode=control,pretty=on > > However, this is rather cumbersome, so this series (its first patch) > adds a shortcut in the form of the new command line option -qmp-pretty. > > Since the argument given to a monitor command line option (such as -qmp) > is parsed depending on its prefix and probably also depending on the > current phase of the moon, this is cleaner than trying to add a "switch" > to -qmp itself (in the form of "-qmp stdio,pretty=on"). > > > Patch 3 makes uses of the new option in qemu-iotest 067 to greatly > increase maintainability of its reference output. Patch 2 extends the > QMP filter for qemu-iotests so it is able to filter out the QMP version > object in pretty mode. Thanks, applied to block-next. Kevin ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH v4 0/3] chardev: Add -qmp-pretty 2014-11-17 12:31 [Qemu-devel] [PATCH v4 0/3] chardev: Add -qmp-pretty Max Reitz ` (3 preceding siblings ...) 2014-11-20 17:02 ` [Qemu-devel] [PATCH v4 0/3] chardev: Add -qmp-pretty Kevin Wolf @ 2014-11-28 15:55 ` Markus Armbruster 2014-11-28 17:21 ` Paolo Bonzini 2014-12-02 9:24 ` Max Reitz 4 siblings, 2 replies; 15+ messages in thread From: Markus Armbruster @ 2014-11-28 15:55 UTC (permalink / raw) To: Max Reitz Cc: Kevin Wolf, Paolo Bonzini, qemu-devel, Stefan Hajnoczi, Luiz Capitulino Copying Luiz. Max Reitz <mreitz@redhat.com> writes: > This series does not add new functionality. Adding a QMP monitor with > prettily formatted JSON output can be done as follows: > > $ qemu -chardev stdio,id=mon0 -mon chardev=mon0,mode=control,pretty=on > > However, this is rather cumbersome, so this series (its first patch) > adds a shortcut in the form of the new command line option -qmp-pretty. > > Since the argument given to a monitor command line option (such as -qmp) > is parsed depending on its prefix and probably also depending on the > current phase of the moon, this is cleaner than trying to add a "switch" > to -qmp itself (in the form of "-qmp stdio,pretty=on"). Yet another "convenience" option *groan* Why can't we simply make -qmp set pretty=on and be done with it? It's a convenience option, i.e. meant for humans, and why would humans *not* want pretty=on? ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH v4 0/3] chardev: Add -qmp-pretty 2014-11-28 15:55 ` Markus Armbruster @ 2014-11-28 17:21 ` Paolo Bonzini 2014-12-02 9:24 ` Max Reitz 1 sibling, 0 replies; 15+ messages in thread From: Paolo Bonzini @ 2014-11-28 17:21 UTC (permalink / raw) To: Markus Armbruster, Max Reitz Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi, Luiz Capitulino On 28/11/2014 16:55, Markus Armbruster wrote: > Copying Luiz. > > Max Reitz <mreitz@redhat.com> writes: > >> This series does not add new functionality. Adding a QMP monitor with >> prettily formatted JSON output can be done as follows: >> >> $ qemu -chardev stdio,id=mon0 -mon chardev=mon0,mode=control,pretty=on >> >> However, this is rather cumbersome, so this series (its first patch) >> adds a shortcut in the form of the new command line option -qmp-pretty. >> >> Since the argument given to a monitor command line option (such as -qmp) >> is parsed depending on its prefix and probably also depending on the >> current phase of the moon, this is cleaner than trying to add a "switch" >> to -qmp itself (in the form of "-qmp stdio,pretty=on"). > > Yet another "convenience" option *groan* > > Why can't we simply make -qmp set pretty=on and be done with it? > It's a convenience option, i.e. meant for humans, and why would humans > *not* want pretty=on? Agreed, it is valid JSON anyway so there is no backwards compatibility issue. Paolo ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH v4 0/3] chardev: Add -qmp-pretty 2014-11-28 15:55 ` Markus Armbruster 2014-11-28 17:21 ` Paolo Bonzini @ 2014-12-02 9:24 ` Max Reitz 2014-12-02 18:17 ` Markus Armbruster 1 sibling, 1 reply; 15+ messages in thread From: Max Reitz @ 2014-12-02 9:24 UTC (permalink / raw) To: Markus Armbruster Cc: Kevin Wolf, Paolo Bonzini, qemu-devel, Stefan Hajnoczi, Luiz Capitulino On 2014-11-28 at 16:55, Markus Armbruster wrote: > Copying Luiz. > > Max Reitz <mreitz@redhat.com> writes: > >> This series does not add new functionality. Adding a QMP monitor with >> prettily formatted JSON output can be done as follows: >> >> $ qemu -chardev stdio,id=mon0 -mon chardev=mon0,mode=control,pretty=on >> >> However, this is rather cumbersome, so this series (its first patch) >> adds a shortcut in the form of the new command line option -qmp-pretty. >> >> Since the argument given to a monitor command line option (such as -qmp) >> is parsed depending on its prefix and probably also depending on the >> current phase of the moon, this is cleaner than trying to add a "switch" >> to -qmp itself (in the form of "-qmp stdio,pretty=on"). > Yet another "convenience" option *groan* > > Why can't we simply make -qmp set pretty=on and be done with it? > It's a convenience option, i.e. meant for humans, and why would humans > *not* want pretty=on? Well, pretty=on produces really long output. I prefer short output if I don't expect to run commands like query-block which are completely unreadable with pretty=off. I personally had a really bad taste when I saw that I couldn't modify -qmp somehow to get it to set pretty=on optionally. But then, after having implemented -qmp-pretty in basically five lines (the "case" statement for that option and the qemu_opt_set_bool() in monitor_parse()), I could send the series with a clear conscience. It's not that there's a new convenience option which is really bad to maintain and will break after the first gentle blow, as it reuses all the code paths from -qmp, which we will keep anyway. Since this is only in block-next, I wouldn't object to it being dropped and making pretty=on the default for -qmp, in principle. But I personally like having a convenience pretty=off option, too, because I as a human often actually don't want pretty=on. Max ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH v4 0/3] chardev: Add -qmp-pretty 2014-12-02 9:24 ` Max Reitz @ 2014-12-02 18:17 ` Markus Armbruster 0 siblings, 0 replies; 15+ messages in thread From: Markus Armbruster @ 2014-12-02 18:17 UTC (permalink / raw) To: Max Reitz Cc: Kevin Wolf, Paolo Bonzini, qemu-devel, Stefan Hajnoczi, Luiz Capitulino Max Reitz <mreitz@redhat.com> writes: > On 2014-11-28 at 16:55, Markus Armbruster wrote: >> Copying Luiz. >> >> Max Reitz <mreitz@redhat.com> writes: >> >>> This series does not add new functionality. Adding a QMP monitor with >>> prettily formatted JSON output can be done as follows: >>> >>> $ qemu -chardev stdio,id=mon0 -mon chardev=mon0,mode=control,pretty=on >>> >>> However, this is rather cumbersome, so this series (its first patch) >>> adds a shortcut in the form of the new command line option -qmp-pretty. >>> >>> Since the argument given to a monitor command line option (such as -qmp) >>> is parsed depending on its prefix and probably also depending on the >>> current phase of the moon, this is cleaner than trying to add a "switch" >>> to -qmp itself (in the form of "-qmp stdio,pretty=on"). >> Yet another "convenience" option *groan* >> >> Why can't we simply make -qmp set pretty=on and be done with it? >> It's a convenience option, i.e. meant for humans, and why would humans >> *not* want pretty=on? > > Well, pretty=on produces really long output. I prefer short output if > I don't expect to run commands like query-block which are completely > unreadable with pretty=off. > > I personally had a really bad taste when I saw that I couldn't modify > -qmp somehow to get it to set pretty=on optionally. But then, after > having implemented -qmp-pretty in basically five lines (the "case" > statement for that option and the qemu_opt_set_bool() in > monitor_parse()), I could send the series with a clear > conscience. It's not that there's a new convenience option which is > really bad to maintain and will break after the first gentle blow, as > it reuses all the code paths from -qmp, which we will keep anyway. The implementation complexity doesn't worry me. I dislike the interface complexity. We're stuck with two sugar options around -mon, but adding a *third* is pushing it too far for me. > Since this is only in block-next, I wouldn't object to it being > dropped and making pretty=on the default for -qmp, in principle. But I > personally like having a convenience pretty=off option, too, because I > as a human often actually don't want pretty=on. -set mon.compat_monitor0.pretty=off ;-P Alternatively, define a pretty and an ugly monitor in a configuration file, and connect to the one you want: [chardev "qmp"] backend = "socket" path = "sock-qmp" server = "on" wait = "off" [mon "qmp"] mode = "control" chardev = "qmp" pretty = "off" [chardev "qmp-pretty"] backend = "socket" path = "sock-qmp-pretty" server = "on" wait = "off" [mon "qmp-pretty"] mode = "control" chardev = "qmp-pretty" pretty = "on" Alternatively^2, create a QMP command to toggle prettiness. ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2014-12-02 18:18 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-11-17 12:31 [Qemu-devel] [PATCH v4 0/3] chardev: Add -qmp-pretty Max Reitz 2014-11-17 12:31 ` [Qemu-devel] [PATCH v4 1/3] " Max Reitz 2014-11-17 12:31 ` [Qemu-devel] [PATCH v4 2/3] iotests: _filter_qmp for pretty JSON output Max Reitz 2014-11-17 16:04 ` Eric Blake 2014-11-17 16:14 ` Max Reitz 2014-11-17 17:06 ` Eric Blake 2014-11-17 12:31 ` [Qemu-devel] [PATCH v4 3/3] iotests: Use -qmp-pretty in 067 Max Reitz 2014-11-20 18:39 ` Kevin Wolf 2014-11-20 18:56 ` Max Reitz 2014-11-20 19:43 ` Eric Blake 2014-11-20 17:02 ` [Qemu-devel] [PATCH v4 0/3] chardev: Add -qmp-pretty Kevin Wolf 2014-11-28 15:55 ` Markus Armbruster 2014-11-28 17:21 ` Paolo Bonzini 2014-12-02 9:24 ` Max Reitz 2014-12-02 18:17 ` Markus Armbruster
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).