[Qemu-devel] [PATCH v4 0/3] chardev: Add -qmp-pretty

[Qemu-devel] [PATCH v4 0/3] chardev: Add -qmp-pretty

[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

[Qemu-devel] [PATCH v4 1/3] chardev: Add -qmp-pretty

[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

[Qemu-devel] [PATCH v4 2/3] iotests: _filter_qmp for pretty JSON output

[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

[Qemu-devel] [PATCH v4 3/3] iotests: Use -qmp-pretty in 067

[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

Re: [Qemu-devel] [PATCH v4 2/3] iotests: _filter_qmp for pretty JSON output

[Eric Blake]

[-- 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 --]

Re: [Qemu-devel] [PATCH v4 2/3] iotests: _filter_qmp for pretty JSON output

[Max Reitz]

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

Re: [Qemu-devel] [PATCH v4 2/3] iotests: _filter_qmp for pretty JSON output

[Eric Blake]

[-- 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 --]

Re: [Qemu-devel] [PATCH v4 0/3] chardev: Add -qmp-pretty

[Kevin Wolf]

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

Re: [Qemu-devel] [PATCH v4 3/3] iotests: Use -qmp-pretty in 067

[Kevin Wolf]

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

Re: [Qemu-devel] [PATCH v4 3/3] iotests: Use -qmp-pretty in 067

[Max Reitz]

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

Re: [Qemu-devel] [PATCH v4 3/3] iotests: Use -qmp-pretty in 067

[Eric Blake]

[-- 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 --]

Re: [Qemu-devel] [PATCH v4 0/3] chardev: Add -qmp-pretty

[Markus Armbruster]

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?

Re: [Qemu-devel] [PATCH v4 0/3] chardev: Add -qmp-pretty

[Paolo Bonzini]

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

Re: [Qemu-devel] [PATCH v4 0/3] chardev: Add -qmp-pretty

[Max Reitz]

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

Re: [Qemu-devel] [PATCH v4 0/3] chardev: Add -qmp-pretty

[Markus Armbruster]

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.