[PATCH 00/27] Add qapi-domain Sphinx extension

[PATCH 00/27] Add qapi-domain Sphinx extension

[John Snow]

This series adds a new qapi-domain extension for Sphinx, which adds a
series of custom directives for documenting QAPI definitions.

GitLab CI: https://gitlab.com/jsnow/qemu/-/pipelines/1259566476

(Link to a demo HTML page at the end of this cover letter, but I want
you to read the cover letter first to explain what you're seeing.)

This adds a new QAPI Index page, cross-references for QMP commands,
events, and data types, and improves the aesthetics of the QAPI/QMP
documentation.

This series adds only the new ReST syntax, *not* the autogenerator. The
ReST syntax used in this series is, in general, not intended for anyone
to actually write by hand. This mimics how Sphinx's own autodoc
extension generates Python domain directives, which are then re-parsed
to produce the final result.

I have prototyped such a generator, but it isn't ready for inclusion
yet. (Rest assured: error context reporting is preserved down to the
line, even in generated ReST. There is no loss in usability for this
approach. It will likely either supplant qapidoc.py or heavily alter
it.) The generator requires only extremely minor changes to
scripts/qapi/parser.py to preserve nested indentation and provide more
accurate line information. It is less invasive than you may
fear. Relying on a secondary ReST parse phase eliminates much of the
complexity of qapidoc.py. Sleep soundly.

The purpose of sending this series in its current form is largely to
solicit feedback on general aesthetics, layout, and features. Sphinx is
a wily beast, and feedback at this stage will dictate how and where
certain features are implemented.

A goal for this syntax (and the generator) is to fully in-line all
command/event/object members, inherited or local, boxed or not, branched
or not. This should provide a boon to using these docs as a reference,
because users will not have to grep around the page looking for various
types, branches, or inherited members. Any arguments types will be
hyperlinked to their definition, further aiding usability. Commands can
be hotlinked from anywhere else in the manual, and will provide a
complete reference directly on the first screenful of information.

(Okay, maybe two screenfuls for commands with a ton of
arguments... There's only so much I can do!)

This RFC series includes a "sandbox" .rst document that showcases the
features of this domain by writing QAPI directives by hand; this
document will be removed from the series before final inclusion. It's
here to serve as a convenient test-bed for y'all to give feedback.

All that said, here's the sandbox document fully rendered:
https://jsnow.gitlab.io/qemu/qapi/index.html

And here's the new QAPI index page created by that sandbox document:
https://jsnow.gitlab.io/qemu/qapi-index.html

Known issues / points of interest:

- The formatting upsets checkpatch. The default line length for the
  "black" formatting tool is a little long. I'll fix it next spin.

- I did my best to preserve cross-version compatibility, but some
  problems have crept in here and there. This series may require
  Sphinx>= 4.0, like the dbus extension. Notably, the Ubuntu build fails
  on Gitlab CI currently. The next version will text against more Sphinx
  versions more rigorously. Sphinx version 5.3.0 and above should work
  perfectly.

- There's a bug in Sphinx itself that may manifest in your testing,
  concerning reported error locations. There's a patch in this series
  that fixes it, but it's later in the series. If you run into the bug
  while testing with this series, try applying that patch first.

- QAPI 'namespaces' aren't yet handled. i.e., there's nothing to
  distinguish entities between QMP, QGA and QSD yet. That feature will
  be added to a future version of this patchset (Likely when the
  generator is ready for inclusion: without it, references will clash
  and the index will gripe about duplicated entries.)

- Per-member features and ifcond are not yet accounted for; though
  definition-scoped features and ifconds are. Please feel free to
  suggest how you'd like to see those represented.

- Inlining all members means there is some ambiguity on what to do with
  doc block sections on inlined entities; features and members have an
  obvious home - body, since, and misc sections are not as obvious on
  how to handle. This will feature more prominently in the generator
  series.

- Some features could be implemented in either the QAPI domain syntax
  *or* the generator, or some combination of both. Depending on
  aesthetic feedback, this may influence where those features should be
  implemented.

- The formatting and aesthetics of branches are a little up in the air,
  see the qapi:union patch for more details.

- It's late, maybe other things. Happy Friday!

Hope you like it!

--js

Harmonie Snow (1):
  docs/qapi-domain: add CSS styling

John Snow (26):
  docs/sphinx: create QAPI domain extension stub
  docs/qapi-domain: add qapi:module directive
  docs/qapi-module: add QAPI domain object registry
  docs/qapi-domain: add QAPI index
  docs/qapi-domain: add resolve_any_xref()
  docs/qapi-domain: add QAPI xref roles
  docs/qapi-domain: add qapi:command directive
  docs/qapi-domain: add :since: directive option
  docs/qapi-domain: add "Arguments:" field lists
  docs/qapi-domain: add "Features:" field lists
  docs/qapi-domain: add "Errors:" field lists
  docs/qapi-domain: add "Returns:" field lists
  docs/qapi-domain: add qapi:enum directive
  docs/qapi-domain: add qapi:alternate directive
  docs/qapi-domain: add qapi:event directive
  docs/qapi-domain: add qapi:struct directive
  docs/qapi-domain: add qapi:union and qapi:branch directives
  docs/qapi-domain: add :deprecated: directive option
  docs/qapi-domain: add :unstable: directive option
  docs/qapi-domain: add :ifcond: directive option
  docs/qapi-domain: RFC patch - add malformed field list entries
  docs/qapi-domain: add warnings for malformed field lists
  docs/qapi-domain: RFC patch - delete malformed field lists
  docs/qapi-domain: add type cross-refs to field lists
  docs/qapi-domain: implement error context reporting fix
  docs/qapi-domain: RFC patch - Add one last sample command

 docs/conf.py                           |   15 +-
 docs/index.rst                         |    1 +
 docs/qapi/index.rst                    |  364 ++++++++
 docs/sphinx-static/theme_overrides.css |   92 +-
 docs/sphinx/qapi-domain.py             | 1061 ++++++++++++++++++++++++
 5 files changed, 1530 insertions(+), 3 deletions(-)
 create mode 100644 docs/qapi/index.rst
 create mode 100644 docs/sphinx/qapi-domain.py

-- 
2.44.0

[PATCH 01/27] docs/sphinx: create QAPI domain extension stub

[John Snow]

It doesn't really do anything yet, we'll get to it brick-by-brick in the
forthcoming commits to keep the series breezy and the git history
informative.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 docs/conf.py               |  3 ++-
 docs/sphinx/qapi-domain.py | 50 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 1 deletion(-)
 create mode 100644 docs/sphinx/qapi-domain.py

diff --git a/docs/conf.py b/docs/conf.py
index aae0304ac6e..b15665d956d 100644
--- a/docs/conf.py
+++ b/docs/conf.py
@@ -61,7 +61,8 @@
 # Add any Sphinx extension module names here, as strings. They can be
 # extensions coming with Sphinx (named 'sphinx.ext.*') or your custom
 # ones.
-extensions = ['kerneldoc', 'qmp_lexer', 'hxtool', 'depfile', 'qapidoc']
+extensions = ['kerneldoc', 'hxtool', 'depfile',
+              'qapidoc', 'qapi-domain', 'qmp_lexer']
 
 if sphinx.version_info[:3] > (4, 0, 0):
     tags.add('sphinx4')
diff --git a/docs/sphinx/qapi-domain.py b/docs/sphinx/qapi-domain.py
new file mode 100644
index 00000000000..163b9ff21c3
--- /dev/null
+++ b/docs/sphinx/qapi-domain.py
@@ -0,0 +1,50 @@
+"""
+QAPI domain extension.
+"""
+
+from __future__ import annotations
+
+from typing import (
+    TYPE_CHECKING,
+    Any,
+    Dict,
+    List,
+    Tuple,
+)
+
+from sphinx.domains import Domain, ObjType
+from sphinx.util import logging
+
+
+if TYPE_CHECKING:
+    from sphinx.application import Sphinx
+
+logger = logging.getLogger(__name__)
+
+
+class QAPIDomain(Domain):
+    """QAPI language domain."""
+
+    name = "qapi"
+    label = "QAPI"
+
+    object_types: Dict[str, ObjType] = {}
+    directives = {}
+    roles = {}
+    initial_data: Dict[str, Dict[str, Tuple[Any]]] = {}
+    indices = []
+
+    def merge_domaindata(self, docnames: List[str], otherdata: Dict[str, Any]) -> None:
+        pass
+
+
+def setup(app: Sphinx) -> Dict[str, Any]:
+    app.setup_extension("sphinx.directives")
+    app.add_domain(QAPIDomain)
+
+    return {
+        "version": "1.0",
+        "env_version": 1,
+        "parallel_read_safe": True,
+        "parallel_write_safe": True,
+    }
-- 
2.44.0

[PATCH 02/27] docs/qapi-domain: add qapi:module directive

[John Snow]

This adds a qapi:module directive, which just notes the current module
being documented and performs a nested parse of the content block, if
present.

This code is based pretty heavily on Sphinx's PyModule directive, but
with the modindex functionality excised.

This commit also adds the _nested_parse helper, which adds cross-version
compatibility for nested parsing while preserving proper line context
information.

For example:

.. qapi:module:: block-core

   Hello, and welcome to block-core!
   =================================

   lorem ipsum, dolor sit amet ...

(For RFC purposes, this commit also adds a test document that
demonstrates the functionality-so-far to allow reviewers to easily test
and experiment with each commit. The eventual submission for inclusion
will remove this playground file.)

Signed-off-by: John Snow <jsnow@redhat.com>
---
 docs/index.rst             |   1 +
 docs/qapi/index.rst        |  38 +++++++++++
 docs/sphinx/qapi-domain.py | 128 ++++++++++++++++++++++++++++++++++++-
 3 files changed, 166 insertions(+), 1 deletion(-)
 create mode 100644 docs/qapi/index.rst

diff --git a/docs/index.rst b/docs/index.rst
index 0b9ee9901d9..11c18c598a8 100644
--- a/docs/index.rst
+++ b/docs/index.rst
@@ -18,3 +18,4 @@ Welcome to QEMU's documentation!
    interop/index
    specs/index
    devel/index
+   qapi/index
diff --git a/docs/qapi/index.rst b/docs/qapi/index.rst
new file mode 100644
index 00000000000..880fd17c709
--- /dev/null
+++ b/docs/qapi/index.rst
@@ -0,0 +1,38 @@
+----------------
+QAPI Domain Test
+----------------
+
+.. qapi:module:: foo-module
+   :no-index:
+
+   This starts a hypothetical module named ``foo-module``, but it
+   doesn't create a cross-reference target and it isn't added to the
+   index.
+
+   Check out the `genindex` for proof that foo-module is not present.
+
+.. qapi:module:: bar-module
+   :no-typesetting:
+
+   This starts a hypothetical module named ``bar-module``, but the
+   contents of the body here will not be rendered in the
+   output. However, any link targets created here or in nested
+   directives will be preserved and functional.
+
+   Check out the `genindex` for proof that bar-module is present in two
+   places! (under both "bar-module" and "QAPI module".)
+
+.. qapi:module:: block-core
+
+   Block core (VM unrelated)
+   =========================
+
+   This starts the documentation section for the ``block-core`` module.
+   All documentation objects that follow belong to the block-core module
+   until another ``qapi:module:`` directive is encountered.
+
+   This directive does not create an entry in the sidebar or the TOC
+   *unless* you create a nested section title within the directive.
+
+   The ``block-core`` module will have two entries in the `genindex`,
+   under both "block-core" and "QAPI module".
diff --git a/docs/sphinx/qapi-domain.py b/docs/sphinx/qapi-domain.py
index 163b9ff21c3..7c5e4407bc1 100644
--- a/docs/sphinx/qapi-domain.py
+++ b/docs/sphinx/qapi-domain.py
@@ -7,21 +7,141 @@
 from typing import (
     TYPE_CHECKING,
     Any,
+    ClassVar,
     Dict,
+    Iterable,
     List,
     Tuple,
+    cast,
 )
 
+from docutils import nodes
+from docutils.parsers.rst import directives
+
+from sphinx import addnodes
 from sphinx.domains import Domain, ObjType
 from sphinx.util import logging
+from sphinx.util.docutils import SphinxDirective, switch_source_input
+from sphinx.util.nodes import make_id, nested_parse_with_titles
 
 
 if TYPE_CHECKING:
+    from docutils.nodes import Element, Node
+
     from sphinx.application import Sphinx
+    from sphinx.util.typing import OptionSpec
 
 logger = logging.getLogger(__name__)
 
 
+def _nested_parse(directive: SphinxDirective, content_node: Element) -> None:
+    """
+    This helper preserves error parsing context across sphinx versions.
+    """
+
+    # necessary so that the child nodes get the right source/line set
+    content_node.document = directive.state.document
+
+    try:
+        # Modern sphinx (6.2.0+) supports proper offsetting for
+        # nested parse error context management
+        nested_parse_with_titles(
+            directive.state,
+            directive.content,
+            content_node,
+            content_offset=directive.content_offset,  # type: ignore[call-arg]
+        )
+    except TypeError:
+        # No content_offset argument. Fall back to SSI method.
+        with switch_source_input(directive.state, directive.content):
+            nested_parse_with_titles(directive.state, directive.content, content_node)
+
+
+class QAPIModule(SphinxDirective):
+    """
+    Directive to mark description of a new module.
+
+    This directive doesn't generate any special formatting, and is just
+    a pass-through for the content body. Named section titles are
+    allowed in the content body.
+
+    Use this directive to associate subsequent definitions with the
+    module they are defined in for purposes of search and QAPI index
+    organization.
+
+    :arg: The name of the module.
+    :opt no-index: Don't add cross-reference targets or index entries.
+    :opt no-typesetting: Don't render the content body (but preserve any
+       cross-reference target IDs in the squelched output.)
+
+    Example::
+
+       .. qapi:module:: block-core
+          :no-index:
+          :no-typesetting:
+
+          Lorem ipsum, dolor sit amet ...
+
+    """
+
+    has_content = True
+    required_arguments = 1
+    optional_arguments = 0
+    final_argument_whitespace = False
+
+    option_spec: ClassVar[OptionSpec] = {
+        # These are universal "Basic" options;
+        # https://www.sphinx-doc.org/en/master/usage/domains/index.html#basic-markup
+        "no-index": directives.flag,
+        "no-typesetting": directives.flag,
+        "no-contents-entry": directives.flag,  # NB: No effect
+        # Deprecated aliases; to be removed in Sphinx 9.0
+        "noindex": directives.flag,
+        "nocontentsentry": directives.flag,  # NB: No effect
+    }
+
+    def run(self) -> List[Node]:
+        modname = self.arguments[0].strip()
+        no_index = "no-index" in self.options or "noindex" in self.options
+
+        self.env.ref_context["qapi:module"] = modname
+
+        content_node: Element = nodes.section()
+        _nested_parse(self, content_node)
+
+        ret: List[Node] = []
+        inode = addnodes.index(entries=[])
+
+        if not no_index:
+            node_id = make_id(self.env, self.state.document, "module", modname)
+            target = nodes.target("", "", ids=[node_id], ismod=True)
+            self.set_source_info(target)
+            self.state.document.note_explicit_target(target)
+
+            indextext = f"QAPI module; {modname}"
+            inode = addnodes.index(
+                entries=[
+                    ("pair", indextext, node_id, "", None),
+                ]
+            )
+            ret.append(inode)
+            content_node.insert(0, target)
+
+        if "no-typesetting" in self.options:
+            if node_ids := [
+                node_id
+                for el in content_node.findall(nodes.Element)
+                for node_id in cast(Iterable[str], el.get("ids", ()))
+            ]:
+                target = nodes.target(ids=node_ids)
+                self.set_source_info(target)
+                ret.append(target)
+        else:
+            ret.extend(content_node.children)
+
+        return ret
+
+
 class QAPIDomain(Domain):
     """QAPI language domain."""
 
@@ -29,7 +149,13 @@ class QAPIDomain(Domain):
     label = "QAPI"
 
     object_types: Dict[str, ObjType] = {}
-    directives = {}
+
+    # Each of these provides a ReST directive,
+    # e.g. .. qapi:module:: block-core
+    directives = {
+        "module": QAPIModule,
+    }
+
     roles = {}
     initial_data: Dict[str, Dict[str, Tuple[Any]]] = {}
     indices = []
-- 
2.44.0

[PATCH 03/27] docs/qapi-module: add QAPI domain object registry

[John Snow]

This is the first step towards QAPI domain cross-references and a QAPI
reference index.

For now, just create the object registry and amend the qapi:module
directive to use that registry. Update the merge_domaindata method now
that we have actual data we may need to merge.

RFC: Much of this code is adapted directly from sphinx.domains.python;
it is unclear to me if there ever would be a collision on merge, hence
the assertion. I haven't been able to trigger it or find better code to
use as a template, so probably I'll just leave the assertion in there.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 docs/sphinx/qapi-domain.py | 76 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 73 insertions(+), 3 deletions(-)

diff --git a/docs/sphinx/qapi-domain.py b/docs/sphinx/qapi-domain.py
index 7c5e4407bc1..ab80fd5f634 100644
--- a/docs/sphinx/qapi-domain.py
+++ b/docs/sphinx/qapi-domain.py
@@ -11,6 +11,7 @@
     Dict,
     Iterable,
     List,
+    NamedTuple,
     Tuple,
     cast,
 )
@@ -20,6 +21,7 @@
 
 from sphinx import addnodes
 from sphinx.domains import Domain, ObjType
+from sphinx.locale import _, __
 from sphinx.util import logging
 from sphinx.util.docutils import SphinxDirective, switch_source_input
 from sphinx.util.nodes import make_id, nested_parse_with_titles
@@ -34,6 +36,13 @@
 logger = logging.getLogger(__name__)
 
 
+class ObjectEntry(NamedTuple):
+    docname: str
+    node_id: str
+    objtype: str
+    aliased: bool
+
+
 def _nested_parse(directive: SphinxDirective, content_node: Element) -> None:
     """
     This helper preserves error parsing context across sphinx versions.
@@ -101,6 +110,7 @@ class QAPIModule(SphinxDirective):
     }
 
     def run(self) -> List[Node]:
+        domain = cast(QAPIDomain, self.env.get_domain("qapi"))
         modname = self.arguments[0].strip()
         no_index = "no-index" in self.options or "noindex" in self.options
 
@@ -113,11 +123,14 @@ def run(self) -> List[Node]:
         inode = addnodes.index(entries=[])
 
         if not no_index:
+            # note module to the domain
             node_id = make_id(self.env, self.state.document, "module", modname)
             target = nodes.target("", "", ids=[node_id], ismod=True)
             self.set_source_info(target)
             self.state.document.note_explicit_target(target)
 
+            domain.note_object(modname, "module", node_id, location=target)
+
             indextext = f"QAPI module; {modname}"
             inode = addnodes.index(
                 entries=[
@@ -148,7 +161,12 @@ class QAPIDomain(Domain):
     name = "qapi"
     label = "QAPI"
 
-    object_types: Dict[str, ObjType] = {}
+    # This table associates cross-reference object types (key) with an
+    # ObjType instance, which defines the valid cross-reference roles
+    # for each object type.
+    object_types: Dict[str, ObjType] = {
+        "module": ObjType(_("module"), "mod", "obj"),
+    }
 
     # Each of these provides a ReST directive,
     # e.g. .. qapi:module:: block-core
@@ -157,11 +175,63 @@ class QAPIDomain(Domain):
     }
 
     roles = {}
-    initial_data: Dict[str, Dict[str, Tuple[Any]]] = {}
+
+    # Moved into the data property at runtime;
+    # this is the internal index of reference-able objects.
+    initial_data: Dict[str, Dict[str, Tuple[Any]]] = {
+        "objects": {},  # fullname -> ObjectEntry
+    }
+
     indices = []
 
+    @property
+    def objects(self) -> Dict[str, ObjectEntry]:
+        return self.data.setdefault("objects", {})  # type: ignore[no-any-return]
+
+    def note_object(
+        self,
+        name: str,
+        objtype: str,
+        node_id: str,
+        aliased: bool = False,
+        location: Any = None,
+    ) -> None:
+        """Note a QAPI object for cross reference."""
+        if name in self.objects:
+            other = self.objects[name]
+            if other.aliased and aliased is False:
+                # The original definition found. Override it!
+                pass
+            elif other.aliased is False and aliased:
+                # The original definition is already registered.
+                return
+            else:
+                # duplicated
+                logger.warning(
+                    __(
+                        "duplicate object description of %s, "
+                        "other instance in %s, use :no-index: for one of them"
+                    ),
+                    name,
+                    other.docname,
+                    location=location,
+                )
+        self.objects[name] = ObjectEntry(self.env.docname, node_id, objtype, aliased)
+
+    def clear_doc(self, docname: str) -> None:
+        for fullname, obj in list(self.objects.items()):
+            if obj.docname == docname:
+                del self.objects[fullname]
+
     def merge_domaindata(self, docnames: List[str], otherdata: Dict[str, Any]) -> None:
-        pass
+        for fullname, obj in otherdata["objects"].items():
+            if obj.docname in docnames:
+                # FIXME: Unsure of the implications of merge conflicts.
+                # Sphinx's own python domain doesn't appear to bother.
+                assert (
+                    fullname not in self.objects
+                ), f"!?!? collision on merge? {fullname=} {obj=} {self.objects[fullname]=}"
+                self.objects[fullname] = obj
 
 
 def setup(app: Sphinx) -> Dict[str, Any]:
-- 
2.44.0

[PATCH 04/27] docs/qapi-domain: add QAPI index

[John Snow]

Use the QAPI object registry to generate a special index just for QAPI
definitions. The index can show entries both by definition type and
alphabetically.

The index can be linked from anywhere in the QEMU manual by using
`qapi-index`.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 docs/qapi/index.rst        |  6 +++-
 docs/sphinx/qapi-domain.py | 66 +++++++++++++++++++++++++++++++++++---
 2 files changed, 66 insertions(+), 6 deletions(-)

diff --git a/docs/qapi/index.rst b/docs/qapi/index.rst
index 880fd17c709..051dc6b3a37 100644
--- a/docs/qapi/index.rst
+++ b/docs/qapi/index.rst
@@ -9,7 +9,8 @@ QAPI Domain Test
    doesn't create a cross-reference target and it isn't added to the
    index.
 
-   Check out the `genindex` for proof that foo-module is not present.
+   Check out the `genindex` or the `qapi-index` for proof that
+   foo-module is not present.
 
 .. qapi:module:: bar-module
    :no-typesetting:
@@ -36,3 +37,6 @@ QAPI Domain Test
 
    The ``block-core`` module will have two entries in the `genindex`,
    under both "block-core" and "QAPI module".
+
+   Modules will also be reported in the `qapi-index`, under the Modules
+   category and in the alphabetical categories that follow.
diff --git a/docs/sphinx/qapi-domain.py b/docs/sphinx/qapi-domain.py
index ab80fd5f634..65409786119 100644
--- a/docs/sphinx/qapi-domain.py
+++ b/docs/sphinx/qapi-domain.py
@@ -12,6 +12,7 @@
     Iterable,
     List,
     NamedTuple,
+    Optional,
     Tuple,
     cast,
 )
@@ -20,7 +21,12 @@
 from docutils.parsers.rst import directives
 
 from sphinx import addnodes
-from sphinx.domains import Domain, ObjType
+from sphinx.domains import (
+    Domain,
+    Index,
+    IndexEntry,
+    ObjType,
+)
 from sphinx.locale import _, __
 from sphinx.util import logging
 from sphinx.util.docutils import SphinxDirective, switch_source_input
@@ -74,9 +80,10 @@ class QAPIModule(SphinxDirective):
     a pass-through for the content body. Named section titles are
     allowed in the content body.
 
-    Use this directive to associate subsequent definitions with the
-    module they are defined in for purposes of search and QAPI index
-    organization.
+    Use this directive to create entries for the QAPI module in the
+    global index and the qapi index; as well as to associate subsequent
+    definitions with the module they are defined in for purposes of
+    search and QAPI index organization.
 
     :arg: The name of the module.
     :opt no-index: Don't add cross-reference targets or index entries.
@@ -155,6 +162,52 @@ def run(self) -> List[Node]:
         return ret
 
 
+class QAPIIndex(Index):
+    """
+    Index subclass to provide the QAPI definition index.
+    """
+
+    name = "index"
+    localname = _("QAPI Index")
+    shortname = _("QAPI Index")
+
+    def generate(
+        self,
+        docnames: Optional[Iterable[str]] = None,
+    ) -> Tuple[List[Tuple[str, List[IndexEntry]]], bool]:
+        assert isinstance(self.domain, QAPIDomain)
+        content: Dict[str, List[IndexEntry]] = {}
+        collapse = False
+
+        # list of all object (name, ObjectEntry) pairs, sorted by name
+        objects = sorted(self.domain.objects.items(), key=lambda x: x[0].lower())
+
+        for objname, obj in objects:
+            if docnames and obj.docname not in docnames:
+                continue
+
+            # Strip the module name out:
+            objname = objname.split(".")[-1]
+
+            # Add an alphabetical entry:
+            entries = content.setdefault(objname[0].upper(), [])
+            entries.append(
+                IndexEntry(objname, 0, obj.docname, obj.node_id, obj.objtype, "", "")
+            )
+
+            # Add a categorical entry:
+            category = obj.objtype.title() + "s"
+            entries = content.setdefault(category, [])
+            entries.append(IndexEntry(objname, 0, obj.docname, obj.node_id, "", "", ""))
+
+        # alphabetically sort categories; type names first, ABC entries last.
+        sorted_content = sorted(
+            content.items(),
+            key=lambda x: (len(x[0]) == 1, x[0]),
+        )
+        return sorted_content, collapse
+
+
 class QAPIDomain(Domain):
     """QAPI language domain."""
 
@@ -182,7 +235,10 @@ class QAPIDomain(Domain):
         "objects": {},  # fullname -> ObjectEntry
     }
 
-    indices = []
+    # Index pages to generate; each entry is an Index class.
+    indices = [
+        QAPIIndex,
+    ]
 
     @property
     def objects(self) -> Dict[str, ObjectEntry]:
-- 
2.44.0

[PATCH 05/27] docs/qapi-domain: add resolve_any_xref()

[John Snow]

Add the ability to resolve cross-references using the `any`
cross-reference syntax. Adding QAPI-specific cross-reference roles will
be added in a forthcoming commit, and will share the same find_obj()
helper.

(There's less code needed for the generic cross-reference resolver, so
it comes first in this series.)

Once again, this code is based very heavily on sphinx.domains.python.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 docs/qapi/index.rst        |  7 +++
 docs/sphinx/qapi-domain.py | 95 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 101 insertions(+), 1 deletion(-)

diff --git a/docs/qapi/index.rst b/docs/qapi/index.rst
index 051dc6b3a37..39ad405fd93 100644
--- a/docs/qapi/index.rst
+++ b/docs/qapi/index.rst
@@ -40,3 +40,10 @@ QAPI Domain Test
 
    Modules will also be reported in the `qapi-index`, under the Modules
    category and in the alphabetical categories that follow.
+
+
+QAPI modules can now be cross-referenced using the ```any```
+cross-referencing syntax. Here's a link to `bar-module`, even though
+the actual output of that directive was suppressed. Here's a link to
+`block-core`. A link to ```foo-module``` won't resolve because of the
+``:no-index:`` option we used for that directive.
diff --git a/docs/sphinx/qapi-domain.py b/docs/sphinx/qapi-domain.py
index 65409786119..4758451ff0e 100644
--- a/docs/sphinx/qapi-domain.py
+++ b/docs/sphinx/qapi-domain.py
@@ -21,6 +21,7 @@
 from docutils.parsers.rst import directives
 
 from sphinx import addnodes
+from sphinx.addnodes import pending_xref
 from sphinx.domains import (
     Domain,
     Index,
@@ -30,13 +31,19 @@
 from sphinx.locale import _, __
 from sphinx.util import logging
 from sphinx.util.docutils import SphinxDirective, switch_source_input
-from sphinx.util.nodes import make_id, nested_parse_with_titles
+from sphinx.util.nodes import (
+    make_id,
+    make_refnode,
+    nested_parse_with_titles,
+)
 
 
 if TYPE_CHECKING:
     from docutils.nodes import Element, Node
 
     from sphinx.application import Sphinx
+    from sphinx.builders import Builder
+    from sphinx.environment import BuildEnvironment
     from sphinx.util.typing import OptionSpec
 
 logger = logging.getLogger(__name__)
@@ -289,6 +296,92 @@ def merge_domaindata(self, docnames: List[str], otherdata: Dict[str, Any]) -> No
                 ), f"!?!? collision on merge? {fullname=} {obj=} {self.objects[fullname]=}"
                 self.objects[fullname] = obj
 
+    def find_obj(
+        self, modname: str, name: str, type: Optional[str]
+    ) -> list[tuple[str, ObjectEntry]]:
+        """
+        Find a QAPI object for "name", perhaps using the given module.
+
+        Returns a list of (name, object entry) tuples.
+
+        :param modname: The current module context (if any!)
+                        under which we are searching.
+        :param name: The name of the x-ref to resolve;
+                     may or may not include a leading module.
+        :param type: The role name of the x-ref we're resolving, if provided.
+                     (This is absent for "any" lookups.)
+        """
+        if not name:
+            return []
+
+        names: list[str] = []
+        matches: list[tuple[str, ObjectEntry]] = []
+
+        fullname = name
+        if "." in fullname:
+            # We're searching for a fully qualified reference;
+            # ignore the contextual module.
+            pass
+        elif modname:
+            # We're searching for something from somewhere;
+            # try searching the current module first.
+            # e.g. :qapi:cmd:`query-block` or `query-block` is being searched.
+            fullname = f"{modname}.{name}"
+
+        if type is None:
+            # type isn't specified, this is a generic xref.
+            # search *all* qapi-specific object types.
+            objtypes: Optional[List[str]] = list(self.object_types)
+        else:
+            # type is specified and will be a role (e.g. obj, mod, cmd)
+            # convert this to eligible object types (e.g. command, module)
+            # using the QAPIDomain.object_types table.
+            objtypes = self.objtypes_for_role(type)
+
+        # Either we should have been given no type, or the type we were
+        # given should correspond to at least one real actual object
+        # type.
+        assert objtypes
+
+        if name in self.objects and self.objects[name].objtype in objtypes:
+            names = [name]
+        elif fullname in self.objects and self.objects[fullname].objtype in objtypes:
+            names = [fullname]
+        else:
+            # exact match wasn't found; e.g. we are searching for
+            # `query-block` from a different (or no) module.
+            searchname = "." + name
+            names = [
+                oname
+                for oname in self.objects
+                if oname.endswith(searchname)
+                and self.objects[oname].objtype in objtypes
+            ]
+
+        matches = [(oname, self.objects[oname]) for oname in names]
+        if len(matches) > 1:
+            matches = [m for m in matches if not m[1].aliased]
+        return matches
+
+    def resolve_any_xref(
+        self,
+        env: BuildEnvironment,
+        fromdocname: str,
+        builder: Builder,
+        target: str,
+        node: pending_xref,
+        contnode: Element,
+    ) -> list[tuple[str, Element]]:
+        results: list[tuple[str, Element]] = []
+        matches = self.find_obj(node.get("qapi:module"), target, None)
+        for name, obj in matches:
+            role = "qapi:" + self.role_for_objtype(obj.objtype)
+            refnode = make_refnode(
+                builder, fromdocname, obj.docname, obj.node_id, contnode, name
+            )
+            results.append((role, refnode))
+        return results
+
 
 def setup(app: Sphinx) -> Dict[str, Any]:
     app.setup_extension("sphinx.directives")
-- 
2.44.0

[PATCH 06/27] docs/qapi-domain: add QAPI xref roles

[John Snow]

Add domain-specific cross-reference syntax. As of this commit, that
means new :qapi:mod:`block-core` and :qapi:obj:`block-core` referencing
syntax.

:mod: will only find modules, but :obj: will find anything registered to
the QAPI domain. (In forthcoming commits, this means commands, events,
enums, etc.)

Creating the cross-references is powered by the QAPIXRefRole class;
resolving them is handled by QAPIDomain.resolve_xref().

QAPIXrefRole is copied almost verbatim from Sphinx's own
PyXrefRole. PyXrefRole (and QAPIXrefRole) adds two features over the
base class:

(1) Creating a cross-reference with e.g. :py:class:`~class.name`
instructs sphinx to omit the fully qualified parts of the resolved name
from the actual link text. This may be useful in the future if we add
namespaces to QAPI documentation, e.g. :qapi:cmd:`~qsd.blockdev-backup`
could link to the QSD-specific documentation for blockdev-backup while
omitting that prefix from the link text.

(2) Prefixing the link target with a "." changes the search behavior to
prefer locally-scoped items first.

I think both of these are worth keeping to help manage future namespace
issues between QEMU, QSD and QGA; but it's possible it's extraneous. It
may possibly be worth keeping just to keep feature parity with Sphinx's
other domains; e.g. "principle of least surprise". Dunno.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 docs/qapi/index.rst        |  4 +++
 docs/sphinx/qapi-domain.py | 67 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 70 insertions(+), 1 deletion(-)

diff --git a/docs/qapi/index.rst b/docs/qapi/index.rst
index 39ad405fd93..e2223d5f363 100644
--- a/docs/qapi/index.rst
+++ b/docs/qapi/index.rst
@@ -47,3 +47,7 @@ cross-referencing syntax. Here's a link to `bar-module`, even though
 the actual output of that directive was suppressed. Here's a link to
 `block-core`. A link to ```foo-module``` won't resolve because of the
 ``:no-index:`` option we used for that directive.
+
+Explicit cross-referencing syntax for QAPI modules is available with
+``:qapi:mod:`foo```, here's a link to :qapi:mod:`bar-module` and one to
+:qapi:mod:`block-core`.
diff --git a/docs/sphinx/qapi-domain.py b/docs/sphinx/qapi-domain.py
index 4758451ff0e..d28ac1cb9d8 100644
--- a/docs/sphinx/qapi-domain.py
+++ b/docs/sphinx/qapi-domain.py
@@ -29,6 +29,7 @@
     ObjType,
 )
 from sphinx.locale import _, __
+from sphinx.roles import XRefRole
 from sphinx.util import logging
 from sphinx.util.docutils import SphinxDirective, switch_source_input
 from sphinx.util.nodes import (
@@ -56,6 +57,34 @@ class ObjectEntry(NamedTuple):
     aliased: bool
 
 
+class QAPIXRefRole(XRefRole):
+    def process_link(
+        self,
+        env: BuildEnvironment,
+        refnode: Element,
+        has_explicit_title: bool,
+        title: str,
+        target: str,
+    ) -> tuple[str, str]:
+        refnode["qapi:module"] = env.ref_context.get("qapi:module")
+        if not has_explicit_title:
+            title = title.lstrip(".")  # only has a meaning for the target
+            target = target.lstrip("~")  # only has a meaning for the title
+            # if the first character is a tilde, don't display the module
+            # parts of the contents
+            if title[0:1] == "~":
+                title = title[1:]
+                dot = title.rfind(".")
+                if dot != -1:
+                    title = title[dot + 1 :]
+        # if the first character is a dot, search more specific namespaces first
+        # else search builtins first
+        if target[0:1] == ".":
+            target = target[1:]
+            refnode["refspecific"] = True
+        return title, target
+
+
 def _nested_parse(directive: SphinxDirective, content_node: Element) -> None:
     """
     This helper preserves error parsing context across sphinx versions.
@@ -234,7 +263,13 @@ class QAPIDomain(Domain):
         "module": QAPIModule,
     }
 
