Kevin Wolf <kw...@redhat.com> writes: [...]
> What I had in mind was using the schema to generate the necessary code, > using the generic keyval parser everywhere, and just providing a hook > where the QDict could be modified after the keyval parser and before the > visitor. Most command line options would not have any explicit code for > parsing input, only the declarative schema and the final option handler > getting a QAPI type (which could actually be the corresponding QMP > command handler in the common case). A walk to the bakery made me see a problem with transforming the qdict between keyval parsing and the visitor: error reporting. On closer investigation, the problem exists even with just aliases. All experiments performed with your complete QAPIfication at https://repo.or.cz/qemu/kevin.git qapi-alias-chardev-v4. Example: flattening leads to suboptimal error $ qemu-system-x86_64 -chardev udp,id=chr0,port=12345,ipv4=om qemu-system-x86_64: -chardev udp,id=chr0,port=12345,ipv4=om: Parameter 'backend.data.remote.data.ipv4' expects 'on' or 'off' We're using "alternate" notation, but the error message barks back in "standard" notation. It comes from the visitor. While less than pleasant, it's still understandable, because the "standard" key ends with the "alternate" key. Example: renaming leads to confusing error So far, we rename only members of type 'str', where the visitor can't fail. Just for illustrating the problem, I'm adding one where it can: diff --git a/qapi/char.json b/qapi/char.json index 0e39840d4f..b436d83cbe 100644 --- a/qapi/char.json +++ b/qapi/char.json @@ -398,7 +398,8 @@ ## { 'struct': 'ChardevRingbuf', 'data': { '*size': 'int' }, - 'base': 'ChardevCommon' } + 'base': 'ChardevCommon', + 'aliases': [ { 'name': 'capacity', 'source': [ 'size' ] } ] } ## # @ChardevQemuVDAgent: With this patch: $ qemu-system-x86_64 -chardev ringbuf,id=chr0,capacity=lots qemu-system-x86_64: -chardev ringbuf,id=chr0,capacity=lots: Parameter 'backend.data.size' expects integer The error message fails to connect to the offending key=value. Example: manual transformation leads to confusion #1 Starting point: $ qemu-system-x86_64 -chardev udp,id=chr0,port=12345,localaddr=localhost Works. host defaults to localhost, localport defaults to 0, same as in git master. Now "forget" to specify @port: $ qemu-system-x86_64 -chardev udp,id=chr0,localaddr=localhost qemu-system-x86_64: -chardev udp,id=chr0,localaddr=localhost: Parameter 'backend.data.remote.data.port' is missing Again, the visitor's error message uses "standard" notation. We obediently do what the error message tells us to do: $ qemu-system-x86_64 -chardev udp,id=chr0,localaddr=localhost,backend.data.remote.data.port=12345 qemu-system-x86_64: -chardev udp,id=chr0,localaddr=localhost,backend.data.remote.data.port=12345: Parameters 'backend.*' used inconsistently Mission accomplished: confusion :) Example: manual transformation leads to confusion #2 Confusion is possible even without tricking the user into mixing "standard" and "alternate" explicitly: $ qemu-system-x86_64 -chardev udp,id=chr0,backend.data.remote.type=udp qemu-system-x86_64: -chardev udp,id=chr0,backend.data.remote.type=udp: Parameters 'backend.*' used inconsistently Here, the user tried to stick to "standard", but forgot to specify a required member. The transformation machinery then "helpfully" transformed nothing into something, which made the visitor throw up. Clear error reporting is a critical part of a human-friendly interface. We have two separate problems with it: 1. The visitor reports errors as if aliases didn't exist Fixing this is "merely" a matter of tracing back alias applications. More complexity... 2. The visitor reports errors as if manual transformation didn't exist Manual transformation can distort the users input beyond recognition. The visitor reports errors for the transformed input. To fix this one, we'd have to augment the parse tree so it points back at the actual user input, and then make the manual transformations preserve that. Uff! I'm afraid I need another round of thinking on how to best drag legacy syntax along when we QAPIfy. [...]
