1. Patchew
  2. Libvirt
  3. View series

[PATCH] 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/b025865f3568816137a2e27ec8c587dd1a903b3e.1631632511.git.pkrempa@redhat.com
There is a newer version of this series
docs/manpages/virsh.rst |  9 ++++++++-
tools/virsh-domain.c    | 31 ++++++++++++++++++++++++++++---
2 files changed, 36 insertions(+), 4 deletions(-)
[PATCH] 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 --qmpwrap $VM query-status

is equivalent to

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

and

 virsh qemu-monitor-command --qmpwrap $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>
---

I originally called the flag '--qmp' but that could be confusing given
that we have '--hmp'. On the other hand it's shorter so I wouldn't mind
turning it back to '--qmp'.

 docs/manpages/virsh.rst |  9 ++++++++-
 tools/virsh-domain.c    | 31 ++++++++++++++++++++++++++++---
 2 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
index 9561b3f59d..b8cb3cfbc2 100644
--- a/docs/manpages/virsh.rst
+++ b/docs/manpages/virsh.rst
@@ -7694,7 +7694,8 @@ qemu-monitor-command

 ::

-   qemu-monitor-command domain { [--hmp] | [--pretty] [--return-value] } command...
+   qemu-monitor-command domain { [--hmp] | [--qmpwrap] [--pretty]
+       [--return-value] } command...

 Send an arbitrary monitor command *command* to domain *domain* through the
 QEMU monitor.  The results of the command will be printed on stdout.
@@ -7705,6 +7706,12 @@ 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.

+If *--qmpwrap* is passed the first argument passed as *command* is used as a QMP
+command name and appropriately wrapped into a JSON block. Additionally a second
+argument passed as *command* is appended as value of the 'arguments' parameter
+of the QMP command verbatim. *Note* that in the future automatic formatting
+*--qmpwrap* of the arguments might be added, so scripts should format their own JSON.
+
 If *--pretty* is given the QMP reply is pretty-printed.

 If *--return-value* is given the 'return' key of the QMP response object is
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index e5bd1fdd75..ac57196b20 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -9380,6 +9380,10 @@ static const vshCmdOptDef opts_qemu_monitor_command[] = {
      .type = VSH_OT_BOOL,
      .help = N_("extract the value of the 'return' key from the returned string")
     },
