Re: [PATCH v3 4/4] migration/qapi: Drop @MigrationParameter enum

[Markus Armbruster]

Peter Xu <peterx@redhat.com> writes:

> On Tue, Sep 26, 2023 at 10:43:22PM +0200, Markus Armbruster wrote:
>> Loophole...  Here's the stupidest solution that could possibly work:
>> 
>>     ##
>>     # @MigrationParameter:
>>     #
>>     # TODO: elide from generated documentation (type is used only
>>     #     internally, and not visible in QMP)
>>     #
>>     # Features:
>>     #
>>     # @unstable: Members @x-checkpoint-delay and @x-vcpu-dirty-limit-period
>>     #     are experimental.
>>     #
>>     # Since: 2.4
>>     ##
>> 
>> Works because the QAPI generator currently doesn't flag missing member
>> documentation, and quietly substitutes "Not documented" instead.
>
> Didn't work for me..
>
> In file included from ../qapi/qapi-schema.json:61:
> ../qapi/migration.json:681:1: unexpected de-indent (expected at least 4 
> spaces)
>
> L681 points to the "Features:".
>
> But maybe I did it wrong somewhere?

Parser bug?  Moving the TODO down some works around the issue:

    ##
    # @MigrationParameter:
    #
    # Features:
    #
    # @unstable: Members @x-checkpoint-delay and @x-vcpu-dirty-limit-period
    #     are experimental.
    #
    # TODO: elide from generated documentation (type is used only
    #     internally, and not visible in QMP)
    #
    # Since: 2.4
    ##

Weird.

Better, because even stupider: drop the feature flags.  They have no
effect on internal use, and there is no external use.

    ##
    # @MigrationParameter:
    #
    # TODO: elide from generated documentation (type is used only
    #     internally, and not visible in QMP)
    #
    # Since: 2.4
    ##
    { 'enum': 'MigrationParameter',
      'data': ['announce-initial', 'announce-max',
               'announce-rounds', 'announce-step',
               'compress-level', 'compress-threads', 'decompress-threads',
               'compress-wait-thread', 'throttle-trigger-threshold',
               'cpu-throttle-initial', 'cpu-throttle-increment',
               'cpu-throttle-tailslow',
               'tls-creds', 'tls-hostname', 'tls-authz', 'max-bandwidth',
               'downtime-limit',
               'x-checkpoint-delay',
               'block-incremental',
               'multifd-channels',
               'xbzrle-cache-size', 'max-postcopy-bandwidth',
               'max-cpu-throttle', 'multifd-compression',
               'multifd-zlib-level', 'multifd-zstd-level',
               'block-bitmap-mapping',
               'x-vcpu-dirty-limit-period',
               'vcpu-dirty-limit'] }


>> Looks like
>> 
>>     "MigrationParameter" (Enum)
>>     ---------------------------
>> 
>> 
>>     Values
>>     ~~~~~~
>> 
>>     "announce-initial"
>>        Not documented
>> 
>>     "announce-max"
>>        Not documented
>> 
>> and so forth.  Sure ugly, but is it really worse than before?  It's now
>> obviously useless, whereas before it was unobviously useless.
>> 
>> This will break when we tighten up the QAPI generator to require member
>> documentation.  Along we a few hundred other violators.
>> 
>> We might want to add a way to say "members intentionally undocumented".
>> Could be useful for qapi/ui.json's QKeyCode.  Most of its 162 members
>> don't really need documentation...
>
> Yes I'd be super happy if we can declare that in qapi/.
>
> Please let me know if I missed something above on the trick; you're right
> it's only about the documentation we want to get rid of.  If we can achieve
> that with qapi generating the helpers that'll definitely be perfect.

Done, I think :)