[PATCH v2] virsh: Add QMP command wrapping for 'qemu-monitor-command'

Peter Krempa posted 1 patch 2 years, 7 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/e7aa6878447da02df1ec5ebbf1a1e2aa78050304.1631885652.git.pkrempa@redhat.com
docs/manpages/virsh.rst | 16 ++++++-
tools/virsh-domain.c    | 98 ++++++++++++++++++++++++++++++++++++-----
2 files changed, 103 insertions(+), 11 deletions(-)
[PATCH v2] virsh: Add QMP command wrapping for 'qemu-monitor-command'
Posted by Peter Krempa 2 years, 7 months ago
Issuing simple QMP commands is pain as they need to be wrapped by the
JSON wrapper:

 { "execute": "COMMAND" }

and optionally also:

 { "execute": "COMMAND", "arguments":...}

For simple commands without arguments we can add syntax sugar to virsh
which allows simple usage of QMP and additionally prepares also for
passing through of the 'arguments' section:

 virsh qemu-monitor-command $VM query-status

is equivalent to

 virsh qemu-monitor-command $VM '{"execute":"query-status"}'

and

 virsh qemu-monitor-command $VM query-named-block-nodes '{"flat":true}'
 or
 virsh qemu-monitor-command $VM query-named-block-nodes '"flat":true'

is equivalent to

 virsh qemu-monitor-command $VM '{"execute":"query-named-block-nodes", "arguments":{"flat":true}}'

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---

v2:
  - dropped the '--qmpwrap' option and do wrapping if we don't get a
  JSON object instead. Similarly for arguments.

 docs/manpages/virsh.rst | 16 ++++++-
 tools/virsh-domain.c    | 98 ++++++++++++++++++++++++++++++++++++-----
 2 files changed, 103 insertions(+), 11 deletions(-)

diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index 350ded2026..b4d31bf884 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -7772,7 +7772,21 @@ If more than one argument is provided for *command*, they are concatenated with
 a space in between before passing the single command to the monitor.

 Note that libvirt uses the QMP to talk to qemu so *command* must be valid JSON
-in QMP format to work properly.
+in QMP format to work properly. If *command* is not a JSON object libvirt tries
+to wrap it as a JSON object to provide convenient interface such as the groups
+of commands with identical handling:
+
+::
+
+   # simple command
+   $ virsh qemu-monitor-command VM commandname
+   $ virsh qemu-monitor-command VM '{"execute":"commandname"}'
+
+   # with arguments
+   $ virsh qemu-monitor-command VM commandname '"arg1":123' '"arg2":"test"'
+   $ virsh qemu-monitor-command VM commandname '{"arg1":123,"arg2":"test"}'
+   $ virsh qemu-monitor-command VM '{"execute":"commandname", "arguments":{"arg1":123,"arg2":"test"}}'
+

 If *--pretty* is given the QMP reply is pretty-printed.

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 70aa4167c2..659aa1a242 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -9445,6 +9445,84 @@ static const vshCmdOptDef opts_qemu_monitor_command[] = {
     {.name = NULL}
 };