+    {.name = "qmpwrap",
+     .type = VSH_OT_BOOL,
+     .help = N_("wrap the 'cmd' argument in JSON wrapper for QMP")
+    },
     {.name = "cmd",
      .type = VSH_OT_ARGV,
      .flags = VSH_OFLAG_REQ,
@@ -9405,14 +9409,35 @@ cmdQemuMonitorCommand(vshControl *ctl, const vshCmd *cmd)

     VSH_EXCLUSIVE_OPTIONS("hmp", "pretty");
     VSH_EXCLUSIVE_OPTIONS("hmp", "return-value");
+    VSH_EXCLUSIVE_OPTIONS("hmp", "qmpwrap");

     if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
         return false;

-    while ((opt = vshCommandOptArgv(ctl, cmd, opt)))
-        virBufferAsprintf(&buf, "%s ", opt->data);
+    if (vshCommandOptBool(cmd, "qmpwrap")) {
+        const char *command = NULL;
+        const char *arguments = NULL;

-    virBufferTrim(&buf, " ");
+        if ((opt = vshCommandOptArgv(ctl, cmd, opt)))
+            command = opt->data;
+        if ((opt = vshCommandOptArgv(ctl, cmd, opt)))
+            arguments = opt->data;
+
+        if (!command || (arguments && vshCommandOptArgv(ctl, cmd, opt))) {
+            vshError(ctl, "%s", _("--qmpwrap option requires 1 or 2 arguments"));
+            return false;
+        }
+
+        virBufferAsprintf(&buf, "{\"execute\":\"%s\"", command);
+        if (arguments)
+            virBufferAsprintf(&buf, ", \"arguments\":%s", arguments);
+        virBufferAddLit(&buf, "}");
+    } else {
+        while ((opt = vshCommandOptArgv(ctl, cmd, opt)))
+            virBufferAsprintf(&buf, "%s ", opt->data);
+
+        virBufferTrim(&buf, " ");
+    }

     monitor_cmd = virBufferContentAndReset(&buf);

-- 
2.31.1
Re: [PATCH] virsh: Add QMP command wrapping for 'qemu-monitor-command'
Posted by Jonathon Jongsma 2 years, 7 months ago 
On Tue, 14 Sep 2021 17:16:11 +0200
Peter Krempa <pkrempa@redhat.com> 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 --qmpwrap $VM query-status
> 
> is equivalent to
> 
>  virsh qemu-monitor-command $VM '{"execute":"query-status"}'
> 
> and
> 
>  virsh qemu-monitor-command --qmpwrap $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>
> ---
> 
> I originally called the flag '--qmp' but that could be confusing given
> that we have '--hmp'. On the other hand it's shorter so I wouldn't
> mind turning it back to '--qmp'.

I like the idea, as I also find the qemu-monitor-command to be
awkward to use. I'm not crazy about the name --qmpwrap, though. I
kind of wish we could do something like 
    virsh qemu-monitor-command --cmd command-name --args '{"arg1":true}'

but --cmd is already taken as an optional flag for the full
json-formatted command. I'm not sure I have any better ideas though.


Tangent: Reading the code for this function leads me the surprising
discovery that these two commands are actually identical:

qemu-monitor-command vm --cmd {\"execute\": \"help\"}
qemu-monitor-command vm {\"execute\": --cmd \"help\"}

Jonathon


> 
>  docs/manpages/virsh.rst |  9 ++++++++-
>  tools/virsh-domain.c    | 31 ++++++++++++++++++++++++++++---
>  2 files changed, 36 insertions(+), 4 deletions(-)
> 
> diff --git a/docs/manpages/virsh.rst b/docs/manpages/virsh.rst
> index 9561b3f59d..b8cb3cfbc2 100644
> --- a/docs/manpages/virsh.rst
> +++ b/docs/manpages/virsh.rst
> @@ -7694,7 +7694,8 @@ qemu-monitor-command
> 
>  ::
> 
> -   qemu-monitor-command domain { [--hmp] | [--pretty]
> [--return-value] } command...
> +   qemu-monitor-command domain { [--hmp] | [--qmpwrap] [--pretty]
> +       [--return-value] } command...
> 
>  Send an arbitrary monitor command *command* to domain *domain*
> through the QEMU monitor.  The results of the command will be printed
> on stdout. @@ -7705,6 +7706,12 @@ 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.
> 
> +If *--qmpwrap* is passed the first argument passed as *command* is
> used as a QMP +command name and appropriately wrapped into a JSON
> block. Additionally a second +argument passed as *command* is
> appended as value of the 'arguments' parameter +of the QMP command
> verbatim. *Note* that in the future automatic formatting +*--qmpwrap*
> of the arguments might be added, so scripts should format their own
> JSON. + If *--pretty* is given the QMP reply is pretty-printed.
> 
>  If *--return-value* is given the 'return' key of the QMP response
> object is diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
> index e5bd1fdd75..ac57196b20 100644
> --- a/tools/virsh-domain.c
> +++ b/tools/virsh-domain.c
> @@ -9380,6 +9380,10 @@ static const vshCmdOptDef
> opts_qemu_monitor_command[] = { .type = VSH_OT_BOOL,
>       .help = N_("extract the value of the 'return' key from the
> returned string") },
> +    {.name = "qmpwrap",
> +     .type = VSH_OT_BOOL,
> +     .help = N_("wrap the 'cmd' argument in JSON wrapper for QMP")
> +    },
>      {.name = "cmd",
>       .type = VSH_OT_ARGV,
>       .flags = VSH_OFLAG_REQ,
> @@ -9405,14 +9409,35 @@ cmdQemuMonitorCommand(vshControl *ctl, const
> vshCmd *cmd)
> 
>      VSH_EXCLUSIVE_OPTIONS("hmp", "pretty");
>      VSH_EXCLUSIVE_OPTIONS("hmp", "return-value");
> +    VSH_EXCLUSIVE_OPTIONS("hmp", "qmpwrap");
> 
>      if (!(dom = virshCommandOptDomain(ctl, cmd, NULL)))
>          return false;
> 
> -    while ((opt = vshCommandOptArgv(ctl, cmd, opt)))
> -        virBufferAsprintf(&buf, "%s ", opt->data);
> +    if (vshCommandOptBool(cmd, "qmpwrap")) {
> +        const char *command = NULL;
> +        const char *arguments = NULL;
> 
> -    virBufferTrim(&buf, " ");
> +        if ((opt = vshCommandOptArgv(ctl, cmd, opt)))
> +            command = opt->data;
> +        if ((opt = vshCommandOptArgv(ctl, cmd, opt)))
> +            arguments = opt->data;
> +
> +        if (!command || (arguments && vshCommandOptArgv(ctl, cmd,
> opt))) {
> +            vshError(ctl, "%s", _("--qmpwrap option requires 1 or 2
> arguments"));
> +            return false;
> +        }
> +
> +        virBufferAsprintf(&buf, "{\"execute\":\"%s\"", command);
> +        if (arguments)
> +            virBufferAsprintf(&buf, ", \"arguments\":%s", arguments);
> +        virBufferAddLit(&buf, "}");
> +    } else {
> +        while ((opt = vshCommandOptArgv(ctl, cmd, opt)))
> +            virBufferAsprintf(&buf, "%s ", opt->data);
> +
> +        virBufferTrim(&buf, " ");
> +    }
> 
>      monitor_cmd = virBufferContentAndReset(&buf);
>
Re: [PATCH] virsh: Add QMP command wrapping for 'qemu-monitor-command'
Posted by Daniel P. Berrangé 2 years, 7 months ago 
On Tue, Sep 14, 2021 at 05:16:11PM +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 --qmpwrap $VM query-status
> 
> is equivalent to
> 
>  virsh qemu-monitor-command $VM '{"execute":"query-status"}'
> 
> and
> 
>  virsh qemu-monitor-command --qmpwrap $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>
> ---
> 
> I originally called the flag '--qmp' but that could be confusing given
> that we have '--hmp'. On the other hand it's shorter so I wouldn't mind
> turning it back to '--qmp'.

How about just following a DWIM approach and not adding any flag

These two cases are trivially distinguished:

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

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

because the second doesn't start with a '{'

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.