On 07/14/2016 02:04 AM, Daniel P. Berrange wrote: > On Wed, Jul 13, 2016 at 09:50:27PM -0600, Eric Blake wrote: >> Currently the QmpInputVisitor assumes that all scalar >> values are directly represented as their final types. >> ie it assumes an 'int' is using QInt, and a 'bool' is >> using QBool. >> >> This adds an alternative mode where a QString can also >> be parsed in place of the native type, by adding a parameter >> and updating all callers. For now, only the testsuite sets >> the new autocast parameter. >> >> This makes it possible to use QmpInputVisitor with a QDict >> produced from QemuOpts, where everything is in string format. >> It also makes it possible for the next patch to support >> back-compat in legacy commands like 'netdev_add' that no >> longer use QemuOpts, but must parse the same set of QMP >> constructs that QemuOpts would historically allow. > > I'm not really a huge fan of the approach there. IMHO the > input visitor should strictly operate in "all native types" > or "all string types" mode. The way you've done it here > means that users can actually mix & match strings vs native > types for each individual parameter :-( > > IMHO, I'd suggest you try to parse netdev_add args with > it in "all native types" mode, if that fails, then re-parse > in "all string types" mode.
Sadly, this idea won't work. Without this series, the existing QMP command 'netdev_add' takes mixed integers and strings in a single call, by virtue of converting QObject into QemuOpts and back into QObject. Forcing the parser to allow only integers or only strings would reject current uses of netdev_add, and we don't yet have a good idea whether any existing clients were depending on that behavior. Also, see patch 17/17 - the easiest way to implement a relaxed QMP parse is by setting a single flag which controls which visitor constructor is used. Your idea of parsing once with integer-only, then falling back to parse a second time with string-only, would complicate the generator to have to create two different visitors, rather than passing a single boolean parameter through to the single visitor constructor. > > For that you'd not have to modify my patch at all. > Also not quite true - your patch no longer applies to master, so it needed rebasing anyway. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