-    roles = {}
+    # These are all cross-reference roles; e.g.
+    # :qapi:cmd:`query-block`. The keys correlate to the names used in
+    # the object_types table values above.
+    roles = {
+        "mod": QAPIXRefRole(),
+        "obj": QAPIXRefRole(),  # reference *any* type of QAPI object.
+    }
 
     # Moved into the data property at runtime;
     # this is the internal index of reference-able objects.
@@ -363,6 +398,36 @@ def find_obj(
             matches = [m for m in matches if not m[1].aliased]
         return matches
 
+    def resolve_xref(
+        self,
+        env: BuildEnvironment,
+        fromdocname: str,
+        builder: Builder,
+        type: str,
+        target: str,
+        node: pending_xref,
+        contnode: Element,
+    ) -> Element | None:
+        modname = node.get("qapi:module")
+        matches = self.find_obj(modname, target, type)
+        multiple_matches = len(matches) > 1
+
+        if not matches:
+            return None
+        elif multiple_matches:
+            logger.warning(
+                __("more than one target found for cross-reference %r: %s"),
+                target,
+                ", ".join(match[0] for match in matches),
+                type="ref",
+                subtype="qapi",
+                location=node,
+            )
+        name, obj = matches[0]
+        return make_refnode(
+            builder, fromdocname, obj.docname, obj.node_id, contnode, name
+        )
+
     def resolve_any_xref(
         self,
         env: BuildEnvironment,
-- 
2.44.0

[PATCH 07/27] docs/qapi-domain: add qapi:command directive

[John Snow]

This commit adds a generic QAPIObject class for use in documenting
various QAPI entities in the Sphinx ecosystem.

It also adds a stubbed version of QAPICommand that utilizes the
QAPIObject class; along with the qapi:command directive, the
:qapi:cmd: cross-reference role, and the "command" object type in the
QAPI object registry.

They don't do anything *particularly* interesting yet, but that will
come in forthcoming commits.

Note: some versions of mypy get a little confused over the difference
between class and instance variables; because sphinx's ObjectDescription
does not declare option_spec as a ClassVar (even though it's obvious
that it is), mypy may produce this error:

qapi-domain.py:125: error: Cannot override instance variable (previously
declared on base class "ObjectDescription") with class variable [misc]

I can't control that; so silence the error with a pragma.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 docs/qapi/index.rst        |  34 ++++++++++
 docs/sphinx/qapi-domain.py | 132 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 165 insertions(+), 1 deletion(-)

diff --git a/docs/qapi/index.rst b/docs/qapi/index.rst
index e2223d5f363..5516f762a24 100644
--- a/docs/qapi/index.rst
+++ b/docs/qapi/index.rst
@@ -51,3 +51,37 @@ the actual output of that directive was suppressed. Here's a link to
 Explicit cross-referencing syntax for QAPI modules is available with
 ``:qapi:mod:`foo```, here's a link to :qapi:mod:`bar-module` and one to
 :qapi:mod:`block-core`.
+
+
+.. qapi:command:: example-command
+
+   This directive creates a QAPI command named `example-command` that
+   appears in both the `genindex` and the `qapi-index`. As of this
+   commit, there aren't any special arguments or options you can give to
+   this directive, it merely parses its content block and handles the
+   TOC/index/xref book-keeping.
+
+   Unlike the QAPI module directive, this directive *does* add a TOC
+   entry by default.
+
+   This object can be referenced in *quite a few ways*:
+
+   * ```example-command``` => `example-command`
+   * ```block-core.example-command``` => `block-core.example-command`
+   * ``:qapi:cmd:`example-command``` => :qapi:cmd:`example-command`
+   * ``:qapi:cmd:`block-core.example-command``` => :qapi:cmd:`block-core.example-command`
+   * ``:qapi:cmd:`~example-command``` => :qapi:cmd:`~example-command`
+   * ``:qapi:cmd:`~block-core.example-command``` => :qapi:cmd:`~block-core.example-command`
+   * ``:qapi:obj:`example-command``` => :qapi:obj:`example-command`
+   * ``:qapi:obj:`block-core.example-command``` => :qapi:obj:`block-core.example-command`
+   * ``:qapi:obj:`~example-command``` => :qapi:obj:`~example-command`
+   * ``:qapi:obj:`~block-core.example-command``` => :qapi:obj:`~block-core.example-command`
+
+   As of Sphinx v7.2.6, there are a few sphinx-standard options this
+   directive has:
+
+   * ``:no-index:`` or ``:noindex:`` Don't add to the `genindex` nor
+     the `qapi-index`; do not register for cross-references.
+   * ``:no-index-entry:`` or ``:noindexentry:``
+   * ``:no-contents-entry:`` or ``:nocontentsentry:``
+   * ``:no-typesetting:``
diff --git a/docs/sphinx/qapi-domain.py b/docs/sphinx/qapi-domain.py
index d28ac1cb9d8..2c1e60290d9 100644
--- a/docs/sphinx/qapi-domain.py
+++ b/docs/sphinx/qapi-domain.py
@@ -21,7 +21,8 @@
 from docutils.parsers.rst import directives
 
 from sphinx import addnodes
-from sphinx.addnodes import pending_xref
+from sphinx.addnodes import desc_signature, pending_xref
+from sphinx.directives import ObjectDescription
 from sphinx.domains import (
     Domain,
     Index,
@@ -108,6 +109,132 @@ def _nested_parse(directive: SphinxDirective, content_node: Element) -> None:
             nested_parse_with_titles(directive.state, directive.content, content_node)
 
 
+# Alias for the return of handle_signature(), which is used in several places.
+# (In the Python domain, this is Tuple[str, str] instead.)
+Signature = str
+
+
+class QAPIObject(ObjectDescription[Signature]):
+    """
+    Description of a generic QAPI object.
+
+    It's not used directly, but is instead subclassed by specific directives.
+    """
+
+    # Inherit some standard options from Sphinx's ObjectDescription
+    option_spec: OptionSpec = ObjectDescription.option_spec.copy()  # type:ignore[misc]
+    option_spec.update(
+        {
+            # Borrowed from the Python domain:
+            "module": directives.unchanged,  # Override contextual module name
+        }
+    )
+
+    def get_signature_prefix(self, sig: str) -> List[nodes.Node]:
+        """Returns a prefix to put before the object name in the signature."""
+        assert self.objtype
+        return [
+            addnodes.desc_sig_keyword("", self.objtype.title()),
+            addnodes.desc_sig_space(),
+        ]
+
+    def get_signature_suffix(self, sig: str) -> list[nodes.Node]:
+        """Returns a suffix to put after the object name in the signature."""
+        return []
+
+    def handle_signature(self, sig: str, signode: desc_signature) -> Signature:
+        """
+        Transform a QAPI definition name into RST nodes.
+
+        This method was originally intended for handling function
+        signatures. In the QAPI domain, however, we only pass the
+        command name as the directive argument and handle everything
+        else in the content body with field lists.
+
+        As such, the only argument here is "sig", which is just the QAPI
+        definition name.
+        """
+        modname = self.options.get("module", self.env.ref_context.get("qapi:module"))
+
+        signode["fullname"] = sig
+        signode["module"] = modname
+        sig_prefix = self.get_signature_prefix(sig)
+        if sig_prefix:
+            signode += addnodes.desc_annotation(str(sig_prefix), "", *sig_prefix)
+        signode += addnodes.desc_name(sig, sig)
+        signode += self.get_signature_suffix(sig)
+
+        return sig
+
+    def _object_hierarchy_parts(self, sig_node: desc_signature) -> Tuple[str, ...]:
+        if "fullname" not in sig_node:
+            return ()
+        modname = sig_node.get("module")
+        fullname = sig_node["fullname"]
+
+        if modname:
+            return (modname, *fullname.split("."))
+        else:
+            return tuple(fullname.split("."))
+
+    def get_index_text(self, modname: str, name: Signature) -> str:
+        """Return the text for the index entry of the object."""
+        # NB this is used for the global index, not the QAPI index.
+        return f"{name} (QMP {self.objtype})"
+
+    def add_target_and_index(
+        self, name: Signature, sig: str, signode: desc_signature
+    ) -> None:
+        # Called by ObjectDescription.run with the result of
+        # handle_signature; name is the return value of handle_signature
+        # where sig is the original argument to handle_signature. In our
+        # case, they're the same for now.
+        assert self.objtype
+
+        modname = self.options.get("module", self.env.ref_context.get("qapi:module"))
+        # Here, sphinx decides to prepend the module name. OK.
+        fullname = (modname + "." if modname else "") + name
+        node_id = make_id(self.env, self.state.document, "", fullname)
+        signode["ids"].append(node_id)
+        self.state.document.note_explicit_target(signode)
+
+        domain = cast(QAPIDomain, self.env.get_domain("qapi"))
+        domain.note_object(fullname, self.objtype, node_id, location=signode)
+
+        if "no-index-entry" not in self.options:
+            indextext = self.get_index_text(modname, name)
+            assert self.indexnode is not None
+            if indextext:
+                self.indexnode["entries"].append(
+                    ("single", indextext, node_id, "", None)
+                )
+
+    def _toc_entry_name(self, sig_node: desc_signature) -> str:
+        # This controls the name in the TOC and on the sidebar.
+
+        # This is the return type of _object_hierarchy_parts().
+        toc_parts = cast(Tuple[str, ...], sig_node.get("_toc_parts", ()))
+        if not toc_parts:
+            return ""
+
+        config = self.env.app.config
+        *parents, name = toc_parts
+        if config.toc_object_entries_show_parents == "domain":
+            return sig_node.get("fullname", name)
+        if config.toc_object_entries_show_parents == "hide":
+            return name
+        if config.toc_object_entries_show_parents == "all":
+            return ".".join(parents + [name])
+        return ""
+
+
+class QAPICommand(QAPIObject):
+    """Description of a QAPI Command."""
+
+    # Nothing unique for now! Changed in later commits O:-)
+    pass
+
+
 class QAPIModule(SphinxDirective):
     """
     Directive to mark description of a new module.
@@ -255,12 +382,14 @@ class QAPIDomain(Domain):
     # for each object type.
     object_types: Dict[str, ObjType] = {
         "module": ObjType(_("module"), "mod", "obj"),
+        "command": ObjType(_("command"), "cmd", "obj"),
     }
 
     # Each of these provides a ReST directive,
     # e.g. .. qapi:module:: block-core
     directives = {
         "module": QAPIModule,
+        "command": QAPICommand,
     }
 
     # These are all cross-reference roles; e.g.
@@ -268,6 +397,7 @@ class QAPIDomain(Domain):
     # the object_types table values above.
     roles = {
         "mod": QAPIXRefRole(),
+        "cmd": QAPIXRefRole(),
         "obj": QAPIXRefRole(),  # reference *any* type of QAPI object.
     }
 
-- 
2.44.0

[PATCH 08/27] docs/qapi-domain: add :since: directive option

[John Snow]

Add a little special markup for registering "Since:" information. Adding
it as an option instead of generic content lets us hoist the information
into the Signature bar, optionally put it in the index, etc.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 docs/qapi/index.rst        |  1 +
 docs/sphinx/qapi-domain.py | 27 +++++++++++++++++++++++++--
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/docs/qapi/index.rst b/docs/qapi/index.rst
index 5516f762a24..33b9349a3ee 100644
--- a/docs/qapi/index.rst
+++ b/docs/qapi/index.rst
@@ -54,6 +54,7 @@ Explicit cross-referencing syntax for QAPI modules is available with
 
 
 .. qapi:command:: example-command
+   :since: 42.0
 
    This directive creates a QAPI command named `example-command` that
    appears in both the `genindex` and the `qapi-index`. As of this
diff --git a/docs/sphinx/qapi-domain.py b/docs/sphinx/qapi-domain.py
index 2c1e60290d9..38a50318d08 100644
--- a/docs/sphinx/qapi-domain.py
+++ b/docs/sphinx/qapi-domain.py
@@ -4,6 +4,7 @@
 
 from __future__ import annotations
 
+import re
 from typing import (
     TYPE_CHECKING,
     Any,
@@ -86,6 +87,18 @@ def process_link(
         return title, target
 
 
+def since_validator(param: str) -> str:
+    """
+    Validate the `:since: X.Y` option field.
+    """
+    match = re.match(r"[0-9]+\.[0-9]+", param)
+    if not match:
+        raise ValueError(
+            f":since: requires a version number in X.Y format; not {param!r}"
+        )
+    return param
+
+
 def _nested_parse(directive: SphinxDirective, content_node: Element) -> None:
     """
     This helper preserves error parsing context across sphinx versions.
@@ -127,6 +140,8 @@ class QAPIObject(ObjectDescription[Signature]):
         {
             # Borrowed from the Python domain:
             "module": directives.unchanged,  # Override contextual module name
+            # These are QAPI originals:
+            "since": since_validator,
         }
     )
 
@@ -138,9 +153,17 @@ def get_signature_prefix(self, sig: str) -> List[nodes.Node]:
             addnodes.desc_sig_space(),
         ]
 
-    def get_signature_suffix(self, sig: str) -> list[nodes.Node]:
+    def get_signature_suffix(self, sig: str) -> List[nodes.Node]:
         """Returns a suffix to put after the object name in the signature."""
-        return []
+        ret: List[nodes.Node] = []
+
+        if "since" in self.options:
+            ret += [
+                addnodes.desc_sig_space(),
+                addnodes.desc_sig_element("", f"(Since: {self.options['since']})"),
+            ]
+
+        return ret
 
     def handle_signature(self, sig: str, signode: desc_signature) -> Signature:
         """
-- 
2.44.0

[PATCH 09/27] docs/qapi-domain: add "Arguments:" field lists

[John Snow]

This adds special rendering for Sphinx's typed field lists.

This patch does not add any QAPI-aware markup, rendering, or
cross-referencing for the type names, yet. That feature requires a
subclass to TypedField which will happen in its own commit quite a bit
later in this series; after all the basic fields and objects have been
established first.

The syntax for this field is:

:arg type name: description
   description cont'd

You can omit the type or the description, but you cannot omit the name
-- if you do so, it degenerates into a "normal field list" entry, and
probably isn't what you want.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 docs/qapi/index.rst        | 15 +++++++++++++++
 docs/sphinx/qapi-domain.py | 14 ++++++++++++--
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/docs/qapi/index.rst b/docs/qapi/index.rst
index 33b9349a3ee..197587bbc81 100644
--- a/docs/qapi/index.rst
+++ b/docs/qapi/index.rst
@@ -86,3 +86,18 @@ Explicit cross-referencing syntax for QAPI modules is available with
    * ``:no-index-entry:`` or ``:noindexentry:``
    * ``:no-contents-entry:`` or ``:nocontentsentry:``
    * ``:no-typesetting:``
+
+.. qapi:command:: fake-command
+   :since: 13.37
+
+   This is a fake command, it's not real. It can't hurt you.
+
+   :arg int foo: normal parameter documentation.
+   :arg str bar: Another normal parameter description.
+   :arg baz: Missing a type.
+   :arg no-descr:
+
+   Field lists can appear anywhere in the directive block, but any field
+   list entries in the same list block that are recognized as special
+   ("arg") will be reformatted and grouped accordingly for rendered
+   output.
diff --git a/docs/sphinx/qapi-domain.py b/docs/sphinx/qapi-domain.py
index 38a50318d08..853bd91b7a8 100644
--- a/docs/sphinx/qapi-domain.py
+++ b/docs/sphinx/qapi-domain.py
@@ -33,6 +33,7 @@
 from sphinx.locale import _, __
 from sphinx.roles import XRefRole
 from sphinx.util import logging
+from sphinx.util.docfields import TypedField
 from sphinx.util.docutils import SphinxDirective, switch_source_input
 from sphinx.util.nodes import (
     make_id,
@@ -254,8 +255,17 @@ def _toc_entry_name(self, sig_node: desc_signature) -> str:
 class QAPICommand(QAPIObject):
     """Description of a QAPI Command."""
 
-    # Nothing unique for now! Changed in later commits O:-)
-    pass
+    doc_field_types = QAPIObject.doc_field_types.copy()
+    doc_field_types.extend(
+        [
+            TypedField(
+                "argument",
+                label=_("Arguments"),
+                names=("arg",),
+                can_collapse=True,
+            ),
+        ]
+    )
 
 
 class QAPIModule(SphinxDirective):
-- 
2.44.0

[PATCH 10/27] docs/qapi-domain: add "Features:" field lists

[John Snow]

Add support for Features field lists. There is no QAPI-specific
functionality here, but this could be changed if desired (if we wanted
the feature names to link somewhere, for instance.)

This feature list doesn't have any restrictions, so it can be used to
document object-wide features or per-member features as deemed
appropriate. It's essentially free-form text.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 docs/qapi/index.rst        |  6 ++++++
 docs/sphinx/qapi-domain.py | 11 ++++++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/docs/qapi/index.rst b/docs/qapi/index.rst
index 197587bbc81..a570c37abb2 100644
--- a/docs/qapi/index.rst
+++ b/docs/qapi/index.rst
@@ -95,9 +95,15 @@ Explicit cross-referencing syntax for QAPI modules is available with
    :arg int foo: normal parameter documentation.
    :arg str bar: Another normal parameter description.
    :arg baz: Missing a type.
+   :feat unstable: More than unstable, this command doesn't even exist!
    :arg no-descr:
+   :feat hallucination: This command is a figment of your imagination.
 
    Field lists can appear anywhere in the directive block, but any field
    list entries in the same list block that are recognized as special
    ("arg") will be reformatted and grouped accordingly for rendered
    output.
+
+   At the moment, the order of grouped sections is based on the order in
+   which each group was encountered. This example will render Arguments
+   first, and then Features; but the order can be any that you choose.
diff --git a/docs/sphinx/qapi-domain.py b/docs/sphinx/qapi-domain.py
index 853bd91b7a8..c0dc6482204 100644
--- a/docs/sphinx/qapi-domain.py
+++ b/docs/sphinx/qapi-domain.py
@@ -33,7 +33,7 @@
 from sphinx.locale import _, __
 from sphinx.roles import XRefRole
 from sphinx.util import logging
-from sphinx.util.docfields import TypedField
+from sphinx.util.docfields import GroupedField, TypedField
 from sphinx.util.docutils import SphinxDirective, switch_source_input
 from sphinx.util.nodes import (
     make_id,
@@ -146,6 +146,15 @@ class QAPIObject(ObjectDescription[Signature]):
         }
     )
 
+    doc_field_types = [
+        GroupedField(
+            "feature",
+            label=_("Features"),
+            names=("feat",),
+            can_collapse=True,
+        ),
+    ]
+
     def get_signature_prefix(self, sig: str) -> List[nodes.Node]:
         """Returns a prefix to put before the object name in the signature."""
         assert self.objtype
-- 
2.44.0

[PATCH 11/27] docs/qapi-domain: add "Errors:" field lists

[John Snow]

``:error type: descr`` can now be used to document error conditions,
naming the type of error object and a description of when the error is
surfaced.

Like the previous Arguments patch, this patch does not apply any special
QAPI syntax highlighting or cross-referencing for the types, but this
can be adjusted in the future if desired.

(At present, I have no commits that add such highlighting. Sphinx also
does not appear to support Grouped fields with optional (or no)
parameters, so the ability to exclude error types is currently not
supported. If you omit the type, Sphinx treats it as a regular field
list and doesn't apply the special Grouping postprocessing to it.)

Signed-off-by: John Snow <jsnow@redhat.com>
---
 docs/qapi/index.rst        | 4 ++++
 docs/sphinx/qapi-domain.py | 6 ++++++
 2 files changed, 10 insertions(+)

diff --git a/docs/qapi/index.rst b/docs/qapi/index.rst
index a570c37abb2..004d02e0437 100644
--- a/docs/qapi/index.rst
+++ b/docs/qapi/index.rst
@@ -98,6 +98,10 @@ Explicit cross-referencing syntax for QAPI modules is available with
    :feat unstable: More than unstable, this command doesn't even exist!
    :arg no-descr:
    :feat hallucination: This command is a figment of your imagination.
+   :error CommandNotFound: When you try to use this command, because it
+      isn't real.
+   :error GenericError: If the system decides it doesn't like the
+      argument values. It's very temperamental.
 
    Field lists can appear anywhere in the directive block, but any field
    list entries in the same list block that are recognized as special
diff --git a/docs/sphinx/qapi-domain.py b/docs/sphinx/qapi-domain.py
index c0dc6482204..1f0b168fa2c 100644
--- a/docs/sphinx/qapi-domain.py
+++ b/docs/sphinx/qapi-domain.py
@@ -273,6 +273,12 @@ class QAPICommand(QAPIObject):
                 names=("arg",),
                 can_collapse=True,
             ),
+            GroupedField(
+                "error",
+                label=_("Errors"),
+                names=("error",),
+                can_collapse=True,
+            ),
         ]
     )
 
-- 
2.44.0

[PATCH 12/27] docs/qapi-domain: add "Returns:" field lists

[John Snow]

Add "Returns:" field list syntax to QAPI Commands.

Like "Arguments:" and "Errors:", the type name isn't currently processed
for cross-referencing, but this will be addressed in a forthcoming
commit.

This patch adds "errors" as a GroupedField, which means that multiple
return values can be annotated - this is only done because Sphinx does
not seemingly (Maybe I missed it?) support mandatory type arguments to
Ungrouped fields. Because we want to cross-reference this type
information later, we want to make the type argument mandatory. As a
result, you can technically add multiple :return: fields, though I'm not
aware of any circumstance in which you'd need or want
to. Recommendation: "Don't do that, then."

Since this field describes an action/event instead of describing a list
of nouns (arguments, features, errors), I added both the imperative and
indicative forms (:return: and :returns:) to allow doc writers to use
whichever mood "feels right" in the source document. The rendered output
will always use the "Returns:" label, however.

I'm sure you'll let me know how you feel about that. O:-)

Signed-off-by: John Snow <jsnow@redhat.com>
---
 docs/qapi/index.rst        | 2 ++
 docs/sphinx/qapi-domain.py | 6 ++++++
 2 files changed, 8 insertions(+)

diff --git a/docs/qapi/index.rst b/docs/qapi/index.rst
index 004d02e0437..39fe4dd2dae 100644
--- a/docs/qapi/index.rst
+++ b/docs/qapi/index.rst
@@ -102,6 +102,8 @@ Explicit cross-referencing syntax for QAPI modules is available with
       isn't real.
    :error GenericError: If the system decides it doesn't like the
       argument values. It's very temperamental.
+   :return SomeTypeName: An esoteric collection of mystical nonsense to
+      both confound and delight.
 
    Field lists can appear anywhere in the directive block, but any field
    list entries in the same list block that are recognized as special
diff --git a/docs/sphinx/qapi-domain.py b/docs/sphinx/qapi-domain.py
index 1f0b168fa2c..5d44dba6cd3 100644
--- a/docs/sphinx/qapi-domain.py
+++ b/docs/sphinx/qapi-domain.py
@@ -279,6 +279,12 @@ class QAPICommand(QAPIObject):
                 names=("error",),
                 can_collapse=True,
             ),
+            GroupedField(
+                "returnvalue",
+                label=_("Returns"),
+                names=("return", "returns"),
+                can_collapse=True,
+            ),
         ]
     )
 
-- 
2.44.0

[PATCH 13/27] docs/qapi-domain: add qapi:enum directive

[John Snow]

Add the .. qapi:enum:: directive, object, and :qapi:enum:`name`
cross-reference role.

Add the :value name: field list for documenting Enum values.

Of note, also introduce a new "type" role that is intended to be used by
other QAPI object directives to cross-reference arbitrary QAPI type
names, but will exclude commands, events, and modules from
consideration.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 docs/qapi/index.rst        | 14 ++++++++++++++
 docs/sphinx/qapi-domain.py | 24 ++++++++++++++++++++++++
 2 files changed, 38 insertions(+)

diff --git a/docs/qapi/index.rst b/docs/qapi/index.rst
index 39fe4dd2dae..cf794e6e739 100644
--- a/docs/qapi/index.rst
+++ b/docs/qapi/index.rst
@@ -113,3 +113,17 @@ Explicit cross-referencing syntax for QAPI modules is available with
    At the moment, the order of grouped sections is based on the order in
    which each group was encountered. This example will render Arguments
    first, and then Features; but the order can be any that you choose.
+
+.. qapi:enum:: BitmapSyncMode
+   :since: 4.2
+
+   An enumeration of possible behaviors for the synchronization of a
+   bitmap when used for data copy operations.
+
+   :value on-success: The bitmap is only synced when the operation is
+      successful. This is the behavior always used for
+      ``INCREMENTAL`` backups.
+   :value never: The bitmap is never synchronized with the operation, and
+      is treated solely as a read-only manifest of blocks to copy.
+   :value always: The bitmap is always synchronized with the operation,
+      regardless of whether or not the operation was successful.
diff --git a/docs/sphinx/qapi-domain.py b/docs/sphinx/qapi-domain.py
index 5d44dba6cd3..6759c39290d 100644
--- a/docs/sphinx/qapi-domain.py
+++ b/docs/sphinx/qapi-domain.py
@@ -289,6 +289,22 @@ class QAPICommand(QAPIObject):
     )
 
 
+class QAPIEnum(QAPIObject):
+    """Description of a QAPI Enum."""
+
+    doc_field_types = QAPIObject.doc_field_types.copy()
+    doc_field_types.extend(
+        [
+            GroupedField(
+                "value",
+                label=_("Values"),
+                names=("value",),
+                can_collapse=True,
+            )
+        ]
+    )
+
+
 class QAPIModule(SphinxDirective):
     """
     Directive to mark description of a new module.
@@ -434,9 +450,14 @@ class QAPIDomain(Domain):
     # This table associates cross-reference object types (key) with an
     # ObjType instance, which defines the valid cross-reference roles
     # for each object type.
+    #
+    # e.g., the :qapi:type: cross-reference role can refer to enum,
+    # struct, union, or alternate objects; but :qapi:obj: can refer to
+    # anything. Each object also gets its own targeted cross-reference role.
     object_types: Dict[str, ObjType] = {
         "module": ObjType(_("module"), "mod", "obj"),
         "command": ObjType(_("command"), "cmd", "obj"),
+        "enum": ObjType(_("enum"), "enum", "obj", "type"),
     }
 
     # Each of these provides a ReST directive,
@@ -444,6 +465,7 @@ class QAPIDomain(Domain):
     directives = {
         "module": QAPIModule,
         "command": QAPICommand,
+        "enum": QAPIEnum,
     }
 
     # These are all cross-reference roles; e.g.
@@ -452,6 +474,8 @@ class QAPIDomain(Domain):
     roles = {
         "mod": QAPIXRefRole(),
         "cmd": QAPIXRefRole(),
+        "enum": QAPIXRefRole(),
+        "type": QAPIXRefRole(),  # reference any data type (excludes modules, commands, events)
         "obj": QAPIXRefRole(),  # reference *any* type of QAPI object.
     }
 
-- 
2.44.0

[PATCH 14/27] docs/qapi-domain: add qapi:alternate directive

[John Snow]

Add the .. qapi:alternate:: directive, object, and qapi:alt:`name`
cross-reference role.

Add the "Choices:" field list for describing alternate choices. Like
other field lists that reference QAPI types, a forthcoming commit will
add cross-referencing support to this field.

RFC: In the future, it would be nice to directly inline Alternates as
part of the type information in the containing object (i.e. directly in
arguments/members) - but that's a task for another series. For now, the
branch "names" are documented just like qapidoc.py does, even though
this information is superfluous for user documentation. Room for future
improvement, but not now.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 docs/qapi/index.rst        |  7 +++++++
 docs/sphinx/qapi-domain.py | 19 +++++++++++++++++++
 2 files changed, 26 insertions(+)

diff --git a/docs/qapi/index.rst b/docs/qapi/index.rst
index cf794e6e739..9bfe4d9f454 100644
--- a/docs/qapi/index.rst
+++ b/docs/qapi/index.rst
@@ -127,3 +127,10 @@ Explicit cross-referencing syntax for QAPI modules is available with
       is treated solely as a read-only manifest of blocks to copy.
    :value always: The bitmap is always synchronized with the operation,
       regardless of whether or not the operation was successful.
+
+.. qapi:alternate:: BlockDirtyBitmapOrStr
+   :since: 4.1
+
+   :choice str local: name of the bitmap, attached to the same node as
+      target bitmap.
+   :choice BlockDirtyBitmap external: bitmap with specified node
diff --git a/docs/sphinx/qapi-domain.py b/docs/sphinx/qapi-domain.py
index 6759c39290d..c6eb6324594 100644
--- a/docs/sphinx/qapi-domain.py
+++ b/docs/sphinx/qapi-domain.py
@@ -305,6 +305,22 @@ class QAPIEnum(QAPIObject):
     )
 
 
