Re: [libvirt] [PATCH 01/19] virsh: Add QMP command wrapping for 'qemu-monitor-command'
On Fri, Dec 13, 2019 at 08:04:51 +0100, Peter Krempa wrote:
On Thu, Dec 12, 2019 at 19:30:27 +0100, Michal Privoznik wrote:
> On 12/12/19 7:16 PM, Eric Blake wrote:
> > On 12/12/19 11:18 AM, Peter Krempa wrote:
[...]
> Since we won't use HMP to talk to qemu ever (even the small
set of HMP
> commands we have are wrapped inside QMP once being sent down the wire), can
Specifically you already must use --hmp if you ever want to use HMP.
> we not use --qmp flag at all? Just look if there's "execute" in the
user's
> input and if not add it there. For instance:
>
> virsh qemu-monitor-command query-machines
>
> will expand to
>
> {"execute":"query-machines"}
At first I wanted to argue that I'd like to support passing raw
unmodified commands to qemu, but in fact libvirt itself parses the
string as JSON so that it can be re-wrapped with the monitor sequence
field, so you have to pass in JSON anyways.
Thus I'm okay with dropping the flag and deciding on whether the opening
'{' is present on input.
I thought about this for a bit and looked into the code. Currently we
pass to qemu anything that parses as valid JSON. It must not be an
object. This means that also things like 'true' are valid JSON and would
be passed to qemu. On the other hand, qemu accepts only JSON objects in
the first place.
This means I'm now unsure if we want to do this without an explicit
switch, because we'd lose some possibilities of passtrhough. (Not very
useful ones though).
Other option would be to try to parse the user-provided string as JSON
and if that fails do the modification. This solution is too magic for my
taste though and could have non-obvious results.
If you think it's okay to lose passthrough of non-objects I'll do it,
but otherwise the extra flag has to stay.