On Wed, Mar 02, 2022 at 09:39:24 +0100, Martin Kletzander wrote:
> 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
> > --pass-fds.
> >
> > Signed-off-by: Peter Krempa <pkrempa@xxxxxxxxxx>
> > ---
> > 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
I've (trivially) re-structured the code so that vshResetLibvirtError
is done conditionally upon NULL return from virJSONValueFromString and
thus it makes it obvious what we are resetting.
>
> Reviewed-by: Martin Kletzander <mkletzan@xxxxxxxxxx>
Thanks!