+
+static char *
+cmdQemuMonitorCommandConcatCmd(vshControl *ctl,
+                               const vshCmd *cmd,
+                               const vshCmdOpt *opt)
+{
+    g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
+
+    while ((opt = vshCommandOptArgv(ctl, cmd, opt)))
+        virBufferAsprintf(&buf, "%s ", opt->data);
+
+    virBufferTrim(&buf, " ");
+
+    return virBufferContentAndReset(&buf);
+}
+
+
+static char *
+cmdQemuMonitorCommandQMPWrap(vshControl *ctl,
+                             const vshCmd *cmd)
+{
+    g_autofree char *fullcmd = cmdQemuMonitorCommandConcatCmd(ctl, cmd, NULL);
+    g_autoptr(virJSONValue) fullcmdjson = virJSONValueFromString(fullcmd);
+    g_autofree char *fullargs = NULL;
+    g_autoptr(virJSONValue) fullargsjson = NULL;
+    const vshCmdOpt *opt = NULL;
+    const char *commandname = NULL;
+    g_autoptr(virJSONValue) command = NULL;
+    g_autoptr(virJSONValue) arguments = NULL;
+
+    /* if we've got a JSON object, pass it through */
+    if (virJSONValueIsObject(fullcmdjson))
+        return g_steal_pointer(&fullcmd);
+
+    /* we try to wrap the command and possible arguments into a JSON object, if
+     * we as fall back we pass through what we've got from the user */
+
+    if ((opt = vshCommandOptArgv(ctl, cmd, opt)))
+        commandname = opt->data;
+
+    /* now we process arguments similarly to how we've dealt with the full command */
+    if ((fullargs = cmdQemuMonitorCommandConcatCmd(ctl, cmd, opt)))
+        fullargsjson = virJSONValueFromString(fullargs);
+
+    /* for empty args or a valid JSON object we just use that */
+    if (!fullargs || virJSONValueIsObject(fullargsjson)) {
+        arguments = g_steal_pointer(&fullargsjson);
+    } else {
+        /* for a non-object we try to concatenate individual _ARGV bits into a
+         * JSON object wrapper and try using that */
+        g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
+
+        virBufferAddLit(&buf, "{");
+        /* opt points to the _ARGV option bit containing the command so we'll
+         * iterate through the arguments now */
+        while ((opt = vshCommandOptArgv(ctl, cmd, opt)))
+            virBufferAsprintf(&buf, "%s,", opt->data);
+
+        virBufferTrim(&buf, ",");
+        virBufferAddLit(&buf, "}");
+
+        if (!(arguments = virJSONValueFromString(virBufferCurrentContent(&buf)))) {
+            vshError(ctl, _("failed to wrap arguments '%s' into a QMP command wrapper"),
+                     fullargs);
+            return NULL;
+        }
+    }
+
+    if (virJSONValueObjectCreate(&command,
+                                 "s:execute", commandname,
+                                 "A:arguments", &arguments,
+                                 NULL) < 0)
+        return NULL;
+
+    return virJSONValueToString(command, false);
+}
+
+
 static bool
 cmdQemuMonitorCommand(vshControl *ctl, const vshCmd *cmd)
 {
@@ -9453,8 +9531,6 @@ cmdQemuMonitorCommand(vshControl *ctl, const vshCmd *cmd)
     g_autofree char *result = NULL;
     g_autoptr(virJSONValue) resultjson = NULL;
     unsigned int flags = 0;
-    const vshCmdOpt *opt = NULL;
-    g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
     bool pretty = vshCommandOptBool(cmd, "pretty");
     bool returnval = vshCommandOptBool(cmd, "return-value");
     virJSONValue *formatjson;
@@ -9466,15 +9542,17 @@ cmdQemuMonitorCommand(vshControl *ctl, const vshCmd *cmd)
     if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
         return false;

-    while ((opt = vshCommandOptArgv(ctl, cmd, opt)))
-        virBufferAsprintf(&buf, "%s ", opt->data);
-
-    virBufferTrim(&buf, " ");
-
-    monitor_cmd = virBufferContentAndReset(&buf);
-
-    if (vshCommandOptBool(cmd, "hmp"))
+    if (vshCommandOptBool(cmd, "hmp")) {
         flags |= VIR_DOMAIN_QEMU_MONITOR_COMMAND_HMP;
+        monitor_cmd = cmdQemuMonitorCommandConcatCmd(ctl, cmd, NULL);
+    } else {
+        monitor_cmd = cmdQemuMonitorCommandQMPWrap(ctl, cmd);
+    }
+
+    if (!monitor_cmd) {
+        vshSaveLibvirtError();
+        return NULL;
+    }

     if (virDomainQemuMonitorCommand(dom, monitor_cmd, &result, flags) < 0)
         return false;
-- 
2.31.1

Re: [PATCH v2] virsh: Add QMP command wrapping for 'qemu-monitor-command'
Posted by Michal Prívozník 2 years, 6 months ago
On 9/17/21 3:34 PM, Peter Krempa wrote:
> Issuing simple QMP commands is pain as they need to be wrapped by the
> JSON wrapper:
> 
>  { "execute": "COMMAND" }
> 
> and optionally also:
> 
>  { "execute": "COMMAND", "arguments":...}
> 
> For simple commands without arguments we can add syntax sugar to virsh
> which allows simple usage of QMP and additionally prepares also for
> passing through of the 'arguments' section:
> 
>  virsh qemu-monitor-command $VM query-status
> 
> is equivalent to
> 
>  virsh qemu-monitor-command $VM '{"execute":"query-status"}'
> 
> and
> 
>  virsh qemu-monitor-command $VM query-named-block-nodes '{"flat":true}'
>  or
>  virsh qemu-monitor-command $VM query-named-block-nodes '"flat":true'
> 
> is equivalent to
> 
>  virsh qemu-monitor-command $VM '{"execute":"query-named-block-nodes", "arguments":{"flat":true}}'
> 
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
> 
> v2:
>   - dropped the '--qmpwrap' option and do wrapping if we don't get a
>   JSON object instead. Similarly for arguments.
> 
>  docs/manpages/virsh.rst | 16 ++++++-
>  tools/virsh-domain.c    | 98 ++++++++++++++++++++++++++++++++++++-----
>  2 files changed, 103 insertions(+), 11 deletions(-)

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>

Michal

Re: [PATCH v2] virsh: Add QMP command wrapping for 'qemu-monitor-command'
Posted by Kashyap Chamarthy 2 years, 6 months ago
On Fri, Sep 17, 2021 at 03:34:56PM +0200, Peter Krempa wrote:
> Issuing simple QMP commands is pain as they need to be wrapped by the
> JSON wrapper:
> 
>  { "execute": "COMMAND" }
> 
> and optionally also:
> 
>  { "execute": "COMMAND", "arguments":...}
> 
> For simple commands without arguments we can add syntax sugar to virsh
> which allows simple usage of QMP and additionally prepares also for
> passing through of the 'arguments' section:
> 
>  virsh qemu-monitor-command $VM query-status
> 
> is equivalent to
> 
>  virsh qemu-monitor-command $VM '{"execute":"query-status"}'
> 
> and
> 
>  virsh qemu-monitor-command $VM query-named-block-nodes '{"flat":true}'
>  or
>  virsh qemu-monitor-command $VM query-named-block-nodes '"flat":true'
> 
> is equivalent to
> 
>  virsh qemu-monitor-command $VM '{"execute":"query-named-block-nodes", "arguments":{"flat":true}}'

Very cool; thanks for this.

> Signed-off-by: Peter Krempa <pkrempa@redhat.com>

FWIW:

  Very-much-appreciated-by: Kashyap Chamarthy <kchamart@redhat.com>

> ---
> 
> v2:
>   - dropped the '--qmpwrap' option and do wrapping if we don't get a
>   JSON object instead. Similarly for arguments.
> 
>  docs/manpages/virsh.rst | 16 ++++++-
>  tools/virsh-domain.c    | 98 ++++++++++++++++++++++++++++++++++++-----
>  2 files changed, 103 insertions(+), 11 deletions(-)
> 
> diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
> index 350ded2026..b4d31bf884 100644
> --- a/docs/manpages/virsh.rst
> +++ b/docs/manpages/virsh.rst
> @@ -7772,7 +7772,21 @@ If more than one argument is provided for *command*, they are concatenated with
>  a space in between before passing the single command to the monitor.
> 
>  Note that libvirt uses the QMP to talk to qemu so *command* must be valid JSON
> -in QMP format to work properly.
> +in QMP format to work properly. If *command* is not a JSON object libvirt tries
> +to wrap it as a JSON object to provide convenient interface such as the groups
> +of commands with identical handling:
> +
> +::
> +
> +   # simple command
> +   $ virsh qemu-monitor-command VM commandname
> +   $ virsh qemu-monitor-command VM '{"execute":"commandname"}'

Do you want to give a proper example here?  E.g. `query-kvm` or
`query-status`, or `query-block`?

> +   # with arguments
> +   $ virsh qemu-monitor-command VM commandname '"arg1":123' '"arg2":"test"'
> +   $ virsh qemu-monitor-command VM commandname '{"arg1":123,"arg2":"test"}'
> +   $ virsh qemu-monitor-command VM '{"execute":"commandname", "arguments":{"arg1":123,"arg2":"test"}}'
> +
> 
>  If *--pretty* is given the QMP reply is pretty-printed.
> 
> diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index 70aa4167c2..659aa1a242 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -9445,6 +9445,84 @@ static const vshCmdOptDef opts_qemu_monitor_command[] = {
>      {.name = NULL}
>  };
> 
> +
> +static char *
> +cmdQemuMonitorCommandConcatCmd(vshControl *ctl,
> +                               const vshCmd *cmd,
> +                               const vshCmdOpt *opt)
> +{
> +    g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
> +
> +    while ((opt = vshCommandOptArgv(ctl, cmd, opt)))
> +        virBufferAsprintf(&buf, "%s ", opt->data);
> +
> +    virBufferTrim(&buf, " ");
> +
> +    return virBufferContentAndReset(&buf);
> +}
> +
> +
> +static char *
> +cmdQemuMonitorCommandQMPWrap(vshControl *ctl,
> +                             const vshCmd *cmd)
> +{
> +    g_autofree char *fullcmd = cmdQemuMonitorCommandConcatCmd(ctl, cmd, NULL);
> +    g_autoptr(virJSONValue) fullcmdjson = virJSONValueFromString(fullcmd);
> +    g_autofree char *fullargs = NULL;
> +    g_autoptr(virJSONValue) fullargsjson = NULL;
> +    const vshCmdOpt *opt = NULL;
> +    const char *commandname = NULL;
> +    g_autoptr(virJSONValue) command = NULL;
> +    g_autoptr(virJSONValue) arguments = NULL;
> +
> +    /* if we've got a JSON object, pass it through */
> +    if (virJSONValueIsObject(fullcmdjson))
> +        return g_steal_pointer(&fullcmd);
> +
> +    /* we try to wrap the command and possible arguments into a JSON object, if
> +     * we as fall back we pass through what we've got from the user */
> +
> +    if ((opt = vshCommandOptArgv(ctl, cmd, opt)))
> +        commandname = opt->data;
> +
> +    /* now we process arguments similarly to how we've dealt with the full command */
> +    if ((fullargs = cmdQemuMonitorCommandConcatCmd(ctl, cmd, opt)))
> +        fullargsjson = virJSONValueFromString(fullargs);
> +
> +    /* for empty args or a valid JSON object we just use that */
> +    if (!fullargs || virJSONValueIsObject(fullargsjson)) {
> +        arguments = g_steal_pointer(&fullargsjson);
> +    } else {
> +        /* for a non-object we try to concatenate individual _ARGV bits into a
> +         * JSON object wrapper and try using that */
> +        g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
> +
> +        virBufferAddLit(&buf, "{");
> +        /* opt points to the _ARGV option bit containing the command so we'll
> +         * iterate through the arguments now */
> +        while ((opt = vshCommandOptArgv(ctl, cmd, opt)))
> +            virBufferAsprintf(&buf, "%s,", opt->data);
> +
> +        virBufferTrim(&buf, ",");
> +        virBufferAddLit(&buf, "}");
> +
> +        if (!(arguments = virJSONValueFromString(virBufferCurrentContent(&buf)))) {
> +            vshError(ctl, _("failed to wrap arguments '%s' into a QMP command wrapper"),
> +                     fullargs);
> +            return NULL;
> +        }
> +    }
> +
> +    if (virJSONValueObjectCreate(&command,
> +                                 "s:execute", commandname,
> +                                 "A:arguments", &arguments,
> +                                 NULL) < 0)
> +        return NULL;
> +
> +    return virJSONValueToString(command, false);
> +}
> +
> +
>  static bool
>  cmdQemuMonitorCommand(vshControl *ctl, const vshCmd *cmd)
>  {
> @@ -9453,8 +9531,6 @@ cmdQemuMonitorCommand(vshControl *ctl, const vshCmd *cmd)
>      g_autofree char *result = NULL;
>      g_autoptr(virJSONValue) resultjson = NULL;
>      unsigned int flags = 0;
> -    const vshCmdOpt *opt = NULL;
> -    g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
>      bool pretty = vshCommandOptBool(cmd, "pretty");
>      bool returnval = vshCommandOptBool(cmd, "return-value");
>      virJSONValue *formatjson;
> @@ -9466,15 +9542,17 @@ cmdQemuMonitorCommand(vshControl *ctl, const vshCmd *cmd)
>      if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
>          return false;
> 
> -    while ((opt = vshCommandOptArgv(ctl, cmd, opt)))
> -        virBufferAsprintf(&buf, "%s ", opt->data);
> -
> -    virBufferTrim(&buf, " ");
> -
> -    monitor_cmd = virBufferContentAndReset(&buf);
> -
> -    if (vshCommandOptBool(cmd, "hmp"))
> +    if (vshCommandOptBool(cmd, "hmp")) {
>          flags |= VIR_DOMAIN_QEMU_MONITOR_COMMAND_HMP;
> +        monitor_cmd = cmdQemuMonitorCommandConcatCmd(ctl, cmd, NULL);
> +    } else {
> +        monitor_cmd = cmdQemuMonitorCommandQMPWrap(ctl, cmd);
> +    }
> +
> +    if (!monitor_cmd) {
> +        vshSaveLibvirtError();
> +        return NULL;
> +    }
> 
>      if (virDomainQemuMonitorCommand(dom, monitor_cmd, &result, flags) < 0)
>          return false;
> -- 
> 2.31.1
> 

-- 
/kashyap

Re: [PATCH v2] virsh: Add QMP command wrapping for 'qemu-monitor-command'
Posted by Peter Krempa 2 years, 6 months ago
On Fri, Sep 17, 2021 at 15:34:56 +0200, Peter Krempa wrote:
> Issuing simple QMP commands is pain as they need to be wrapped by the
> JSON wrapper:
> 
>  { "execute": "COMMAND" }
> 
> and optionally also:
> 
>  { "execute": "COMMAND", "arguments":...}
> 
> For simple commands without arguments we can add syntax sugar to virsh
> which allows simple usage of QMP and additionally prepares also for
> passing through of the 'arguments' section:
> 
>  virsh qemu-monitor-command $VM query-status
> 
> is equivalent to
> 
>  virsh qemu-monitor-command $VM '{"execute":"query-status"}'
> 
> and
> 
>  virsh qemu-monitor-command $VM query-named-block-nodes '{"flat":true}'
>  or
>  virsh qemu-monitor-command $VM query-named-block-nodes '"flat":true'
> 
> is equivalent to
> 
>  virsh qemu-monitor-command $VM '{"execute":"query-named-block-nodes", "arguments":{"flat":true}}'
> 
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
> 
> v2:
>   - dropped the '--qmpwrap' option and do wrapping if we don't get a
>   JSON object instead. Similarly for arguments.
> 
>  docs/manpages/virsh.rst | 16 ++++++-
>  tools/virsh-domain.c    | 98 ++++++++++++++++++++++++++++++++++++-----
>  2 files changed, 103 insertions(+), 11 deletions(-)

Ping? :)