+class QAPIAlternate(QAPIObject):
+    """Description of a QAPI Alternate."""
+
+    doc_field_types = QAPIObject.doc_field_types.copy()
+    doc_field_types.extend(
+        [
+            TypedField(
+                "choice",
+                label=_("Choices"),
+                names=("choice",),
+                can_collapse=True,
+            ),
+        ]
+    )
+
+
 class QAPIModule(SphinxDirective):
     """
     Directive to mark description of a new module.
@@ -458,6 +474,7 @@ class QAPIDomain(Domain):
         "module": ObjType(_("module"), "mod", "obj"),
         "command": ObjType(_("command"), "cmd", "obj"),
         "enum": ObjType(_("enum"), "enum", "obj", "type"),
+        "alternate": ObjType(_("alternate"), "alt", "obj", "type"),
     }
 
     # Each of these provides a ReST directive,
@@ -466,6 +483,7 @@ class QAPIDomain(Domain):
         "module": QAPIModule,
         "command": QAPICommand,
         "enum": QAPIEnum,
+        "alternate": QAPIAlternate,
     }
 
     # These are all cross-reference roles; e.g.
@@ -475,6 +493,7 @@ class QAPIDomain(Domain):
         "mod": QAPIXRefRole(),
         "cmd": QAPIXRefRole(),
         "enum": QAPIXRefRole(),
+        "alt": QAPIXRefRole(),
         "type": QAPIXRefRole(),  # reference any data type (excludes modules, commands, events)
         "obj": QAPIXRefRole(),  # reference *any* type of QAPI object.
     }
-- 
2.44.0

[PATCH 15/27] docs/qapi-domain: add qapi:event directive

[John Snow]

Adds the .. qapi:event:: directive, object, and :qapi:event:`name`
cross-referencing role.

Adds the :memb type name: field list syntax for documenting event data
members. As this syntax and phrasing will be shared with Structs and
Unions as well, add the field list definition to a shared abstract
class.

As per usual, QAPI cross-referencing for types in the member field list
will be added in a forthcoming commit.

NOTE 1: The "str?" type annotation syntax sneaks into this commit in the
demonstration doc. It isn't processed yet, so just ignore it for
now. The non-RFC version of this series won't include the sandbox doc,
so that inconsistency will sort itself out later. (Skip ahead to the
commit that adds XRef support for TypedField and GroupedField lists to
learn what the deal is there and leave feedback on that syntax.)

NOTE 2: We already have a QMP lexer, turn it on for example blocks in this
sample demo.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 docs/qapi/index.rst        | 40 ++++++++++++++++++++++++++++++++++++++
 docs/sphinx/qapi-domain.py | 25 ++++++++++++++++++++++++
 2 files changed, 65 insertions(+)

diff --git a/docs/qapi/index.rst b/docs/qapi/index.rst
index 9bfe4d9f454..d81bccfb06a 100644
--- a/docs/qapi/index.rst
+++ b/docs/qapi/index.rst
@@ -2,6 +2,12 @@
 QAPI Domain Test
 ----------------
 
+.. this sets the code-highlighting language to QMP for this *document*.
+   I wonder if I can set a domain default...?
+
+.. highlight:: QMP
+
+
 .. qapi:module:: foo-module
    :no-index:
 
@@ -134,3 +140,37 @@ Explicit cross-referencing syntax for QAPI modules is available with
    :choice str local: name of the bitmap, attached to the same node as
       target bitmap.
    :choice BlockDirtyBitmap external: bitmap with specified node
+
+.. qapi:event:: BLOCK_JOB_COMPLETED
+   :since: 1.1
+
+   Emitted when a block job has completed.
+
+   :memb JobType type: job type
+   :memb str device: The job identifier. Originally the device name but
+      other values are allowed since QEMU 2.7
+   :memb int len: maximum progress value
+   :memb int offset: current progress value. On success this is equal to
+      len. On failure this is less than len
+   :memb int speed: rate limit, bytes per second
+   :memb str? error: error message. Only present on failure. This field
+      contains a human-readable error message. There are no semantics
+      other than that streaming has failed and clients should not try to
+      interpret the error string
+
+   Example::
+
+     <- {
+       "event": "BLOCK_JOB_COMPLETED",
+       "data": {
+         "type": "stream",
+         "device": "virtio-disk0",
+         "len": 10737418240,
+         "offset": 10737418240,
+         "speed": 0
+       },
+       "timestamp": {
+         "seconds": 1267061043,
+         "microseconds": 959568
+       }
+     }
diff --git a/docs/sphinx/qapi-domain.py b/docs/sphinx/qapi-domain.py
index c6eb6324594..74dc578b3c7 100644
--- a/docs/sphinx/qapi-domain.py
+++ b/docs/sphinx/qapi-domain.py
@@ -321,6 +321,28 @@ class QAPIAlternate(QAPIObject):
     )
 
 
+class QAPIObjectWithMembers(QAPIObject):
+    """Base class for Events/Structs/Unions"""
+
+    doc_field_types = QAPIObject.doc_field_types.copy()
+    doc_field_types.extend(
+        [
+            TypedField(
+                "member",
+                label=_("Members"),
+                names=("memb",),
+                can_collapse=True,
+            ),
+        ]
+    )
+
+
+class QAPIEvent(QAPIObjectWithMembers):
+    """Description of a QAPI Event."""
+
+    pass
+
+
 class QAPIModule(SphinxDirective):
     """
     Directive to mark description of a new module.
@@ -473,6 +495,7 @@ class QAPIDomain(Domain):
     object_types: Dict[str, ObjType] = {
         "module": ObjType(_("module"), "mod", "obj"),
         "command": ObjType(_("command"), "cmd", "obj"),
+        "event": ObjType(_("event"), "event", "obj"),
         "enum": ObjType(_("enum"), "enum", "obj", "type"),
         "alternate": ObjType(_("alternate"), "alt", "obj", "type"),
     }
@@ -482,6 +505,7 @@ class QAPIDomain(Domain):
     directives = {
         "module": QAPIModule,
         "command": QAPICommand,
+        "event": QAPIEvent,
         "enum": QAPIEnum,
         "alternate": QAPIAlternate,
     }
@@ -492,6 +516,7 @@ class QAPIDomain(Domain):
     roles = {
         "mod": QAPIXRefRole(),
         "cmd": QAPIXRefRole(),
+        "event": QAPIXRefRole(),
         "enum": QAPIXRefRole(),
         "alt": QAPIXRefRole(),
         "type": QAPIXRefRole(),  # reference any data type (excludes modules, commands, events)
-- 
2.44.0

[PATCH 16/27] docs/qapi-domain: add qapi:struct directive

[John Snow]

Adds the .. qapi:struct:: directive, object, and :qapi:struct:`name`
cross-referencing role.

As per usual, QAPI cross-referencing for types in the member field list
will be added in a forthcoming commit.

RFC Note: The "?" syntax sneaks into the example document again. Please
ignore that for now.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 docs/qapi/index.rst        | 16 ++++++++++++++++
 docs/sphinx/qapi-domain.py |  9 +++++++++
 2 files changed, 25 insertions(+)

diff --git a/docs/qapi/index.rst b/docs/qapi/index.rst
index d81bccfb06a..b07e6e9e2e3 100644
--- a/docs/qapi/index.rst
+++ b/docs/qapi/index.rst
@@ -174,3 +174,19 @@ Explicit cross-referencing syntax for QAPI modules is available with
          "microseconds": 959568
        }
      }
+
+.. qapi:struct:: BackupPerf
+   :since: 6.0
+
+   Optional parameters for backup.  These parameters don't affect
+   functionality, but may significantly affect performance.
+
+   :memb bool? use-copy-range: Use copy offloading.  Default false.
+   :memb int? max-workers: Maximum number of parallel requests for the
+      sustained background copying process.  Doesn't influence
+      copy-before-write operations.  Default 64.
+   :memb int64? max-chunk: Maximum request length for the sustained
+     background copying process.  Doesn't influence copy-before-write
+     operations.  0 means unlimited.  If max-chunk is non-zero then it
+     should not be less than job cluster size which is calculated as
+     maximum of target image cluster size and 64k.  Default 0.
diff --git a/docs/sphinx/qapi-domain.py b/docs/sphinx/qapi-domain.py
index 74dc578b3c7..b46faeaceef 100644
--- a/docs/sphinx/qapi-domain.py
+++ b/docs/sphinx/qapi-domain.py
@@ -343,6 +343,12 @@ class QAPIEvent(QAPIObjectWithMembers):
     pass
 
 
+class QAPIStruct(QAPIObjectWithMembers):
+    """Description of a QAPI Struct."""
+
+    pass
+
+
 class QAPIModule(SphinxDirective):
     """
     Directive to mark description of a new module.
@@ -497,6 +503,7 @@ class QAPIDomain(Domain):
         "command": ObjType(_("command"), "cmd", "obj"),
         "event": ObjType(_("event"), "event", "obj"),
         "enum": ObjType(_("enum"), "enum", "obj", "type"),
+        "struct": ObjType(_("struct"), "struct", "obj", "type"),
         "alternate": ObjType(_("alternate"), "alt", "obj", "type"),
     }
 
@@ -507,6 +514,7 @@ class QAPIDomain(Domain):
         "command": QAPICommand,
         "event": QAPIEvent,
         "enum": QAPIEnum,
+        "struct": QAPIStruct,
         "alternate": QAPIAlternate,
     }
 
@@ -518,6 +526,7 @@ class QAPIDomain(Domain):
         "cmd": QAPIXRefRole(),
         "event": QAPIXRefRole(),
         "enum": QAPIXRefRole(),
+        "struct": QAPIXRefRole(),
         "alt": QAPIXRefRole(),
         "type": QAPIXRefRole(),  # reference any data type (excludes modules, commands, events)
         "obj": QAPIXRefRole(),  # reference *any* type of QAPI object.
-- 
2.44.0

[PATCH 17/27] docs/qapi-domain: add qapi:union and qapi:branch directives

[John Snow]

Adds the .. qapi:union:: directive, object, and :qapi:union:`name`
cross-referencing role.

In order to support discriminated branches of unions, a new qapi:branch
directive is created whose only purpose is to create a dynamically named
field list section based on the name of the branch key and value.

Because the label for these branch sections is dynamically generated,
this patch allows the use of either :memb: or :arg: in branches; they
are simply aliases and do not do anything different.

(This is to allow the directive to be used for either Commands or Unions
as needed; i.e. for commands whose argument type is a discriminated
union.)

For example:

.. qapi:union:: foo-union

   :memb foo-enum key: Discriminator field
   :memb foo-type fieldname: Some base type field that is always present

   .. qapi:branch:: key value1

      :memb type name: lorem ipsum ...

   .. qapi:branch:: key value2

      :memb type2 name2: dolor sit amet ...

In order to support this syntax, the root QAPIObject class needs to
perform some post-processing on field lists to merge adjecent field
lists. because each branch directive will return a separate field list,
and we want to combine them into one unified list for proper rendering.

NOTE: Technically, the branch directive will allow you to put arbitrary
ReST besides field lists inside of it, but it almost certainly won't do
anything you'd consider useful. I don't recommend it, but
programmatically forbidding it is expensive. Recommendation: "Don't!"

... and now ...

RFC: The branches abuse the field list system to create new categories
of field list entries that are dynamically generated. At the moment, I
do not have code to apply appropriate formatting to these headings, but
would like to apply ``literal`` syntax at a minimum to the enum values.

The other idea I have is to create a "blank" members value that doesn't
add a "title" (field label) on the left column and instead adds a
horizontal info bar into the arguments list in the right, but the
problem with this approach is that I won't be able to get rid of the "*
..." list bullet syntax, so it may look odd. Otherwise, a benefit to
this syntax is that it would allow us to write a longer explanation,
e.g. "When ``{key}`` is ``{value}``".

It's possible I could split the argument list into several nested lists,
but I haven't experimented in that direction yet - it's also likely that
Sphinx will want to append a ":" even to blank section titles, so this
may prove tricky to do within the constraints of Sphinx's existing Field
list system. (I believe the HTML formatter is responsible for appending
the colons.)

I'm open to suggestions, but wrestling with Sphinx, html and css is time
consuming (for me, at least), so ultimately, I'm afraid I must say:
***PATCHES WELCOME***.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 docs/qapi/index.rst        | 37 ++++++++++++++
 docs/sphinx/qapi-domain.py | 98 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 134 insertions(+), 1 deletion(-)

diff --git a/docs/qapi/index.rst b/docs/qapi/index.rst
index b07e6e9e2e3..d37ceac497f 100644
--- a/docs/qapi/index.rst
+++ b/docs/qapi/index.rst
@@ -103,6 +103,18 @@ Explicit cross-referencing syntax for QAPI modules is available with
    :arg baz: Missing a type.
    :feat unstable: More than unstable, this command doesn't even exist!
    :arg no-descr:
+   :arg BitmapSyncMode discrim: How about branches in commands?
+
+   .. qapi:branch:: discrim on-success
+
+      :arg str foobar: This is an argument that belongs to a tagged union branch.
+      :arg int? foobaz: This is another argument belonging to the same branch.
+
+   .. qapi:branch:: discrim never
+
+      :arg str barfoo: This is an argument that belongs to a *different* tagged union branch.
+      :arg int64 zzxyz: And this is another argument belonging to that same branch.
+
    :feat hallucination: This command is a figment of your imagination.
    :error CommandNotFound: When you try to use this command, because it
       isn't real.
@@ -190,3 +202,28 @@ Explicit cross-referencing syntax for QAPI modules is available with
      operations.  0 means unlimited.  If max-chunk is non-zero then it
      should not be less than job cluster size which is calculated as
      maximum of target image cluster size and 64k.  Default 0.
+
+.. qapi:union:: RbdEncryptionOptions
+   :since: 6.1
+
+   :memb RbdImageEncryptionFormat format: Encryption format.
+   :memb RbdEncryptionOptions? parent: Parent image encryption options
+      (for cloned images).  Can be left unspecified if this cloned image
+      is encrypted using the same format and secret as its parent image
+      (i.e. not explicitly formatted) or if its parent image is not
+      encrypted.  (Since 8.0)
+
+   .. qapi:branch:: format luks
+
+      :memb str key-secret: ID of a QCryptoSecret object providing a
+         passphrase for unlocking the encryption
+
+   .. qapi:branch:: format luks2
+
+      :memb str key-secret: ID of a QCryptoSecret object providing a
+         passphrase for unlocking the encryption
+
+   .. qapi:branch:: format luks-any
+
+      :memb str key-secret: ID of a QCryptoSecret object providing a
+         passphrase for unlocking the encryption
diff --git a/docs/sphinx/qapi-domain.py b/docs/sphinx/qapi-domain.py
index b46faeaceef..f0094300c03 100644
--- a/docs/sphinx/qapi-domain.py
+++ b/docs/sphinx/qapi-domain.py
@@ -33,7 +33,12 @@
 from sphinx.locale import _, __
 from sphinx.roles import XRefRole
 from sphinx.util import logging
-from sphinx.util.docfields import GroupedField, TypedField
+from sphinx.util.docfields import (
+    DocFieldTransformer,
+    Field,
+    GroupedField,
+    TypedField,
+)
 from sphinx.util.docutils import SphinxDirective, switch_source_input
 from sphinx.util.nodes import (
     make_id,
@@ -52,6 +57,8 @@
 
 logger = logging.getLogger(__name__)
 
+quack = cast  # O:-)
+
 
 class ObjectEntry(NamedTuple):
     docname: str
@@ -242,6 +249,30 @@ def add_target_and_index(
                     ("single", indextext, node_id, "", None)
                 )
 
+    def _merge_adjoining_field_lists(self, contentnode: addnodes.desc_content) -> None:
+        # Take any adjacent field lists and glue them together into
+        # one list for further processing by Sphinx. This is done so
+        # that field lists declared in nested directives can be
+        # flattened into non-nested field lists.
+
+        first_list = None
+        delete_queue: List[nodes.field_list] = []
+        for child in contentnode:
+            if isinstance(child, nodes.field_list):
+                if not first_list:
+                    first_list = child
+                else:
+                    first_list += child.children
+                    delete_queue.append(child)
+            else:
+                first_list = None
+
+        for child in delete_queue:
+            contentnode.remove(child)
+
+    def transform_content(self, contentnode: addnodes.desc_content) -> None:
+        self._merge_adjoining_field_lists(contentnode)
+
     def _toc_entry_name(self, sig_node: desc_signature) -> str:
         # This controls the name in the TOC and on the sidebar.
 
@@ -349,6 +380,12 @@ class QAPIStruct(QAPIObjectWithMembers):
     pass
 
 
+class QAPIUnion(QAPIObjectWithMembers):
+    """Description of a QAPI Union."""
+
+    pass
+
+
 class QAPIModule(SphinxDirective):
     """
     Directive to mark description of a new module.
@@ -439,6 +476,59 @@ def run(self) -> List[Node]:
         return ret
 
 
+class Branch(SphinxDirective):
+    """
+    Nested directive which only serves to introduce temporary
+    metadata but return its parsed content nodes unaltered otherwise.
+
+    Technically, you can put whatever you want in here, but doing so may
+    prevent proper merging of adjacent field lists.
+    """
+
+    doc_field_types: List[Field] = []
+    has_content = True
+    required_arguments = 2
+    optional_arguments = 0
+    domain = "qapi"
+
+    def get_field_type_map(self) -> Dict[str, Tuple[Field, bool]]:
+        ret = {}
+        for field in self.doc_field_types:
+            for name in field.names:
+                ret[name] = (field, False)
+
+            if field.is_typed:
+                typed_field = cast(TypedField, field)
+                for name in typed_field.typenames:
+                    ret[name] = (field, True)
+        return ret
+
+    def run(self) -> list[Node]:
+        discrim = self.arguments[0].strip()
+        value = self.arguments[1].strip()
+
+        # The label name is dynamically generated per-instance instead
+        # of per-class to incorporate the branch conditions as a label
+        # name.
+        self.doc_field_types = [
+            TypedField(
+                "branch-arg-or-memb",
+                label=f"[{discrim} = {value}]",
+                # In a branch, we don't actually use the name of the
+                # field name to generate the label; so allow either-or.
+                names=("arg", "memb"),
+            ),
+        ]
+
+        content_node: addnodes.desc_content = addnodes.desc_content()
+        _nested_parse(self, content_node)
+        # DocFieldTransformer usually expects ObjectDescription, but... quack!
+        transformer = DocFieldTransformer(quack(ObjectDescription, self))
+        transformer.transform_all(content_node)
+
+        return content_node.children
+
+
 class QAPIIndex(Index):
     """
     Index subclass to provide the QAPI definition index.
@@ -505,6 +595,7 @@ class QAPIDomain(Domain):
         "enum": ObjType(_("enum"), "enum", "obj", "type"),
         "struct": ObjType(_("struct"), "struct", "obj", "type"),
         "alternate": ObjType(_("alternate"), "alt", "obj", "type"),
+        "union": ObjType(_("union"), "union", "obj", "type"),
     }
 
     # Each of these provides a ReST directive,
@@ -516,6 +607,10 @@ class QAPIDomain(Domain):
         "enum": QAPIEnum,
         "struct": QAPIStruct,
         "alternate": QAPIAlternate,
+        "union": QAPIUnion,
+        # This is not an object in its own right;
+        # It's a directive for documenting branch members of Union types.
+        "branch": Branch,
     }
 
     # These are all cross-reference roles; e.g.
@@ -528,6 +623,7 @@ class QAPIDomain(Domain):
         "enum": QAPIXRefRole(),
         "struct": QAPIXRefRole(),
         "alt": QAPIXRefRole(),
+        "union": QAPIXRefRole(),
         "type": QAPIXRefRole(),  # reference any data type (excludes modules, commands, events)
         "obj": QAPIXRefRole(),  # reference *any* type of QAPI object.
     }
-- 
2.44.0

[PATCH 18/27] docs/qapi-domain: add :deprecated: directive option

[John Snow]

Although "deprecated" is a feature (and *will* appear in the features
list), add a special :deprecated: option to generate an eye-catch that
makes this information very hard to miss.

(The intent is to modify qapidoc.py to add this option whenever it
detects that the features list attached to a definition contains the
"deprecated" entry.)

-

RFC: Technically, this object-level option is un-needed and could be
replaced with a standard content-level directive that e.g. qapidoc.py
could insert at the beginning of the content block. I've done it here as
an option to demonstrate how it would be possible to do.

It's a matter of taste for "where" we feel like implementing it.

One benefit of doing it this way is that we can create a single
containing box to set CSS style options controlling the flow of multiple
infoboxes. The other way to achieve that would be to create a directive
that allows us to set multiple options instead, e.g.:

.. qapi:infoboxes:: deprecated unstable

or possibly:

.. qapi:infoboxes::
   :deprecated:
   :unstable:

For now, I've left these as top-level QAPI object options. "Hey, it works."

P.S., I outsourced the CSS ;)

Signed-off-by: Harmonie Snow <harmonie@gmail.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
 docs/qapi/index.rst                    |  4 ++++
 docs/sphinx-static/theme_overrides.css | 25 +++++++++++++++++++++++++
 docs/sphinx/qapi-domain.py             | 24 ++++++++++++++++++++++++
 3 files changed, 53 insertions(+)

diff --git a/docs/qapi/index.rst b/docs/qapi/index.rst
index d37ceac497f..b9a69f6bd17 100644
--- a/docs/qapi/index.rst
+++ b/docs/qapi/index.rst
@@ -95,6 +95,7 @@ Explicit cross-referencing syntax for QAPI modules is available with
 
 .. qapi:command:: fake-command
    :since: 13.37
+   :deprecated:
 
    This is a fake command, it's not real. It can't hurt you.
 
@@ -116,6 +117,9 @@ Explicit cross-referencing syntax for QAPI modules is available with
       :arg int64 zzxyz: And this is another argument belonging to that same branch.
 
    :feat hallucination: This command is a figment of your imagination.
+   :feat deprecated: Although this command is fake, you should know that
+      it's also deprecated. That's great news! Maybe it will go away and
+      stop haunting you someday.
    :error CommandNotFound: When you try to use this command, because it
       isn't real.
    :error GenericError: If the system decides it doesn't like the
diff --git a/docs/sphinx-static/theme_overrides.css b/docs/sphinx-static/theme_overrides.css
index c70ef951286..97b8c1c60e6 100644
--- a/docs/sphinx-static/theme_overrides.css
+++ b/docs/sphinx-static/theme_overrides.css
@@ -159,3 +159,28 @@ div[class^="highlight"] pre {
         color: inherit;
     }
 }
+
+/* QAPI domain theming */
+
+.qapi-infopips {
+    margin-bottom: 1em;
+}
+
+.qapi-infopip {
+    display: inline-block;
+    padding: 0em 0.5em 0em 0.5em;
+    margin: 0.25em;
+}
+
+.qapi-deprecated {
+    background-color: #fffef5;
+    border: solid #fff176 6px;
+    font-weight: bold;
+    padding: 8px;
+    border-radius: 15px;
+    margin: 5px;
+}
+
+.qapi-deprecated::before {
+    content: '⚠️ ';
+}
diff --git a/docs/sphinx/qapi-domain.py b/docs/sphinx/qapi-domain.py
index f0094300c03..065ad501960 100644
--- a/docs/sphinx/qapi-domain.py
+++ b/docs/sphinx/qapi-domain.py
@@ -150,6 +150,7 @@ class QAPIObject(ObjectDescription[Signature]):
             "module": directives.unchanged,  # Override contextual module name
             # These are QAPI originals:
             "since": since_validator,
+            "deprecated": directives.flag,
         }
     )
 
@@ -249,6 +250,28 @@ def add_target_and_index(
                     ("single", indextext, node_id, "", None)
                 )
 
+    def _add_infopips(self, contentnode: addnodes.desc_content) -> None:
+        # Add various eye-catches and things that go below the signature
+        # bar, but precede the user-defined content.
+        infopips = nodes.container()
+        infopips.attributes["classes"].append("qapi-infopips")
+
+        def _add_pip(source: str, content: str, classname: str) -> None:
+            node = nodes.container(source)
+            node.append(nodes.Text(content))
+            node.attributes["classes"].extend(["qapi-infopip", classname])
+            infopips.append(node)
+
+        if "deprecated" in self.options:
+            _add_pip(
+                ":deprecated:",
+                f"This {self.objtype} is deprecated.",
+                "qapi-deprecated",
+            )
+
+        if infopips.children:
+            contentnode.insert(0, infopips)
+
     def _merge_adjoining_field_lists(self, contentnode: addnodes.desc_content) -> None:
         # Take any adjacent field lists and glue them together into
         # one list for further processing by Sphinx. This is done so
@@ -271,6 +294,7 @@ def _merge_adjoining_field_lists(self, contentnode: addnodes.desc_content) -> No
             contentnode.remove(child)
 
     def transform_content(self, contentnode: addnodes.desc_content) -> None:
+        self._add_infopips(contentnode)
         self._merge_adjoining_field_lists(contentnode)
 
     def _toc_entry_name(self, sig_node: desc_signature) -> str:
-- 
2.44.0

[PATCH 19/27] docs/qapi-domain: add :unstable: directive option

[John Snow]

Although "unstable" is a feature (and *will* appear in the features
list), add a special :unstable: option to generate an eye-catch that
makes this information very hard to miss.

(The intent is to modify qapidoc.py to add this option whenever it
detects that the features list attached to a definition contains the
"unstable" entry.)

RFC: Same comments as last patch.

Signed-off-by: Harmonie Snow <harmonie@gmail.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
 docs/qapi/index.rst                    | 4 +++-
 docs/sphinx-static/theme_overrides.css | 6 +++++-
 docs/sphinx/qapi-domain.py             | 8 ++++++++
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/docs/qapi/index.rst b/docs/qapi/index.rst
index b9a69f6bd17..6350791a3ed 100644
--- a/docs/qapi/index.rst
+++ b/docs/qapi/index.rst
@@ -96,13 +96,13 @@ Explicit cross-referencing syntax for QAPI modules is available with
 .. qapi:command:: fake-command
    :since: 13.37
    :deprecated:
+   :unstable:
 
    This is a fake command, it's not real. It can't hurt you.
 
    :arg int foo: normal parameter documentation.
    :arg str bar: Another normal parameter description.
    :arg baz: Missing a type.
-   :feat unstable: More than unstable, this command doesn't even exist!
    :arg no-descr:
    :arg BitmapSyncMode discrim: How about branches in commands?
 
@@ -120,6 +120,8 @@ Explicit cross-referencing syntax for QAPI modules is available with
    :feat deprecated: Although this command is fake, you should know that
       it's also deprecated. That's great news! Maybe it will go away and
       stop haunting you someday.
+   :feat unstable: This command, as a figment of your imagination, is
+      highly unstable and should not be relied upon.
    :error CommandNotFound: When you try to use this command, because it
       isn't real.
    :error GenericError: If the system decides it doesn't like the
diff --git a/docs/sphinx-static/theme_overrides.css b/docs/sphinx-static/theme_overrides.css
index 97b8c1c60e6..acdf11675db 100644
--- a/docs/sphinx-static/theme_overrides.css
+++ b/docs/sphinx-static/theme_overrides.css
@@ -172,7 +172,7 @@ div[class^="highlight"] pre {
     margin: 0.25em;
 }
 
-.qapi-deprecated {
+.qapi-deprecated,.qapi-unstable {
     background-color: #fffef5;
     border: solid #fff176 6px;
     font-weight: bold;
@@ -181,6 +181,10 @@ div[class^="highlight"] pre {
     margin: 5px;
 }
 
+.qapi-unstable::before {
+    content: '🚧 ';
+}
+
 .qapi-deprecated::before {
     content: '⚠️ ';
 }
diff --git a/docs/sphinx/qapi-domain.py b/docs/sphinx/qapi-domain.py
index 065ad501960..76157476e02 100644
--- a/docs/sphinx/qapi-domain.py
+++ b/docs/sphinx/qapi-domain.py
@@ -151,6 +151,7 @@ class QAPIObject(ObjectDescription[Signature]):
             # These are QAPI originals:
             "since": since_validator,
             "deprecated": directives.flag,
+            "unstable": directives.flag,
         }
     )
 
@@ -269,6 +270,13 @@ def _add_pip(source: str, content: str, classname: str) -> None:
                 "qapi-deprecated",
             )
 
+        if "unstable" in self.options:
+            _add_pip(
+                ":unstable:",
+                f"This {self.objtype} is unstable/experimental.",
+                "qapi-unstable",
+            )
+
         if infopips.children:
             contentnode.insert(0, infopips)
 
-- 
2.44.0

[PATCH 20/27] docs/qapi-domain: add :ifcond: directive option

[John Snow]

Add a special :ifcond: option that allows us to annotate the
definition-level conditionals.

RFC: This patch renders IFCOND information in two places, because I'm
undecided about how to style this information. One option is in the
signature bar, and another option is in an eye-catch, like :deprecated:
or :unstable:.

A benefit to having this be a directive option is that we can put it in
the signature bar, the QAPI index, etc. However, if we merely want it in
the content section, a directive would work just as well,
e.g. ".. qapi:ifcond:: CONFIG_LINUX".

(Though, having it be in the same containing box as the unstable/ifcond
boxes might require some extra fiddling/post-processing to
achieve. Generally, the less docutils tree muddling I have to do, the
happier I am.)

The syntax of the argument is currently undefined, but it is possible to
parse it back down into constituent parts to avoid applying literal
formatting to "AND" or "&&" or whichever syntax we formalize. (Or, in
the future, applying cross-reference links to the config values for
additional reading on some of those build options. Not for this series.)

"Vote now on your phones!"

Signed-off-by: Harmonie Snow <harmonie@gmail.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
 docs/qapi/index.rst                    |  1 +
 docs/sphinx-static/theme_overrides.css | 13 +++++++++
 docs/sphinx/qapi-domain.py             | 37 ++++++++++++++++++++++++--
 3 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/docs/qapi/index.rst b/docs/qapi/index.rst
index 6350791a3ed..c68e2044890 100644
--- a/docs/qapi/index.rst
+++ b/docs/qapi/index.rst
@@ -97,6 +97,7 @@ Explicit cross-referencing syntax for QAPI modules is available with
    :since: 13.37
    :deprecated:
    :unstable:
+   :ifcond: CONFIG_LINUX
 
    This is a fake command, it's not real. It can't hurt you.
 
diff --git a/docs/sphinx-static/theme_overrides.css b/docs/sphinx-static/theme_overrides.css
index acdf11675db..b239a762a9e 100644
--- a/docs/sphinx-static/theme_overrides.css
+++ b/docs/sphinx-static/theme_overrides.css
@@ -188,3 +188,16 @@ div[class^="highlight"] pre {
 .qapi-deprecated::before {
     content: '⚠️ ';
 }
+
+.qapi-ifcond::before {
+    /* gaze ye into the crystal ball to determine feature availability */
+    content: '🔮 ';
+}
+
+.qapi-ifcond {
+    background-color: #f9f5ff;
+    border: solid #dac2ff 6px;
+    padding: 8px;
+    border-radius: 15px;
+    margin: 5px;
+}
diff --git a/docs/sphinx/qapi-domain.py b/docs/sphinx/qapi-domain.py
index 76157476e02..e44db10488f 100644
--- a/docs/sphinx/qapi-domain.py
+++ b/docs/sphinx/qapi-domain.py
@@ -15,6 +15,7 @@
     NamedTuple,
     Optional,
     Tuple,
+    Union,
     cast,
 )
 
@@ -150,6 +151,7 @@ class QAPIObject(ObjectDescription[Signature]):
             "module": directives.unchanged,  # Override contextual module name
             # These are QAPI originals:
             "since": since_validator,
+            "ifcond": directives.unchanged,
             "deprecated": directives.flag,
             "unstable": directives.flag,
         }
@@ -176,6 +178,20 @@ def get_signature_suffix(self, sig: str) -> List[nodes.Node]:
         """Returns a suffix to put after the object name in the signature."""
         ret: List[nodes.Node] = []
 
+        if "ifcond" in self.options:
+            ret += [
+                addnodes.desc_sig_space(),
+                nodes.inline(
+                    self.options["ifcond"],
+                    "",
+                    nodes.Text("["),
+                    nodes.literal("", "#if"),
+                    addnodes.desc_sig_space(),
+                    nodes.literal(self.options["ifcond"], self.options["ifcond"]),
+                    nodes.Text("]"),
+                ),
+            ]
+
         if "since" in self.options:
             ret += [
                 addnodes.desc_sig_space(),
@@ -257,9 +273,14 @@ def _add_infopips(self, contentnode: addnodes.desc_content) -> None:
         infopips = nodes.container()
         infopips.attributes["classes"].append("qapi-infopips")
 
-        def _add_pip(source: str, content: str, classname: str) -> None:
+        def _add_pip(
+            source: str, content: Union[str, List[nodes.Node]], classname: str
+        ) -> None:
             node = nodes.container(source)
-            node.append(nodes.Text(content))
+            if isinstance(content, str):
+                node.append(nodes.Text(content))
+            else:
+                node.extend(content)
             node.attributes["classes"].extend(["qapi-infopip", classname])
             infopips.append(node)
 
@@ -277,6 +298,18 @@ def _add_pip(source: str, content: str, classname: str) -> None:
                 "qapi-unstable",
             )
 
+        if self.options.get("ifcond", ""):
+            ifcond = self.options["ifcond"]
+            _add_pip(
+                f":ifcond: {ifcond}",
+                [
+                    nodes.emphasis("", "Availability"),
+                    nodes.Text(": "),
+                    nodes.literal(ifcond, ifcond),
+                ],
+                "qapi-ifcond",
+            )
+
         if infopips.children:
             contentnode.insert(0, infopips)
 
-- 
2.44.0

[PATCH 21/27] docs/qapi-domain: RFC patch - add malformed field list entries

[John Snow]

This patch demonstrates what happens when you mess up a field list
entry. The next patch adds a safeguard against this.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 docs/qapi/index.rst | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/docs/qapi/index.rst b/docs/qapi/index.rst
index c68e2044890..5bb1c37a5ed 100644
--- a/docs/qapi/index.rst
+++ b/docs/qapi/index.rst
@@ -129,6 +129,10 @@ Explicit cross-referencing syntax for QAPI modules is available with
       argument values. It's very temperamental.
    :return SomeTypeName: An esoteric collection of mystical nonsense to
       both confound and delight.
+   :arg: this is malformed.
+   :memb: this is malformed and unrecognized.
+   :choice type name: This is unrecognized.
+   :errors FooError: Also malformed.
 
    Field lists can appear anywhere in the directive block, but any field
    list entries in the same list block that are recognized as special
-- 
2.44.0

[PATCH 22/27] docs/qapi-domain: add warnings for malformed field lists

[John Snow]

Normally, Sphinx will silently fall back to its standard field list
processing if it doesn't match one of your defined fields. A lot of the
time, that's not what we want - we want to be warned if we goof
something up.

For instance, the canonical argument field list form is:

:arg type name: descr

This form is captured by Sphinx and transformed so that the field label
will become "Arguments:". It's possible to omit the type name and descr
and still have it be processed correctly. However, if you omit the type
name, Sphinx no longer recognizes it:

:arg: this is not recognized.

This will turn into an arbitrary field list entry whose label is "Arg:",
and it otherwise silently fails. You may also see failures for doing
things like using :values: instead of :value:, or :errors: instead of
:error:, and so on. It's also case sensitive, and easy to trip up.

Add a validator that guarantees all field list entries that are the
direct child of an ObjectDescription use only recognized forms of field
lists, and emit a warning (treated as error by default in most build
configurations) whenever we detect one that is goofed up.

However, there's still benefit to allowing arbitrary fields -- they are
after all not a Sphinx invention, but perfectly normal docutils
syntax. Create an allow list for known spellings we don't mind letting
through, but warn against anything else.

-

RFC: Yes, this patch breaks the build! I thought it'd be helpful for you
to see the error checking in action. The next commit drops the erroneous
fields.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 docs/conf.py               | 12 ++++++++
 docs/qapi/index.rst        | 27 +++++++++++++++++
 docs/sphinx/qapi-domain.py | 59 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 98 insertions(+)

diff --git a/docs/conf.py b/docs/conf.py
index b15665d956d..7a7d7005780 100644
--- a/docs/conf.py
+++ b/docs/conf.py
@@ -148,6 +148,18 @@
 with open(os.path.join(qemu_docdir, 'defs.rst.inc')) as f:
     rst_epilog += f.read()
 
+
+# Normally, the QAPI domain is picky about what field lists you use to
+# describe a QAPI entity. If you'd like to use arbitrary additional
+# fields in source documentation, add them here.
+qapi_allowed_fields = {
+    "example",
+    "note",
+    "see also",
+    "TODO",
+}
+
+
 # -- Options for HTML output ----------------------------------------------
 
 # The theme to use for HTML and HTML Help pages.  See the documentation for
diff --git a/docs/qapi/index.rst b/docs/qapi/index.rst
index 5bb1c37a5ed..ef58dfc4bcd 100644
--- a/docs/qapi/index.rst
+++ b/docs/qapi/index.rst
@@ -133,6 +133,33 @@ Explicit cross-referencing syntax for QAPI modules is available with
    :memb: this is malformed and unrecognized.
    :choice type name: This is unrecognized.
    :errors FooError: Also malformed.
+   :example: This isn't a "semantic" field, but it's been added to the
+      allowed field names list. you can use whatever field names you'd
+      like; but to prevent accidental typos, there is an allow list of
+      "arbitrary" section names.
+
+      You can nestle code-blocks in here, too, by using the ``::``
+      syntax::
+
+         -> { [ "bidirectional QMP example" ] }
+         <- { [ "hello world!"] }
+
+      Or use explicit ``.. code-block:: QMP`` syntax, but it must start
+      on its own line with a blank line both before and after the
+      directive to render correctly:
+
+      .. code-block:: QMP
+
+         -> "Hello friend!"
+
+      Note that the QMP highlighter is merely garden-variety JSON, but
+      with the addition of ``->``, ``<-`` and ``...`` symbols to help
+      denote bidirectionality and elided segments. Eduardo Habkost and I
+      wrote this lexer many moons ago to support the
+      :doc:`/interop/bitmaps` documentation.
+   :see also: This is also not a "semantic" field. The only limit is
+      your imagination and what you can convince others to let you check
+      into conf.py.
 
    Field lists can appear anywhere in the directive block, but any field
    list entries in the same list block that are recognized as special
diff --git a/docs/sphinx/qapi-domain.py b/docs/sphinx/qapi-domain.py
index e44db10488f..bf8bb933345 100644
--- a/docs/sphinx/qapi-domain.py
+++ b/docs/sphinx/qapi-domain.py
@@ -334,10 +334,63 @@ def _merge_adjoining_field_lists(self, contentnode: addnodes.desc_content) -> No
         for child in delete_queue:
             contentnode.remove(child)
 
+    def _validate_field(self, field: nodes.field) -> None:
+        field_name = field.children[0]
+        assert isinstance(field_name, nodes.field_name)
+        allowed_fields = set(self.env.app.config.qapi_allowed_fields)
+
+        field_label = field_name.astext()
+        if re.match(r"\[\S+ = \S+\]", field_label) or field_label in allowed_fields:
+            # okie-dokey. branch entry or known good allowed name.
+            return
+
+        try:
+            # split into field type and argument
+            field_type, fieldarg = field_label.split(None, 1)
+        except ValueError:
+            # No arguments provided
+            field_type = field_label
+            fieldarg = ""
+
+        typemap = self.get_field_type_map()
+        if field_type in typemap:
+            # This is a semantic field, yet-to-be-processed. Catch
+            # correct names, but incorrect arguments (which WILL fail to
+            # process correctly).
+            typedesc = typemap[field_type][0]
+            if typedesc.has_arg != bool(fieldarg):
+                msg = f"semantic field list type {field_type!r} "
+                if typedesc.has_arg:
+                    msg += "requires an argument."
+                else:
+                    msg += "takes no arguments."
+                logger.warning(msg, location=field)
+        else:
+            # This is unrecognized entirely.
+            valid = ", ".join(sorted(set(typemap) | allowed_fields))
+            msg = (
+                f"Unrecognized field list name {field_label!r}.\n"
+                f"Valid fields for qapi:{self.objtype} are: {valid}\n"
+                "\n"
+                "If this usage is intentional, please add it to "
+                "'qapi_allowed_fields' in docs/conf.py."
+            )
+            logger.warning(msg, location=field)
+
     def transform_content(self, contentnode: addnodes.desc_content) -> None:
         self._add_infopips(contentnode)
         self._merge_adjoining_field_lists(contentnode)
 
+        # Validate field lists. Note that this hook runs after any
+        # branch directives have processed and transformed their field
+        # lists, but before the main object directive has had a chance
+        # to do so.
+        for child in contentnode:
+            if isinstance(child, nodes.field_list):
+                for field in child.children:
+                    assert isinstance(field, nodes.field)
+                    self._validate_field(field)
+
     def _toc_entry_name(self, sig_node: desc_signature) -> str:
         # This controls the name in the TOC and on the sidebar.
 
@@ -872,6 +925,12 @@ def resolve_any_xref(
 
 def setup(app: Sphinx) -> Dict[str, Any]:
     app.setup_extension("sphinx.directives")
+    app.add_config_value(
+        "qapi_allowed_fields",
+        set(),
+        "env",  # Setting impacts parsing phase
+        types=set,
+    )
     app.add_domain(QAPIDomain)
 
     return {
-- 
2.44.0

[PATCH 23/27] docs/qapi-domain: RFC patch - delete malformed field lists

[John Snow]

Cleanup of the last patch to fix the build before closing out this RFC
series.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 docs/qapi/index.rst | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/docs/qapi/index.rst b/docs/qapi/index.rst
index ef58dfc4bcd..8352a27d4a5 100644
--- a/docs/qapi/index.rst
+++ b/docs/qapi/index.rst
@@ -129,10 +129,6 @@ Explicit cross-referencing syntax for QAPI modules is available with
       argument values. It's very temperamental.
    :return SomeTypeName: An esoteric collection of mystical nonsense to
       both confound and delight.
-   :arg: this is malformed.
-   :memb: this is malformed and unrecognized.
-   :choice type name: This is unrecognized.
-   :errors FooError: Also malformed.
    :example: This isn't a "semantic" field, but it's been added to the
       allowed field names list. you can use whatever field names you'd
       like; but to prevent accidental typos, there is an allow list of
-- 
2.44.0

[PATCH 24/27] docs/qapi-domain: add type cross-refs to field lists

[John Snow]

This commit, finally, adds cross-referencing support to various field
lists; modeled tightly after Sphinx's own Python domain code.

Cross-referencing support is added to type names provided to :arg:,
:memb:, :returns: and :choice:.

:feat:, :error: and :value:, which do not take type names, do not
support this syntax.

The general syntax is simple:

:arg TypeName ArgName: Lorem Ipsum ...

The domain will transform TypeName into :qapi:type:`TypeName` in this
basic case, and also apply the ``literal`` decoration to indicate that
this is a type cross-reference.

For Optional arguments, the special "?" suffix is used. Because "*" has
special meaning in ReST that would cause parsing errors, we elect to use
"?" instead. The special syntax processing in QAPIXrefMixin strips this
character from the end of any type name argument and will append ",
Optional" to the rendered output, applying the cross-reference only to
the actual type name.

The intent here is that the actual syntax in doc-blocks need not change;
but e.g. qapidoc.py will need to process and transform "@arg foo lorem
ipsum" into ":arg type? foo: lorem ipsum" based on the schema
information. Therefore, nobody should ever actually witness this
intermediate syntax unless they are writing manual documentation or the
doc transmogrifier breaks.

For array arguments, type names can similarly be surrounded by "[]",
which are stripped off and then re-appended outside of the
cross-reference.

Note: The mixin pattern here (borrowed from Sphinx) confuses mypy
because it cannot tell that it will be mixed into a descendent of
Field. Doing that instead causes more errors, because many versions of
Sphinx erroneously did not mark various arguments as Optional, so we're
a bit hosed either way. Do the simpler thing.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 docs/qapi/index.rst        |  34 ++++++++++++
 docs/sphinx/qapi-domain.py | 110 +++++++++++++++++++++++++++++++++++--
 2 files changed, 138 insertions(+), 6 deletions(-)

diff --git a/docs/qapi/index.rst b/docs/qapi/index.rst
index 8352a27d4a5..6e85ea5280d 100644
--- a/docs/qapi/index.rst
+++ b/docs/qapi/index.rst
@@ -105,6 +105,11 @@ Explicit cross-referencing syntax for QAPI modules is available with
    :arg str bar: Another normal parameter description.
    :arg baz: Missing a type.
    :arg no-descr:
+   :arg int? oof: Testing optional argument parsing.
+   :arg [XDbgBlockGraphNode] rab: Testing array argument parsing.
+   :arg [BitmapSyncMode]? zab: Testing optional array argument parsing,
+      even though Markus said this should never happen. I believe him,
+      but I didn't *forbid* the syntax either.
    :arg BitmapSyncMode discrim: How about branches in commands?
 
    .. qapi:branch:: discrim on-success
@@ -261,3 +266,32 @@ Explicit cross-referencing syntax for QAPI modules is available with
 
       :memb str key-secret: ID of a QCryptoSecret object providing a
          passphrase for unlocking the encryption
+
+.. qapi:command:: x-debug-query-block-graph
+   :since: 4.0
+   :unstable:
+
+   Get the block graph.
+
+   :feat unstable: This command is meant for debugging.
+   :return XDbgBlockGraph: lorem ipsum ...
+
+.. qapi:struct:: XDbgBlockGraph
+   :since: 4.0
+
+   Block Graph - list of nodes and list of edges.
+
+   :memb [XDbgBlockGraphNode] nodes:
+   :memb [XDbgBlockGraphEdge] edges:
+
+.. qapi:struct:: XDbgBlockGraphNode
+   :since: 4.0
+
+   :memb uint64 id: Block graph node identifier.  This @id is generated only for
+      x-debug-query-block-graph and does not relate to any other
+      identifiers in Qemu.
+   :memb XDbgBlockGraphNodeType type: Type of graph node.  Can be one of
+      block-backend, block-job or block-driver-state.
+   :memb str name: Human readable name of the node.  Corresponds to
+      node-name for block-driver-state nodes; is not guaranteed to be
+      unique in the whole graph (with block-jobs and block-backends).
diff --git a/docs/sphinx/qapi-domain.py b/docs/sphinx/qapi-domain.py
index bf8bb933345..074453193ce 100644
--- a/docs/sphinx/qapi-domain.py
+++ b/docs/sphinx/qapi-domain.py
@@ -50,11 +50,12 @@
 
 if TYPE_CHECKING:
     from docutils.nodes import Element, Node
+    from docutils.parsers.rst.states import Inliner
 
     from sphinx.application import Sphinx
     from sphinx.builders import Builder
     from sphinx.environment import BuildEnvironment
-    from sphinx.util.typing import OptionSpec
+    from sphinx.util.typing import OptionSpec, TextlikeNode
 
 logger = logging.getLogger(__name__)
 
@@ -68,6 +69,90 @@ class ObjectEntry(NamedTuple):
     aliased: bool
 
 
+class QAPIXrefMixin:
+    def make_xref(
+        self,
+        rolename: str,
+        domain: str,
+        target: str,
+        innernode: type[TextlikeNode] = nodes.literal,
+        contnode: Optional[Node] = None,
+        env: Optional[BuildEnvironment] = None,
+        inliner: Optional[Inliner] = None,
+        location: Optional[Node] = None,
+    ) -> Node:
+        result = super().make_xref(  # type: ignore[misc]
+            rolename,
+            domain,
+            target,
+            innernode=innernode,
+            contnode=contnode,
+            env=env,
+            inliner=None,
+            location=None,
+        )
+        if isinstance(result, pending_xref):
+            assert env is not None
+            result["refspecific"] = True
+            result["qapi:module"] = env.ref_context.get("qapi:module")
+
+        assert isinstance(result, Node)
+        return result
+
+    def make_xrefs(
+        self,
+        rolename: str,
+        domain: str,
+        target: str,
+        innernode: type[TextlikeNode] = nodes.literal,
+        contnode: Optional[Node] = None,
+        env: Optional[BuildEnvironment] = None,
+        inliner: Optional[Inliner] = None,
+        location: Optional[Node] = None,
+    ) -> list[Node]:
+        # Note: this function is called on up to three fields of text:
+        # (1) The field name argument (e.g. member/arg name)
+        # (2) The field name type (e.g. member/arg type)
+        # (3) The field *body* text, for Fields that do not take arguments.
+
+        list_type = False
+        optional = False
+
+        # If the rolename is qapi:type, we know we are processing a type
+        # and not an arg/memb name or field body text.
+        if rolename == "type":
+            # force the innernode class to be a literal.
+            innernode = nodes.literal
+
+            # Type names that end with "?" are considered Optional
+            # arguments and should be documented as such, but it's not
+            # part of the xref itself.
+            if target.endswith("?"):
+                optional = True
+                target = target[:-1]
+
+            # Type names wrapped in brackets denote lists. strip the
+            # brackets and remember to add them back later.
+            if target.startswith("[") and target.endswith("]"):
+                list_type = True
+                target = target[1:-1]
+
+        results = []
+        result = self.make_xref(
+            rolename, domain, target, innernode, contnode, env, inliner, location
+        )
+        results.append(result)
+
+        if list_type:
+            results.insert(0, nodes.literal("[", "["))
+            results.append(nodes.literal("]", "]"))
+        if optional:
+            results.append(nodes.Text(", "))
+            results.append(nodes.emphasis("?", "Optional"))
+
+        return results
+
+
 class QAPIXRefRole(XRefRole):
     def process_link(
         self,
@@ -96,6 +181,14 @@ def process_link(
         return title, target
 
 
+class QAPIGroupedField(QAPIXrefMixin, GroupedField):
+    pass
+
+
+class QAPITypedField(QAPIXrefMixin, TypedField):
+    pass
+
+
 def since_validator(param: str) -> str:
     """
     Validate the `:since: X.Y` option field.
