Re: Notes on Generating Python signatures for QMP RPCs

[Markus Armbruster <armbru@redhat.com>]

John Snow <jsnow@redhat.com> writes:

> 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:

[...]

>> I think it is not unreasonable to expose the struct names
>> on introspection though, and just accept that it ties our
>> hands a little more to avoid renaming structs. I don't
>> think we rename frequently enough that this matters.
>
> I feel like I don't have a real stake in this issue yet. Maybe we can
> discuss bolstering the introspection data if we decide that we really,
> really would like the ability to generate bindings dynamically on the
> client side. I'm not sure *I* even want that enough to really push for
> this change yet. (Again, I think I need to come forward with something
> more concrete than an experiment before I dive too deeply into this
> issue. I'll get back to you.)
>
> I wouldn't mind hearing from Markus on what he believes the value of
> anonymizing the types names is. My current understanding is: "The type
> names aren't necessary to speak QMP, so they aren't necessary to
> reveal. We operate on a need-to-know basis, and nobody has needed to
> know."
>
> (The most tangible story I can think of is that we don't want clients
> to do things like assume they can index the introspection data in a
> hashmap and rely on looking up specific type names.)

QMP's compatibility promise (which predates schema introspection)
applies to commands and events.

When I designed schema introspection, I didn't want to grow the existing
compatibility promise, because that would restrict what we can do in the
schema.  Instead, I opted to limit the new compatibility promise for
introspection.  Section "Client JSON Protocol introspection" in
docs/devel/qapi-code-gen.rst has a paragraph on it.

Since telling users what not to do has a somewhat disappointing success
rate, I additionally looked for easy ways to make things not to be done
impractical.  I found "hide the type names".  Tiny bonus: saves a bit of
bandwidth, which probably doesn't matter beyond pleasing me.

Deterring users from bad practice was arguably more important when
schema introspection was new, and good practice wasn't established.

commit 1a9a507b2e3e90aa719c96b4c092e7fad7215f21 (tag: pull-qapi-2015-09-21)
Author: Markus Armbruster <armbru@redhat.com>
Date:   Wed Sep 16 13:06:29 2015 +0200

    qapi-introspect: Hide type names
    
    To eliminate the temptation for clients to look up types by name
    (which are not ABI), replace all type names by meaningless strings.
    
    Reduces output of query-schema by 13 out of 85KiB.
    
    As a debugging aid, provide option -u to suppress the hiding.
    
    Signed-off-by: Markus Armbruster <armbru@redhat.com>
    Reviewed-by: Eric Blake <eblake@redhat.com>
    Message-Id: <1442401589-24189-27-git-send-email-armbru@redhat.com>

>> A "NORETURN" flag sounds like a reasonable idea.
>
> Markus has gently pointed out that we do have this information in the
> schema, but it isn't revealed over introspection data *and* we do not
> have introspection for QGA anyway.
>
> We /could/ remove success-response and add a 'NORETURN' feature flag,
> modifying the generator to use that flag (instead of
> success-response), and then we'd get away with not having to modify
> the introspection schema. But we'd still have to add introspection in
> general to QGA, which rather sounds like where the bulk of the work is
> anyway.

If we had feature flags from the start, we might not have added other
flags to the syntax, such as @gen, @success-response, @boxed.

Feature flags are exposed in introspection, the others aren't.  That
dictates which kind to use when adding a flag.

Flags that aren't exposed in introspection can only be used by the
generator itself (d'oh).

A few special feature flags are also used by the generator.  Currently
'deprecated' and 'unstable'.

>>
>> Regards,
>> Daniel
>
> Thanks! I've got a lot to think about.

I might have pile on some more, forgive me ;)