* Markus Armbruster (arm...@redhat.com) wrote:
> "Dr. David Alan Gilbert" <dgilb...@redhat.com> writes:
>
> > * Markus Armbruster (arm...@redhat.com) wrote:
> >> "Dr. David Alan Gilbert" <dgilb...@redhat.com> writes:
> >>
> >> > * Philippe Mathieu-Daudé (phi...@redhat.com) wrote:
> >> >> On 5/20/20 5:11 PM, Dr. David Alan Gilbert (git) wrote:
> >> >> > From: "Dr. David Alan Gilbert" <dgilb...@redhat.com>
> >> >> >
> >> >> > Simplify qom_set by making it use qmp_qom_set and the JSON parser.
> >> >> >
> >> >> > Note that qom-set likes JSON strings quoted with ' not ", e.g.:
> >> >> >
> >> >> > (qemu) qom-get /machine smm
> >> >> > "auto"
> >> >> > (qemu) qom-set /machine smm 'auto'
> >> >>
> >> >> Will I get this output using "?
> >> >>
> >> >> (qemu) qom-set /machine smm "auto"
> >> >> Error: Expecting a JSON value
> >> >
> >> > The error you get is:
> >> >
> >> > (qemu) qom-set /machine smm "auto"
> >> > Error: JSON parse error, invalid keyword 'auto'
> >> >
> >> > I think, having seen alphanumerics, it's expecting a keyword;
> >> > i.e. a true/false making a bool, or a null.
> >>
> >> The command parses its argument as JSON.
> >>
> >> Before we get there, the HMP core extracts the argument from the line of
> >> input. The extraction is guided by the command's .args_type, in this
> >> case the 's' in "value:s" in
> >>
> >> {
> >> .name = "qom-set",
> >> .args_type = "path:s,property:s,value:s",
> >> [...]
> >> },
> >>
> >> monitor/monitor-internal.h documents type code 's' as
> >>
> >> * 's' string (accept optional quote)
> >>
> >> The implementation boils down to:
> >>
> >> 1. Skip whitespace.
> >>
> >> 2. If looking at '"', get the string enclosed in '"', with C-like escape
> >> sequences \n, \r, \\, \', \".
> >>
> >> 3. Else, get the string up to the next whitespace.
> >>
> >> See get_str().
> >>
> >> Therefore, argument "auto" is the same as auto. Parsing auto as JSON
> >> duly fails.
> >>
> >> Argument 'auto' works, but only because qobject_from_json() recognizes
> >> single-quoted strings. This is as extension over RFC 8259.
> >>
> >> Using single quotes falls apart when you want to pass something
> >> containing whitespace. Then you'd have to use
> >>
> >> "\"ugly and unintuitive\""
> >>
> >> or, again relying on the extension
> >>
> >> "'ugly and unintuitive'"
> >>
> >> There's a better way, and Paolo pointed it out in
> >>
> >> Subject: Re: [RFC PATCH] qom: Implement qom-get HMP command
> >> Date: Thu, 21 May 2020 16:24:12 +0200
> >> Message-ID: <2c148331-78ae-31f7-8702-d65c37a09...@redhat.com>
> >>
> >> Use argument type 'S'. Documented as
> >>
> >> * 'S' it just appends the rest of the string (accept optional
> >> quote)
> >>
> >> but the parenthesis is confusing. It really just skips whitespace, then
> >> extracts the remainder of the line. Can't do string with leading
> >> whitespace, but that's just fine for us.
> >
> > Yep, thanks - I spotted Paolo's response but thanks for the deeper
> > explanation.
> > With that would you give me a Review-by?
>
> Also fix the typo in the title "hmp: Simplify qom_set": it's qom-set.
Done.
> Reviewed-by: Markus Armbruster <arm...@redhat.com>
Thanks.
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
