John Snow <[email protected]> writes:
> On Fri, Mar 20, 2026, 9:45 AM Markus Armbruster <[email protected]> wrote:
>
>> John Snow <[email protected]> writes:
>>
>> > When a documentation block consists only of plaintext, there is nothing
>> > to semantically differentiate the "intro" from the "details"
>> > section. For the purposes of the inliner, the intro section is likely to
>> > be dropped while the details section will be merged onto the end of the
>> > parent's details section.
>> >
>> > When the delineation between "intro" and "details" is not clear because
>> > there is no intervening "members", "features", "errors", "returns",
>> > "TODO", or "Since" section, the parser assumes the entire text section
>> > is an "intro" section. This may not always be semantically true, so this
>> > patch clarifies certain sections explicitly as "details" sections by
>> > using an empty "Details:" marker.
>> >
>> > Signed-off-by: John Snow <[email protected]>
>> > ---
>> > qapi/block-core.json | 3 +++
>> > qapi/machine.json | 2 +-
>> > qapi/migration.json | 8 ++++++--
>> > qapi/net.json | 2 +-
>> > qapi/qom.json | 4 ++++
>> > qapi/yank.json | 2 +-
>> > scripts/qapi/parser.py | 8 ++++++++
>> > 7 files changed, 24 insertions(+), 5 deletions(-)
>>
>> Missing: update to docs/devel/qapi-code-gen.rst. Suggest to put in a
>> FIXME, so we don't forget.
>>
>
> ACK
>
>
>> >
>> > diff --git a/qapi/block-core.json b/qapi/block-core.json
>> > index f8d446b3d6e..c5ff15ae7a1 100644
>> > --- a/qapi/block-core.json
>> > +++ b/qapi/block-core.json
>> > @@ -5007,6 +5007,9 @@
>> > # @blockdev-reopen:
>> > #
>> > # Reopens one or more block devices using the given set of options.
>> > +#
>> > +# Details:
>> > +#
>> > # Any option not specified will be reset to its default value
>> > # regardless of its previous status. If an option cannot be changed
>> > # or a particular driver does not support reopening then the command
>>
>> Rendered documentation before the patch:
>>
>> Command blockdev-reopen (Since: 6.1)
>>
>> Reopens one or more block devices using the given set of options.
>> Any option not specified will be reset to its default value
>> regardless of its previous status. If an option cannot be changed
>> or a particular driver does not support reopening then the command
>> will return an error. All devices in the list are reopened in one
>> transaction, so if one of them fails then the whole transaction is
>> cancelled.
>>
>> [...]
>>
>> Unlike with "blockdev-add", the "backing" option must always be
>> present unless the node being reopened does not have a backing file
>> and its image does not have a default backing file name as part of
>> its metadata.
>>
>> Arguments:
>> * options ([BlockdevOptions]) -- Not documented
>>
>> The command's single argument is undocumented, and the doc generator
>> papered over this (sadly common defect) by splicing in this "Not
>> documented" description.
>>
>> Aside: why not boxed, like blockdev-add? Design mistake?
>>
>> The patch moves the splice point. Not wrong, but we should really fill
>> in the document gap instead.
>>
>
> Sure, but I'm worried about other things for the time being. I don't
> disagree but I think I'm the wrong person for intricate doc cleanup work of
> that sort.
Put in a TODO for now?
>> > diff --git a/qapi/machine.json b/qapi/machine.json
>> > index 685e4e29b87..bc2279b2526 100644
>> > --- a/qapi/machine.json
>> > +++ b/qapi/machine.json
>> > @@ -1259,7 +1259,7 @@
>> > # Return the amount of initially allocated and present hotpluggable
>> > # (if enabled) memory in bytes.
>> > #
>> > -# TODO: This line is a hack to separate the example from the body
>> > +# Details:
>>
>> This replaces a hack by a proper solution. More of the same below, not
>> mentioning this again.
>>
>> > #
>> > # .. qmp-example::
>> > #
>> > diff --git a/qapi/migration.json b/qapi/migration.json
>> > index 7134d4ce47e..558b4f145ed 100644
>> > --- a/qapi/migration.json
>> > +++ b/qapi/migration.json
>> > @@ -1633,7 +1633,7 @@
>> > #
>> > # Query replication status while the vm is running.
>> > #
>> > -# TODO: This line is a hack to separate the example from the body
>> > +# Details:
>> > #
>> > # .. qmp-example::
>> > #
>> > @@ -1651,6 +1651,8 @@
>> > #
>> > # Xen uses this command to notify replication to trigger a checkpoint.
>> > #
>> > +# Details:
>> > +#
>> > # .. qmp-example::
>> > #
>> > # -> { "execute": "xen-colo-do-checkpoint" }
>>
>> No change to generated documentation. I guess this is needed to avoid
>> the yelping added in the next patch. Correct?
>>
>
> Yes, probably. I didn't differentiate the cases...
>
>
>> > @@ -1687,7 +1689,7 @@
>> > #
>> > # Query COLO status while the vm is running.
>> > #
>> > -# TODO: This line is a hack to separate the example from the body
>> > +# Details:
>> > #
>> > # .. qmp-example::
>> > #
>> > @@ -1724,6 +1726,8 @@
>> > #
>> > # Pause a migration. Currently it only supports postcopy.
>> > #
>> > +# Details:
>> > +#
>> > # .. qmp-example::
>> > #
>> > # -> { "execute": "migrate-pause" }
>>
>> Likewise?
>>
>> > diff --git a/qapi/net.json b/qapi/net.json
>> > index 118bd349651..c011d6dc1a9 100644
>> > --- a/qapi/net.json
>> > +++ b/qapi/net.json
>> > @@ -1070,7 +1070,7 @@
>> > # switches. This can be useful when network bonds fail-over the
>> > # active slave.
>> > #
>> > -# TODO: This line is a hack to separate the example from the body
>> > +# Details:
>> > #
>> > # .. qmp-example::
>> > #
>> > diff --git a/qapi/qom.json b/qapi/qom.json
>> > index 1b47abd44e9..568b7d4b997 100644
>> > --- a/qapi/qom.json
>> > +++ b/qapi/qom.json
>> > @@ -777,6 +777,8 @@
>> > #
>> > # Properties for memory-backend-shm objects.
>> > #
>> > +# Details:
>> > +#
>> > # This memory backend supports only shared memory, which is the
>> > # default.
>> > #
>>
>> Rendered documentation changes from
>>
>> Object MemoryBackendShmProperties (Since: 9.1)
>> Availability: CONFIG_POSIX
>>
>> Properties for memory-backend-shm objects.
>>
>> This memory backend supports only shared memory, which is the
>> default.
>>
>> Members:
>> * The members of MemoryBackendProperties.
>>
>> to
>>
>> Object MemoryBackendShmProperties (Since: 9.1)
>> Availability: CONFIG_POSIX
>>
>> Properties for memory-backend-shm objects.
>>
>> Members:
>> * The members of MemoryBackendProperties.
>>
>> This memory backend supports only shared memory, which is the
>> default.
>>
>> Feels like a wash.
>>
>> I guess we need a "Details:" somewhere to avoid yelping. Correct?
>>
>
> If we choose to formally include the yelper patch. I included it to show
> all of the cases where this *might* make a difference.
>
> There's likely other heuristics we could employ to reduce how often we
> yelp. For instance, things that are never inlined from, or never inline,
> could likely be skipped as it won't make a difference.
>
> I think the trick here is to come up with a blanket "rule" that makes sense
> and is easy to understand for the uninitiated, even if that rule covers
> more cases than strictly necessary.
Yes.
I think it's better to keep the rules around "Details:" as simple as
possible, even if that means we have to write it more often. The
decision whether and where to write it should be as obvious as we can
make it. The less obvious is it, the more often we'll get it wrong[*].
> This patch used a very broad algorithm with almost no edge cases to yelp.
>
> Concern: if I do not make the parser yelp, we may see new cases added in
> the future that render in unintended ways. If I make the yelper too yelpy,
> it's just annoying without providing sufficient benefit.
Yes, this is still an open problem.
> Truth is likely in the middle somewhere. This patch is an effort to find
> and flush out that truth by brute force.
Good.
>> > @@ -792,6 +794,8 @@
>> > #
>> > # Properties for memory-backend-epc objects.
>> > #
>> > +# Details:
>> > +#
>> > # The @merge boolean option is false by default with epc
>> > #
>> > # The @dump boolean option is false by default with epc
>>
>> Similar change to rendered documentation, same request to confirm
>> yelping.
>>
>> > diff --git a/qapi/yank.json b/qapi/yank.json
>> > index f3cd5c15d60..2854a8a9d2a 100644
>> > --- a/qapi/yank.json
>> > +++ b/qapi/yank.json
>> > @@ -104,7 +104,7 @@
>> > #
>> > # Query yank instances. See `YankInstance` for more information.
>> > #
>> > -# TODO: This line is a hack to separate the example from the body
>> > +# Details:
>> > #
>> > # .. qmp-example::
>> > #
>>
>> [Skipping the Python code in my first pass...]
>>
>> I'd be tempted to split the patch into hack replacement and yelping
>> avoidance.
>>
>
> ACK, I can do that. It will take a moment to audit which cause real html
> output changes and which are solely yelp avoidance, but it can be done.
Thanks!
> Note that some changes may appear to be yelp avoidance only without the
> inliner active. Some cases *may* produce tangible differences with the
> inliner active.
>
> I don't have an example at hand or a method currently to identify them,
> it's just a heads up for the possibility.
[*] A glance at rendered documentation should catch most instances of
wrong, but in developers generally don't do that.
