Re: [RFC v2 1/5] qapi/qom: Introduce kvm-pmu-filter object

[Markus Armbruster <armbru@redhat.com>]

Zhao Liu <zhao1.liu@intel.com> writes:

> Hi Mrkus,
>
> I'm really sorry I completely missed your reply (and your patient
> advice). It wasn't until I looked back at the lore archives that I
> realized my mistake. Thinking it over again, I see that your reply,
> which I missed, really helped clear up my confusion:

I'm glad I was able to help some!

> On Fri, Feb 07, 2025 at 02:02:44PM +0100, Markus Armbruster wrote:
>> Date: Fri, 07 Feb 2025 14:02:44 +0100
>> From: Markus Armbruster <armbru@redhat.com>
>> Subject: Re: [RFC v2 1/5] qapi/qom: Introduce kvm-pmu-filter object
>> 
>> Zhao Liu <zhao1.liu@intel.com> writes:
>> 
>> >> Let's ignore how to place it for now, and focus on where we would *like*
>> >> to place it.
>> >> 
>> >> Is it related to anything other than ObjectType / ObjectOptions in the
>> >> QMP reference manual?
>> >
>> > Yes!
>> 
>> Now I'm confused :)
>> 
>> It is related to ObjectType / ObjectType.
>> 
>> Is it related to anything else in the QMP reference manual, and if yes,
>> to what exactly is it related?
>
> I misunderstood your point. The PMU stuff and the QAPI definitions for
> ObjectType/ObjectOptions are not related. They should belong to separate
> categories or sections.
>
>> >> I guess qapi/kvm.json is for KVM-specific stuff in general, not just the
>> >> KVM PMU filter.  Should we have a section for accelerator-specific
>> >> stuff, with subsections for the various accelerators?
>> >> 
>> >> [...]
>> >
>> > If we consider the accelerator from a top-down perspective, I understand
>> > that we need to add accelerator.json, kvm.json, and kvm-pmu-filter.json.
>> >
>> > The first two files are just to include subsections without any additional
>> > content. Is this overkill? Could we just add a single kvm-pmu-filter.json
>> > (I also considered this name, thinking that kvm might need to add more
>> > things in the future)?
>> >
>> > Of course, I lack experience with the file organization here. If you think
>> > the three-level sections (accelerator.json, kvm.json, and kvm-pmu-filter.json)
>> > is necessary, I am happy to try this way. :-)
>> 
>> We don't have to create files just to get a desired section structure.
>> 
>> I'll show you how in a jiffie, but before I do that, let me stress: we
>> should figure out what we want *first*, and only then how to get it.
>> So, what section structure would make the most sense for the QMP
>> reference manual?
>
> Thank you for your patience. I have revisited and carefully considered
> the "QEMU QMP Reference Manual," especially from a reader's perspective.
> Indeed, I agree that, as you mentioned, a three-level directory
> (accelerator - kvm - kvm stuff) is more readable and easier to maintain.

Sounds good to me.

> For this question "what we want *first*, and only then how to get it", I
> think my thought is:
>
> First, the structure should be considered, and then the specific content
> can be added. Once the structure is clearly defined, categorizing items
> into their appropriate places becomes a natural process...
>
> Then for this question "what section structure would make the most sense
> for the QMP reference manual?", I understand that a top-down, clearly
> defined hierarchical directory makes the most sense, allowing readers to
> follow the structure to find what they want. Directly adding
> kvm-pmu-filter.json or kvm.json would disrupt the entire structure, because
> KVM is just one of the accelerators supported by QEMU. Using "accelerator"
> as the entry point for the documentation, similar to the "accel" directory
> in QEMU's source code, would make indexing more convenient.

I think so, too.

>> A few hints on how...
>> 
>> Consider how qapi/block.json includes qapi/block-core.json:
>> 
>>     ##
>>     # = Block devices
>>     ##
>> 
>>     { 'include': 'block-core.json' }
>> 
>>     ##
>>     # == Additional block stuff (VM related)
>>     ##
>> 
>> block-core.json starts with
>> 
>>     ##
>>     # == Block core (VM unrelated)
>>     ##
>> 
>> Together, this produces this section structure
>> 
>>     = Block devices
>>     == 
>>     ##
>> 
>> Together, this produces this section structure
>> 
>>     = Block devices
>>     == Block core (VM unrelated)
>>     == Additional block stuff (VM related)
>> 
>> Note that qapi/block-core.json isn't included anywhere else.
>> qapi/qapi-schema.json advises:
>> 
>>     # Documentation generated with qapi-gen.py is in source order, with
>>     # included sub-schemas inserted at the first include directive
>>     # (subsequent include directives have no effect).  To get a sane and
>>     # stable order, it's best to include each sub-schema just once, or
>>     # include it first right here.
>
> Thank you very much!!
>
> Based on your inspiration, I think the ideal section structure for my
> issue could be:
>
>     = accelerator
>     == KVM
>     === PMU
>
> Firstly, I should have a new accelerator.json () to include KVM stuff:
>
>     ##
>     # = Accelerator
>     ##
>
>     { 'include': 'kvm.json' }
>
> Next, in kvm.json, I could organize stuffs like:
>
>     ##
>     # == KVM
>     ##
>
>     ##
>     # === PMU stuff
>     ##
>
>     ... (the below are my current QPAI definitions.)
>
> Is such a structure reasonable?

Yes.

I'm not a fan of schema files with nothing but includes, like
accelerator.json.  But the alternative here would be putting its
contents into qapi-schema.json, which I really don't like, or keeping
the KVM contents there instead of in a separate kvm.json, which would
interfere with tracking maintainers in the MAINTAINERS file.

> Thank you again for your guidance!

You're welcome!