Re: Notes on Generating Python signatures for QMP RPCs

[Markus Armbruster]

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

> On Thu, Feb 03, 2022 at 05:52:10PM -0500, John Snow wrote:
>> On Thu, Feb 3, 2022 at 5:40 AM Daniel P. Berrangé <berrange@redhat.com> 
>> wrote:
>> >
>> > On Wed, Jan 26, 2022 at 01:58:19PM -0500, John Snow wrote:
>> >
>> > As you mention though, bear in mind that a command returning
>> > nothing today, might return something tomorrow. IOW, we'd be
>> > going from a empty dict to a non-empty dict. If you use "None"
>> > then it'd be gonig from None to a non-empty dict.
>> >
>> > I think you can argue both of these approaches are backwards
>> > compatible. The python app is not likely to be looking at
>> > the return type at all initially - unlike C, errors get
>> > raised as exceptions, so you don't need to look at return
>> > type to distinguish succes from failure.
>> >
>> > So I'd consider it merely a documentation problem to say
>> > that a "None" return type might later change to a dict.
>> > Dunno how you represent that in python type annotations,
>> > but I presume there's a way.
>> 
>> I don't think type hints offer a temporal dimension to them yet :)
>> 
>> I started writing a much lengthier response, but the subject of
>> compatibility is really complex and I am not prepared to give a
>> comprehensive response to some of the issues you raise ... so instead
>> I will say "Wow, good points!" and I will get back to you on some of
>> it. A lot of things will only make sense if we are talking about a
>> very specific type of project, with very specific goals that are
>> clearly established. I don't really have that ready, here; I am just
>> experimenting to learn where some of the pain points are, still.
>> 
>> So... I'll get back to you on this.
>> 
>> > We do allow fields to be deleted, which is a *non-compatible*
>> > evolution, but they MUST have been marked as deprecated for
>> > 2 cycles first.
>> 
>> Good point.
>> 
>> > I'd say sorting required vs optional arguments is doomed as
>> > a strategy. Stuff that is mandatory today can be made optional
>> > tomorrow and I think that's reasonable to want todo as we
>> > evolve an API design.
>> 
>> Also a good point. Python requires all mandatory arguments precede all
>> optional ones, so you're probably right that in order to maximize
>> cross-version compatibility, keyword-only arguments for *all*
>> arguments both mandatory and optional is the only real way to fly.
>> 
>> I think this might cause problems for Marc-Andre in rust/dbus land,
>> but it's tractable in Python. I am unclear on the ramifications for
>> golang. (Victor, Marc-Andre, any input here?)
>
> Well as a general point, if we consider usage outside of
> qemu.git, I'm far from convinced that generating code from
> the schema as it exists today is going to be broadly usable
> enough to make it worthwhile.
>
> The problem is precisely that the code that is generated is
> only ensured to work with the specific version of QEMU that
> it was generated from. The key problem here is the removal
> of features after deprecation.  That removal is fine if you
> only ever need an API to talk to the current QEMU, or only
> need to be able to introspect the current QEMU.
>
> Management apps are likely to want to write code that works
> with more than 1 version of QEMU, and be able to decide
> whether they provide the params needed by the old command
> or the new command.   The introspection data lets them
> make the decision about which command needs to be invoked,
> but it can't be used to generate the code needed for the
> old command.
>
> I don't know how exactly you're dealing with the Python
> mapping. If you're literally statically generating Python
> code you'll face this same problem. If on the other hand
> you've just got a stub object that does dynamic dispatch
> then it can dynamically adapt at runtime to work with
> whatever version of the schema it queried from the QEMU
> it is talking to. I'm hoping you're doing the latter
> for this reason.
>
> One strategy for compiled languages is to generate multiple
> copies of the APIs, one for each QEMU version. This could
> be made to work but I feel it is pretty horrific as an
> approach.  Libvirt has one set of client APIs that we've
> extended over time so they can be used to call both old
> and new variants of commands - we just need to use the
> introspected schema to decide which to use.
>
> To make the latter viable for generating compiled code
> though, we need to change how we deal with removals, by
> essentially never removing things at all. Instead we
> would need some way to express that "field A" or "type T"
> is not present after some point in time / release.
>
> Overall I don't think we're really in a position to use
> compiled auto generated bindings, except for QMP clients
> that are released in lockstep with specific QEMU versions,
> and I don't think lockstep releases are a viable strategy
> in general.

You described two choices.  I believe there's a third one.

A management application can choose to target a single QEMU version, and
require lockstep upgrade, i.e. upgrading QEMU requires upgrading to the
matching management application, and vice versa.  This is quite a hassle
for users.

The other extreme is to target a range of QEMU versions that is so wide
that the management application has to deal both with old and new
interfaces.  QEMU provides schema introspection to help with that, and
libvirt makes use of it.

A third choice is to target the narrow range of QEMU versions where the
deprecation policy protects you.  If a management application release
for QEMU version N uses no deprecated interfaces, it should be good up
to version N+2.  That's a full year.  Less wiggle room than libvirt
provides.  Whether the extra wiggle room is worth the extra effort is
for the management application developers to decide.

Note that this *can* use bindings generated for version N.  These
bindings talk wire format version N to QEMU, which QEMU up to version
N+2 must understand as per deprecation policy.

The difference between the first and the last choice is some extra
analysis and testing: you have to ensure no uses of deprecated
interfaces are left (QEMU provides -compat for that since v6.0), and you
have to keep testing with new QEMU releases (preferably *before* they
come out), to guard against deprecation policy violations slipping
through.  Same testing as for the second choice, just for fewer QEMU
releases.

>> > It sounds like you need a wrapper type.  This StrOrNull scenario
>> > is QMP's "alternate" type IIUC, but you're trying to avoid
>> > expressing the existance fo the "alternate" type in the API
>> 
>> Yes. This is a very clean way to type it, but it is a little more
>> laborious for the user to have to remember to wrap certain strings in
>> a special constructor first. Still, this is a good trick that I hadn't
>> considered. I'll keep it in mind for future experiments.
>
> Bear in mind that this situation is pretty rare, so I don't think
> the user is especially burdened by needing an extra level of
> indirection for using 'alternate' types
>
> $ git grep "'alternate'" qapi
> qapi/block-core.json:{ 'alternate': 'BlockDirtyBitmapMergeSource',
> qapi/block-core.json:{ 'alternate': 'Qcow2OverlapChecks',
> qapi/block-core.json:{ 'alternate': 'BlockdevRef',
> qapi/block-core.json:{ 'alternate': 'BlockdevRefOrNull',
> qapi/common.json:{ 'alternate': 'StrOrNull',
>
> $ git grep "'StrOrNull'" qapi
> qapi/block-core.json:             'iothread': 'StrOrNull',
> qapi/common.json:{ 'alternate': 'StrOrNull',
> qapi/migration.json:            '*tls-creds': 'StrOrNull',
> qapi/migration.json:            '*tls-hostname': 'StrOrNull',
> qapi/migration.json:            '*tls-authz': 'StrOrNull',

Yes.

Apropos StrOrNull.  The natural C binding would be a nullable const *
(plain str is *not* nullable).  But StrOrNull is too rare to bother.