Message ID | 20250214202944.69897-1-victortoso@redhat.com (mailing list archive) |
---|---|
Headers | show |
Series | [v4,01/11] qapi: golang: first level unmarshalling type | expand |
On Fri, Feb 14, 2025 at 09:29:33PM +0100, Victor Toso wrote: > Hi again, > > This patch series intent is to introduce a generator that produces a Go > module for Go applications to interact over QMP with QEMU. > > Previous version (10 Jan 2025) > https://lists.gnu.org/archive/html/qemu-devel/2025-01/msg01530.html > > The generated code was mostly tested using existing examples in the QAPI > documentation, 192 instances that might have multiple QMP messages each. snip > 1. Daniel wrote a demo on top of v3 and proposed changes that would > result in more interesting module to build on top: > https://lists.gnu.org/archive/html/qemu-devel/2025-01/msg03052.html > > I've implemented all the suggestions that are relevant for this > introductory series, they are: > > a. New struct type Message, that shall be used for a 1st level > unmarshalling of the JSON message. > b. Removal of Marshal/Unmarshal code in both Events and Comands, > together with utility code that is not relevant anymore. > c. Declaration of 3 new interfaces: > i. Events > ii. Commands > iii. CommandsAsync > > 2. I've moved the code to a new folder: scripts/qapi/golang. This > allowed me to move templates out of golang.py, keeping go related > code self-contained in the new directory. When I think about the code generator and how this will all evolve over time, I have a strong feeling that none of this should be in qemu.git. Taking those three interfaces in (1)(c), when we come to actually generate implementations of them, the generated code is going to be intimately tied to the client/server API framework we need to plumb into. IMHO qemu.git should not have any knowledge of this, as it will create a bidirectional dependency betweeen qemu.git and the qemu-go.git repo, requiring them to evolve in lockstep. We'll need 3 releases of the Go code per year, to match the introduction the new QAPI schema versions, but outside that I think there ought to be the freedom to have other Go releases independently. The need for this to be part of qemu.git is fairly narrow. It provides a new hook into the QAPI generator code, and the QAPI generator is not installed by 'make install', it is in-tree only. Can we solve this linkage ? We would need to be able to: * Have a mechanism for the QAPI code generator to load new genrator backends * Have a mechanism to tell the QAPI code generator to only run a single backend. * Have a mechanism to consume the QAPI code genrfator from outside qemu.git The first point there could be addressable using the python "entry-points" concept https://packaging.python.org/en/latest/specifications/entry-points/ https://setuptools.pypa.io/en/latest/pkg_resources.html#entry-points Or, alternatively by passing the name of a python module on the CLI. The second point is just a bit of glue code to wire up a new CLI arg The third point either involves having 'make install' put the QAPI code generator into /usr/share/qemu, or /usr/lib/python3.x. The latter would be if QAPI code generator were a formally release python module on pypi. The former would be if we're just trying a QAPI code generator as a local tool, not for pypi. A fallback would be to consume qemu.git as a git submodule from qapi-go.git. The gitmodule commit would not have to match the version of the QAPI schema we are generating code for. The least effort approach would be making QAPI code genrator accept a python module name, and call it via a git submodule. This would avoid declaring a long term support policy for the QAPI code gen python APIs as a blocker. > > 3. As mentioned in (2), created protocol.go and utils.go that are 100% > hand generated Go code. Message mentioned in (1a) is under > protocol.go > > 4. Defined license using SPDX-License-Identifier. > a. Every Go source code written by hand is 100% MIT-0 > b. Every Go source code generated is dual licensed as MIT-0 and > GPL-2.0-or-later > c. The binary code is expected to be MIT-0 only but not really > relevant for this series. > > If you want more information, please check the thread: > https://lists.gnu.org/archive/html/qemu-devel/2024-11/msg01621.html > > 5. I've renamed the generated files. > a. Any type related file is now prefixed with "gen_type_" > b. Any interface related file is prefixed as "gen_iface_" > > 6. Relevant changes were made to the doc but it is not complete. I plan > that follow-up proposals would add to the documentation. > > 7. Improvements to the generator were made to. > > 8. Also worth to mention that resulting generated code does not have any > diff with gofmt and goimport tools, as requested in the past. With regards, Daniel
On Fri, Feb 14, 2025 at 09:29:33PM +0100, Victor Toso wrote: > Hi again, > > This patch series intent is to introduce a generator that produces a Go > module for Go applications to interact over QMP with QEMU. > > Previous version (10 Jan 2025) > https://lists.gnu.org/archive/html/qemu-devel/2025-01/msg01530.html > > The generated code was mostly tested using existing examples in the QAPI > documentation, 192 instances that might have multiple QMP messages each. > > You can find the the tests and the generated code in my personal repo, > main branch: > > https://gitlab.com/victortoso/qapi-go > > If you want to see the generated code from QEMU's master but per patch: > > https://gitlab.com/victortoso/qapi-go/-/commits/qapi-golang-v4-by-patch In terms of generated code, my only real feedback is that the re-wrapping of docs comments is having undesirable effets on formatting ## # @add_client: # # Allow client connections for VNC, Spice and socket based character # devices to be passed in to QEMU via SCM_RIGHTS. # # If the FD associated with @fdname is not a socket, the command will # fail and the FD will be closed. # # @protocol: protocol name. Valid names are "vnc", "spice", # "@dbus-display" or the name of a character device (e.g. from # -chardev id=XXXX) # # @fdname: file descriptor name previously passed via 'getfd' command # # @skipauth: whether to skip authentication. Only applies to "vnc" # and "spice" protocols # # @tls: whether to perform TLS. Only applies to the "spice" protocol # # Since: 0.14 # # .. qmp-example:: # # -> { "execute": "add_client", "arguments": { "protocol": "vnc", # "fdname": "myclient" } } # <- { "return": {} } ## is getting turned into // Allow client connections for VNC, Spice and socket based character // devices to be passed in to QEMU via SCM_RIGHTS. If the FD // associated with @fdname is not a socket, the command will fail and // the FD will be closed. // // Since: 0.14 // // .. qmp-example:: -> { "execute": "add_client", "arguments": { // "protocol": "vnc", "fdname": "myclient" } // } <- { "return": {} } the '.. qmp-example' bit is what's particularly badly affected. If we assume that the input QAPI schemas are nicely lined wrapped, we could probably just preserve the docs lines as-is with no change in wrapping. That said I'm not sure if we'll need some docs syntax changes to make it render nicely - hard to say until the code appears up on pkg.go.dev, so can probably worry about that aspect later. > ################ > # Expectations # > ################ > > As is, this still is a PoC that works. I'd like to have the generated > code included in QEMU's gitlab [0] in order to write library and tools > on top. Initial version should be considered alpha. Moving to > beta/stable would require functional libraries and tools, but this work > needs to be merged before one commit to that. We don't need to overthink this. I don't think we're best served by continuing to post many more rounds of this series. Better to just get it into a dedicated git repo and iterate via pull requests IMHO. Golang uses semver, so we could start publishing the generated code in a Go module as it is today, as long as we pick a v0.XXX.0 version number. 'XXX' would be a packing of QEMU's 3 digit version into the semver 2nd digit. This lets us indicate this is not considered a stable API, letting us iterate on further imlp details, while also getting us in the habit of publishing releases to track schema updates for each new QEMU. We just need the patch for qapi-gen.py to support plugins for code generation to make this happen, so we can decouple ongoing development from QEMU's main git repo & release cycle. With regards, Daniel
Hi, On Mon, Feb 17, 2025 at 02:58:22PM +0000, Daniel P. Berrangé wrote: > On Fri, Feb 14, 2025 at 09:29:33PM +0100, Victor Toso wrote: > > Hi again, > > > > This patch series intent is to introduce a generator that produces a Go > > module for Go applications to interact over QMP with QEMU. > > > > Previous version (10 Jan 2025) > > https://lists.gnu.org/archive/html/qemu-devel/2025-01/msg01530.html > > > > The generated code was mostly tested using existing examples in the QAPI > > documentation, 192 instances that might have multiple QMP messages each. > > > > You can find the the tests and the generated code in my personal repo, > > main branch: > > > > https://gitlab.com/victortoso/qapi-go > > > > If you want to see the generated code from QEMU's master but per patch: > > > > https://gitlab.com/victortoso/qapi-go/-/commits/qapi-golang-v4-by-patch > > In terms of generated code, my only real feedback is that the > re-wrapping of docs comments is having undesirable effets > on formatting > > ## > # @add_client: > # > # Allow client connections for VNC, Spice and socket based character > # devices to be passed in to QEMU via SCM_RIGHTS. > # > # If the FD associated with @fdname is not a socket, the command will > # fail and the FD will be closed. > # > # @protocol: protocol name. Valid names are "vnc", "spice", > # "@dbus-display" or the name of a character device (e.g. from > # -chardev id=XXXX) > # > # @fdname: file descriptor name previously passed via 'getfd' command > # > # @skipauth: whether to skip authentication. Only applies to "vnc" > # and "spice" protocols > # > # @tls: whether to perform TLS. Only applies to the "spice" protocol > # > # Since: 0.14 > # > # .. qmp-example:: > # > # -> { "execute": "add_client", "arguments": { "protocol": "vnc", > # "fdname": "myclient" } } > # <- { "return": {} } > ## > > > is getting turned into > > > // Allow client connections for VNC, Spice and socket based character > // devices to be passed in to QEMU via SCM_RIGHTS. If the FD > // associated with @fdname is not a socket, the command will fail and > // the FD will be closed. > // > // Since: 0.14 > // > // .. qmp-example:: -> { "execute": "add_client", "arguments": { > // "protocol": "vnc", "fdname": "myclient" } > // } <- { "return": {} } > > > the '.. qmp-example' bit is what's particularly badly affected. > > If we assume that the input QAPI schemas are nicely lined wrapped, > we could probably just preserve the docs lines as-is with no change > in wrapping. > > That said I'm not sure if we'll need some docs syntax changes to > make it render nicely - hard to say until the code appears up on > pkg.go.dev, so can probably worry about that aspect later. My preference is that the Go code has nicely formatted sections, like the qmp-example one. The decision to not work on this now was made together with Markus as he pointed out this formatting on documentation part is still a work in progress, besides the fact that it can be done as a follow-up. Having examples is a nice thing even if the format is not great. > > ################ > > # Expectations # > > ################ > > > > As is, this still is a PoC that works. I'd like to have the generated > > code included in QEMU's gitlab [0] in order to write library and tools > > on top. Initial version should be considered alpha. Moving to > > beta/stable would require functional libraries and tools, but this work > > needs to be merged before one commit to that. > > We don't need to overthink this. I don't think we're best served by > continuing to post many more rounds of this series. Better to just > get it into a dedicated git repo and iterate via pull requests IMHO. Well, I'm happy to hear it. How the repo get created so we can move the discussion and patches there? > Golang uses semver, so we could start publishing the generated code in > a Go module as it is today, as long as we pick a v0.XXX.0 version number. > 'XXX' would be a packing of QEMU's 3 digit version into the semver 2nd > digit. This lets us indicate this is not considered a stable API, letting > us iterate on further imlp details, while also getting us in the habit of > publishing releases to track schema updates for each new QEMU. Sure. > We just need the patch for qapi-gen.py to support plugins for > code generation to make this happen, so we can decouple ongoing > development from QEMU's main git repo & release cycle. Looking forward to it. Cheers, Victor
Daniel P. Berrangé <berrange@redhat.com> writes: [...] > When I think about the code generator and how this will all > evolve over time, I have a strong feeling that none of this > should be in qemu.git. Yes, keeping it in qemu.git has its drawbacks. Testing is awkward there. The coupling could cause friction. I'll gladly support exploring alternatives. I saw your "[PATCH] qapi: pluggable backend code generators", and just sent my review. Thanks! [...]