@@ -416,10 +509,11 @@ class QAPICommand(QAPIObject):
     doc_field_types = QAPIObject.doc_field_types.copy()
     doc_field_types.extend(
         [
-            TypedField(
+            QAPITypedField(
                 "argument",
                 label=_("Arguments"),
                 names=("arg",),
+                typerolename="type",
                 can_collapse=True,
             ),
             GroupedField(
@@ -428,9 +522,10 @@ class QAPICommand(QAPIObject):
                 names=("error",),
                 can_collapse=True,
             ),
-            GroupedField(
+            QAPIGroupedField(
                 "returnvalue",
                 label=_("Returns"),
+                rolename="type",
                 names=("return", "returns"),
                 can_collapse=True,
             ),
@@ -460,10 +555,11 @@ class QAPIAlternate(QAPIObject):
     doc_field_types = QAPIObject.doc_field_types.copy()
     doc_field_types.extend(
         [
-            TypedField(
+            QAPITypedField(
                 "choice",
                 label=_("Choices"),
                 names=("choice",),
+                typerolename="type",
                 can_collapse=True,
             ),
         ]
@@ -476,10 +572,11 @@ class QAPIObjectWithMembers(QAPIObject):
     doc_field_types = QAPIObject.doc_field_types.copy()
     doc_field_types.extend(
         [
-            TypedField(
+            QAPITypedField(
                 "member",
                 label=_("Members"),
                 names=("memb",),
+                typerolename="type",
                 can_collapse=True,
             ),
         ]
@@ -629,12 +726,13 @@ def run(self) -> list[Node]:
         # of per-class to incorporate the branch conditions as a label
         # name.
         self.doc_field_types = [
-            TypedField(
+            QAPITypedField(
                 "branch-arg-or-memb",
                 label=f"[{discrim} = {value}]",
                 # In a branch, we don't actually use the name of the
                 # field name to generate the label; so allow either-or.
                 names=("arg", "memb"),
+                typerolename="type",
             ),
         ]
 
-- 
2.44.0

[PATCH 25/27] docs/qapi-domain: implement error context reporting fix

[John Snow]

Sphinx 5.3.0 to Sphinx 6.2.0 has a bug where nested content in an
ObjectDescription content block has its error position reported
incorrectly due to an oversight when they added nested section support
to this directive.

(This bug is present in Sphinx's own Python and C domains; test it
yourself by creating a py:func directive and creating a syntax error in
the directive's content block.)

To avoid overriding and re-implementing the entirety of the run()
method, a workaround is employed where we parse the content block
ourselves in before_content(), then null the content block to make
Sphinx's own parsing a no-op. Then, in transform_content (which occurs
after Sphinx's nested parse), we simply swap our own parsed content tree
back in for Sphinx's.

It appears a little tricky, but it's the nicest solution I can find.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 docs/sphinx/qapi-domain.py | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/docs/sphinx/qapi-domain.py b/docs/sphinx/qapi-domain.py
index 074453193ce..7d8911c635f 100644
--- a/docs/sphinx/qapi-domain.py
+++ b/docs/sphinx/qapi-domain.py
@@ -21,7 +21,9 @@
 
 from docutils import nodes
 from docutils.parsers.rst import directives
+from docutils.statemachine import StringList
 
+import sphinx
 from sphinx import addnodes
 from sphinx.addnodes import desc_signature, pending_xref
 from sphinx.directives import ObjectDescription
@@ -470,7 +472,27 @@ def _validate_field(self, field: nodes.field) -> None:
             )
             logger.warning(msg, location=field)
 
+    def before_content(self) -> None:
+        # Work around a sphinx bug and parse the content ourselves.
+        self._temp_content = self.content
+        self._temp_offset = self.content_offset
+        self._temp_node = None
+
+        if sphinx.version_info[:3] >= (5, 3, 0) and sphinx.version_info[:3] < (6, 2, 0):
+            self._temp_node = addnodes.desc_content()
+            self.state.nested_parse(self.content, self.content_offset, self._temp_node)
+            # Sphinx will try to parse the content block itself,
+            # Give it nothingness to parse instead.
+            self.content = StringList()
+            self.content_offset = 0
+
     def transform_content(self, contentnode: addnodes.desc_content) -> None:
+        # Sphinx workaround: Inject our parsed content and restore state.
+        if self._temp_node:
+            contentnode += self._temp_node.children
+            self.content = self._temp_content
+            self.content_offset = self._temp_offset
+
         self._add_infopips(contentnode)
         self._merge_adjoining_field_lists(contentnode)
 
-- 
2.44.0

[PATCH 26/27] docs/qapi-domain: RFC patch - Add one last sample command

[John Snow]

Just to have a bit more to look at in the generated doc, here's a fairly
complex command with a lot of bells and whistles.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 docs/qapi/index.rst | 67 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 67 insertions(+)

diff --git a/docs/qapi/index.rst b/docs/qapi/index.rst
index 6e85ea5280d..4562e755d21 100644
--- a/docs/qapi/index.rst
+++ b/docs/qapi/index.rst
@@ -267,6 +267,73 @@ Explicit cross-referencing syntax for QAPI modules is available with
       :memb str key-secret: ID of a QCryptoSecret object providing a
          passphrase for unlocking the encryption
 
+.. qapi:command:: blockdev-backup
+   :since: 4.2
+
+   Start a point-in-time copy of a block device to a new
+   destination. The status of ongoing blockdev-backup operations can be
+   checked with query-block-jobs where the BlockJobInfo.type field has
+   the value ‘backup’. The operation can be stopped before it has
+   completed using the block-job-cancel command.
+
+   :arg str target:
+      the device name or node-name of the backup target node.
+   :arg str? job-id:
+      identifier for the newly-created block job. If omitted, the device
+      name will be used. (Since 2.7)
+   :arg str device:
+      the device name or node-name of a root node which should be copied.
+   :arg MirrorSyncMode sync:
+      what parts of the disk image should be copied to the destination
+      (all the disk, only the sectors allocated in the topmost image,
+      from a dirty bitmap, or only new I/O).
+   :arg int? speed:
+      the maximum speed, in bytes per second. The default is 0, for unlimited.
+   :arg str? bitmap:
+      The name of a dirty bitmap to use. Must be present if sync is
+      ``bitmap`` or ``incremental``. Can be present if sync is ``full`` or
+      ``top``. Must not be present otherwise. (Since 2.4 (drive-backup),
+      3.1 (blockdev-backup))
+   :arg BitmapSyncMode? bitmap-mode:
+      Specifies the type of data the bitmap should contain after the
+      operation concludes. Must be present if a bitmap was provided,
+      Must NOT be present otherwise. (Since 4.2)
+   :arg bool? compress:
+      true to compress data, if the target format supports it. (default:
+      false) (since 2.8)
+   :arg BlockdevOnError? on-source-error:
+      the action to take on an error on the source, default
+      ``report``. ``stop`` and ``enospc`` can only be used if the block device
+      supports io-status (see BlockInfo).
+   :arg BlockdevOnError? on-target-error:
+      the action to take on an error on the target, default ``report`` (no
+      limitations, since this applies to a different block device than
+      device).
+   :arg bool? auto-finalize:
+      When false, this job will wait in a ``PENDING`` state after it has
+      finished its work, waiting for block-job-finalize before making
+      any block graph changes. When true, this job will automatically
+      perform its abort or commit actions. Defaults to true. (Since
+      2.12)
+   :arg bool? auto-dismiss:
+      When false, this job will wait in a ``CONCLUDED`` state after it has
+      completely ceased all work, and awaits block-job-dismiss. When
+      true, this job will automatically disappear from the query list
+      without user intervention. Defaults to true. (Since 2.12)
+   :arg str? filter-node-name:
+      the node name that should be assigned to the filter driver that
+      the backup job inserts into the graph above node specified by
+      drive. If this option is not given, a node name is
+      autogenerated. (Since: 4.2)
+   :arg BackupPerf? x-perf:
+      Performance options. (Since 6.0)
+   :feat unstable: Member ``x-perf`` is experimental.
+   :error DeviceNotFound: if ``device`` is not a valid block device.
+
+   .. note:: ``on-source-error`` and ``on-target-error only`` affect
+             background I/O. If an error occurs during a guest write
+             request, the device's rerror/werror actions will be used.
+
 .. qapi:command:: x-debug-query-block-graph
    :since: 4.0
    :unstable:
-- 
2.44.0

[PATCH 27/27] docs/qapi-domain: add CSS styling

[John Snow]

From: Harmonie Snow <harmonie@gmail.com>

Improve the general look and feel of generated QAPI docs.

Attempt to limit line lengths to offer a more comfortable measure on
maximized windows, and improve some margin and spacing for field lists.

Signed-off-by: Harmonie Snow <harmonie@gmail.com>
Signed-off-by: John Snow <jsnow@redhat.com>
---
 docs/sphinx-static/theme_overrides.css | 50 ++++++++++++++++++++++++--
 1 file changed, 48 insertions(+), 2 deletions(-)

diff --git a/docs/sphinx-static/theme_overrides.css b/docs/sphinx-static/theme_overrides.css
index b239a762a9e..9a58cf86b5c 100644
--- a/docs/sphinx-static/theme_overrides.css
+++ b/docs/sphinx-static/theme_overrides.css
@@ -18,8 +18,8 @@ h1, h2, .rst-content .toctree-wrapper p.caption, h3, h4, h5, h6, legend {
 
 .rst-content dl:not(.docutils) dt {
     border-top: none;
-    border-left: solid 3px #ccc;
-    background-color: #f0f0f0;
+    border-left: solid 5px #bcc6d2;
+    background-color: #eaedf1;
     color: black;
 }
 
@@ -162,6 +162,18 @@ div[class^="highlight"] pre {
 
 /* QAPI domain theming */
 
+/* most content in a qapi object definition should not eclipse about
+   80ch, but nested field lists are explicitly exempt due to their
+   two-column nature */
+.qapi dd *:not(dl) {
+    max-width: 80ch;
+}
+
+/* but the content column itself should still be less than ~80ch. */
+.qapi .field-list dd {
+    max-width: 80ch;
+}
+
 .qapi-infopips {
     margin-bottom: 1em;
 }
@@ -201,3 +213,37 @@ div[class^="highlight"] pre {
     border-radius: 15px;
     margin: 5px;
 }
+
+/* code blocks */
+.qapi div[class^="highlight"] {
+    width: fit-content;
+    background-color: #fffafd;
+    border: 2px solid #ffe1f3;
+}
+
+/* note, warning, etc. */
+.qapi .admonition {
+    width: fit-content;
+}
+
+/* pad the top of the field-list so the text doesn't start directly at
+   the top border; primarily for the field list labels, but adjust the
+   field bodies as well for parity. */
+dl.field-list > dt:first-of-type, dl.field-list > dd:first-of-type {
+    padding-top: 0.3em;
+}
+
+dl.field-list > dt:last-of-type, dl.field-list > dd:last-of-type {
+    padding-bottom: 0.3em;
+}
+
+/* pad the field list labels so they don't crash into the border */
+dl.field-list > dt {
+    padding-left: 0.5em;
+    padding-right: 0.5em;
+}
+
+/* Add a little padding between field list sections */
+dl.field-list > dd:not(:last-child) {
+    padding-bottom: 1em;
+}
-- 
2.44.0

Re: [PATCH 00/27] Add qapi-domain Sphinx extension

[Markus Armbruster]

John Snow <jsnow@redhat.com> writes:

> This series adds a new qapi-domain extension for Sphinx, which adds a
> series of custom directives for documenting QAPI definitions.
>
> GitLab CI: https://gitlab.com/jsnow/qemu/-/pipelines/1259566476
>
> (Link to a demo HTML page at the end of this cover letter, but I want
> you to read the cover letter first to explain what you're seeing.)
>
> This adds a new QAPI Index page, cross-references for QMP commands,
> events, and data types, and improves the aesthetics of the QAPI/QMP
> documentation.

Cross-references alone will be a *massive* improvement!  I'm sure
readers will appreciate better looks and an index, too.

> This series adds only the new ReST syntax, *not* the autogenerator. The
> ReST syntax used in this series is, in general, not intended for anyone
> to actually write by hand. This mimics how Sphinx's own autodoc
> extension generates Python domain directives, which are then re-parsed
> to produce the final result.
>
> I have prototyped such a generator, but it isn't ready for inclusion
> yet. (Rest assured: error context reporting is preserved down to the
> line, even in generated ReST. There is no loss in usability for this
> approach. It will likely either supplant qapidoc.py or heavily alter
> it.) The generator requires only extremely minor changes to
> scripts/qapi/parser.py to preserve nested indentation and provide more
> accurate line information. It is less invasive than you may
> fear. Relying on a secondary ReST parse phase eliminates much of the
> complexity of qapidoc.py. Sleep soundly.

I'm a Sphinx noob.  Let me paraphrase you to make sure I understand.

You proprose to generate formatted documentation in two steps:

• First, the QAPI generator generates .rst from the QAPI schema.  The
  generated .rst makes use of a custom directives.

• Second, Sphinx turns the .rst into formatted documentation.  A Sphinx
  qapi-domain extension implements the custom directives

This mirrors how Sphinx works for Python docs.  Which is its original
use case.

Your series demonstrates the second step, with test input you wrote
manually.

You have code for the first step, but you'd prefer to show it later.

Fair?

> The purpose of sending this series in its current form is largely to
> solicit feedback on general aesthetics, layout, and features. Sphinx is
> a wily beast, and feedback at this stage will dictate how and where
> certain features are implemented.

I'd appreciate help with that.  Opinions?

> A goal for this syntax (and the generator) is to fully in-line all
> command/event/object members, inherited or local, boxed or not, branched
> or not. This should provide a boon to using these docs as a reference,
> because users will not have to grep around the page looking for various
> types, branches, or inherited members. Any arguments types will be
> hyperlinked to their definition, further aiding usability. Commands can
> be hotlinked from anywhere else in the manual, and will provide a
> complete reference directly on the first screenful of information.

Let me elaborate a bit here.

A command's arguments can be specified inline, like so:

    { 'command': 'job-cancel', 'data': { 'id': 'str' } }

The arguments are then documented right with the command.

But they can also be specified by referencing an object type, like so:

    { 'command': 'block-dirty-bitmap-remove',
      'data': 'BlockDirtyBitmap' }

Reasons for doing it this way:

• Several commands take the same arguments, and you don't want to repeat
  yourself.

• You want generated C take a single struct argument ('boxed': true).

• The arguments are a union (which requires 'boxed': true).

Drawback: the arguments are then documented elsewhere.  Not nice.

Bug: the generated documentation fails to point there.

You're proposing to inline the argument documentation, so it appears
right with the command.

An event's data is just like a command's argument.

A command's return value can only specified by referencing a type.  Same
doc usability issue.

Similarly, a union type's base can specified inline or by referencing a
struct type, and a union's branches must be specified by referencing a
struct type.  Same doc usability issue.

At least, the generated documentation does point to the referenced
types.

> (Okay, maybe two screenfuls for commands with a ton of
> arguments... There's only so much I can do!)

*cough* blockdev-add *cough*

> This RFC series includes a "sandbox" .rst document that showcases the
> features of this domain by writing QAPI directives by hand; this
> document will be removed from the series before final inclusion. It's
> here to serve as a convenient test-bed for y'all to give feedback.
>
> All that said, here's the sandbox document fully rendered:
> https://jsnow.gitlab.io/qemu/qapi/index.html
>
> And here's the new QAPI index page created by that sandbox document:
> https://jsnow.gitlab.io/qemu/qapi-index.html
>
> Known issues / points of interest:
>
> - The formatting upsets checkpatch. The default line length for the
>   "black" formatting tool is a little long. I'll fix it next spin.
>
> - I did my best to preserve cross-version compatibility, but some
>   problems have crept in here and there. This series may require
>   Sphinx>= 4.0, like the dbus extension. Notably, the Ubuntu build fails
>   on Gitlab CI currently. The next version will text against more Sphinx
>   versions more rigorously. Sphinx version 5.3.0 and above should work
>   perfectly.
>
> - There's a bug in Sphinx itself that may manifest in your testing,
>   concerning reported error locations. There's a patch in this series
>   that fixes it, but it's later in the series. If you run into the bug
>   while testing with this series, try applying that patch first.
>
> - QAPI 'namespaces' aren't yet handled. i.e., there's nothing to
>   distinguish entities between QMP, QGA and QSD yet. That feature will
>   be added to a future version of this patchset (Likely when the
>   generator is ready for inclusion: without it, references will clash
>   and the index will gripe about duplicated entries.)

qemu-storage-daemon's QMP is a proper subset of qemu-system-FOO's.
Regardless, each of them has its own, independent reference manual.
That's defensible.

But the way we build them can complicate matters.  For instance, when I
tried to elide types not used for QMP from the reference manuals, I got
defeated by Sphinx caching.

> - Per-member features and ifcond are not yet accounted for; though
>   definition-scoped features and ifconds are. Please feel free to
>   suggest how you'd like to see those represented.
>
> - Inlining all members means there is some ambiguity on what to do with
>   doc block sections on inlined entities; features and members have an
>   obvious home - body, since, and misc sections are not as obvious on
>   how to handle. This will feature more prominently in the generator
>   series.

Yes, this is a real problem.

The member documentation gets written in the context of the type.  It
may make sense only in that context.  Inlining copies it into a
different context.

Features may need to combine.  Say a command's arguments are a union
type, and several branches of the union both contain deprecated members.
These branch types all document feature @deprecated.  Simply inlining
their feature documentation results in feature @deprecated documented
several times.  Ugh.  Combining them would be better.  But how?  Do we
need to rethink how we document features?

> - Some features could be implemented in either the QAPI domain syntax
>   *or* the generator, or some combination of both. Depending on
>   aesthetic feedback, this may influence where those features should be
>   implemented.
>
> - The formatting and aesthetics of branches are a little up in the air,
>   see the qapi:union patch for more details.
>
> - It's late, maybe other things. Happy Friday!
>
> Hope you like it!

Looks promising!

Re: [PATCH 00/27] Add qapi-domain Sphinx extension

[Markus Armbruster]

Markus Armbruster <armbru@redhat.com> writes:

[...]

>> The purpose of sending this series in its current form is largely to
>> solicit feedback on general aesthetics, layout, and features. Sphinx is
>> a wily beast, and feedback at this stage will dictate how and where
>> certain features are implemented.
>
> I'd appreciate help with that.  Opinions?

Less than clear, let me try again: I'm soliciting opinions on the new
look.  Check it out...

[...]

>> This RFC series includes a "sandbox" .rst document that showcases the
>> features of this domain by writing QAPI directives by hand; this
>> document will be removed from the series before final inclusion. It's
>> here to serve as a convenient test-bed for y'all to give feedback.

... here:

>> All that said, here's the sandbox document fully rendered:
>> https://jsnow.gitlab.io/qemu/qapi/index.html
>>
>> And here's the new QAPI index page created by that sandbox document:
>> https://jsnow.gitlab.io/qemu/qapi-index.html

[...]

Re: [PATCH 00/27] Add qapi-domain Sphinx extension

[John Snow]

[-- Attachment #1: Type: text/plain, Size: 10664 bytes --]

On Fri, Apr 19, 2024, 10:45 AM Markus Armbruster <armbru@redhat.com> wrote:

> John Snow <jsnow@redhat.com> writes:
>
> > This series adds a new qapi-domain extension for Sphinx, which adds a
> > series of custom directives for documenting QAPI definitions.
> >
> > GitLab CI: https://gitlab.com/jsnow/qemu/-/pipelines/1259566476
> >
> > (Link to a demo HTML page at the end of this cover letter, but I want
> > you to read the cover letter first to explain what you're seeing.)
> >
> > This adds a new QAPI Index page, cross-references for QMP commands,
> > events, and data types, and improves the aesthetics of the QAPI/QMP
> > documentation.
>
> Cross-references alone will be a *massive* improvement!  I'm sure
> readers will appreciate better looks and an index, too.
>
> > This series adds only the new ReST syntax, *not* the autogenerator. The
> > ReST syntax used in this series is, in general, not intended for anyone
> > to actually write by hand. This mimics how Sphinx's own autodoc
> > extension generates Python domain directives, which are then re-parsed
> > to produce the final result.
> >
> > I have prototyped such a generator, but it isn't ready for inclusion
> > yet. (Rest assured: error context reporting is preserved down to the
> > line, even in generated ReST. There is no loss in usability for this
> > approach. It will likely either supplant qapidoc.py or heavily alter
> > it.) The generator requires only extremely minor changes to
> > scripts/qapi/parser.py to preserve nested indentation and provide more
> > accurate line information. It is less invasive than you may
> > fear. Relying on a secondary ReST parse phase eliminates much of the
> > complexity of qapidoc.py. Sleep soundly.
>
> I'm a Sphinx noob.  Let me paraphrase you to make sure I understand.
>
> You proprose to generate formatted documentation in two steps:
>
> • First, the QAPI generator generates .rst from the QAPI schema.  The
>   generated .rst makes use of a custom directives.
>

Yes, but this .rst file is built in-memory and never makes it to disk, like
Sphinx's autodoc for Python.

(We can add a debug knob to log it or save it out to disk if needed.)


> • Second, Sphinx turns the .rst into formatted documentation.  A Sphinx
>   qapi-domain extension implements the custom directives
>

Yes.


> This mirrors how Sphinx works for Python docs.  Which is its original
> use case.
>
> Your series demonstrates the second step, with test input you wrote
> manually.
>
> You have code for the first step, but you'd prefer to show it later.
>

Right, it's not fully finished, although I have events, commands, and
objects working. Unions, Alternates and Events need work.


> Fair?
>

Bingo!


> > The purpose of sending this series in its current form is largely to
> > solicit feedback on general aesthetics, layout, and features. Sphinx is
> > a wily beast, and feedback at this stage will dictate how and where
> > certain features are implemented.
>
> I'd appreciate help with that.  Opinions?


> > A goal for this syntax (and the generator) is to fully in-line all
> > command/event/object members, inherited or local, boxed or not, branched
> > or not. This should provide a boon to using these docs as a reference,
> > because users will not have to grep around the page looking for various
> > types, branches, or inherited members. Any arguments types will be
> > hyperlinked to their definition, further aiding usability. Commands can
> > be hotlinked from anywhere else in the manual, and will provide a
> > complete reference directly on the first screenful of information.
>
> Let me elaborate a bit here.
>
> A command's arguments can be specified inline, like so:
>
>     { 'command': 'job-cancel', 'data': { 'id': 'str' } }
>
> The arguments are then documented right with the command.
>
> But they can also be specified by referencing an object type, like so:
>
>     { 'command': 'block-dirty-bitmap-remove',
>       'data': 'BlockDirtyBitmap' }
>
> Reasons for doing it this way:
>
> • Several commands take the same arguments, and you don't want to repeat
>   yourself.
>
> • You want generated C take a single struct argument ('boxed': true).
>
> • The arguments are a union (which requires 'boxed': true).
>
> Drawback: the arguments are then documented elsewhere.  Not nice.
>
> Bug: the generated documentation fails to point there.
>
> You're proposing to inline the argument documentation, so it appears
> right with the command.
>
> An event's data is just like a command's argument.
>
> A command's return value can only specified by referencing a type.  Same
> doc usability issue.
>
> Similarly, a union type's base can specified inline or by referencing a
> struct type, and a union's branches must be specified by referencing a
> struct type.  Same doc usability issue.
>
> At least, the generated documentation does point to the referenced
> types.
>

Right. My proposal is to recursively inline referenced bases for the
top-level members so that this manual is useful as a user reference,
without worrying about the actual QAPI structure.

Return types will just be hotlinked.


> > (Okay, maybe two screenfuls for commands with a ton of
> > arguments... There's only so much I can do!)
>
> *cough* blockdev-add *cough*


> > This RFC series includes a "sandbox" .rst document that showcases the
> > features of this domain by writing QAPI directives by hand; this
> > document will be removed from the series before final inclusion. It's
> > here to serve as a convenient test-bed for y'all to give feedback.
> >
> > All that said, here's the sandbox document fully rendered:
> > https://jsnow.gitlab.io/qemu/qapi/index.html
> >
> > And here's the new QAPI index page created by that sandbox document:
> > https://jsnow.gitlab.io/qemu/qapi-index.html
> >
> > Known issues / points of interest:
> >
> > - The formatting upsets checkpatch. The default line length for the
> >   "black" formatting tool is a little long. I'll fix it next spin.
> >
> > - I did my best to preserve cross-version compatibility, but some
> >   problems have crept in here and there. This series may require
> >   Sphinx>= 4.0, like the dbus extension. Notably, the Ubuntu build fails
> >   on Gitlab CI currently. The next version will text against more Sphinx
> >   versions more rigorously. Sphinx version 5.3.0 and above should work
> >   perfectly.
> >
> > - There's a bug in Sphinx itself that may manifest in your testing,
> >   concerning reported error locations. There's a patch in this series
> >   that fixes it, but it's later in the series. If you run into the bug
> >   while testing with this series, try applying that patch first.
> >
> > - QAPI 'namespaces' aren't yet handled. i.e., there's nothing to
> >   distinguish entities between QMP, QGA and QSD yet. That feature will
> >   be added to a future version of this patchset (Likely when the
> >   generator is ready for inclusion: without it, references will clash
> >   and the index will gripe about duplicated entries.)
>
> qemu-storage-daemon's QMP is a proper subset of qemu-system-FOO's.
> Regardless, each of them has its own, independent reference manual.
> That's defensible.
>
> But the way we build them can complicate matters.  For instance, when I
> tried to elide types not used for QMP from the reference manuals, I got
> defeated by Sphinx caching.
>

I haven't tried yet, but I believe it should be possible to "tag" each
referenced QAPI object and mark it as "visited". Each namespaced definition
would be tagged separately.

(i.e. qemu.block-core.blockdev-backup and qsd.block-core.blockdev-backup
would be two distinct entities.)

Then, the renderer could ignore/squelch untagged entities. (I think.)

Maybe some work to do to un-index unvisited entities.

Or we can go the other route and bake this info into the schema and squelch
stuff earlier. We can add this feature later. I am still not sure why your
prototype for this ran into cache issues, but I am convinced it should be
fixable by making namespaced entities distinct.

We could perhaps bake the namespace into the qapidoc directive, e.g.:

 .. qapi:autodoc:: schema.json
   :namespace: QSD

(And the default adds no namespace.)


> > - Per-member features and ifcond are not yet accounted for; though
> >   definition-scoped features and ifconds are. Please feel free to
> >   suggest how you'd like to see those represented.
> >
> > - Inlining all members means there is some ambiguity on what to do with
> >   doc block sections on inlined entities; features and members have an
> >   obvious home - body, since, and misc sections are not as obvious on
> >   how to handle. This will feature more prominently in the generator
> >   series.
>
> Yes, this is a real problem.
>
> The member documentation gets written in the context of the type.  It
> may make sense only in that context.  Inlining copies it into a
> different context.


> Features may need to combine.  Say a command's arguments are a union
> type, and several branches of the union both contain deprecated members.
> These branch types all document feature @deprecated.  Simply inlining
> their feature documentation results in feature @deprecated documented
> several times.  Ugh.  Combining them would be better.  But how?  Do we
> need to rethink how we document features?
>

To be honest, I figured I'd plow ahead and then find the counter-examples
programmatically and decide what to do once I had my pile of edge cases.

It might involve rewriting docs in some places, but I think the usability
win is completely worth the hassle.

It's possible there might be some rework needed to maximize cogency of the
generated docs, but I think prioritizing a user-facing reference for QMP is
the objectively correct end goal and I'm more than happy to work backwards
from that desired state.


> > - Some features could be implemented in either the QAPI domain syntax
> >   *or* the generator, or some combination of both. Depending on
> >   aesthetic feedback, this may influence where those features should be
> >   implemented.
> >
> > - The formatting and aesthetics of branches are a little up in the air,
> >   see the qapi:union patch for more details.
> >
> > - It's late, maybe other things. Happy Friday!
> >
> > Hope you like it!
>
> Looks promising!
>

Fingers crossed. This has bothered me about QEMU for years.

--js

>

[-- Attachment #2: Type: text/html, Size: 15333 bytes --]

Re: [PATCH 24/27] docs/qapi-domain: add type cross-refs to field lists

[John Snow]

On Fri, Apr 19, 2024 at 12:38 AM John Snow <jsnow@redhat.com> wrote:
>
> This commit, finally, adds cross-referencing support to various field
> lists; modeled tightly after Sphinx's own Python domain code.
>
> Cross-referencing support is added to type names provided to :arg:,
> :memb:, :returns: and :choice:.
>
> :feat:, :error: and :value:, which do not take type names, do not
> support this syntax.
>
> The general syntax is simple:
>
> :arg TypeName ArgName: Lorem Ipsum ...
>
> The domain will transform TypeName into :qapi:type:`TypeName` in this
> basic case, and also apply the ``literal`` decoration to indicate that
> this is a type cross-reference.
>
> For Optional arguments, the special "?" suffix is used. Because "*" has
> special meaning in ReST that would cause parsing errors, we elect to use
> "?" instead. The special syntax processing in QAPIXrefMixin strips this
> character from the end of any type name argument and will append ",
> Optional" to the rendered output, applying the cross-reference only to
> the actual type name.
>
> The intent here is that the actual syntax in doc-blocks need not change;
> but e.g. qapidoc.py will need to process and transform "@arg foo lorem
> ipsum" into ":arg type? foo: lorem ipsum" based on the schema
> information. Therefore, nobody should ever actually witness this
> intermediate syntax unless they are writing manual documentation or the
> doc transmogrifier breaks.
>
> For array arguments, type names can similarly be surrounded by "[]",
> which are stripped off and then re-appended outside of the
> cross-reference.
>
> Note: The mixin pattern here (borrowed from Sphinx) confuses mypy
> because it cannot tell that it will be mixed into a descendent of
> Field. Doing that instead causes more errors, because many versions of
> Sphinx erroneously did not mark various arguments as Optional, so we're
> a bit hosed either way. Do the simpler thing.
>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  docs/qapi/index.rst        |  34 ++++++++++++
>  docs/sphinx/qapi-domain.py | 110 +++++++++++++++++++++++++++++++++++--
>  2 files changed, 138 insertions(+), 6 deletions(-)
>
> diff --git a/docs/qapi/index.rst b/docs/qapi/index.rst
> index 8352a27d4a5..6e85ea5280d 100644
> --- a/docs/qapi/index.rst
> +++ b/docs/qapi/index.rst
> @@ -105,6 +105,11 @@ Explicit cross-referencing syntax for QAPI modules is available with
>     :arg str bar: Another normal parameter description.
>     :arg baz: Missing a type.
>     :arg no-descr:
> +   :arg int? oof: Testing optional argument parsing.
> +   :arg [XDbgBlockGraphNode] rab: Testing array argument parsing.
> +   :arg [BitmapSyncMode]? zab: Testing optional array argument parsing,
> +      even though Markus said this should never happen. I believe him,
> +      but I didn't *forbid* the syntax either.
>     :arg BitmapSyncMode discrim: How about branches in commands?
>
>     .. qapi:branch:: discrim on-success
> @@ -261,3 +266,32 @@ Explicit cross-referencing syntax for QAPI modules is available with
>
>        :memb str key-secret: ID of a QCryptoSecret object providing a
>           passphrase for unlocking the encryption
> +
> +.. qapi:command:: x-debug-query-block-graph
> +   :since: 4.0
> +   :unstable:
> +
> +   Get the block graph.
> +
> +   :feat unstable: This command is meant for debugging.
> +   :return XDbgBlockGraph: lorem ipsum ...
> +
> +.. qapi:struct:: XDbgBlockGraph
> +   :since: 4.0
> +
> +   Block Graph - list of nodes and list of edges.
> +
> +   :memb [XDbgBlockGraphNode] nodes:
> +   :memb [XDbgBlockGraphEdge] edges:
> +
> +.. qapi:struct:: XDbgBlockGraphNode
> +   :since: 4.0
> +
> +   :memb uint64 id: Block graph node identifier.  This @id is generated only for
> +      x-debug-query-block-graph and does not relate to any other
> +      identifiers in Qemu.
> +   :memb XDbgBlockGraphNodeType type: Type of graph node.  Can be one of
> +      block-backend, block-job or block-driver-state.
> +   :memb str name: Human readable name of the node.  Corresponds to
> +      node-name for block-driver-state nodes; is not guaranteed to be
> +      unique in the whole graph (with block-jobs and block-backends).
> diff --git a/docs/sphinx/qapi-domain.py b/docs/sphinx/qapi-domain.py
> index bf8bb933345..074453193ce 100644
> --- a/docs/sphinx/qapi-domain.py
> +++ b/docs/sphinx/qapi-domain.py
> @@ -50,11 +50,12 @@
>
>  if TYPE_CHECKING:
>      from docutils.nodes import Element, Node
> +    from docutils.parsers.rst.states import Inliner
>
>      from sphinx.application import Sphinx
>      from sphinx.builders import Builder
>      from sphinx.environment import BuildEnvironment
> -    from sphinx.util.typing import OptionSpec
> +    from sphinx.util.typing import OptionSpec, TextlikeNode
>
>  logger = logging.getLogger(__name__)
>
> @@ -68,6 +69,90 @@ class ObjectEntry(NamedTuple):
>      aliased: bool
>
>
> +class QAPIXrefMixin:
> +    def make_xref(
> +        self,
> +        rolename: str,
> +        domain: str,
> +        target: str,
> +        innernode: type[TextlikeNode] = nodes.literal,
> +        contnode: Optional[Node] = None,
> +        env: Optional[BuildEnvironment] = None,
> +        inliner: Optional[Inliner] = None,
> +        location: Optional[Node] = None,
> +    ) -> Node:
> +        result = super().make_xref(  # type: ignore[misc]
> +            rolename,
> +            domain,
> +            target,
> +            innernode=innernode,
> +            contnode=contnode,
> +            env=env,
> +            inliner=None,
> +            location=None,
> +        )
> +        if isinstance(result, pending_xref):
> +            assert env is not None
> +            result["refspecific"] = True
> +            result["qapi:module"] = env.ref_context.get("qapi:module")
> +
> +        assert isinstance(result, Node)

A bug snuck in because I made edits after I published to GitLab; this
line should be:

assert isinstance(result, nodes.Node)

> +        return result
> +
> +    def make_xrefs(
> +        self,
> +        rolename: str,
> +        domain: str,
> +        target: str,
> +        innernode: type[TextlikeNode] = nodes.literal,
> +        contnode: Optional[Node] = None,
> +        env: Optional[BuildEnvironment] = None,
> +        inliner: Optional[Inliner] = None,
> +        location: Optional[Node] = None,
> +    ) -> list[Node]:
> +        # Note: this function is called on up to three fields of text:
> +        # (1) The field name argument (e.g. member/arg name)
> +        # (2) The field name type (e.g. member/arg type)
> +        # (3) The field *body* text, for Fields that do not take arguments.
> +
> +        list_type = False
> +        optional = False
> +
> +        # If the rolename is qapi:type, we know we are processing a type
> +        # and not an arg/memb name or field body text.
> +        if rolename == "type":
> +            # force the innernode class to be a literal.
> +            innernode = nodes.literal
> +
> +            # Type names that end with "?" are considered Optional
> +            # arguments and should be documented as such, but it's not
> +            # part of the xref itself.
> +            if target.endswith("?"):
> +                optional = True
> +                target = target[:-1]
> +
> +            # Type names wrapped in brackets denote lists. strip the
> +            # brackets and remember to add them back later.
> +            if target.startswith("[") and target.endswith("]"):
> +                list_type = True
> +                target = target[1:-1]
> +
> +        results = []
> +        result = self.make_xref(
> +            rolename, domain, target, innernode, contnode, env, inliner, location
> +        )
> +        results.append(result)
> +
> +        if list_type:
> +            results.insert(0, nodes.literal("[", "["))
> +            results.append(nodes.literal("]", "]"))
> +        if optional:
> +            results.append(nodes.Text(", "))
> +            results.append(nodes.emphasis("?", "Optional"))
> +
> +        return results
> +
> +
>  class QAPIXRefRole(XRefRole):
>      def process_link(
>          self,
> @@ -96,6 +181,14 @@ def process_link(
>          return title, target
>
>
> +class QAPIGroupedField(QAPIXrefMixin, GroupedField):
> +    pass
> +
> +
> +class QAPITypedField(QAPIXrefMixin, TypedField):
> +    pass
> +
> +
>  def since_validator(param: str) -> str:
>      """
>      Validate the `:since: X.Y` option field.
> @@ -416,10 +509,11 @@ class QAPICommand(QAPIObject):
>      doc_field_types = QAPIObject.doc_field_types.copy()
>      doc_field_types.extend(
>          [
> -            TypedField(
> +            QAPITypedField(
>                  "argument",
>                  label=_("Arguments"),
>                  names=("arg",),
> +                typerolename="type",
>                  can_collapse=True,
>              ),
>              GroupedField(
> @@ -428,9 +522,10 @@ class QAPICommand(QAPIObject):
>                  names=("error",),
>                  can_collapse=True,
>              ),
> -            GroupedField(
> +            QAPIGroupedField(
>                  "returnvalue",
>                  label=_("Returns"),
> +                rolename="type",
>                  names=("return", "returns"),
>                  can_collapse=True,
>              ),
> @@ -460,10 +555,11 @@ class QAPIAlternate(QAPIObject):
>      doc_field_types = QAPIObject.doc_field_types.copy()
>      doc_field_types.extend(
>          [
> -            TypedField(
> +            QAPITypedField(
>                  "choice",
>                  label=_("Choices"),
>                  names=("choice",),
> +                typerolename="type",
>                  can_collapse=True,
>              ),
>          ]
> @@ -476,10 +572,11 @@ class QAPIObjectWithMembers(QAPIObject):
>      doc_field_types = QAPIObject.doc_field_types.copy()
>      doc_field_types.extend(
>          [
> -            TypedField(
> +            QAPITypedField(
>                  "member",
>                  label=_("Members"),
>                  names=("memb",),
> +                typerolename="type",
>                  can_collapse=True,
>              ),
>          ]
> @@ -629,12 +726,13 @@ def run(self) -> list[Node]:
>          # of per-class to incorporate the branch conditions as a label
>          # name.
>          self.doc_field_types = [
> -            TypedField(
> +            QAPITypedField(
>                  "branch-arg-or-memb",
>                  label=f"[{discrim} = {value}]",
>                  # In a branch, we don't actually use the name of the
>                  # field name to generate the label; so allow either-or.
>                  names=("arg", "memb"),
> +                typerolename="type",
>              ),
>          ]
>
> --
> 2.44.0
>

Re: [PATCH 00/27] Add qapi-domain Sphinx extension

[Markus Armbruster]

John Snow <jsnow@redhat.com> writes:

> On Fri, Apr 19, 2024, 10:45 AM Markus Armbruster <armbru@redhat.com> wrote:
>
>> John Snow <jsnow@redhat.com> writes:
>>
>> > This series adds a new qapi-domain extension for Sphinx, which adds a
>> > series of custom directives for documenting QAPI definitions.
>> >
>> > GitLab CI: https://gitlab.com/jsnow/qemu/-/pipelines/1259566476
>> >
>> > (Link to a demo HTML page at the end of this cover letter, but I want
>> > you to read the cover letter first to explain what you're seeing.)
>> >
>> > This adds a new QAPI Index page, cross-references for QMP commands,
>> > events, and data types, and improves the aesthetics of the QAPI/QMP
>> > documentation.
>>
>> Cross-references alone will be a *massive* improvement!  I'm sure
>> readers will appreciate better looks and an index, too.
>>
>> > This series adds only the new ReST syntax, *not* the autogenerator. The
>> > ReST syntax used in this series is, in general, not intended for anyone
>> > to actually write by hand. This mimics how Sphinx's own autodoc
>> > extension generates Python domain directives, which are then re-parsed
>> > to produce the final result.
>> >
>> > I have prototyped such a generator, but it isn't ready for inclusion
>> > yet. (Rest assured: error context reporting is preserved down to the
>> > line, even in generated ReST. There is no loss in usability for this
>> > approach. It will likely either supplant qapidoc.py or heavily alter
>> > it.) The generator requires only extremely minor changes to
>> > scripts/qapi/parser.py to preserve nested indentation and provide more
>> > accurate line information. It is less invasive than you may
>> > fear. Relying on a secondary ReST parse phase eliminates much of the
>> > complexity of qapidoc.py. Sleep soundly.
>>
>> I'm a Sphinx noob.  Let me paraphrase you to make sure I understand.
>>
>> You proprose to generate formatted documentation in two steps:
>>
>> • First, the QAPI generator generates .rst from the QAPI schema.  The
>>   generated .rst makes use of a custom directives.
>>
>
> Yes, but this .rst file is built in-memory and never makes it to disk, like
> Sphinx's autodoc for Python.

Makes sense.

> (We can add a debug knob to log it or save it out to disk if needed.)

Likely useful at least occasionally.

>> • Second, Sphinx turns the .rst into formatted documentation.  A Sphinx
>>   qapi-domain extension implements the custom directives
>
> Yes.
>
>
>> This mirrors how Sphinx works for Python docs.  Which is its original
>> use case.
>>
>> Your series demonstrates the second step, with test input you wrote
>> manually.
>>
>> You have code for the first step, but you'd prefer to show it later.
>
> Right, it's not fully finished, although I have events, commands, and
> objects working. Unions, Alternates and Events need work.
>
>
>> Fair?
>
> Bingo!

Thanks!

>> > The purpose of sending this series in its current form is largely to
>> > solicit feedback on general aesthetics, layout, and features. Sphinx is
>> > a wily beast, and feedback at this stage will dictate how and where
>> > certain features are implemented.
>>
>> I'd appreciate help with that.  Opinions?
>
>
>> > A goal for this syntax (and the generator) is to fully in-line all
>> > command/event/object members, inherited or local, boxed or not, branched
>> > or not. This should provide a boon to using these docs as a reference,
>> > because users will not have to grep around the page looking for various
>> > types, branches, or inherited members. Any arguments types will be
>> > hyperlinked to their definition, further aiding usability. Commands can
>> > be hotlinked from anywhere else in the manual, and will provide a
>> > complete reference directly on the first screenful of information.
>>
>> Let me elaborate a bit here.
>>
>> A command's arguments can be specified inline, like so:
>>
>>     { 'command': 'job-cancel', 'data': { 'id': 'str' } }
>>
>> The arguments are then documented right with the command.
>>
>> But they can also be specified by referencing an object type, like so:
>>
>>     { 'command': 'block-dirty-bitmap-remove',
>>       'data': 'BlockDirtyBitmap' }
>>
>> Reasons for doing it this way:
>>
>> • Several commands take the same arguments, and you don't want to repeat
>>   yourself.
>>
>> • You want generated C take a single struct argument ('boxed': true).
>>
>> • The arguments are a union (which requires 'boxed': true).
>>
>> Drawback: the arguments are then documented elsewhere.  Not nice.
>>
>> Bug: the generated documentation fails to point there.
>>
>> You're proposing to inline the argument documentation, so it appears
>> right with the command.
>>
>> An event's data is just like a command's argument.
>>
>> A command's return value can only specified by referencing a type.  Same
>> doc usability issue.
>>
>> Similarly, a union type's base can specified inline or by referencing a
>> struct type, and a union's branches must be specified by referencing a
>> struct type.  Same doc usability issue.
>>
>> At least, the generated documentation does point to the referenced
>> types.
>>
>
> Right. My proposal is to recursively inline referenced bases for the
> top-level members so that this manual is useful as a user reference,
> without worrying about the actual QAPI structure.
>
> Return types will just be hotlinked.

The argument for inlining the arguments equally applies to results:
"users will not have to grep around the page".

102 out of 236 commands return something, usually some object type or an
array of some object type.  Most of these types are used for exactly one
command's return and nothing else.

Regardless, it's fine to explore inlining just for arguments.  If we can
make that work, extending it to return results shouldn't be too hard.

>> > (Okay, maybe two screenfuls for commands with a ton of
>> > arguments... There's only so much I can do!)
>>
>> *cough* blockdev-add *cough*
>
>
>> > This RFC series includes a "sandbox" .rst document that showcases the
>> > features of this domain by writing QAPI directives by hand; this
>> > document will be removed from the series before final inclusion. It's
>> > here to serve as a convenient test-bed for y'all to give feedback.
>> >
>> > All that said, here's the sandbox document fully rendered:
>> > https://jsnow.gitlab.io/qemu/qapi/index.html
>> >
>> > And here's the new QAPI index page created by that sandbox document:
>> > https://jsnow.gitlab.io/qemu/qapi-index.html
>> >
>> > Known issues / points of interest:
>> >
>> > - The formatting upsets checkpatch. The default line length for the
>> >   "black" formatting tool is a little long. I'll fix it next spin.
>> >
>> > - I did my best to preserve cross-version compatibility, but some
>> >   problems have crept in here and there. This series may require
>> >   Sphinx>= 4.0, like the dbus extension. Notably, the Ubuntu build fails
>> >   on Gitlab CI currently. The next version will text against more Sphinx
>> >   versions more rigorously. Sphinx version 5.3.0 and above should work
>> >   perfectly.
>> >
>> > - There's a bug in Sphinx itself that may manifest in your testing,
>> >   concerning reported error locations. There's a patch in this series
>> >   that fixes it, but it's later in the series. If you run into the bug
>> >   while testing with this series, try applying that patch first.
>> >
>> > - QAPI 'namespaces' aren't yet handled. i.e., there's nothing to
>> >   distinguish entities between QMP, QGA and QSD yet. That feature will
>> >   be added to a future version of this patchset (Likely when the
>> >   generator is ready for inclusion: without it, references will clash
>> >   and the index will gripe about duplicated entries.)
>>
>> qemu-storage-daemon's QMP is a proper subset of qemu-system-FOO's.
>> Regardless, each of them has its own, independent reference manual.
>> That's defensible.
>>
>> But the way we build them can complicate matters.  For instance, when I
>> tried to elide types not used for QMP from the reference manuals, I got
>> defeated by Sphinx caching.
>
> I haven't tried yet, but I believe it should be possible to "tag" each
> referenced QAPI object and mark it as "visited". Each namespaced definition
> would be tagged separately.
>
> (i.e. qemu.block-core.blockdev-backup and qsd.block-core.blockdev-backup
> would be two distinct entities.)
>
> Then, the renderer could ignore/squelch untagged entities. (I think.)
>
> Maybe some work to do to un-index unvisited entities.
>
> Or we can go the other route and bake this info into the schema and squelch
> stuff earlier. We can add this feature later. I am still not sure why your
> prototype for this ran into cache issues, but I am convinced it should be
> fixable by making namespaced entities distinct.

From (hazy) memory: when full QMP and storage daemon QMP both include a
sub-module for its types, but full QMP uses more of its types than
storage daemon QMP does, the storage daemon manual elides more of its
types than the QMP manual does.  However, sphinx-build's caching uses
whatever got built first for *both* manuals.  When we happen to build
the full QMP manual first, we end up with extra types in the storage
daemon manual.  When we happen to build the storage daemon manual first,
the full manual is missing types.

> We could perhaps bake the namespace into the qapidoc directive, e.g.:
>
>  .. qapi:autodoc:: schema.json
>    :namespace: QSD
>
> (And the default adds no namespace.)

Is it worth it?  How useful is the separate QEMU Storage Daemon QMP
Reference Manual?  It's an exact duplicate of selected chapters of the
QEMU QMP Reference Manual...  Could we instead document "QMP, but only
these chapters"?

Diversion: can we come up with a better way of subsetting the full QAPI
schema for the storage daemon?

We currently subset by having storage-daemon/qapi/qapi-schema.json a
subset of the submodules qapi/qapi-schema.json includes.  The code
generated for the entire schema differs (basically qapi-init-commands.c,
qapi-emit-events.[ch], and qapi-introspect.c).  The code generated for
the submodules should be identical (modulo unused array types, but
that's harmless detail).  So we ignore the code generated for the
storage daemon's submodules.

End of diversion.

>> > - Per-member features and ifcond are not yet accounted for; though
>> >   definition-scoped features and ifconds are. Please feel free to
>> >   suggest how you'd like to see those represented.
>> >
>> > - Inlining all members means there is some ambiguity on what to do with
>> >   doc block sections on inlined entities; features and members have an
>> >   obvious home - body, since, and misc sections are not as obvious on
>> >   how to handle. This will feature more prominently in the generator
>> >   series.
>>
>> Yes, this is a real problem.
>>
>> The member documentation gets written in the context of the type.  It
>> may make sense only in that context.  Inlining copies it into a
>> different context.
>>
>> Features may need to combine.  Say a command's arguments are a union
>> type, and several branches of the union both contain deprecated members.
>> These branch types all document feature @deprecated.  Simply inlining
>> their feature documentation results in feature @deprecated documented
>> several times.  Ugh.  Combining them would be better.  But how?  Do we
>> need to rethink how we document features?
>
> To be honest, I figured I'd plow ahead and then find the counter-examples
> programmatically and decide what to do once I had my pile of edge cases.
>
> It might involve rewriting docs in some places, but I think the usability
> win is completely worth the hassle.
>
> It's possible there might be some rework needed to maximize cogency of the
> generated docs, but I think prioritizing a user-facing reference for QMP is
> the objectively correct end goal and I'm more than happy to work backwards
> from that desired state.

I'm not disputing the idea that documenting the arguments right with the
command is good.  I'm merely pointing out obstacles to pulling that off.

Adjusting existing documentation is only half the battle.  The other
half is making sure documentation stays adjusted.  We may have to come
up with new documentation rules, and ways to enforce them.

>> > - Some features could be implemented in either the QAPI domain syntax
>> >   *or* the generator, or some combination of both. Depending on
>> >   aesthetic feedback, this may influence where those features should be
>> >   implemented.
>> >
>> > - The formatting and aesthetics of branches are a little up in the air,
>> >   see the qapi:union patch for more details.
>> >
>> > - It's late, maybe other things. Happy Friday!
>> >
>> > Hope you like it!
>>
>> Looks promising!
>
> Fingers crossed. This has bothered me about QEMU for years.

Thanks!

Re: [PATCH 00/27] Add qapi-domain Sphinx extension

[John Snow]

On Mon, Apr 22, 2024 at 5:20 AM Markus Armbruster <armbru@redhat.com> wrote:
>
> John Snow <jsnow@redhat.com> writes:
>
> > On Fri, Apr 19, 2024, 10:45 AM Markus Armbruster <armbru@redhat.com> wrote:
> >
> >> John Snow <jsnow@redhat.com> writes:
> >>
> >> > This series adds a new qapi-domain extension for Sphinx, which adds a
> >> > series of custom directives for documenting QAPI definitions.
> >> >
> >> > GitLab CI: https://gitlab.com/jsnow/qemu/-/pipelines/1259566476
> >> >
> >> > (Link to a demo HTML page at the end of this cover letter, but I want
> >> > you to read the cover letter first to explain what you're seeing.)
> >> >
> >> > This adds a new QAPI Index page, cross-references for QMP commands,
> >> > events, and data types, and improves the aesthetics of the QAPI/QMP
> >> > documentation.
> >>
> >> Cross-references alone will be a *massive* improvement!  I'm sure
> >> readers will appreciate better looks and an index, too.
> >>
> >> > This series adds only the new ReST syntax, *not* the autogenerator. The
> >> > ReST syntax used in this series is, in general, not intended for anyone
> >> > to actually write by hand. This mimics how Sphinx's own autodoc
> >> > extension generates Python domain directives, which are then re-parsed
> >> > to produce the final result.
> >> >
> >> > I have prototyped such a generator, but it isn't ready for inclusion
> >> > yet. (Rest assured: error context reporting is preserved down to the
> >> > line, even in generated ReST. There is no loss in usability for this
> >> > approach. It will likely either supplant qapidoc.py or heavily alter
> >> > it.) The generator requires only extremely minor changes to
> >> > scripts/qapi/parser.py to preserve nested indentation and provide more
> >> > accurate line information. It is less invasive than you may
> >> > fear. Relying on a secondary ReST parse phase eliminates much of the
> >> > complexity of qapidoc.py. Sleep soundly.
> >>
> >> I'm a Sphinx noob.  Let me paraphrase you to make sure I understand.
> >>
> >> You proprose to generate formatted documentation in two steps:
> >>
> >> • First, the QAPI generator generates .rst from the QAPI schema.  The
> >>   generated .rst makes use of a custom directives.
> >>
> >
> > Yes, but this .rst file is built in-memory and never makes it to disk, like
> > Sphinx's autodoc for Python.
>
> Makes sense.
>
> > (We can add a debug knob to log it or save it out to disk if needed.)
>
> Likely useful at least occasionally.

Yep, python's autodoc has such a knob to use the debugging log for
this. I just want to point out that avoiding the intermediate file
on-disk is actually the mechanism by which I can preserve source
lines, so this is how it's gotta be.

I build an intermediate doc in-memory with source filenames and source
lines along with the modified doc block content so that ReST errors
can be tracked back directly to the QAPI json files. If we saved to
disk and parsed that, it'd obliterate that information.

>
> >> • Second, Sphinx turns the .rst into formatted documentation.  A Sphinx
> >>   qapi-domain extension implements the custom directives
> >
> > Yes.
> >
> >
> >> This mirrors how Sphinx works for Python docs.  Which is its original
> >> use case.
> >>
> >> Your series demonstrates the second step, with test input you wrote
> >> manually.
> >>
> >> You have code for the first step, but you'd prefer to show it later.
> >
> > Right, it's not fully finished, although I have events, commands, and
> > objects working. Unions, Alternates and Events need work.
> >
> >
> >> Fair?
> >
> > Bingo!
>
> Thanks!
>
> >> > The purpose of sending this series in its current form is largely to
> >> > solicit feedback on general aesthetics, layout, and features. Sphinx is
> >> > a wily beast, and feedback at this stage will dictate how and where
> >> > certain features are implemented.
> >>
> >> I'd appreciate help with that.  Opinions?
> >
> >
> >> > A goal for this syntax (and the generator) is to fully in-line all
> >> > command/event/object members, inherited or local, boxed or not, branched
> >> > or not. This should provide a boon to using these docs as a reference,
> >> > because users will not have to grep around the page looking for various
> >> > types, branches, or inherited members. Any arguments types will be
> >> > hyperlinked to their definition, further aiding usability. Commands can
> >> > be hotlinked from anywhere else in the manual, and will provide a
> >> > complete reference directly on the first screenful of information.
> >>
> >> Let me elaborate a bit here.
> >>
> >> A command's arguments can be specified inline, like so:
> >>
> >>     { 'command': 'job-cancel', 'data': { 'id': 'str' } }
> >>
> >> The arguments are then documented right with the command.
> >>
> >> But they can also be specified by referencing an object type, like so:
> >>
> >>     { 'command': 'block-dirty-bitmap-remove',
> >>       'data': 'BlockDirtyBitmap' }
> >>
> >> Reasons for doing it this way:
> >>
> >> • Several commands take the same arguments, and you don't want to repeat
> >>   yourself.
> >>
> >> • You want generated C take a single struct argument ('boxed': true).
> >>
> >> • The arguments are a union (which requires 'boxed': true).
> >>
> >> Drawback: the arguments are then documented elsewhere.  Not nice.
> >>
> >> Bug: the generated documentation fails to point there.
> >>
> >> You're proposing to inline the argument documentation, so it appears
> >> right with the command.
> >>
> >> An event's data is just like a command's argument.
> >>
> >> A command's return value can only specified by referencing a type.  Same
> >> doc usability issue.
> >>
> >> Similarly, a union type's base can specified inline or by referencing a
> >> struct type, and a union's branches must be specified by referencing a
> >> struct type.  Same doc usability issue.
> >>
> >> At least, the generated documentation does point to the referenced
> >> types.
> >>
> >
> > Right. My proposal is to recursively inline referenced bases for the
> > top-level members so that this manual is useful as a user reference,
> > without worrying about the actual QAPI structure.
> >
> > Return types will just be hotlinked.
>
> The argument for inlining the arguments equally applies to results:
> "users will not have to grep around the page".
>
> 102 out of 236 commands return something, usually some object type or an
> array of some object type.  Most of these types are used for exactly one
> command's return and nothing else.
>
> Regardless, it's fine to explore inlining just for arguments.  If we can
> make that work, extending it to return results shouldn't be too hard.

Yeah. Future work, if we want it. Otherwise, I think it's not *too*
bad to hotlink to the return arg type; but if that's info that you
want to "hide" from the user API, I get it - we can work on squanching
that in too. (For a follow-up though, please.)

>
> >> > (Okay, maybe two screenfuls for commands with a ton of
> >> > arguments... There's only so much I can do!)
> >>
> >> *cough* blockdev-add *cough*
> >
> >
> >> > This RFC series includes a "sandbox" .rst document that showcases the
> >> > features of this domain by writing QAPI directives by hand; this
> >> > document will be removed from the series before final inclusion. It's
> >> > here to serve as a convenient test-bed for y'all to give feedback.
> >> >
> >> > All that said, here's the sandbox document fully rendered:
> >> > https://jsnow.gitlab.io/qemu/qapi/index.html
> >> >
> >> > And here's the new QAPI index page created by that sandbox document:
> >> > https://jsnow.gitlab.io/qemu/qapi-index.html
> >> >
> >> > Known issues / points of interest:
> >> >
> >> > - The formatting upsets checkpatch. The default line length for the
> >> >   "black" formatting tool is a little long. I'll fix it next spin.
> >> >
> >> > - I did my best to preserve cross-version compatibility, but some
> >> >   problems have crept in here and there. This series may require
> >> >   Sphinx>= 4.0, like the dbus extension. Notably, the Ubuntu build fails
> >> >   on Gitlab CI currently. The next version will text against more Sphinx
> >> >   versions more rigorously. Sphinx version 5.3.0 and above should work
> >> >   perfectly.
> >> >
> >> > - There's a bug in Sphinx itself that may manifest in your testing,
> >> >   concerning reported error locations. There's a patch in this series
> >> >   that fixes it, but it's later in the series. If you run into the bug
> >> >   while testing with this series, try applying that patch first.
> >> >
> >> > - QAPI 'namespaces' aren't yet handled. i.e., there's nothing to
> >> >   distinguish entities between QMP, QGA and QSD yet. That feature will
> >> >   be added to a future version of this patchset (Likely when the
> >> >   generator is ready for inclusion: without it, references will clash
> >> >   and the index will gripe about duplicated entries.)
> >>
> >> qemu-storage-daemon's QMP is a proper subset of qemu-system-FOO's.
> >> Regardless, each of them has its own, independent reference manual.
> >> That's defensible.
> >>
> >> But the way we build them can complicate matters.  For instance, when I
> >> tried to elide types not used for QMP from the reference manuals, I got
> >> defeated by Sphinx caching.
> >
> > I haven't tried yet, but I believe it should be possible to "tag" each
> > referenced QAPI object and mark it as "visited". Each namespaced definition
> > would be tagged separately.
> >
> > (i.e. qemu.block-core.blockdev-backup and qsd.block-core.blockdev-backup
> > would be two distinct entities.)
> >
> > Then, the renderer could ignore/squelch untagged entities. (I think.)
> >
> > Maybe some work to do to un-index unvisited entities.
> >
> > Or we can go the other route and bake this info into the schema and squelch
> > stuff earlier. We can add this feature later. I am still not sure why your
> > prototype for this ran into cache issues, but I am convinced it should be
> > fixable by making namespaced entities distinct.
>
> From (hazy) memory: when full QMP and storage daemon QMP both include a
> sub-module for its types, but full QMP uses more of its types than
> storage daemon QMP does, the storage daemon manual elides more of its
> types than the QMP manual does.  However, sphinx-build's caching uses
> whatever got built first for *both* manuals.  When we happen to build
> the full QMP manual first, we end up with extra types in the storage
> daemon manual.  When we happen to build the storage daemon manual first,
> the full manual is missing types.

Weird. I still don't understand where that info is getting cached, to
be honest ...

>
> > We could perhaps bake the namespace into the qapidoc directive, e.g.:
> >
> >  .. qapi:autodoc:: schema.json
> >    :namespace: QSD
> >
> > (And the default adds no namespace.)
>
> Is it worth it?  How useful is the separate QEMU Storage Daemon QMP
> Reference Manual?  It's an exact duplicate of selected chapters of the
> QEMU QMP Reference Manual...  Could we instead document "QMP, but only
> these chapters"?

I dunno. That's up to you and Kevin, I think? For now, I'm just
following "What we've got" but with cross-referencing and indexing
improvements. Those improvements require distinguishing QMP and QSD
somehow. Adding simple namespace support seems like the most flexible
*and* dead simple way to do it. Other improvements can come later if
desired, I think.

If you need help adding Sphinx features to make it happen, you know
where I live.

>
> Diversion: can we come up with a better way of subsetting the full QAPI
> schema for the storage daemon?

Out of scope for me I'm afraid, but clue me in to the discussion
if/when you have it.

>
> We currently subset by having storage-daemon/qapi/qapi-schema.json a
> subset of the submodules qapi/qapi-schema.json includes.  The code
> generated for the entire schema differs (basically qapi-init-commands.c,
> qapi-emit-events.[ch], and qapi-introspect.c).  The code generated for
> the submodules should be identical (modulo unused array types, but
> that's harmless detail).  So we ignore the code generated for the
> storage daemon's submodules.

Ah, so you are debating a type of pragma or other meta-include that
just fishes out specific definitions? That could work, I think - I'm
not sure how the QAPI Schema end would look, but the Sphinx domain
could provide a meta-directive for adding a QSD support tag to
specific entities or some such thing, and the single QMP reference
could advertise a qemu-storage-daemon support flag.

Or something - I'm not really privy to the big picture when it comes
to how we want to handle QMP modularization and differentiation
between different binaries.

>
> End of diversion.
>
> >> > - Per-member features and ifcond are not yet accounted for; though
> >> >   definition-scoped features and ifconds are. Please feel free to
> >> >   suggest how you'd like to see those represented.
> >> >
> >> > - Inlining all members means there is some ambiguity on what to do with
> >> >   doc block sections on inlined entities; features and members have an
> >> >   obvious home - body, since, and misc sections are not as obvious on
> >> >   how to handle. This will feature more prominently in the generator
> >> >   series.
> >>
> >> Yes, this is a real problem.
> >>
> >> The member documentation gets written in the context of the type.  It
> >> may make sense only in that context.  Inlining copies it into a
> >> different context.
> >>
> >> Features may need to combine.  Say a command's arguments are a union
> >> type, and several branches of the union both contain deprecated members.
> >> These branch types all document feature @deprecated.  Simply inlining
> >> their feature documentation results in feature @deprecated documented
> >> several times.  Ugh.  Combining them would be better.  But how?  Do we
> >> need to rethink how we document features?
> >
> > To be honest, I figured I'd plow ahead and then find the counter-examples
> > programmatically and decide what to do once I had my pile of edge cases.
> >
> > It might involve rewriting docs in some places, but I think the usability
> > win is completely worth the hassle.
> >
> > It's possible there might be some rework needed to maximize cogency of the
> > generated docs, but I think prioritizing a user-facing reference for QMP is
> > the objectively correct end goal and I'm more than happy to work backwards
> > from that desired state.
>
> I'm not disputing the idea that documenting the arguments right with the
> command is good.  I'm merely pointing out obstacles to pulling that off.
>
> Adjusting existing documentation is only half the battle.  The other
> half is making sure documentation stays adjusted.  We may have to come
> up with new documentation rules, and ways to enforce them.

For the sake of argument, let's say we forbid everything except
arg/features from definitions destined to be used as base/inherited
types. This would be very easy to enforce at the qapidoc level where
the doc inlining is performed by yelping when the base type contains
additional documentation sections.

Now, in the real world, maybe sometimes those sections are useful and
we don't want to get rid of them, esp. because they may contain useful
documentation that we don't want to duplicate in the source files.

My plan is to just forbid them at first and enumerate the cases where
they occur, then decide which ones are better off being moved
elsewhere or explicitly tolerated. The generator's tolerance can be
adjusted accordingly and we can formulate a rule for exactly how doc
blocks are combined and merged. I think it won't be a problem to
enforce it programmatically.

I'll get back to you on how often and precisely where these cases
occur so you can take a look and see how you feel.

>
> >> > - Some features could be implemented in either the QAPI domain syntax
> >> >   *or* the generator, or some combination of both. Depending on
> >> >   aesthetic feedback, this may influence where those features should be
> >> >   implemented.
> >> >
> >> > - The formatting and aesthetics of branches are a little up in the air,
> >> >   see the qapi:union patch for more details.
> >> >
> >> > - It's late, maybe other things. Happy Friday!
> >> >
> >> > Hope you like it!
> >>
> >> Looks promising!
> >
> > Fingers crossed. This has bothered me about QEMU for years.
>
> Thanks!
>

Re: [PATCH 00/27] Add qapi-domain Sphinx extension

[John Snow]

On Mon, Apr 22, 2024 at 12:38 PM John Snow <jsnow@redhat.com> wrote:
>
> On Mon, Apr 22, 2024 at 5:20 AM Markus Armbruster <armbru@redhat.com> wrote:
> >
> > John Snow <jsnow@redhat.com> writes:
> >
> > > On Fri, Apr 19, 2024, 10:45 AM Markus Armbruster <armbru@redhat.com> wrote:
> > >
> > >> John Snow <jsnow@redhat.com> writes:
> > >>
> > >> > This series adds a new qapi-domain extension for Sphinx, which adds a
> > >> > series of custom directives for documenting QAPI definitions.
> > >> >
> > >> > GitLab CI: https://gitlab.com/jsnow/qemu/-/pipelines/1259566476
> > >> >
> > >> > (Link to a demo HTML page at the end of this cover letter, but I want
> > >> > you to read the cover letter first to explain what you're seeing.)
> > >> >
> > >> > This adds a new QAPI Index page, cross-references for QMP commands,
> > >> > events, and data types, and improves the aesthetics of the QAPI/QMP
> > >> > documentation.
> > >>
> > >> Cross-references alone will be a *massive* improvement!  I'm sure
> > >> readers will appreciate better looks and an index, too.
> > >>
> > >> > This series adds only the new ReST syntax, *not* the autogenerator. The
> > >> > ReST syntax used in this series is, in general, not intended for anyone
> > >> > to actually write by hand. This mimics how Sphinx's own autodoc
> > >> > extension generates Python domain directives, which are then re-parsed
> > >> > to produce the final result.
> > >> >
> > >> > I have prototyped such a generator, but it isn't ready for inclusion
> > >> > yet. (Rest assured: error context reporting is preserved down to the
> > >> > line, even in generated ReST. There is no loss in usability for this
> > >> > approach. It will likely either supplant qapidoc.py or heavily alter
> > >> > it.) The generator requires only extremely minor changes to
> > >> > scripts/qapi/parser.py to preserve nested indentation and provide more
> > >> > accurate line information. It is less invasive than you may
> > >> > fear. Relying on a secondary ReST parse phase eliminates much of the
> > >> > complexity of qapidoc.py. Sleep soundly.
> > >>
> > >> I'm a Sphinx noob.  Let me paraphrase you to make sure I understand.
> > >>
> > >> You proprose to generate formatted documentation in two steps:
> > >>
> > >> • First, the QAPI generator generates .rst from the QAPI schema.  The
> > >>   generated .rst makes use of a custom directives.
> > >>
> > >
> > > Yes, but this .rst file is built in-memory and never makes it to disk, like
> > > Sphinx's autodoc for Python.
> >
> > Makes sense.
> >
> > > (We can add a debug knob to log it or save it out to disk if needed.)
> >
> > Likely useful at least occasionally.
>
> Yep, python's autodoc has such a knob to use the debugging log for
> this. I just want to point out that avoiding the intermediate file
> on-disk is actually the mechanism by which I can preserve source
> lines, so this is how it's gotta be.
>
> I build an intermediate doc in-memory with source filenames and source
> lines along with the modified doc block content so that ReST errors
> can be tracked back directly to the QAPI json files. If we saved to
> disk and parsed that, it'd obliterate that information.
>
> >
> > >> • Second, Sphinx turns the .rst into formatted documentation.  A Sphinx
> > >>   qapi-domain extension implements the custom directives
> > >
> > > Yes.
> > >
> > >
> > >> This mirrors how Sphinx works for Python docs.  Which is its original
> > >> use case.
> > >>
> > >> Your series demonstrates the second step, with test input you wrote
> > >> manually.
> > >>
> > >> You have code for the first step, but you'd prefer to show it later.
> > >
> > > Right, it's not fully finished, although I have events, commands, and
> > > objects working. Unions, Alternates and Events need work.
> > >
> > >
> > >> Fair?
> > >
> > > Bingo!
> >
> > Thanks!
> >
> > >> > The purpose of sending this series in its current form is largely to
> > >> > solicit feedback on general aesthetics, layout, and features. Sphinx is
> > >> > a wily beast, and feedback at this stage will dictate how and where
> > >> > certain features are implemented.
> > >>
> > >> I'd appreciate help with that.  Opinions?
> > >
> > >
> > >> > A goal for this syntax (and the generator) is to fully in-line all
> > >> > command/event/object members, inherited or local, boxed or not, branched
> > >> > or not. This should provide a boon to using these docs as a reference,
> > >> > because users will not have to grep around the page looking for various
> > >> > types, branches, or inherited members. Any arguments types will be
> > >> > hyperlinked to their definition, further aiding usability. Commands can
> > >> > be hotlinked from anywhere else in the manual, and will provide a
> > >> > complete reference directly on the first screenful of information.
> > >>
> > >> Let me elaborate a bit here.
> > >>
> > >> A command's arguments can be specified inline, like so:
> > >>
> > >>     { 'command': 'job-cancel', 'data': { 'id': 'str' } }
> > >>
> > >> The arguments are then documented right with the command.
> > >>
> > >> But they can also be specified by referencing an object type, like so:
> > >>
> > >>     { 'command': 'block-dirty-bitmap-remove',
> > >>       'data': 'BlockDirtyBitmap' }
> > >>
> > >> Reasons for doing it this way:
> > >>
> > >> • Several commands take the same arguments, and you don't want to repeat
> > >>   yourself.
> > >>
> > >> • You want generated C take a single struct argument ('boxed': true).
> > >>
> > >> • The arguments are a union (which requires 'boxed': true).
> > >>
> > >> Drawback: the arguments are then documented elsewhere.  Not nice.
> > >>
> > >> Bug: the generated documentation fails to point there.
> > >>
> > >> You're proposing to inline the argument documentation, so it appears
> > >> right with the command.
> > >>
> > >> An event's data is just like a command's argument.
> > >>
> > >> A command's return value can only specified by referencing a type.  Same
> > >> doc usability issue.
> > >>
> > >> Similarly, a union type's base can specified inline or by referencing a
> > >> struct type, and a union's branches must be specified by referencing a
> > >> struct type.  Same doc usability issue.
> > >>
> > >> At least, the generated documentation does point to the referenced
> > >> types.
> > >>
> > >
> > > Right. My proposal is to recursively inline referenced bases for the
> > > top-level members so that this manual is useful as a user reference,
> > > without worrying about the actual QAPI structure.
> > >
> > > Return types will just be hotlinked.
> >
> > The argument for inlining the arguments equally applies to results:
> > "users will not have to grep around the page".
> >
> > 102 out of 236 commands return something, usually some object type or an
> > array of some object type.  Most of these types are used for exactly one
> > command's return and nothing else.
> >
> > Regardless, it's fine to explore inlining just for arguments.  If we can
> > make that work, extending it to return results shouldn't be too hard.
>
> Yeah. Future work, if we want it. Otherwise, I think it's not *too*
> bad to hotlink to the return arg type; but if that's info that you
> want to "hide" from the user API, I get it - we can work on squanching
> that in too. (For a follow-up though, please.)
>
> >
> > >> > (Okay, maybe two screenfuls for commands with a ton of
> > >> > arguments... There's only so much I can do!)
> > >>
> > >> *cough* blockdev-add *cough*
> > >
> > >
> > >> > This RFC series includes a "sandbox" .rst document that showcases the
> > >> > features of this domain by writing QAPI directives by hand; this
> > >> > document will be removed from the series before final inclusion. It's
> > >> > here to serve as a convenient test-bed for y'all to give feedback.
> > >> >
> > >> > All that said, here's the sandbox document fully rendered:
> > >> > https://jsnow.gitlab.io/qemu/qapi/index.html
> > >> >
> > >> > And here's the new QAPI index page created by that sandbox document:
> > >> > https://jsnow.gitlab.io/qemu/qapi-index.html
> > >> >
> > >> > Known issues / points of interest:
> > >> >
> > >> > - The formatting upsets checkpatch. The default line length for the
> > >> >   "black" formatting tool is a little long. I'll fix it next spin.
> > >> >
> > >> > - I did my best to preserve cross-version compatibility, but some
> > >> >   problems have crept in here and there. This series may require
> > >> >   Sphinx>= 4.0, like the dbus extension. Notably, the Ubuntu build fails
> > >> >   on Gitlab CI currently. The next version will text against more Sphinx
> > >> >   versions more rigorously. Sphinx version 5.3.0 and above should work
> > >> >   perfectly.
> > >> >
> > >> > - There's a bug in Sphinx itself that may manifest in your testing,
> > >> >   concerning reported error locations. There's a patch in this series
> > >> >   that fixes it, but it's later in the series. If you run into the bug
> > >> >   while testing with this series, try applying that patch first.
> > >> >
> > >> > - QAPI 'namespaces' aren't yet handled. i.e., there's nothing to
> > >> >   distinguish entities between QMP, QGA and QSD yet. That feature will
> > >> >   be added to a future version of this patchset (Likely when the
> > >> >   generator is ready for inclusion: without it, references will clash
> > >> >   and the index will gripe about duplicated entries.)
> > >>
> > >> qemu-storage-daemon's QMP is a proper subset of qemu-system-FOO's.
> > >> Regardless, each of them has its own, independent reference manual.
> > >> That's defensible.
> > >>
> > >> But the way we build them can complicate matters.  For instance, when I
> > >> tried to elide types not used for QMP from the reference manuals, I got
> > >> defeated by Sphinx caching.
> > >
> > > I haven't tried yet, but I believe it should be possible to "tag" each
> > > referenced QAPI object and mark it as "visited". Each namespaced definition
> > > would be tagged separately.
> > >
> > > (i.e. qemu.block-core.blockdev-backup and qsd.block-core.blockdev-backup
> > > would be two distinct entities.)
> > >
> > > Then, the renderer could ignore/squelch untagged entities. (I think.)
> > >
> > > Maybe some work to do to un-index unvisited entities.
> > >
> > > Or we can go the other route and bake this info into the schema and squelch
> > > stuff earlier. We can add this feature later. I am still not sure why your
> > > prototype for this ran into cache issues, but I am convinced it should be
> > > fixable by making namespaced entities distinct.
> >
> > From (hazy) memory: when full QMP and storage daemon QMP both include a
> > sub-module for its types, but full QMP uses more of its types than
> > storage daemon QMP does, the storage daemon manual elides more of its
> > types than the QMP manual does.  However, sphinx-build's caching uses
> > whatever got built first for *both* manuals.  When we happen to build
> > the full QMP manual first, we end up with extra types in the storage
> > daemon manual.  When we happen to build the storage daemon manual first,
> > the full manual is missing types.
>
> Weird. I still don't understand where that info is getting cached, to
> be honest ...
>
> >
> > > We could perhaps bake the namespace into the qapidoc directive, e.g.:
> > >
> > >  .. qapi:autodoc:: schema.json
> > >    :namespace: QSD
> > >
> > > (And the default adds no namespace.)
> >
> > Is it worth it?  How useful is the separate QEMU Storage Daemon QMP
> > Reference Manual?  It's an exact duplicate of selected chapters of the
> > QEMU QMP Reference Manual...  Could we instead document "QMP, but only
> > these chapters"?
>
> I dunno. That's up to you and Kevin, I think? For now, I'm just
> following "What we've got" but with cross-referencing and indexing
> improvements. Those improvements require distinguishing QMP and QSD
> somehow. Adding simple namespace support seems like the most flexible
> *and* dead simple way to do it. Other improvements can come later if
> desired, I think.
>
> If you need help adding Sphinx features to make it happen, you know
> where I live.
>
> >
> > Diversion: can we come up with a better way of subsetting the full QAPI
> > schema for the storage daemon?
>
> Out of scope for me I'm afraid, but clue me in to the discussion
> if/when you have it.
>
> >
> > We currently subset by having storage-daemon/qapi/qapi-schema.json a
> > subset of the submodules qapi/qapi-schema.json includes.  The code
> > generated for the entire schema differs (basically qapi-init-commands.c,
> > qapi-emit-events.[ch], and qapi-introspect.c).  The code generated for
> > the submodules should be identical (modulo unused array types, but
> > that's harmless detail).  So we ignore the code generated for the
> > storage daemon's submodules.
>
> Ah, so you are debating a type of pragma or other meta-include that
> just fishes out specific definitions? That could work, I think - I'm
> not sure how the QAPI Schema end would look, but the Sphinx domain
> could provide a meta-directive for adding a QSD support tag to
> specific entities or some such thing, and the single QMP reference
> could advertise a qemu-storage-daemon support flag.
>
> Or something - I'm not really privy to the big picture when it comes
> to how we want to handle QMP modularization and differentiation
> between different binaries.
>
> >
> > End of diversion.
> >
> > >> > - Per-member features and ifcond are not yet accounted for; though
> > >> >   definition-scoped features and ifconds are. Please feel free to
> > >> >   suggest how you'd like to see those represented.
> > >> >
> > >> > - Inlining all members means there is some ambiguity on what to do with
> > >> >   doc block sections on inlined entities; features and members have an
> > >> >   obvious home - body, since, and misc sections are not as obvious on
> > >> >   how to handle. This will feature more prominently in the generator
> > >> >   series.
> > >>
> > >> Yes, this is a real problem.
> > >>
> > >> The member documentation gets written in the context of the type.  It
> > >> may make sense only in that context.  Inlining copies it into a
> > >> different context.
> > >>
> > >> Features may need to combine.  Say a command's arguments are a union
> > >> type, and several branches of the union both contain deprecated members.
> > >> These branch types all document feature @deprecated.  Simply inlining
> > >> their feature documentation results in feature @deprecated documented
> > >> several times.  Ugh.  Combining them would be better.  But how?  Do we
> > >> need to rethink how we document features?
> > >
> > > To be honest, I figured I'd plow ahead and then find the counter-examples
> > > programmatically and decide what to do once I had my pile of edge cases.
> > >
> > > It might involve rewriting docs in some places, but I think the usability
> > > win is completely worth the hassle.
> > >
> > > It's possible there might be some rework needed to maximize cogency of the
> > > generated docs, but I think prioritizing a user-facing reference for QMP is
> > > the objectively correct end goal and I'm more than happy to work backwards
> > > from that desired state.
> >
> > I'm not disputing the idea that documenting the arguments right with the
> > command is good.  I'm merely pointing out obstacles to pulling that off.
> >
> > Adjusting existing documentation is only half the battle.  The other
> > half is making sure documentation stays adjusted.  We may have to come
> > up with new documentation rules, and ways to enforce them.
>
> For the sake of argument, let's say we forbid everything except
> arg/features from definitions destined to be used as base/inherited
> types. This would be very easy to enforce at the qapidoc level where
> the doc inlining is performed by yelping when the base type contains
> additional documentation sections.
>
> Now, in the real world, maybe sometimes those sections are useful and
> we don't want to get rid of them, esp. because they may contain useful
> documentation that we don't want to duplicate in the source files.
>
> My plan is to just forbid them at first and enumerate the cases where
> they occur, then decide which ones are better off being moved
> elsewhere or explicitly tolerated. The generator's tolerance can be
> adjusted accordingly and we can formulate a rule for exactly how doc
> blocks are combined and merged. I think it won't be a problem to
> enforce it programmatically.
>
> I'll get back to you on how often and precisely where these cases
> occur so you can take a look and see how you feel.
>

For a warmup, let's look at every unique instance of non-empty
paragraph text on an object that is used as a base anywhere:

Warning: AudiodevPerDirectionOptions - inlined paragraph
Warning: BlockdevOptionsCurlBase - inlined paragraph
Warning: BlockdevOptionsGenericCOWFormat - inlined paragraph
Warning: BlockdevOptionsGenericFormat - inlined paragraph
Warning: BlockExportOptionsNbdBase - inlined paragraph
Warning: BlockNodeInfo - inlined paragraph
Warning: ChardevCommon - inlined paragraph
Warning: CpuInstanceProperties - inlined paragraph
Warning: CryptodevBackendProperties - inlined paragraph
Warning: EventLoopBaseProperties - inlined paragraph
Warning: MemoryBackendProperties - inlined paragraph
Warning: NetfilterProperties - inlined paragraph
Warning: QCryptoBlockAmendOptionsLUKS - inlined paragraph
Warning: QCryptoBlockCreateOptionsLUKS - inlined paragraph
Warning: QCryptoBlockInfoBase - inlined paragraph
Warning: QCryptoBlockOptionsBase - inlined paragraph
Warning: QCryptoBlockOptionsLUKS - inlined paragraph
Warning: RngProperties - inlined paragraph
Warning: SecretCommonProperties - inlined paragraph
Warning: SpiceBasicInfo - inlined paragraph
Warning: TlsCredsProperties - inlined paragraph
Warning: VncBasicInfo - inlined paragraph

There's 22 instances.

Let's look at what they say:

AudiodevPerDirectionOptions: "General audio backend options that are
used for both playback and recording."
BlockdevOptionsCurlBase: "Driver specific block device options shared
by all protocols supported by the curl backend."
BlockdevOptionsGenericCOWFormat: "Driver specific block device options
for image format that have no option besides their data source and an
optional backing file."
BlockdevOptionsGenericFormat: "Driver specific block device options
for image format that have no option besides their data source."
BlockExportOptionsNbdBase: "An NBD block export (common options shared
between nbd-server-add and the NBD branch of block-export-add)."
BlockNodeInfo: "Information about a QEMU image file"
ChardevCommon: "Configuration shared across all chardev backends"

CpuInstanceProperties:
"List of properties to be used for hotplugging a CPU instance, it
should be passed by management with device_add command when a CPU is
being hotplugged.

Which members are optional and which mandatory depends on the
architecture and board.

For s390x see :ref:`cpu-topology-s390x`.

The ids other than the node-id specify the position of the CPU
within the CPU topology (as defined by the machine property "smp",
thus see also type @SMPConfiguration)"

CryptodevBackendProperties: "Properties for cryptodev-backend and
cryptodev-backend-builtin objects."
EventLoopBaseProperties: "Common properties for event loops"
MemoryBackendProperties: "Properties for objects of classes derived
from memory-backend."
NetfilterProperties: "Properties for objects of classes derived from netfilter."
QCryptoBlockAmendOptionsLUKS: "This struct defines the update
parameters that activate/de-activate set of keyslots"
QCryptoBlockCreateOptionsLUKS: "The options that apply to LUKS
encryption format initialization"
QCryptoBlockInfoBase: "The common information that applies to all full
disk encryption formats"
QCryptoBlockOptionsBase: "The common options that apply to all full
disk encryption formats"
QCryptoBlockOptionsLUKS: "The options that apply to LUKS encryption format"
RngProperties: "Properties for objects of classes derived from rng."
SecretCommonProperties: "Properties for objects of classes derived
from secret-common."
SpiceBasicInfo: "The basic information for SPICE network connection"
TlsCredsProperties: "Properties for objects of classes derived from tls-creds."
VncBasicInfo: "The basic information for vnc network connection"

... Alright. So like 98% of the time, it's functionally useless
information for the end-user. The only thing that looks mildly
interesting is CpuInstanceProperties and *maybe*
QCryptoBlockAmendOptionsLUKS.

I propose we add a new section to QAPI doc blocks that gets ignored
from rendered documentation, like "Comment:" -- to keep any info that
might be mildly relevant to a developer that explains the *factoring*
of the object, but won't be exposed in user-facing documents.

On the occasion we DO want to inline documentation paragraphs, we can
leave them in and have the doc generator inline them. There's probably
very few cases where we actually want this.

Let's take a look at any tagged sections now, excluding "Since":

Warning: BackupCommon - inlined tag section Note
Warning: CpuInstanceProperties - inlined, tag section Note
Warning: MemoryBackendProperties - inlined tag section Note

Not very many! Just three.

BackupCommon:

Note: @on-source-error and @on-target-error only affect background
    I/O.  If an error occurs during a guest write request, the
    device's rerror/werror actions will be used.

BackupCommon is used as a base for DriveBackup and BlockdevBackup. In
this case, this note probably does belong on both. It should be
inlined and included.

CpuInstanceProperties:

Note: management should be prepared to pass through additional
    properties with device_add.

This is only used as a base with no other addon args for
NumaCpuOptions, in turn only used for an argument. This is probably
right to pass through, too. (Though I admit I don't really know what
it means... we can discuss the doc *quality* another day.)

MemoryBackendProperties:

Note: prealloc=true and reserve=false cannot be set at the same
    time.  With reserve=true, the behavior depends on the operating
    system: for example, Linux will not reserve swap space for
    shared file mappings -- "not applicable". In contrast,
    reserve=false will bail out if it cannot be configured
    accordingly.

This is used in MemoryBackendFileProperties,
MemoryBackendMemfdProperties, and MemoryBackendEpcProperties. None are
commands. I think the note here should also be inlined into each
object.

So, I think that:

- Most naked paragraphs are usually useless to the end-user, and we
should put them behind a Comment section to prevent them from being
inlined.
- Any *other* paragraph or section should be included in the
descendent. We'll just review with that eye going forward.
- Since: ... we'll talk about later this week.

--js

Re: [PATCH 00/27] Add qapi-domain Sphinx extension

[Markus Armbruster]

John Snow <jsnow@redhat.com> writes:

> On Mon, Apr 22, 2024 at 5:20 AM Markus Armbruster <armbru@redhat.com> wrote:
>>
>> John Snow <jsnow@redhat.com> writes:
>>
>> > On Fri, Apr 19, 2024, 10:45 AM Markus Armbruster <armbru@redhat.com> wrote:
>> >
>> >> John Snow <jsnow@redhat.com> writes:
>> >>
>> >> > This series adds a new qapi-domain extension for Sphinx, which adds a
>> >> > series of custom directives for documenting QAPI definitions.
>> >> >
>> >> > GitLab CI: https://gitlab.com/jsnow/qemu/-/pipelines/1259566476
>> >> >
>> >> > (Link to a demo HTML page at the end of this cover letter, but I want
>> >> > you to read the cover letter first to explain what you're seeing.)
>> >> >
>> >> > This adds a new QAPI Index page, cross-references for QMP commands,
>> >> > events, and data types, and improves the aesthetics of the QAPI/QMP
>> >> > documentation.
>> >>
>> >> Cross-references alone will be a *massive* improvement!  I'm sure
>> >> readers will appreciate better looks and an index, too.
>> >>
>> >> > This series adds only the new ReST syntax, *not* the autogenerator. The
>> >> > ReST syntax used in this series is, in general, not intended for anyone
>> >> > to actually write by hand. This mimics how Sphinx's own autodoc
>> >> > extension generates Python domain directives, which are then re-parsed
>> >> > to produce the final result.
>> >> >
>> >> > I have prototyped such a generator, but it isn't ready for inclusion
>> >> > yet. (Rest assured: error context reporting is preserved down to the
>> >> > line, even in generated ReST. There is no loss in usability for this
>> >> > approach. It will likely either supplant qapidoc.py or heavily alter
>> >> > it.) The generator requires only extremely minor changes to
>> >> > scripts/qapi/parser.py to preserve nested indentation and provide more
>> >> > accurate line information. It is less invasive than you may
>> >> > fear. Relying on a secondary ReST parse phase eliminates much of the
>> >> > complexity of qapidoc.py. Sleep soundly.
>> >>
>> >> I'm a Sphinx noob.  Let me paraphrase you to make sure I understand.
>> >>
>> >> You proprose to generate formatted documentation in two steps:
>> >>
>> >> • First, the QAPI generator generates .rst from the QAPI schema.  The
>> >>   generated .rst makes use of a custom directives.
>> >>
>> >
>> > Yes, but this .rst file is built in-memory and never makes it to disk, like
>> > Sphinx's autodoc for Python.
>>
>> Makes sense.
>>
>> > (We can add a debug knob to log it or save it out to disk if needed.)
>>
>> Likely useful at least occasionally.
>
> Yep, python's autodoc has such a knob to use the debugging log for
> this. I just want to point out that avoiding the intermediate file
> on-disk is actually the mechanism by which I can preserve source
> lines, so this is how it's gotta be.
>
> I build an intermediate doc in-memory with source filenames and source
> lines along with the modified doc block content so that ReST errors
> can be tracked back directly to the QAPI json files. If we saved to
> disk and parsed that, it'd obliterate that information.

Sounds just fine to me.

>> >> • Second, Sphinx turns the .rst into formatted documentation.  A Sphinx
>> >>   qapi-domain extension implements the custom directives
>> >
>> > Yes.
>> >
>> >
>> >> This mirrors how Sphinx works for Python docs.  Which is its original
>> >> use case.
>> >>
>> >> Your series demonstrates the second step, with test input you wrote
>> >> manually.
>> >>
>> >> You have code for the first step, but you'd prefer to show it later.
>> >
>> > Right, it's not fully finished, although I have events, commands, and
>> > objects working. Unions, Alternates and Events need work.
>> >
>> >
>> >> Fair?
>> >
>> > Bingo!
>>
>> Thanks!
>>
>> >> > The purpose of sending this series in its current form is largely to
>> >> > solicit feedback on general aesthetics, layout, and features. Sphinx is
>> >> > a wily beast, and feedback at this stage will dictate how and where
>> >> > certain features are implemented.
>> >>
>> >> I'd appreciate help with that.  Opinions?
>> >
>> >
>> >> > A goal for this syntax (and the generator) is to fully in-line all
>> >> > command/event/object members, inherited or local, boxed or not, branched
>> >> > or not. This should provide a boon to using these docs as a reference,
>> >> > because users will not have to grep around the page looking for various
>> >> > types, branches, or inherited members. Any arguments types will be
>> >> > hyperlinked to their definition, further aiding usability. Commands can
>> >> > be hotlinked from anywhere else in the manual, and will provide a
>> >> > complete reference directly on the first screenful of information.
>> >>
>> >> Let me elaborate a bit here.
>> >>
>> >> A command's arguments can be specified inline, like so:
>> >>
>> >>     { 'command': 'job-cancel', 'data': { 'id': 'str' } }
>> >>
>> >> The arguments are then documented right with the command.
>> >>
>> >> But they can also be specified by referencing an object type, like so:
>> >>
>> >>     { 'command': 'block-dirty-bitmap-remove',
>> >>       'data': 'BlockDirtyBitmap' }
>> >>
>> >> Reasons for doing it this way:
>> >>
>> >> • Several commands take the same arguments, and you don't want to repeat
>> >>   yourself.
>> >>
>> >> • You want generated C take a single struct argument ('boxed': true).
>> >>
>> >> • The arguments are a union (which requires 'boxed': true).
>> >>
>> >> Drawback: the arguments are then documented elsewhere.  Not nice.
>> >>
>> >> Bug: the generated documentation fails to point there.
>> >>
>> >> You're proposing to inline the argument documentation, so it appears
>> >> right with the command.
>> >>
>> >> An event's data is just like a command's argument.
>> >>
>> >> A command's return value can only specified by referencing a type.  Same
>> >> doc usability issue.
>> >>
>> >> Similarly, a union type's base can specified inline or by referencing a
>> >> struct type, and a union's branches must be specified by referencing a
>> >> struct type.  Same doc usability issue.
>> >>
>> >> At least, the generated documentation does point to the referenced
>> >> types.
>> >>
>> >
>> > Right. My proposal is to recursively inline referenced bases for the
>> > top-level members so that this manual is useful as a user reference,
>> > without worrying about the actual QAPI structure.
>> >
>> > Return types will just be hotlinked.
>>
>> The argument for inlining the arguments equally applies to results:
>> "users will not have to grep around the page".
>>
>> 102 out of 236 commands return something, usually some object type or an
>> array of some object type.  Most of these types are used for exactly one
>> command's return and nothing else.
>>
>> Regardless, it's fine to explore inlining just for arguments.  If we can
>> make that work, extending it to return results shouldn't be too hard.
>
> Yeah. Future work, if we want it. Otherwise, I think it's not *too*
> bad to hotlink to the return arg type;

I think hiding a command's results behind a link is roughly as bad as
hiding its arguments behind a link.

>                                        but if that's info that you
> want to "hide" from the user API, I get it - we can work on squanching
> that in too. (For a follow-up though, please.)

It's not about hiding type names.  Sure, type names are not ABI, since
there's nothing the user can do with them.  But as long as that's
understood, they can be useful in documentation.  We use them as text
for links to and a title for the documentation of a type.

Messing with type names affects link texts and titles in documentation,
which is fine.

>> >> > (Okay, maybe two screenfuls for commands with a ton of
>> >> > arguments... There's only so much I can do!)
>> >>
>> >> *cough* blockdev-add *cough*
>> >
>> >
>> >> > This RFC series includes a "sandbox" .rst document that showcases the
>> >> > features of this domain by writing QAPI directives by hand; this
>> >> > document will be removed from the series before final inclusion. It's
>> >> > here to serve as a convenient test-bed for y'all to give feedback.
>> >> >
>> >> > All that said, here's the sandbox document fully rendered:
>> >> > https://jsnow.gitlab.io/qemu/qapi/index.html
>> >> >
>> >> > And here's the new QAPI index page created by that sandbox document:
>> >> > https://jsnow.gitlab.io/qemu/qapi-index.html
>> >> >
>> >> > Known issues / points of interest:
>> >> >
>> >> > - The formatting upsets checkpatch. The default line length for the
>> >> >   "black" formatting tool is a little long. I'll fix it next spin.
>> >> >
>> >> > - I did my best to preserve cross-version compatibility, but some
>> >> >   problems have crept in here and there. This series may require
>> >> >   Sphinx>= 4.0, like the dbus extension. Notably, the Ubuntu build fails
>> >> >   on Gitlab CI currently. The next version will text against more Sphinx
>> >> >   versions more rigorously. Sphinx version 5.3.0 and above should work
>> >> >   perfectly.
>> >> >
>> >> > - There's a bug in Sphinx itself that may manifest in your testing,
>> >> >   concerning reported error locations. There's a patch in this series
>> >> >   that fixes it, but it's later in the series. If you run into the bug
>> >> >   while testing with this series, try applying that patch first.
>> >> >
>> >> > - QAPI 'namespaces' aren't yet handled. i.e., there's nothing to
>> >> >   distinguish entities between QMP, QGA and QSD yet. That feature will
>> >> >   be added to a future version of this patchset (Likely when the
>> >> >   generator is ready for inclusion: without it, references will clash
>> >> >   and the index will gripe about duplicated entries.)
>> >>
>> >> qemu-storage-daemon's QMP is a proper subset of qemu-system-FOO's.
>> >> Regardless, each of them has its own, independent reference manual.
>> >> That's defensible.
>> >>
>> >> But the way we build them can complicate matters.  For instance, when I
>> >> tried to elide types not used for QMP from the reference manuals, I got
>> >> defeated by Sphinx caching.
>> >
>> > I haven't tried yet, but I believe it should be possible to "tag" each
>> > referenced QAPI object and mark it as "visited". Each namespaced definition
>> > would be tagged separately.
>> >
>> > (i.e. qemu.block-core.blockdev-backup and qsd.block-core.blockdev-backup
>> > would be two distinct entities.)
>> >
>> > Then, the renderer could ignore/squelch untagged entities. (I think.)
>> >
>> > Maybe some work to do to un-index unvisited entities.
>> >
>> > Or we can go the other route and bake this info into the schema and squelch
>> > stuff earlier. We can add this feature later. I am still not sure why your
>> > prototype for this ran into cache issues, but I am convinced it should be
>> > fixable by making namespaced entities distinct.
>>
>> From (hazy) memory: when full QMP and storage daemon QMP both include a
>> sub-module for its types, but full QMP uses more of its types than
>> storage daemon QMP does, the storage daemon manual elides more of its
>> types than the QMP manual does.  However, sphinx-build's caching uses
>> whatever got built first for *both* manuals.  When we happen to build
>> the full QMP manual first, we end up with extra types in the storage
>> daemon manual.  When we happen to build the storage daemon manual first,
>> the full manual is missing types.
>
> Weird. I still don't understand where that info is getting cached, to
> be honest ...

Me neither.

>> > We could perhaps bake the namespace into the qapidoc directive, e.g.:
>> >
>> >  .. qapi:autodoc:: schema.json
>> >    :namespace: QSD
>> >
>> > (And the default adds no namespace.)
>>
>> Is it worth it?  How useful is the separate QEMU Storage Daemon QMP
>> Reference Manual?  It's an exact duplicate of selected chapters of the
>> QEMU QMP Reference Manual...  Could we instead document "QMP, but only
>> these chapters"?
>
> I dunno. That's up to you and Kevin, I think? For now, I'm just
> following "What we've got" but with cross-referencing and indexing
> improvements. Those improvements require distinguishing QMP and QSD
> somehow. Adding simple namespace support seems like the most flexible
> *and* dead simple way to do it. Other improvements can come later if
> desired, I think.

If you want namespace support to keep QMP and QGA apart, go for it.

If it's just for full QMP vs. qemu-storage-daemon QMP, I suggest to
ignore the issue for now.

> If you need help adding Sphinx features to make it happen, you know
> where I live.
>
>> Diversion: can we come up with a better way of subsetting the full QAPI
>> schema for the storage daemon?
>
> Out of scope for me I'm afraid, but clue me in to the discussion
> if/when you have it.

Sure!

>> We currently subset by having storage-daemon/qapi/qapi-schema.json a
>> subset of the submodules qapi/qapi-schema.json includes.  The code
>> generated for the entire schema differs (basically qapi-init-commands.c,
>> qapi-emit-events.[ch], and qapi-introspect.c).  The code generated for
>> the submodules should be identical (modulo unused array types, but
>> that's harmless detail).  So we ignore the code generated for the
>> storage daemon's submodules.
>
> Ah, so you are debating a type of pragma or other meta-include that
> just fishes out specific definitions? That could work, I think - I'm
> not sure how the QAPI Schema end would look, but the Sphinx domain
> could provide a meta-directive for adding a QSD support tag to
> specific entities or some such thing, and the single QMP reference
> could advertise a qemu-storage-daemon support flag.
>
> Or something - I'm not really privy to the big picture when it comes
> to how we want to handle QMP modularization and differentiation
> between different binaries.
>
>> End of diversion.
>>
>> >> > - Per-member features and ifcond are not yet accounted for; though
>> >> >   definition-scoped features and ifconds are. Please feel free to
>> >> >   suggest how you'd like to see those represented.
>> >> >
>> >> > - Inlining all members means there is some ambiguity on what to do with
>> >> >   doc block sections on inlined entities; features and members have an
>> >> >   obvious home - body, since, and misc sections are not as obvious on
>> >> >   how to handle. This will feature more prominently in the generator
>> >> >   series.
>> >>
>> >> Yes, this is a real problem.
>> >>
>> >> The member documentation gets written in the context of the type.  It
>> >> may make sense only in that context.  Inlining copies it into a
>> >> different context.
>> >>
>> >> Features may need to combine.  Say a command's arguments are a union
>> >> type, and several branches of the union both contain deprecated members.
>> >> These branch types all document feature @deprecated.  Simply inlining
>> >> their feature documentation results in feature @deprecated documented
>> >> several times.  Ugh.  Combining them would be better.  But how?  Do we
>> >> need to rethink how we document features?
>> >
>> > To be honest, I figured I'd plow ahead and then find the counter-examples
>> > programmatically and decide what to do once I had my pile of edge cases.
>> >
>> > It might involve rewriting docs in some places, but I think the usability
>> > win is completely worth the hassle.
>> >
>> > It's possible there might be some rework needed to maximize cogency of the
>> > generated docs, but I think prioritizing a user-facing reference for QMP is
>> > the objectively correct end goal and I'm more than happy to work backwards
>> > from that desired state.
>>
>> I'm not disputing the idea that documenting the arguments right with the
>> command is good.  I'm merely pointing out obstacles to pulling that off.
>>
>> Adjusting existing documentation is only half the battle.  The other
>> half is making sure documentation stays adjusted.  We may have to come
>> up with new documentation rules, and ways to enforce them.
>
> For the sake of argument, let's say we forbid everything except
> arg/features from definitions destined to be used as base/inherited
> types. This would be very easy to enforce at the qapidoc level where
> the doc inlining is performed by yelping when the base type contains
> additional documentation sections.

Yes.

The type's doc comment is then no longer fit for generating standalone
documentation for the type, only for generating inlined documentation.

> Now, in the real world, maybe sometimes those sections are useful and
> we don't want to get rid of them, esp. because they may contain useful
> documentation that we don't want to duplicate in the source files.

Or a type is used both in a position where its documentation is to be
inlined, and a position where it isn't.  The latter requires us to
generate standalone documentation for the type, so we can link to it.

Whether this is a problem in practice I can't say.

> My plan is to just forbid them at first and enumerate the cases where
> they occur, then decide which ones are better off being moved
> elsewhere or explicitly tolerated. The generator's tolerance can be
> adjusted accordingly and we can formulate a rule for exactly how doc
> blocks are combined and merged. I think it won't be a problem to
> enforce it programmatically.
>
> I'll get back to you on how often and precisely where these cases
> occur so you can take a look and see how you feel.
>
>>
>> >> > - Some features could be implemented in either the QAPI domain syntax
>> >> >   *or* the generator, or some combination of both. Depending on
>> >> >   aesthetic feedback, this may influence where those features should be
>> >> >   implemented.
>> >> >
>> >> > - The formatting and aesthetics of branches are a little up in the air,
>> >> >   see the qapi:union patch for more details.
>> >> >
>> >> > - It's late, maybe other things. Happy Friday!
>> >> >
>> >> > Hope you like it!
>> >>
>> >> Looks promising!
>> >
>> > Fingers crossed. This has bothered me about QEMU for years.
>>
>> Thanks!

Re: [PATCH 00/27] Add qapi-domain Sphinx extension

[Markus Armbruster]

John Snow <jsnow@redhat.com> writes:

> On Mon, Apr 22, 2024 at 12:38 PM John Snow <jsnow@redhat.com> wrote:
>>
>> On Mon, Apr 22, 2024 at 5:20 AM Markus Armbruster <armbru@redhat.com> wrote:
>> >
>> > John Snow <jsnow@redhat.com> writes:
>> >
>> > > On Fri, Apr 19, 2024, 10:45 AM Markus Armbruster <armbru@redhat.com> wrote:
>> > >
>> > >> John Snow <jsnow@redhat.com> writes:
>> > >>
>> > >> > This series adds a new qapi-domain extension for Sphinx, which adds a
>> > >> > series of custom directives for documenting QAPI definitions.

[...]

>> > >> > Known issues / points of interest:

[...]

>> > >> > - Inlining all members means there is some ambiguity on what to do with
>> > >> >   doc block sections on inlined entities; features and members have an
>> > >> >   obvious home - body, since, and misc sections are not as obvious on
>> > >> >   how to handle. This will feature more prominently in the generator
>> > >> >   series.
>> > >>
>> > >> Yes, this is a real problem.
>> > >>
>> > >> The member documentation gets written in the context of the type.  It
>> > >> may make sense only in that context.  Inlining copies it into a
>> > >> different context.
>> > >>
>> > >> Features may need to combine.  Say a command's arguments are a union
>> > >> type, and several branches of the union both contain deprecated members.
>> > >> These branch types all document feature @deprecated.  Simply inlining
>> > >> their feature documentation results in feature @deprecated documented
>> > >> several times.  Ugh.  Combining them would be better.  But how?  Do we
>> > >> need to rethink how we document features?
>> > >
>> > > To be honest, I figured I'd plow ahead and then find the counter-examples
>> > > programmatically and decide what to do once I had my pile of edge cases.
>> > >
>> > > It might involve rewriting docs in some places, but I think the usability
>> > > win is completely worth the hassle.
>> > >
>> > > It's possible there might be some rework needed to maximize cogency of the
>> > > generated docs, but I think prioritizing a user-facing reference for QMP is
>> > > the objectively correct end goal and I'm more than happy to work backwards
>> > > from that desired state.
>> >
>> > I'm not disputing the idea that documenting the arguments right with the
>> > command is good.  I'm merely pointing out obstacles to pulling that off.
>> >
>> > Adjusting existing documentation is only half the battle.  The other
>> > half is making sure documentation stays adjusted.  We may have to come
>> > up with new documentation rules, and ways to enforce them.
>>
>> For the sake of argument, let's say we forbid everything except
>> arg/features from definitions destined to be used as base/inherited
>> types. This would be very easy to enforce at the qapidoc level where
>> the doc inlining is performed by yelping when the base type contains
>> additional documentation sections.
>>
>> Now, in the real world, maybe sometimes those sections are useful and
>> we don't want to get rid of them, esp. because they may contain useful
>> documentation that we don't want to duplicate in the source files.
>>
>> My plan is to just forbid them at first and enumerate the cases where
>> they occur, then decide which ones are better off being moved
>> elsewhere or explicitly tolerated. The generator's tolerance can be
>> adjusted accordingly and we can formulate a rule for exactly how doc
>> blocks are combined and merged. I think it won't be a problem to
>> enforce it programmatically.
>>
>> I'll get back to you on how often and precisely where these cases
>> occur so you can take a look and see how you feel.
>>
>
> For a warmup, let's look at every unique instance of non-empty
> paragraph text on an object that is used as a base anywhere:
>
> Warning: AudiodevPerDirectionOptions - inlined paragraph
> Warning: BlockdevOptionsCurlBase - inlined paragraph
> Warning: BlockdevOptionsGenericCOWFormat - inlined paragraph
> Warning: BlockdevOptionsGenericFormat - inlined paragraph
> Warning: BlockExportOptionsNbdBase - inlined paragraph
> Warning: BlockNodeInfo - inlined paragraph
> Warning: ChardevCommon - inlined paragraph
> Warning: CpuInstanceProperties - inlined paragraph
> Warning: CryptodevBackendProperties - inlined paragraph
> Warning: EventLoopBaseProperties - inlined paragraph
> Warning: MemoryBackendProperties - inlined paragraph
> Warning: NetfilterProperties - inlined paragraph
> Warning: QCryptoBlockAmendOptionsLUKS - inlined paragraph
> Warning: QCryptoBlockCreateOptionsLUKS - inlined paragraph
> Warning: QCryptoBlockInfoBase - inlined paragraph
> Warning: QCryptoBlockOptionsBase - inlined paragraph
> Warning: QCryptoBlockOptionsLUKS - inlined paragraph
> Warning: RngProperties - inlined paragraph
> Warning: SecretCommonProperties - inlined paragraph
> Warning: SpiceBasicInfo - inlined paragraph
> Warning: TlsCredsProperties - inlined paragraph
> Warning: VncBasicInfo - inlined paragraph
>
> There's 22 instances.
>
> Let's look at what they say:
>
> AudiodevPerDirectionOptions: "General audio backend options that are
> used for both playback and recording."
> BlockdevOptionsCurlBase: "Driver specific block device options shared
> by all protocols supported by the curl backend."
> BlockdevOptionsGenericCOWFormat: "Driver specific block device options
> for image format that have no option besides their data source and an
> optional backing file."
> BlockdevOptionsGenericFormat: "Driver specific block device options
> for image format that have no option besides their data source."
> BlockExportOptionsNbdBase: "An NBD block export (common options shared
> between nbd-server-add and the NBD branch of block-export-add)."
> BlockNodeInfo: "Information about a QEMU image file"
> ChardevCommon: "Configuration shared across all chardev backends"
>
> CpuInstanceProperties:
> "List of properties to be used for hotplugging a CPU instance, it
> should be passed by management with device_add command when a CPU is
> being hotplugged.
>
> Which members are optional and which mandatory depends on the
> architecture and board.
>
> For s390x see :ref:`cpu-topology-s390x`.
>
> The ids other than the node-id specify the position of the CPU
> within the CPU topology (as defined by the machine property "smp",
> thus see also type @SMPConfiguration)"
>
> CryptodevBackendProperties: "Properties for cryptodev-backend and
> cryptodev-backend-builtin objects."
> EventLoopBaseProperties: "Common properties for event loops"
> MemoryBackendProperties: "Properties for objects of classes derived
> from memory-backend."
> NetfilterProperties: "Properties for objects of classes derived from netfilter."
> QCryptoBlockAmendOptionsLUKS: "This struct defines the update
> parameters that activate/de-activate set of keyslots"
> QCryptoBlockCreateOptionsLUKS: "The options that apply to LUKS
> encryption format initialization"
> QCryptoBlockInfoBase: "The common information that applies to all full
> disk encryption formats"
> QCryptoBlockOptionsBase: "The common options that apply to all full
> disk encryption formats"
> QCryptoBlockOptionsLUKS: "The options that apply to LUKS encryption format"
> RngProperties: "Properties for objects of classes derived from rng."
> SecretCommonProperties: "Properties for objects of classes derived
> from secret-common."
> SpiceBasicInfo: "The basic information for SPICE network connection"
> TlsCredsProperties: "Properties for objects of classes derived from tls-creds."
> VncBasicInfo: "The basic information for vnc network connection"
>
> ... Alright. So like 98% of the time, it's functionally useless
> information for the end-user. The only thing that looks mildly
> interesting is CpuInstanceProperties and *maybe*
> QCryptoBlockAmendOptionsLUKS.
>
> I propose we add a new section to QAPI doc blocks that gets ignored
> from rendered documentation, like "Comment:" -- to keep any info that
> might be mildly relevant to a developer that explains the *factoring*
> of the object, but won't be exposed in user-facing documents.

Two existing ways to add comments that don't go into generated
documentation:

1. Write a non-doc comment.

   ##
   # This is a doc comment.
   ##

   #
   # This isn't.
   #

2. TODO sections in a doc comment.

Not sure we need more ways, but if we do, we'll create them.

> On the occasion we DO want to inline documentation paragraphs, we can
> leave them in and have the doc generator inline them. There's probably
> very few cases where we actually want this.
>
> Let's take a look at any tagged sections now, excluding "Since":
>
> Warning: BackupCommon - inlined tag section Note
> Warning: CpuInstanceProperties - inlined, tag section Note
> Warning: MemoryBackendProperties - inlined tag section Note
>
> Not very many! Just three.
>
> BackupCommon:
>
> Note: @on-source-error and @on-target-error only affect background
>     I/O.  If an error occurs during a guest write request, the
>     device's rerror/werror actions will be used.
>
> BackupCommon is used as a base for DriveBackup and BlockdevBackup. In
> this case, this note probably does belong on both. It should be
> inlined and included.
>
> CpuInstanceProperties:
>
> Note: management should be prepared to pass through additional
>     properties with device_add.
>
> This is only used as a base with no other addon args for
> NumaCpuOptions, in turn only used for an argument. This is probably
> right to pass through, too. (Though I admit I don't really know what
> it means... we can discuss the doc *quality* another day.)

Oh boy, don't get me started!

> MemoryBackendProperties:
>
> Note: prealloc=true and reserve=false cannot be set at the same
>     time.  With reserve=true, the behavior depends on the operating
>     system: for example, Linux will not reserve swap space for
>     shared file mappings -- "not applicable". In contrast,
>     reserve=false will bail out if it cannot be configured
>     accordingly.
>
> This is used in MemoryBackendFileProperties,
> MemoryBackendMemfdProperties, and MemoryBackendEpcProperties. None are
> commands. I think the note here should also be inlined into each
> object.
>
> So, I think that:
>
> - Most naked paragraphs are usually useless to the end-user, and we
> should put them behind a Comment section to prevent them from being
> inlined.

We may want to delete them instead.

> - Any *other* paragraph or section should be included in the
> descendent. We'll just review with that eye going forward.

But where exactly does it go?

The question applies not just to tagged sections such as "Note:", but to
argument, member, and feature descriptions, too.

Our current answer for argument / member descriptions is "right after
the body section".  Works because we permit only the body section before
argument / member descriptions .  Pretty arbitrary restriction, though.

Fine print: can be a bit more complicated for unions, but let's ignore
that here.

I guess our current answer for feature descriptions is something like
"right after argument / member descriptions if they exist, else right
after the body section".

What could our answer be for other sections?

In my experience, the people writing the doc comments rarely check how
they come out in generated documentation.

The closer the doc comments are to the generated documentation, the
higher the chance it comes out as intended.

The more complex the inlining gets, the higher the chance of mishaps.

If cases of complex inlining are rare, we could sidestep complex
inlining by inlining manually.

If that's undesirable or impractical, we could require explicit
instructions where the inlined material should go.

I'm merely thinking aloud; these are certainly not requests, just ideas,
and possibly bad ones.

> - Since: ... we'll talk about later this week.

Yes.

Re: [PATCH 00/27] Add qapi-domain Sphinx extension

[John Snow]

[-- Attachment #1: Type: text/plain, Size: 17591 bytes --]

On Tue, Apr 23, 2024, 3:48 AM Markus Armbruster <armbru@redhat.com> wrote:

> John Snow <jsnow@redhat.com> writes:
>
> > On Mon, Apr 22, 2024 at 12:38 PM John Snow <jsnow@redhat.com> wrote:
> >>
> >> On Mon, Apr 22, 2024 at 5:20 AM Markus Armbruster <armbru@redhat.com>
> wrote:
> >> >
> >> > John Snow <jsnow@redhat.com> writes:
> >> >
> >> > > On Fri, Apr 19, 2024, 10:45 AM Markus Armbruster <armbru@redhat.com>
> wrote:
> >> > >
> >> > >> John Snow <jsnow@redhat.com> writes:
> >> > >>
> >> > >> > This series adds a new qapi-domain extension for Sphinx, which
> adds a
> >> > >> > series of custom directives for documenting QAPI definitions.
>
> [...]
>
> >> > >> > Known issues / points of interest:
>
> [...]
>
> >> > >> > - Inlining all members means there is some ambiguity on what to
> do with
> >> > >> >   doc block sections on inlined entities; features and members
> have an
> >> > >> >   obvious home - body, since, and misc sections are not as
> obvious on
> >> > >> >   how to handle. This will feature more prominently in the
> generator
> >> > >> >   series.
> >> > >>
> >> > >> Yes, this is a real problem.
> >> > >>
> >> > >> The member documentation gets written in the context of the type.
> It
> >> > >> may make sense only in that context.  Inlining copies it into a
> >> > >> different context.
> >> > >>
> >> > >> Features may need to combine.  Say a command's arguments are a
> union
> >> > >> type, and several branches of the union both contain deprecated
> members.
> >> > >> These branch types all document feature @deprecated.  Simply
> inlining
> >> > >> their feature documentation results in feature @deprecated
> documented
> >> > >> several times.  Ugh.  Combining them would be better.  But how?
> Do we
> >> > >> need to rethink how we document features?
> >> > >
> >> > > To be honest, I figured I'd plow ahead and then find the
> counter-examples
> >> > > programmatically and decide what to do once I had my pile of edge
> cases.
> >> > >
> >> > > It might involve rewriting docs in some places, but I think the
> usability
> >> > > win is completely worth the hassle.
> >> > >
> >> > > It's possible there might be some rework needed to maximize cogency
> of the
> >> > > generated docs, but I think prioritizing a user-facing reference
> for QMP is
> >> > > the objectively correct end goal and I'm more than happy to work
> backwards
> >> > > from that desired state.
> >> >
> >> > I'm not disputing the idea that documenting the arguments right with
> the
> >> > command is good.  I'm merely pointing out obstacles to pulling that
> off.
> >> >
> >> > Adjusting existing documentation is only half the battle.  The other
> >> > half is making sure documentation stays adjusted.  We may have to come
> >> > up with new documentation rules, and ways to enforce them.
> >>
> >> For the sake of argument, let's say we forbid everything except
> >> arg/features from definitions destined to be used as base/inherited
> >> types. This would be very easy to enforce at the qapidoc level where
> >> the doc inlining is performed by yelping when the base type contains
> >> additional documentation sections.
> >>
> >> Now, in the real world, maybe sometimes those sections are useful and
> >> we don't want to get rid of them, esp. because they may contain useful
> >> documentation that we don't want to duplicate in the source files.
> >>
> >> My plan is to just forbid them at first and enumerate the cases where
> >> they occur, then decide which ones are better off being moved
> >> elsewhere or explicitly tolerated. The generator's tolerance can be
> >> adjusted accordingly and we can formulate a rule for exactly how doc
> >> blocks are combined and merged. I think it won't be a problem to
> >> enforce it programmatically.
> >>
> >> I'll get back to you on how often and precisely where these cases
> >> occur so you can take a look and see how you feel.
> >>
> >
> > For a warmup, let's look at every unique instance of non-empty
> > paragraph text on an object that is used as a base anywhere:
> >
> > Warning: AudiodevPerDirectionOptions - inlined paragraph
> > Warning: BlockdevOptionsCurlBase - inlined paragraph
> > Warning: BlockdevOptionsGenericCOWFormat - inlined paragraph
> > Warning: BlockdevOptionsGenericFormat - inlined paragraph
> > Warning: BlockExportOptionsNbdBase - inlined paragraph
> > Warning: BlockNodeInfo - inlined paragraph
> > Warning: ChardevCommon - inlined paragraph
> > Warning: CpuInstanceProperties - inlined paragraph
> > Warning: CryptodevBackendProperties - inlined paragraph
> > Warning: EventLoopBaseProperties - inlined paragraph
> > Warning: MemoryBackendProperties - inlined paragraph
> > Warning: NetfilterProperties - inlined paragraph
> > Warning: QCryptoBlockAmendOptionsLUKS - inlined paragraph
> > Warning: QCryptoBlockCreateOptionsLUKS - inlined paragraph
> > Warning: QCryptoBlockInfoBase - inlined paragraph
> > Warning: QCryptoBlockOptionsBase - inlined paragraph
> > Warning: QCryptoBlockOptionsLUKS - inlined paragraph
> > Warning: RngProperties - inlined paragraph
> > Warning: SecretCommonProperties - inlined paragraph
> > Warning: SpiceBasicInfo - inlined paragraph
> > Warning: TlsCredsProperties - inlined paragraph
> > Warning: VncBasicInfo - inlined paragraph
> >
> > There's 22 instances.
> >
> > Let's look at what they say:
> >
> > AudiodevPerDirectionOptions: "General audio backend options that are
> > used for both playback and recording."
> > BlockdevOptionsCurlBase: "Driver specific block device options shared
> > by all protocols supported by the curl backend."
> > BlockdevOptionsGenericCOWFormat: "Driver specific block device options
> > for image format that have no option besides their data source and an
> > optional backing file."
> > BlockdevOptionsGenericFormat: "Driver specific block device options
> > for image format that have no option besides their data source."
> > BlockExportOptionsNbdBase: "An NBD block export (common options shared
> > between nbd-server-add and the NBD branch of block-export-add)."
> > BlockNodeInfo: "Information about a QEMU image file"
> > ChardevCommon: "Configuration shared across all chardev backends"
> >
> > CpuInstanceProperties:
> > "List of properties to be used for hotplugging a CPU instance, it
> > should be passed by management with device_add command when a CPU is
> > being hotplugged.
> >
> > Which members are optional and which mandatory depends on the
> > architecture and board.
> >
> > For s390x see :ref:`cpu-topology-s390x`.
> >
> > The ids other than the node-id specify the position of the CPU
> > within the CPU topology (as defined by the machine property "smp",
> > thus see also type @SMPConfiguration)"
> >
> > CryptodevBackendProperties: "Properties for cryptodev-backend and
> > cryptodev-backend-builtin objects."
> > EventLoopBaseProperties: "Common properties for event loops"
> > MemoryBackendProperties: "Properties for objects of classes derived
> > from memory-backend."
> > NetfilterProperties: "Properties for objects of classes derived from
> netfilter."
> > QCryptoBlockAmendOptionsLUKS: "This struct defines the update
> > parameters that activate/de-activate set of keyslots"
> > QCryptoBlockCreateOptionsLUKS: "The options that apply to LUKS
> > encryption format initialization"
> > QCryptoBlockInfoBase: "The common information that applies to all full
> > disk encryption formats"
> > QCryptoBlockOptionsBase: "The common options that apply to all full
> > disk encryption formats"
> > QCryptoBlockOptionsLUKS: "The options that apply to LUKS encryption
> format"
> > RngProperties: "Properties for objects of classes derived from rng."
> > SecretCommonProperties: "Properties for objects of classes derived
> > from secret-common."
> > SpiceBasicInfo: "The basic information for SPICE network connection"
> > TlsCredsProperties: "Properties for objects of classes derived from
> tls-creds."
> > VncBasicInfo: "The basic information for vnc network connection"
> >
> > ... Alright. So like 98% of the time, it's functionally useless
> > information for the end-user. The only thing that looks mildly
> > interesting is CpuInstanceProperties and *maybe*
> > QCryptoBlockAmendOptionsLUKS.
> >
> > I propose we add a new section to QAPI doc blocks that gets ignored
> > from rendered documentation, like "Comment:" -- to keep any info that
> > might be mildly relevant to a developer that explains the *factoring*
> > of the object, but won't be exposed in user-facing documents.
>
> Two existing ways to add comments that don't go into generated
> documentation:
>
> 1. Write a non-doc comment.
>
>    ##
>    # This is a doc comment.
>    ##
>
>    #
>    # This isn't.
>    #
>
> 2. TODO sections in a doc comment.
>
> Not sure we need more ways, but if we do, we'll create them.
>

Yeah. we could just delete them. Just offering a section in case you don't
want to lose a mandate that every entity should be documented.

I don't have strong feelings beyond "I think we don't need to inline these,
but we should make it obvious when we do or do not."

If you want to just pull out these paragraphs to be comments before/after
the doc block or just delete them, I don't have strong feelings.


> > On the occasion we DO want to inline documentation paragraphs, we can
> > leave them in and have the doc generator inline them. There's probably
> > very few cases where we actually want this.
> >
> > Let's take a look at any tagged sections now, excluding "Since":
> >
> > Warning: BackupCommon - inlined tag section Note
> > Warning: CpuInstanceProperties - inlined, tag section Note
> > Warning: MemoryBackendProperties - inlined tag section Note
> >
> > Not very many! Just three.
> >
> > BackupCommon:
> >
> > Note: @on-source-error and @on-target-error only affect background
> >     I/O.  If an error occurs during a guest write request, the
> >     device's rerror/werror actions will be used.
> >
> > BackupCommon is used as a base for DriveBackup and BlockdevBackup. In
> > this case, this note probably does belong on both. It should be
> > inlined and included.
> >
> > CpuInstanceProperties:
> >
> > Note: management should be prepared to pass through additional
> >     properties with device_add.
> >
> > This is only used as a base with no other addon args for
> > NumaCpuOptions, in turn only used for an argument. This is probably
> > right to pass through, too. (Though I admit I don't really know what
> > it means... we can discuss the doc *quality* another day.)
>
> Oh boy, don't get me started!
>

:)


> > MemoryBackendProperties:
> >
> > Note: prealloc=true and reserve=false cannot be set at the same
> >     time.  With reserve=true, the behavior depends on the operating
> >     system: for example, Linux will not reserve swap space for
> >     shared file mappings -- "not applicable". In contrast,
> >     reserve=false will bail out if it cannot be configured
> >     accordingly.
> >
> > This is used in MemoryBackendFileProperties,
> > MemoryBackendMemfdProperties, and MemoryBackendEpcProperties. None are
> > commands. I think the note here should also be inlined into each
> > object.
> >
> > So, I think that:
> >
> > - Most naked paragraphs are usually useless to the end-user, and we
> > should put them behind a Comment section to prevent them from being
> > inlined.
>
> We may want to delete them instead.
>

Sure, yeah. You're the maintainer, so your call.

(1) New comment section
(2) Pulled out as a non-doc-block comment
(3) Just deleted.


> > - Any *other* paragraph or section should be included in the
> > descendent. We'll just review with that eye going forward.
>
> But where exactly does it go?
>

"Into the HTML. Any other questions?"

;)


> The question applies not just to tagged sections such as "Note:", but to
> argument, member, and feature descriptions, too.


> Our current answer for argument / member descriptions is "right after
> the body section".  Works because we permit only the body section before
> argument / member descriptions .  Pretty arbitrary restriction, though.
>
> Fine print: can be a bit more complicated for unions, but let's ignore
> that here.
>
> I guess our current answer for feature descriptions is something like
> "right after argument / member descriptions if they exist, else right
> after the body section".
>
> What could our answer be for other sections?
>

Inherited members: Injected *before* the member section *when already
present*. i.e., members are source order, ancestor-to-descendent.

Features are the same. The system does not care if they are duplicated.

The only ambiguity arises when the final descendent does not *have* member
or feature sections.

Current prototype's hack: if we leave the doc sections and members/features
remain undocumented, they are appended to the end of the block.

I do agree this isn't ideal. We had a call off-list earlier today where you
suggested the idea of a section or token that would mark the location for
such things to be inlined in the event that there was no obvious place
otherwise. I don't have a better idea; and it would make this action
explicit rather than implicit. It would only be needed in very few doc
blocks and it would be possible to write a very, very detailed error
message in the circumstances where this was missed.

i.e. a simple section that's just simply:

Inherited-Members: ...

or

Inherited-Features: ...

would do perfectly well.

Example error message: 'QAPI definition foo has inherited
<features/members> [from QAPI <meta-type> bar] but no local
<features/members> section in its documentation block. Please annotate
where they should be included in generated documentation by including the
line "Inherited-<Features/Members>: ..." in the doc block.'

(With a pointer to the source file and line where the doc block starts.)

If this line is erroneously *included* in a doc block that doesn't need it,
we can also emit a (fatal) warning urging its removal and an auditing of
the rendered HTML output.

That leaves other sections. We also spoke about the possibility of
eliminating "notes" and "examples" in exchange for explicit ReST/sphinx
syntax. I think that's probably a good idea overall, because it increases
flexibility in how we can annotate examples and remove more custom qapidoc
parsing code.

So, let's assume the only thing left would be "where do we inline inherited
paragraphs?"

How about this:

If we inherit from an object that *has* such paragraphs, then the
descendent needs to mark the spot for its inclusion.

Copying from the above ideas for members/features, how about:

Inherited-paragraphs: ...

The docgen uses this as the splice point. If the splice point is absent and
there are no paragraphs to splice in, there's no problem. If there are, we
can emit a very detailed warning that explains exactly what went wrong and
how to fix it. Similarly, if the splice point is indicated but absent, we
can warn and urge its removal alongside an audit of the generated
documentation.

This way, the feature is self-documenting and self-correcting. It will
catch regressions written before the feature existed and give guidance on
how to fix it. It will catch the large majority of rebase and refactoring
mistakes.

It adds a little complexity to the QAPIDoc parser, but not very much.

(And in fact, with the removal of Notes and Examples, it may indeed be
possible to refactor the parser to be drastically simpler. Alongside using
all_sections, once qapidoc.py is rewritten/replaced, even more drastic
simplifications are possible.)


> In my experience, the people writing the doc comments rarely check how
> they come out in generated documentation.
>
> The closer the doc comments are to the generated documentation, the
> higher the chance it comes out as intended.
>
> The more complex the inlining gets, the higher the chance of mishaps.
>
> If cases of complex inlining are rare, we could sidestep complex
> inlining by inlining manually.
>
> If that's undesirable or impractical, we could require explicit
> instructions where the inlined material should go.
>
> I'm merely thinking aloud; these are certainly not requests, just ideas,
> and possibly bad ones.
>
> > - Since: ... we'll talk about later this week.
>
> Yes.
>

Also discussed on call, but I'm leaving this alone for right now.

Current prototype: inherited "since" sections are ignored but emit a
non-fatal warning. We can audit these cases and decide if that's an
acceptable shortcoming while we work on something more robust, or if it
needs addressing before merge.

(Or if we just issue manual source corrections in the interim before your
proposed changes to generate such info are merged.)

OK.

So: I quite like the idea of inherited placeholders if and only if they are
required, and would like to proceed with prototyping this mechanism. Are
you OK with me proceeding to prototype this?

(It's okay to have reservations and we may still change our mind, but I
want a tentative ACK before I embark on this as it is categorically more
disruptive than any of the changes I've made thus far. i.e. does it
*conceptually* work for you?)

--js

[-- Attachment #2: Type: text/html, Size: 23554 bytes --]

Re: [PATCH 00/27] Add qapi-domain Sphinx extension

[Markus Armbruster]

John Snow <jsnow@redhat.com> writes:

> On Tue, Apr 23, 2024, 3:48 AM Markus Armbruster <armbru@redhat.com> wrote:
>
>> John Snow <jsnow@redhat.com> writes:
>>
>> > On Mon, Apr 22, 2024 at 12:38 PM John Snow <jsnow@redhat.com> wrote:
>> >>
>> >> On Mon, Apr 22, 2024 at 5:20 AM Markus Armbruster <armbru@redhat.com> wrote:
>> >> >
>> >> > John Snow <jsnow@redhat.com> writes:
>> >> >
>> >> > > On Fri, Apr 19, 2024, 10:45 AM Markus Armbruster <armbru@redhat.com> wrote:
>> >> > >
>> >> > >> John Snow <jsnow@redhat.com> writes:
>> >> > >>
>> >> > >> > This series adds a new qapi-domain extension for Sphinx, which adds a
>> >> > >> > series of custom directives for documenting QAPI definitions.
>>
>> [...]
>>
>> >> > >> > Known issues / points of interest:
>>
>> [...]
>>
>> >> > >> > - Inlining all members means there is some ambiguity on what to do with
>> >> > >> >   doc block sections on inlined entities; features and members have an
>> >> > >> >   obvious home - body, since, and misc sections are not as obvious on
>> >> > >> >   how to handle. This will feature more prominently in the generator
>> >> > >> >   series.
>> >> > >>
>> >> > >> Yes, this is a real problem.
>> >> > >>
>> >> > >> The member documentation gets written in the context of the type. It
>> >> > >> may make sense only in that context.  Inlining copies it into a
>> >> > >> different context.
>> >> > >>
>> >> > >> Features may need to combine.  Say a command's arguments are a union
>> >> > >> type, and several branches of the union both contain deprecated members.
>> >> > >> These branch types all document feature @deprecated.  Simply inlining
>> >> > >> their feature documentation results in feature @deprecated documented
>> >> > >> several times.  Ugh.  Combining them would be better.  But how? Do we
>> >> > >> need to rethink how we document features?
>> >> > >
>> >> > > To be honest, I figured I'd plow ahead and then find the counter-examples
>> >> > > programmatically and decide what to do once I had my pile of edge cases.
>> >> > >
>> >> > > It might involve rewriting docs in some places, but I think the usability
>> >> > > win is completely worth the hassle.
>> >> > >
>> >> > > It's possible there might be some rework needed to maximize cogency of the
>> >> > > generated docs, but I think prioritizing a user-facing reference for QMP is
>> >> > > the objectively correct end goal and I'm more than happy to work backwards
>> >> > > from that desired state.
>> >> >
>> >> > I'm not disputing the idea that documenting the arguments right with the
>> >> > command is good.  I'm merely pointing out obstacles to pulling that off.
>> >> >
>> >> > Adjusting existing documentation is only half the battle.  The other
>> >> > half is making sure documentation stays adjusted.  We may have to come
>> >> > up with new documentation rules, and ways to enforce them.
>> >>
>> >> For the sake of argument, let's say we forbid everything except
>> >> arg/features from definitions destined to be used as base/inherited
>> >> types. This would be very easy to enforce at the qapidoc level where
>> >> the doc inlining is performed by yelping when the base type contains
>> >> additional documentation sections.
>> >>
>> >> Now, in the real world, maybe sometimes those sections are useful and
>> >> we don't want to get rid of them, esp. because they may contain useful
>> >> documentation that we don't want to duplicate in the source files.
>> >>
>> >> My plan is to just forbid them at first and enumerate the cases where
>> >> they occur, then decide which ones are better off being moved
>> >> elsewhere or explicitly tolerated. The generator's tolerance can be
>> >> adjusted accordingly and we can formulate a rule for exactly how doc
>> >> blocks are combined and merged. I think it won't be a problem to
>> >> enforce it programmatically.
>> >>
>> >> I'll get back to you on how often and precisely where these cases
>> >> occur so you can take a look and see how you feel.
>> >>
>> >
>> > For a warmup, let's look at every unique instance of non-empty
>> > paragraph text on an object that is used as a base anywhere:
>> >
>> > Warning: AudiodevPerDirectionOptions - inlined paragraph
>> > Warning: BlockdevOptionsCurlBase - inlined paragraph
>> > Warning: BlockdevOptionsGenericCOWFormat - inlined paragraph
>> > Warning: BlockdevOptionsGenericFormat - inlined paragraph
>> > Warning: BlockExportOptionsNbdBase - inlined paragraph
>> > Warning: BlockNodeInfo - inlined paragraph
>> > Warning: ChardevCommon - inlined paragraph
>> > Warning: CpuInstanceProperties - inlined paragraph
>> > Warning: CryptodevBackendProperties - inlined paragraph
>> > Warning: EventLoopBaseProperties - inlined paragraph
>> > Warning: MemoryBackendProperties - inlined paragraph
>> > Warning: NetfilterProperties - inlined paragraph
>> > Warning: QCryptoBlockAmendOptionsLUKS - inlined paragraph
>> > Warning: QCryptoBlockCreateOptionsLUKS - inlined paragraph
>> > Warning: QCryptoBlockInfoBase - inlined paragraph
>> > Warning: QCryptoBlockOptionsBase - inlined paragraph
>> > Warning: QCryptoBlockOptionsLUKS - inlined paragraph
>> > Warning: RngProperties - inlined paragraph
>> > Warning: SecretCommonProperties - inlined paragraph
>> > Warning: SpiceBasicInfo - inlined paragraph
>> > Warning: TlsCredsProperties - inlined paragraph
>> > Warning: VncBasicInfo - inlined paragraph
>> >
>> > There's 22 instances.
>> >
>> > Let's look at what they say:
>> >
>> > AudiodevPerDirectionOptions: "General audio backend options that are
>> > used for both playback and recording."
>> > BlockdevOptionsCurlBase: "Driver specific block device options shared
>> > by all protocols supported by the curl backend."
>> > BlockdevOptionsGenericCOWFormat: "Driver specific block device options
>> > for image format that have no option besides their data source and an
>> > optional backing file."
>> > BlockdevOptionsGenericFormat: "Driver specific block device options
>> > for image format that have no option besides their data source."
>> > BlockExportOptionsNbdBase: "An NBD block export (common options shared
>> > between nbd-server-add and the NBD branch of block-export-add)."
>> > BlockNodeInfo: "Information about a QEMU image file"
>> > ChardevCommon: "Configuration shared across all chardev backends"
>> >
>> > CpuInstanceProperties:
>> > "List of properties to be used for hotplugging a CPU instance, it
>> > should be passed by management with device_add command when a CPU is
>> > being hotplugged.
>> >
>> > Which members are optional and which mandatory depends on the
>> > architecture and board.
>> >
>> > For s390x see :ref:`cpu-topology-s390x`.
>> >
>> > The ids other than the node-id specify the position of the CPU
>> > within the CPU topology (as defined by the machine property "smp",
>> > thus see also type @SMPConfiguration)"
>> >
>> > CryptodevBackendProperties: "Properties for cryptodev-backend and
>> > cryptodev-backend-builtin objects."
>> > EventLoopBaseProperties: "Common properties for event loops"
>> > MemoryBackendProperties: "Properties for objects of classes derived
>> > from memory-backend."
>> > NetfilterProperties: "Properties for objects of classes derived from netfilter."
>> > QCryptoBlockAmendOptionsLUKS: "This struct defines the update
>> > parameters that activate/de-activate set of keyslots"
>> > QCryptoBlockCreateOptionsLUKS: "The options that apply to LUKS
>> > encryption format initialization"
>> > QCryptoBlockInfoBase: "The common information that applies to all full
>> > disk encryption formats"
>> > QCryptoBlockOptionsBase: "The common options that apply to all full
>> > disk encryption formats"
>> > QCryptoBlockOptionsLUKS: "The options that apply to LUKS encryption format"
>> > RngProperties: "Properties for objects of classes derived from rng."
>> > SecretCommonProperties: "Properties for objects of classes derived
>> > from secret-common."
>> > SpiceBasicInfo: "The basic information for SPICE network connection"
>> > TlsCredsProperties: "Properties for objects of classes derived from tls-creds."
>> > VncBasicInfo: "The basic information for vnc network connection"
>> >
>> > ... Alright. So like 98% of the time, it's functionally useless
>> > information for the end-user. The only thing that looks mildly
>> > interesting is CpuInstanceProperties and *maybe*
>> > QCryptoBlockAmendOptionsLUKS.
>> >
>> > I propose we add a new section to QAPI doc blocks that gets ignored
>> > from rendered documentation, like "Comment:" -- to keep any info that
>> > might be mildly relevant to a developer that explains the *factoring*
>> > of the object, but won't be exposed in user-facing documents.
>>
>> Two existing ways to add comments that don't go into generated
>> documentation:
>>
>> 1. Write a non-doc comment.
>>
>>    ##
>>    # This is a doc comment.
>>    ##
>>
>>    #
>>    # This isn't.
>>    #
>>
>> 2. TODO sections in a doc comment.
>>
>> Not sure we need more ways, but if we do, we'll create them.
>
> Yeah. we could just delete them. Just offering a section in case you don't
> want to lose a mandate that every entity should be documented.

I don't want to think about things like "does this need documentation",
or "where does the documentation go".  I want our tools to be able to
decide that for us.

> I don't have strong feelings beyond "I think we don't need to inline these,
> but we should make it obvious when we do or do not."
>
> If you want to just pull out these paragraphs to be comments before/after
> the doc block or just delete them, I don't have strong feelings.

I figure almost all of them are thoroughly uninteresting.  If that turns
out to be the case, we delete the uninteresting ones, then decide what
to do about each of the few interesting ones.

>> > On the occasion we DO want to inline documentation paragraphs, we can
>> > leave them in and have the doc generator inline them. There's probably
>> > very few cases where we actually want this.
>> >
>> > Let's take a look at any tagged sections now, excluding "Since":
>> >
>> > Warning: BackupCommon - inlined tag section Note
>> > Warning: CpuInstanceProperties - inlined, tag section Note
>> > Warning: MemoryBackendProperties - inlined tag section Note
>> >
>> > Not very many! Just three.
>> >
>> > BackupCommon:
>> >
>> > Note: @on-source-error and @on-target-error only affect background
>> >     I/O.  If an error occurs during a guest write request, the
>> >     device's rerror/werror actions will be used.
>> >
>> > BackupCommon is used as a base for DriveBackup and BlockdevBackup. In
>> > this case, this note probably does belong on both. It should be
>> > inlined and included.
>> >
>> > CpuInstanceProperties:
>> >
>> > Note: management should be prepared to pass through additional
>> >     properties with device_add.
>> >
>> > This is only used as a base with no other addon args for
>> > NumaCpuOptions, in turn only used for an argument. This is probably
>> > right to pass through, too. (Though I admit I don't really know what
>> > it means... we can discuss the doc *quality* another day.)
>>
>> Oh boy, don't get me started!
>
> :)
>
>
>> > MemoryBackendProperties:
>> >
>> > Note: prealloc=true and reserve=false cannot be set at the same
>> >     time.  With reserve=true, the behavior depends on the operating
>> >     system: for example, Linux will not reserve swap space for
>> >     shared file mappings -- "not applicable". In contrast,
>> >     reserve=false will bail out if it cannot be configured
>> >     accordingly.
>> >
>> > This is used in MemoryBackendFileProperties,
>> > MemoryBackendMemfdProperties, and MemoryBackendEpcProperties. None are
>> > commands. I think the note here should also be inlined into each
>> > object.
>> >
>> > So, I think that:
>> >
>> > - Most naked paragraphs are usually useless to the end-user, and we
>> > should put them behind a Comment section to prevent them from being
>> > inlined.
>>
>> We may want to delete them instead.
>
> Sure, yeah. You're the maintainer, so your call.
>
> (1) New comment section
> (2) Pulled out as a non-doc-block comment
> (3) Just deleted.
>
>
>> > - Any *other* paragraph or section should be included in the
>> > descendent. We'll just review with that eye going forward.
>>
>> But where exactly does it go?
>
> "Into the HTML. Any other questions?"
>
> ;)

Would you be interested in a product manager role?  ;-P

>> The question applies not just to tagged sections such as "Note:", but to
>> argument, member, and feature descriptions, too.
>
>
>> Our current answer for argument / member descriptions is "right after
>> the body section".  Works because we permit only the body section before
>> argument / member descriptions .  Pretty arbitrary restriction, though.
>>
>> Fine print: can be a bit more complicated for unions, but let's ignore
>> that here.
>>
>> I guess our current answer for feature descriptions is something like
>> "right after argument / member descriptions if they exist, else right
>> after the body section".
>>
>> What could our answer be for other sections?
>
> Inherited members: Injected *before* the member section *when already
> present*. i.e., members are source order, ancestor-to-descendent.
>
> Features are the same. The system does not care if they are duplicated.

The reader might.  We'll see.

> The only ambiguity arises when the final descendent does not *have* member
> or feature sections.

Yes.

> Current prototype's hack: if we leave the doc sections and members/features
> remain undocumented, they are appended to the end of the block.
>
> I do agree this isn't ideal. We had a call off-list earlier today where you
> suggested the idea of a section or token that would mark the location for
> such things to be inlined in the event that there was no obvious place
> otherwise. I don't have a better idea; and it would make this action
> explicit rather than implicit. It would only be needed in very few doc
> blocks and it would be possible to write a very, very detailed error
> message in the circumstances where this was missed.
>
> i.e. a simple section that's just simply:
>
> Inherited-Members: ...
>
> or
>
> Inherited-Features: ...
>
> would do perfectly well.
>
> Example error message: 'QAPI definition foo has inherited
> <features/members> [from QAPI <meta-type> bar] but no local
> <features/members> section in its documentation block. Please annotate
> where they should be included in generated documentation by including the
> line "Inherited-<Features/Members>: ..." in the doc block.'
>
> (With a pointer to the source file and line where the doc block starts.)
>
> If this line is erroneously *included* in a doc block that doesn't need it,
> we can also emit a (fatal) warning urging its removal and an auditing of
> the rendered HTML output.
>
> That leaves other sections. We also spoke about the possibility of
> eliminating "notes" and "examples" in exchange for explicit ReST/sphinx
> syntax. I think that's probably a good idea overall, because it increases
> flexibility in how we can annotate examples and remove more custom qapidoc
> parsing code.

Yes.  We're reinventing reST there for historical reasons only.  And our
reinvented wheel is decidedly less round than reST's.

> So, let's assume the only thing left would be "where do we inline inherited
> paragraphs?"
>
> How about this:
>
> If we inherit from an object that *has* such paragraphs, then the
> descendent needs to mark the spot for its inclusion.
>
> Copying from the above ideas for members/features, how about:
>
> Inherited-paragraphs: ...
>
> The docgen uses this as the splice point. If the splice point is absent and
> there are no paragraphs to splice in, there's no problem. If there are, we
> can emit a very detailed warning that explains exactly what went wrong and
> how to fix it. Similarly, if the splice point is indicated but absent, we
> can warn and urge its removal alongside an audit of the generated
> documentation.
>
> This way, the feature is self-documenting and self-correcting. It will
> catch regressions written before the feature existed and give guidance on
> how to fix it. It will catch the large majority of rebase and refactoring
> mistakes.

Worth exploring.

> It adds a little complexity to the QAPIDoc parser, but not very much.
>
> (And in fact, with the removal of Notes and Examples, it may indeed be
> possible to refactor the parser to be drastically simpler. Alongside using
> all_sections, once qapidoc.py is rewritten/replaced, even more drastic
> simplifications are possible.)

When I rewrote the doc parser, I aimed for such simplifications, but
qapidoc.py resisted change too much to be worth it, so I shelved the
idea.

>> In my experience, the people writing the doc comments rarely check how
>> they come out in generated documentation.
>>
>> The closer the doc comments are to the generated documentation, the
>> higher the chance it comes out as intended.
>>
>> The more complex the inlining gets, the higher the chance of mishaps.
>>
>> If cases of complex inlining are rare, we could sidestep complex
>> inlining by inlining manually.
>>
>> If that's undesirable or impractical, we could require explicit
>> instructions where the inlined material should go.
>>
>> I'm merely thinking aloud; these are certainly not requests, just ideas,
>> and possibly bad ones.
>>
>> > - Since: ... we'll talk about later this week.
>>
>> Yes.
>
> Also discussed on call, but I'm leaving this alone for right now.
>
> Current prototype: inherited "since" sections are ignored but emit a
> non-fatal warning. We can audit these cases and decide if that's an
> acceptable shortcoming while we work on something more robust, or if it
> needs addressing before merge.
>
> (Or if we just issue manual source corrections in the interim before your
> proposed changes to generate such info are merged.)
>
> OK.
>
> So: I quite like the idea of inherited placeholders if and only if they are
> required, and would like to proceed with prototyping this mechanism. Are
> you OK with me proceeding to prototype this?

Yes.

I'm curious how many of them we'll need in practice.

> (It's okay to have reservations and we may still change our mind, but I
> want a tentative ACK before I embark on this as it is categorically more
> disruptive than any of the changes I've made thus far. i.e. does it
> *conceptually* work for you?)

Well, we need *some* solution to make inlining viable.  Since inlining
is desirable, and no better solution comes to mind, let's explore this
one.