On Mon, Aug 01, 2016 at 06:30:07AM +0200, Peter Krempa wrote:
The prettified JSON string already contains a newline so don't print another one. This allows to pipe the json output (in conjunction with the --quiet option) to files without having to truncate them afterwards. --- tools/virsh-domain.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 6c1bc2f..45fce76 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -8944,6 +8944,7 @@ cmdQemuMonitorCommand(vshControl *ctl, const vshCmd *cmd) virBuffer buf = VIR_BUFFER_INITIALIZER; bool pad = false; virJSONValuePtr pretty = NULL; + char *prettystr = NULL; VSH_EXCLUSIVE_OPTIONS("hmp", "pretty"); @@ -8969,22 +8970,20 @@ cmdQemuMonitorCommand(vshControl *ctl, const vshCmd *cmd) if (virDomainQemuMonitorCommand(dom, monitor_cmd, &result, flags) < 0) goto cleanup; - if (vshCommandOptBool(cmd, "pretty")) { - char *tmp; - pretty = virJSONValueFromString(result); - if (pretty && (tmp = virJSONValueToString(pretty, true))) { - VIR_FREE(result); - result = tmp; - } else { - vshResetLibvirtError(); - } + if (vshCommandOptBool(cmd, "pretty") && + (pretty = virJSONValueFromString(result)) &&
I find naming the variable pretty pretty confusing. virJSONValuePtr is in machine-friendly format and while it has the potential to be formatted as pretty, it is not pretty right now.
+ (prettystr = virJSONValueToString(pretty, true))) { + vshPrint(ctl, "%s", prettystr); + } else { + vshResetLibvirtError();
Resetting it even when --pretty was not specified feels strange. Also, grouping the if (--pretty) condition with the virJSONValue error checks makes the flow hard to read. Do we need the fallback? AFAIK there are only two issues possible when prettifying the string: * the system is out of memory possibly impossible, erroring out or even aborting virsh are acceptable here * virDomainQemuMonitorCommand did not return a JSON output For a domain with JSON monitor, the remote side calls virJSONValueToString, so this could only reasonably happen with a domain that only has HMP. If we error out on the --hmp --pretty combination, we can error out here too and do not need to fall back to raw output.
+ vshPrint(ctl, "%s\n", result); } - vshPrint(ctl, "%s\n", result);
There is also the option of keeping the \n in vshPrint and calling virTrimSpaces on the prettyfied output. Jan -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list