Re: [RFC PATCH v1 0/8] qapi: add generator for Golang interface

[Markus Armbruster]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Tue, May 10, 2022 at 02:02:56PM +0200, Markus Armbruster wrote:
>> Daniel P. Berrangé <berrange@redhat.com> writes:
>> 
>> > On Tue, Apr 26, 2022 at 01:14:28PM +0200, Markus Armbruster wrote:
>> >> 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.
>> >> 
>> >> 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 might work for some projects, but in the general case I find it pretty
>> > unappealing as a restriction. Mixing and matching new QEMU with old 
>> > libvirt,
>> > or vica-verca has been an incredibly common thing todo when both developing
>> > and perhaps more importantly debugging problems. For example I have one
>> > libvirt build and I use it against any QEMU from Fedora / any RHEL-8.x
>> > update, which spans a great many QEMU releases. 
>> 
>> I'd like to propose that for compatibility with a wide range of QEMU
>> versions, you use or reinvent libvirt.
>
> Implicit in that statement though is that libvirt will not be able
> to make use of the QAPI code generator as proposed though. If we are
> designing something to make our application consumer's lives easier,
> but we exclude such a major application, is our solution actually
> a good one.

A solution for a narrow problem we can actually deliver and maintain is
better than a solution for a wider problem we can't.

>> > For a minimum viable use case, this doesn't feel all that difficult, as
>> > conceptually instead of deleting the field from QAPI, we just need to
>> > annotate it to say when it was deleted from the QEMU side.  The QAPI
>> > generator for internal QEMU usage, can omit any fields annotated as
>> > deleted in QAPI schema. The QAPI generator for external app usage,
>> > can (optionally) be told to include deleted fields ranging back to
>> > a given version number. So apps can chooses what degree of compat
>> > they wish to retain.
>> 
>> Consider this evolution of command block_resize
>
> To help us understand, I'll illustrate some possible interfaces
> in both Go and Python, since that covers dynamic and static
> languages
>
>> * Initially, it has a mandatory argument @device[*].
>
> Python definition:
>
>    def block_resize(device, size)
>
> Caller:
>
>   block_resize('dev0', 1*GiB)
>
>
> Golang definition
>
>    type BlockResizeCommand struct {
>        Device string
>        Size int
>    }
>
> Caller
>
>    cmd := &BlockResizeCommand{
>        Device: "dev0",
>        Size: 1 * GiB,
>    }
>
>> * An alternative way to specify the command's object emerges: new
>>   argument @node-name.  Both old @device and new @node-name become
>>   optional, and exactly one of them must be specified.  This is commit
>>   3b1dbd11a6 "qmp: Allow block_resize to manipulate bs graph nodes."
>
> Python definition. Tricky, as non-optional params must be before
> optional params, but size is naturally the last arg. One option
> is to pointlessly mark 'size' as optional
>
>    def block_resize(device=None, node_name=None, size=None)

Who needs compile-time checking anyway.

Back to serious.  Keyword arguments might be a better match for Python
bindings.

> Caller
>
>     block_resize(device="dev0", size=1*GiB)
>     block_resize(node_name="devnode0", size=1*GiB)
>
>
> In golang definition
>
>    type BlockResizeArguments struct {
>        Device string
>        NodeName string
>        Size int
>    }
>
> Caller choice of
>
>    cmd := &BlockResizeCommand{
>        Device: "dev0",
>        Size: 1 * GiB,
>    }
>
>    cmd := &BlockResizeCommand{
>        NodeName: "devnode0",
>        Size: 1 * GiB,
>    }

Note that the Go bindings you sketched effectively use (poor man's)
keyword arguments.

> Neither case can easily prevent passing Device and NodeName
> at same time.

That defect lies at the schema's feet.

>> * At some future date, the old way gets deprecated: argument @device
>>   acquires feature @deprecated.
>
> Ok, no change needed to the APIs in either case. Possibly have
> code emit a warning if a deprecated field is set.
>
>> * Still later, the old way gets removed: @device is deleted, and
>>   @node-name becomes mandatory.
>
> Again no change needed to APIs, but QEMU will throw back an
> error if the wrong one is used. 
>
>> What is the proper version-spanning interface?
>> 
>> I figure it's both arguments optional, must specify the right one for
>> the version of QEMU actually in use.  This spans versions, but it fails
>> to abstract from them.
>
> Yep, I think that's inevitable in this scenario. THe plus side
> is that apps that want to span versions can do so. The downside
> is that apps that don't want smarts to span version, may loose
> compile time warnings about use of the now deleted field. 

The version-spanning interface will arguably be a bad interface for any
version.

> I suggested the code generator have an option to say what level
> of compat to use for generated code, so that apps can request an
> API without compat, which will result in compile errors. This
> though assumes every consumer app is embedding their own
> generated copy of the code. Not neccessarily desirable.
>
> At the C level you can play games with __deprecated__ to get
> compile time warnings in some cases. 
>
> #define GLIB_VERSION_MIN_REQUIRED GLIB_VERSION_2_56
>
> causes QEMU to get compile time warnings (or errors) if it
> attempts to use a API feature deprecated in 2.56, even if
> the API exists in the header & library. 
>
>
>> Note that it's not enough to replace "delete member" by "mark member
>> deleted in <version>".  You also have to keep full history for "is it
>> optional".  And for types, because those can evolve compatibly, too,
>> e.g. from struct to flat union, or from string to alternate of string
>> and something else.  What is the proper version-spanning interface in
>> all the possible cases?
>
> I've not thought through all possible scenarios, but there may end
> up being restrictions, such that changes that were previously possible
> may have to be forbidden.

"There may be restrictions" is not exactly a confidence-inspring design
assumption.  We need a reasonably dependable idea on what exactly we're
intending to sacrifice.

> One example,  in the past we could do deprecate a field 'foo', then
> delete 'foo' and then some time re-introduce 'foo' with a completely
> different type. That would not be possible if we wanted to maintain
> compat, but in this example that's probably a good thing, as it'll
> be super confusing to have the same field name change type like that
> over time. Easier to just use a different name.
>
> So the question to me is not whether all our previous changes are
> still possible, but whether enough of the typwes of change are
> possible, such that we can cope with the ongoing maint in a
> reasonable way. I don't think we've explored the possibility enough
> to say one way or the other.
>
>> > Apps that wish to have version compat, would of course need to write
>> > their code to be aware of which fields they need to seend for which
>> > QEMU version.
>> 
>> At which point we're reinventing libvirt.
>
> The premise of the code generators is that there are applications
> that want to consume QEMU that are not libvirt. With this line of
> reasoning we could easily say that all such applications should
> just use libvirt and then we don't need to provide any of these
> code generators.  The fact that we're considering these code
> generators though, says that we're accepting there are valid use
> cases that don't want to use libvirt for whatever reasons.

Can't resist any longer: "What has libvirt ever done for us?"
https://www.youtube.com/watch?v=Qc7HmhrgTuQ

>                                                            It is
> reasonable that some of those applications may wish to target
> a wide range of QEMU versions, just like libvirt does.

At which point they're comitting to reinventing the relevant parts of
libvirt.

I'd expect marshalling and umarshalling QMP to be among the smaller
sub-problems then.  It may look relatively large at first, because it's
among the first ones to be solved.  More so when you hand-wave away the
more interesting ones of *abstraction* until they bite you in the
posterior.

> It is also reasonable to say that libvirt would be better off if
> it could auto-generate a client API for QEMU too, instead of
> writing it by hand from a human reading the QAPI
>
> With regards,
> Daniel