Hi, Thanks for the quick review Markus. Sorry for taking quite a bit of time to get back to you.
On Tue, Apr 26, 2022 at 01:14:28PM +0200, Markus Armbruster wrote: > Victor Toso <victort...@redhat.com> writes: > > > Hi, > > > > Happy 1st April. Not a joke :) /* ugh, took me too long to send */ > > > > This series is about adding a generator in scripts/qapi to produce > > Go data structures that can be used to communicate with QEMU over > > QMP. > > > > > > * Why Go? > > > > There are quite a few Go projects that interact with QEMU over QMP > > and they endup using a mix of different libraries with their own > > code. > > > > > > ** Which projects? > > > > The ones I've found so far: > > > > - podman machine > > https://github.com/containers/podman/tree/main/pkg/machine/qemu > > > > - kata-containers (govmm) > > > > https://github.com/kata-containers/kata-containers/tree/main/src/runtime/pkg/govmm > > > > - lxd > > https://github.com/lxc/lxd/tree/master/lxd/instance/drivers > > > > - kubevirt (plain json strings) > > https://github.com/kubevirt/kubevirt > > > > (let me know if you know others) > > > > > > * But Why? > > > > I'm particularly interested in 3 out of 4 of the projects above and > > only Kubevirt uses libvirt to handle QEMU. That means that every > > QEMU releases where a QMP command, event or other data struct is > > added, removed or changed, those projects need to check what changed > > in QEMU and then address those changes in their projects, if needed. > > > > The idea behind generating Go data structures is that we can keep a > > Go module which can have releases that follow QEMU releases. > > We need to look at "following the QEMU releases" a bit more > closely. > > Merging your patches gives us the capability to generate a Go > interface to HEAD's version of QMP. Right, just to put it up here, it should be expected that the qapi-go project is to have releases that match QEMU's release. > The obvious way for an out-of-tree Go program to use this > generated Go interface is to build with a specific version of > it. It can then talk QMP to any compatible QEMU version. > > Compatibility with older QEMUs is not assured: stuff added > since is present on the Go QMP client end, but not on the QEMU > QMP server end. > > Compatibility with newer QEMUs is subject to our deprecation > policy: > > In general features are intended to be supported > indefinitely once introduced into QEMU. In the event that > a feature needs to be removed, it will be listed in this > section. The feature will remain functional for the > release in which it was deprecated and one further release. > After these two releases, the feature is liable to be > removed. > > So, if you stay away from deprecated stuff, you're good for two > more releases at least. > > Does this work for the projects you have in mind? It depends on how the project will be using qapi-go, so I can't say for them all. There are projects that will be targeting specific QEMU version (e.g: Kubevirt, Kata containers) and for those, I think they don't mind only bumping qapi-go when they plan to change the target QEMU version or perhaps to keep separated binary versions for a limited amount of time (just my guess). Some other projects like Podman, will likely be talking with the running version of QEMU they have in that host. The possibilities are quite broad here. > Aside: graceful degradation in case of incompatibility seems > desirable. I agree. I haven't thought much on how to handle those scenarios yet and suggestions are more than welcomed. I know that, those projects are already hardcoding commands and expected return types by hand. My first goal is to provide well defined types, QAPI/QMP compliant, with easy to reach documentation as provided by QAPI docs. I expect that, step by step, we can improve things all around but I don't expect it to be done all at once. > > The project that uses this Go module, only need to bump the > > module version and it shall receive all the changes in their > > own vendored code base. > > Ideally, incompatible changes that affect the Go program show > up as compile errors. Do they? It depends. A field/type that they were using but is removed, for sure a compile-time error. What about a new mandatory field? If you run the unit tests in qapi-go, there will be failures but it wouldn't be compile time error. If user doesn't define a mandatory field in a Go struct, the default value is used for the over-the-wire message. Perhaps some tooling can be developed to help users check that something they are using has changed. I'll look into it. > > * Status > > > > There are a few rough edges to work on but this is usable. The major > > thing I forgot to add is handling Error from Commands. It'll be the > > first thing I'll work on next week. > > > > If you want to start using this Today you can fetch it in at > > > > https://gitlab.com/victortoso/qapi-go/ > > > > There are quite a few tests that I took from the examples in the > > qapi schema. Coverage using go's cover tool is giving `28.6% of > > statements` > > > > I've uploaded the a static generated godoc output of the above Go > > module here: > > > > > > https://fedorapeople.org/~victortoso/qapi-go/rfc/victortoso.com/qapi-go/pkg/qapi/ > > > > > > * License > > > > While the generator (golang.py in this series) is GPL v2, the > > I'd make it v2+, just to express my displeasure with the > decision to make the initial QAPI generator v2 only for no good > reason at all. > > > generated code needs to be compatible with other Golang projects, > > such as the ones mentioned above. My intention is to keep a Go > > module with a MIT license. > > Meh. Can't be helped, I guess. :) > > * Disclaimer to reviewers > > > > This is my first serious python project so there'll be lots of > > suggetions that I'll be happy to take and learn from. > > > > > > Thanks for taking a look, let me know if you have questions, ideas > > or suggestions. > > > > Cheers, > > Victor Thanks again, Victor
signature.asc
Description: PGP signature