Re: [PATCH v2 1/2] qdev: add IOThreadVirtQueueMappingList property type

[Markus Armbruster <armbru@redhat.com>]

Kevin Wolf <kwolf@redhat.com> writes:

> Am 19.12.2023 um 16:13 hat Stefan Hajnoczi geschrieben:
>> On Sat, Oct 14, 2023 at 09:35:14AM +0200, Markus Armbruster wrote:
>> > Stefan Hajnoczi <stefanha@redhat.com> writes:
>> > > +##
>> > > +# @IOThreadVirtQueueMappings:
>> > > +#
>> > > +# IOThreadVirtQueueMapping list. This struct is not actually used but the
>> > > +# IOThreadVirtQueueMappingList type it generates is!
>> > 
>> > Two spaces between sentences for consistency, please.
>> > 
>> > Doc comments are QMP reference documentation for users.  Does this
>> > paragraph belong there?
>> 
>> Someone might wonder why a type exists that is never used, so I think
>> including this in the documentation is acceptable.
>
> I seem to remember that we had a similar remark elsewhere, but maybe it
> doesn't exist any more today?

Working up more context...  alright, now I see.

When the QAPI schema defines a type 'T', the QAPI generator generates
code and documentation for it whether it is used in the schema or not.
We occasionally use this to generate types with QAPI goodies for purely
internal use.  In other words, the generator assumes there is a use of T
in C code.

However, the QAPI generator generates code for ['T'] only if there's a
use of ['T'] in the schema.  This is because it's actually needed only
for about one in seven types.  See commit 9f08c8ec738 (qapi: Lazy
creation of array types).

Once in a blue moon, ['T'] is only used in C code.  The QAPI generator
won't generate code for it then, and the C code won't compile.  Then we
have to add a dummy use to the schema to force the array into existence.
Not exactly elegant, but it works.

I can see two blue moons in the schema before this patch.

In qapi/block-core.json:

    ##
    # @DummyBlockCoreForceArrays:
    #
    # Not used by QMP; hack to let us use BlockGraphInfoList internally
    #
    # Since: 8.0
    ##
    { 'struct': 'DummyBlockCoreForceArrays',
      'data': { 'unused-block-graph-info': ['BlockGraphInfo'] } }

It's called Dummy<NameOfModule>ForceArrays, it's at the end of the file,
and it collects all the arrays being forced into existence.

In qapi/machine.json:

    ##
    # @DummyForceArrays:
    #
    # Not used by QMP; hack to let us use X86CPUFeatureWordInfoList
    # internally
    #
    # Since: 2.5
    ##
    { 'struct': 'DummyForceArrays',
      'data': { 'unused': ['X86CPUFeatureWordInfo'] } }

Less good: it has a clash-prone name, and it's not at the end of the
file.  It was the first use of this trick, and we still had a single
schema file back then.

I recommend you do yours just like the first one.

>> My comment is mostly intended to stop someone from removing this
>> "unused" type from the schema though. If there is a better way to do
>> that I can do that instead.
>
> Won't the QAPI generator or the compiler stop them soon enough?

The compiler would.