[PATCH] cmdQemuMonitorCommandQMPWrap: Reset ignored errors from JSON parsing

Martin Kletzander mkletzan at redhat.com
Wed Mar 2 08:39:24 UTC 2022

On Tue, Mar 01, 2022 at 02:33:28PM +0100, Peter Krempa wrote:
>'cmdQemuMonitorCommandQMPWrap' is checking whether the user provided
>string is not valid JSON to avoid wrapping it. In cases where it's not
>JSON we ignore the error and add the wrapper.
>If the caller then reports a different non-libvirt error the error from
>the JSON parsing would be printed as well. Reset errors we ignore:
> # virsh qemu-monitor-command cd --pass-fds a asdf
> error: Unable to parse FD number 'a'
> error: internal error: cannot parse json asdf: lexical error: invalid char in json text.
>                                        asdf
>                      (right here) ------^
>In the above case 'asdf' is not valid JSON, but the code did wrap it
>into '{"execute":"asdf"}', the only problem is the argument for
>Signed-off-by: Peter Krempa <pkrempa at redhat.com>
> tools/virsh-domain.c | 4 ++++
> 1 file changed, 4 insertions(+)
>diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
>index 9b1b14cdc2..743660e794 100644
>--- a/tools/virsh-domain.c
>+++ b/tools/virsh-domain.c
>@@ -9769,6 +9769,8 @@ cmdQemuMonitorCommandQMPWrap(vshControl *ctl,
>     if (virJSONValueIsObject(fullcmdjson))
>         return g_steal_pointer(&fullcmd);
>+    vshResetLibvirtError();
>     /* 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 */
>@@ -9787,6 +9789,8 @@ cmdQemuMonitorCommandQMPWrap(vshControl *ctl,
>          * JSON object wrapper and try using that */
>         g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
>+        vshResetLibvirtError();

This is a little bit out of place and could be seen as confusing since
it is here just because the error could be set in a function that's in
an if condition.  I, for one, would vote for extracting that and making
it more clear why that is, but this fixes the immediate issue, so

Reviewed-by: Martin Kletzander <mkletzan at redhat.com>

>         virBufferAddLit(&buf, "{");
>         /* opt points to the _ARGV option bit containing the command so we'll
>          * iterate through the arguments now */
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20220302/ca4f0216/attachment-0001.sig>

More information about the libvir-list mailing list