[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] virsh: add qemu-monitor-command --pretty



On Thu, Oct 04, 2012 at 05:22:20PM -0600, Eric Blake wrote:
> I was using qemu-monitor-command during development, and found it quite
> hard to use.  Compare the results of this patch on ease of reading:
> 
> $ virsh qemu-monitor-command          dom '{"execute":"query-version"}'
> {"return":{"qemu":{"micro":1,"minor":12,"major":0},"package":"(qemu-kvm-0.12.1.2)"},"id":"libvirt-7683"}
> 
> $ virsh qemu-monitor-command --pretty dom '{"execute":"query-version"}'
> {
>     "return": {
>         "qemu": {
>             "micro": 1,
>             "minor": 12,
>             "major": 0
>         },
>         "package": "(qemu-kvm-0.12.1.2)"
>     },
>     "id": "libvirt-7674"
> }
> 
> * tools/virsh-host.c (cmdQemuMonitorCommand): New option.
> * tools/virsh.pod (qemu-monitor-command): Document it.
> ---
> 
> Note that 'qemu-monitor-command dom --pretty --hmp info version'
> happens to truncate 0.12.1(qemu-kvm-0.12.1.2) into just 0.12 (since
> that is the portion of the prefix of the string that forms a valid
> JSON subsequence); we probably have a bug in virJSONValueFromString
> for not rejecting trailing junk, but that's a matter for another
> patch; for now, I just documented that --hmp and --pretty don't
> make sense together in the man page.

I'd tweak the patch to reject the combination of --hmp and --pretty;
other than that it looks like a great readability enhancement.

Dave

>  tools/virsh-host.c | 15 +++++++++++++++
>  tools/virsh.pod    | 10 ++++++----
>  2 files changed, 21 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/virsh-host.c b/tools/virsh-host.c
> index 2c46336..40ad8de 100644
> --- a/tools/virsh-host.c
> +++ b/tools/virsh-host.c
> @@ -38,6 +38,7 @@
>  #include "virsh-domain.h"
>  #include "xml.h"
>  #include "virtypedparam.h"
> +#include "json.h"
> 
>  /*
>   * "capabilities" command
> @@ -529,6 +530,8 @@ static const vshCmdInfo info_qemu_monitor_command[] = {
>  static const vshCmdOptDef opts_qemu_monitor_command[] = {
>      {"domain", VSH_OT_DATA, VSH_OFLAG_REQ, N_("domain name, id or uuid")},
>      {"hmp", VSH_OT_BOOL, 0, N_("command is in human monitor protocol")},
> +    {"pretty", VSH_OT_BOOL, 0,
> +     N_("pretty-print any qemu monitor protocol output")},
>      {"cmd", VSH_OT_ARGV, VSH_OFLAG_REQ, N_("command")},
>      {NULL, 0, 0, NULL}
>  };
> @@ -544,6 +547,7 @@ cmdQemuMonitorCommand(vshControl *ctl, const vshCmd *cmd)
>      const vshCmdOpt *opt = NULL;
>      virBuffer buf = VIR_BUFFER_INITIALIZER;
>      bool pad = false;
> +    virJSONValuePtr pretty = NULL;
> 
>      dom = vshCommandOptDomain(ctl, cmd, NULL);
>      if (dom == NULL)
> @@ -567,6 +571,16 @@ 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();
> +        }
> +    }
>      vshPrint(ctl, "%s\n", result);
> 
>      ret = true;
> @@ -574,6 +588,7 @@ cmdQemuMonitorCommand(vshControl *ctl, const vshCmd *cmd)
>  cleanup:
>      VIR_FREE(result);
>      VIR_FREE(monitor_cmd);
> +    virJSONValueFree(pretty);
>      if (dom)
>          virDomainFree(dom);
> 
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index c02ffe8..ac8a00f 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
> @@ -2910,15 +2910,17 @@ attaching to an externally launched QEMU process. There may be
>  issues with the guest ABI changing upon migration, and hotunplug
>  may not work.
> 
> -=item B<qemu-monitor-command> I<domain> [I<--hmp>] I<command>...
> +=item B<qemu-monitor-command> I<domain> { [I<--hmp>] | [I<--pretty>] }
> +I<command>...
> 
>  Send an arbitrary monitor command I<command> to domain I<domain> through the
>  qemu monitor.  The results of the command will be printed on stdout.  If
>  I<--hmp> is passed, the command is considered to be a human monitor command
>  and libvirt will automatically convert it into QMP if needed.  In that case
> -the result will also be converted back from QMP.  If more than one argument
> -is provided for I<command>, they are concatenated with a space in between
> -before passing the single command to the monitor.
> +the result will also be converted back from QMP.  If I<--pretty> is given,
> +and the monitor uses QMP, then the output will be pretty-printed.  If more
> +than one argument is provided for I<command>, they are concatenated with a
> +space in between before passing the single command to the monitor.
> 
>  =back
> 
> -- 
> 1.7.11.4
> 
> --
> libvir-list mailing list
> libvir-list redhat com
> https://www.redhat.com/mailman/listinfo/libvir-list


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]