qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] qapi: add auto-generated return docs
@ 2025-03-26 19:57 John Snow
  2025-03-26 19:57 ` [PATCH v2 1/4] docs/qapi-domain: add return-nodesc John Snow
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: John Snow @ 2025-03-26 19:57 UTC (permalink / raw)
  To: qemu-devel
  Cc: Jason Wang, Zhao Liu, Fabiano Rosas, Paolo Bonzini, John Snow,
	Mads Ynddal, Hanna Reitz, Eduardo Habkost, Michael S. Tsirkin,
	qemu-trivial, Markus Armbruster, Marc-André Lureau,
	Yanan Wang, qemu-block, Lukas Straub, Jiri Pirko, Stefan Berger,
	Gerd Hoffmann, Peter Maydell, Philippe Mathieu-Daudé,
	Michael Tokarev, Daniel P. Berrangé, Laurent Vivier,
	Zhenwei Pi, Eric Blake, Peter Xu, Ani Sinha, Marcel Apfelbaum,
	Vladimir Sementsov-Ogievskiy, Kevin Wolf, Michael Roth,
	Stefan Hajnoczi, Gonglei (Arei)

v2: fix multi-return-sections bug :(

John Snow (4):
  docs/qapi-domain: add return-nodesc
  docs, qapi: generate undocumented return sections
  qapi: remove trivial "Returns:" sections
  qapi: rephrase return docs to avoid type name

 docs/devel/qapi-domain.rst | 30 ++++++++++++++++++++++++++++++
 docs/sphinx/qapi_domain.py |  8 ++++++++
 docs/sphinx/qapidoc.py     | 14 ++++++++------
 qapi/audio.json            |  2 --
 qapi/block-core.json       | 14 +++-----------
 qapi/block-export.json     |  2 +-
 qapi/block.json            |  2 +-
 qapi/char.json             |  8 --------
 qapi/control.json          |  5 ++---
 qapi/cryptodev.json        |  2 --
 qapi/dump.json             |  5 ++---
 qapi/introspect.json       |  6 +++---
 qapi/job.json              |  2 +-
 qapi/machine-target.json   |  9 +++------
 qapi/machine.json          | 22 ----------------------
 qapi/migration.json        | 12 ------------
 qapi/misc-target.json      | 14 +-------------
 qapi/misc.json             | 12 ++----------
 qapi/net.json              |  2 +-
 qapi/pci.json              |  2 +-
 qapi/qdev.json             |  3 +--
 qapi/qom.json              |  8 +++-----
 qapi/rocker.json           |  4 ----
 qapi/run-state.json        |  2 --
 qapi/stats.json            |  2 +-
 qapi/tpm.json              |  4 ----
 qapi/trace.json            |  2 +-
 qapi/ui.json               | 10 +---------
 qapi/virtio.json           |  8 +++-----
 qapi/yank.json             |  1 -
 scripts/qapi/parser.py     | 15 +++++++++++++++
 scripts/qapi/schema.py     |  3 +++
 32 files changed, 95 insertions(+), 140 deletions(-)

-- 
2.48.1




^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2 1/4] docs/qapi-domain: add return-nodesc
  2025-03-26 19:57 [PATCH v2 0/4] qapi: add auto-generated return docs John Snow
@ 2025-03-26 19:57 ` John Snow
  2025-03-26 19:57 ` [PATCH v2 2/4] docs, qapi: generate undocumented return sections John Snow
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 12+ messages in thread
From: John Snow @ 2025-03-26 19:57 UTC (permalink / raw)
  To: qemu-devel
  Cc: Jason Wang, Zhao Liu, Fabiano Rosas, Paolo Bonzini, John Snow,
	Mads Ynddal, Hanna Reitz, Eduardo Habkost, Michael S. Tsirkin,
	qemu-trivial, Markus Armbruster, Marc-André Lureau,
	Yanan Wang, qemu-block, Lukas Straub, Jiri Pirko, Stefan Berger,
	Gerd Hoffmann, Peter Maydell, Philippe Mathieu-Daudé,
	Michael Tokarev, Daniel P. Berrangé, Laurent Vivier,
	Zhenwei Pi, Eric Blake, Peter Xu, Ani Sinha, Marcel Apfelbaum,
	Vladimir Sementsov-Ogievskiy, Kevin Wolf, Michael Roth,
	Stefan Hajnoczi, Gonglei (Arei)

This form is used to annotate a return type without an accompanying
description, for when there is no "Returns:" information in the source
doc, but we have a return type we want to generate a cross-reference to.

The syntax is:

:return-nodesc: TypeName

It's primarily necessary because Sphinx always expects both a type and a
description for the prior form and will format it accordingly. To have a
reasonable rendering when the body is missing, we need to use a
different info field list entirely.

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

diff --git a/docs/devel/qapi-domain.rst b/docs/devel/qapi-domain.rst
index a748529f515..5ca060fa04c 100644
--- a/docs/devel/qapi-domain.rst
+++ b/docs/devel/qapi-domain.rst
@@ -242,6 +242,36 @@ Example::
              }
 
 
+``:return-nodesc:``
+-------------------
+
+Document the return type of a QAPI command, without an accompanying description.
+
+:availability: This field list is only available in the body of the
+               Command directive.
+:syntax: ``:return-nodesc: type``
+:type: `sphinx.util.docfields.Field
+       <https://pydoc.dev/sphinx/latest/sphinx.util.docfields.Field.html?private=1>`_
+
+
+Example::
+
+   .. qapi:command:: query-replay
+      :since: 5.2
+
+      Retrieve the record/replay information.  It includes current
+      instruction count which may be used for ``replay-break`` and
+      ``replay-seek`` commands.
+
+      :return-nodesc: ReplayInfo
+
+      .. qmp-example::
+
+          -> { "execute": "query-replay" }
+          <- { "return": {
+                 "mode": "play", "filename": "log.rr", "icount": 220414 }
+             }
+
 ``:value:``
 -----------
 
diff --git a/docs/sphinx/qapi_domain.py b/docs/sphinx/qapi_domain.py
index c94af5719ca..d6d4a70f3df 100644
--- a/docs/sphinx/qapi_domain.py
+++ b/docs/sphinx/qapi_domain.py
@@ -529,6 +529,14 @@ class QAPICommand(QAPIObject):
                 names=("return",),
                 can_collapse=True,
             ),
+            # :return-nodesc: TypeName
+            CompatField(
+                "returnvalue",
+                label=_("Return"),
+                names=("return-nodesc",),
+                bodyrolename="type",
+                has_arg=False,
+            ),
         ]
     )
 
-- 
2.48.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 2/4] docs, qapi: generate undocumented return sections
  2025-03-26 19:57 [PATCH v2 0/4] qapi: add auto-generated return docs John Snow
  2025-03-26 19:57 ` [PATCH v2 1/4] docs/qapi-domain: add return-nodesc John Snow
@ 2025-03-26 19:57 ` John Snow
  2025-03-27  9:11   ` Markus Armbruster
  2025-03-26 19:57 ` [PATCH v2 3/4] qapi: remove trivial "Returns:" sections John Snow
  2025-03-26 19:57 ` [PATCH v2 4/4] qapi: rephrase return docs to avoid type name John Snow
  3 siblings, 1 reply; 12+ messages in thread
From: John Snow @ 2025-03-26 19:57 UTC (permalink / raw)
  To: qemu-devel
  Cc: Jason Wang, Zhao Liu, Fabiano Rosas, Paolo Bonzini, John Snow,
	Mads Ynddal, Hanna Reitz, Eduardo Habkost, Michael S. Tsirkin,
	qemu-trivial, Markus Armbruster, Marc-André Lureau,
	Yanan Wang, qemu-block, Lukas Straub, Jiri Pirko, Stefan Berger,
	Gerd Hoffmann, Peter Maydell, Philippe Mathieu-Daudé,
	Michael Tokarev, Daniel P. Berrangé, Laurent Vivier,
	Zhenwei Pi, Eric Blake, Peter Xu, Ani Sinha, Marcel Apfelbaum,
	Vladimir Sementsov-Ogievskiy, Kevin Wolf, Michael Roth,
	Stefan Hajnoczi, Gonglei (Arei)

This patch changes the qapidoc transmogrifier to generate Return value
documentation for any command that has a return value but hasn't
explicitly documented that return value.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 docs/sphinx/qapidoc.py | 14 ++++++++------
 scripts/qapi/parser.py | 15 +++++++++++++++
 scripts/qapi/schema.py |  3 +++
 3 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
index 0930307bc73..aaf9921c06c 100644
--- a/docs/sphinx/qapidoc.py
+++ b/docs/sphinx/qapidoc.py
@@ -255,16 +255,18 @@ def visit_feature(self, section: QAPIDoc.ArgSection) -> None:
     def visit_returns(self, section: QAPIDoc.Section) -> None:
         assert isinstance(self.entity, QAPISchemaCommand)
         rtype = self.entity.ret_type
-        # q_empty can produce None, but we won't be documenting anything
-        # without an explicit return statement in the doc block, and we
-        # should not have any such explicit statements when there is no
-        # return value.
+        # return statements will not be present (and won't be
+        # autogenerated) for any command that doesn't return
+        # *something*, so ret_type will always be defined here.
         assert rtype
 
         typ = self.format_type(rtype)
         assert typ
-        assert section.text
-        self.add_field("return", typ, section.text, section.info)
+
+        if section.text:
+            self.add_field("return", typ, section.text, section.info)
+        else:
+            self.add_lines(f":return-nodesc: {typ}", section.info)
 
     def visit_errors(self, section: QAPIDoc.Section) -> None:
         # FIXME: the formatting for errors may be inconsistent and may
diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index 949d9e8bff7..6db08f82409 100644
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -815,6 +815,21 @@ def connect_feature(self, feature: 'QAPISchemaFeature') -> None:
                                % feature.name)
         self.features[feature.name].connect(feature)
 
+    def ensure_returns(self, info: QAPISourceInfo) -> None:
+        if not any(s.kind == QAPIDoc.Kind.RETURNS for s in self.all_sections):
+
+            stub = QAPIDoc.Section(info, QAPIDoc.Kind.RETURNS)
+
+            # Stub "Returns" section for undocumented returns value.
+            # Insert stub after the last non-PLAIN section.
+            for sect in reversed(self.all_sections):
+                if sect.kind != QAPIDoc.Kind.PLAIN:
+                    idx = self.all_sections.index(sect) + 1
+                    self.all_sections.insert(idx, stub)
+                    break
+            else:
+                self.all_sections.append(stub)
+
     def check_expr(self, expr: QAPIExpression) -> None:
         if 'command' in expr:
             if self.returns and 'returns' not in expr:
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
index cbe3b5aa91e..3abddea3525 100644
--- a/scripts/qapi/schema.py
+++ b/scripts/qapi/schema.py
@@ -1062,6 +1062,9 @@ def connect_doc(self, doc: Optional[QAPIDoc] = None) -> None:
             if self.arg_type and self.arg_type.is_implicit():
                 self.arg_type.connect_doc(doc)
 
+            if self.ret_type and self.info:
+                doc.ensure_returns(self.info)
+
     def visit(self, visitor: QAPISchemaVisitor) -> None:
         super().visit(visitor)
         visitor.visit_command(
-- 
2.48.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 3/4] qapi: remove trivial "Returns:" sections
  2025-03-26 19:57 [PATCH v2 0/4] qapi: add auto-generated return docs John Snow
  2025-03-26 19:57 ` [PATCH v2 1/4] docs/qapi-domain: add return-nodesc John Snow
  2025-03-26 19:57 ` [PATCH v2 2/4] docs, qapi: generate undocumented return sections John Snow
@ 2025-03-26 19:57 ` John Snow
  2025-03-26 19:57 ` [PATCH v2 4/4] qapi: rephrase return docs to avoid type name John Snow
  3 siblings, 0 replies; 12+ messages in thread
From: John Snow @ 2025-03-26 19:57 UTC (permalink / raw)
  To: qemu-devel
  Cc: Jason Wang, Zhao Liu, Fabiano Rosas, Paolo Bonzini, John Snow,
	Mads Ynddal, Hanna Reitz, Eduardo Habkost, Michael S. Tsirkin,
	qemu-trivial, Markus Armbruster, Marc-André Lureau,
	Yanan Wang, qemu-block, Lukas Straub, Jiri Pirko, Stefan Berger,
	Gerd Hoffmann, Peter Maydell, Philippe Mathieu-Daudé,
	Michael Tokarev, Daniel P. Berrangé, Laurent Vivier,
	Zhenwei Pi, Eric Blake, Peter Xu, Ani Sinha, Marcel Apfelbaum,
	Vladimir Sementsov-Ogievskiy, Kevin Wolf, Michael Roth,
	Stefan Hajnoczi, Gonglei (Arei)

The new qapidoc transmogrifier can generate "Returns" statements with
type information just fine, so we can remove it from the source where it
doesn't add anything particularly novel or helpful and just repeats the
type info.

This patch does not touch Returns: lines that add some information
(potentially helpful, potentially not) but repeats the type information
to remove that type.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 qapi/audio.json          |  2 --
 qapi/block-core.json     |  8 --------
 qapi/char.json           |  8 --------
 qapi/cryptodev.json      |  2 --
 qapi/machine-target.json |  2 --
 qapi/machine.json        | 22 ----------------------
 qapi/migration.json      | 12 ------------
 qapi/misc-target.json    | 12 ------------
 qapi/misc.json           |  7 -------
 qapi/rocker.json         |  4 ----
 qapi/run-state.json      |  2 --
 qapi/tpm.json            |  4 ----
 qapi/ui.json             |  8 --------
 qapi/virtio.json         |  2 --
 qapi/yank.json           |  1 -
 15 files changed, 96 deletions(-)

diff --git a/qapi/audio.json b/qapi/audio.json
index dd5a58d13e6..d49ca4cae47 100644
--- a/qapi/audio.json
+++ b/qapi/audio.json
@@ -535,8 +535,6 @@
 #
 # Returns information about audiodev configuration
 #
-# Returns: array of @Audiodev
-#
 # Since: 8.0
 ##
 { 'command': 'query-audiodevs',
diff --git a/qapi/block-core.json b/qapi/block-core.json
index b1937780e19..92b2e368b72 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1949,8 +1949,6 @@
 # @flat: Omit the nested data about backing image ("backing-image"
 #     key) if true.  Default is false (Since 5.0)
 #
-# Returns: the list of BlockDeviceInfo
-#
 # Since: 2.0
 #
 # .. qmp-example::
@@ -2464,9 +2462,6 @@
 #
 # @unstable: This command is meant for debugging.
 #
-# Returns:
-#     BlockDirtyBitmapSha256
-#
 # Errors:
 #     - If @node is not a valid block device, DeviceNotFound
 #     - If @name is not found or if hashing has failed, GenericError
@@ -6157,9 +6152,6 @@
 #
 # @name: optional the snapshot's name to be deleted
 #
-# Returns:
-#     SnapshotInfo
-#
 # Errors:
 #     - If @device is not a valid block device, GenericError
 #     - If snapshot not found, GenericError
diff --git a/qapi/char.json b/qapi/char.json
index dde2f9538f8..6a82db0d156 100644
--- a/qapi/char.json
+++ b/qapi/char.json
@@ -36,8 +36,6 @@
 #
 # Returns information about current character devices.
 #
-# Returns: a list of @ChardevInfo
-#
 # Since: 0.14
 #
 # .. qmp-example::
@@ -82,8 +80,6 @@
 #
 # Returns information about character device backends.
 #
-# Returns: a list of @ChardevBackendInfo
-#
 # Since: 2.0
 #
 # .. qmp-example::
@@ -772,8 +768,6 @@
 #
 # @backend: backend type and parameters
 #
-# Returns: ChardevReturn.
-#
 # Since: 1.4
 #
 # .. qmp-example::
@@ -812,8 +806,6 @@
 #
 # @backend: new backend type and parameters
 #
-# Returns: ChardevReturn.
-#
 # Since: 2.10
 #
 # .. qmp-example::
diff --git a/qapi/cryptodev.json b/qapi/cryptodev.json
index 04d0e21d209..e8371b9d950 100644
--- a/qapi/cryptodev.json
+++ b/qapi/cryptodev.json
@@ -96,8 +96,6 @@
 #
 # Returns information about current crypto devices.
 #
-# Returns: a list of @QCryptodevInfo
-#
 # Since: 8.0
 ##
 { 'command': 'query-cryptodev', 'returns': ['QCryptodevInfo']}
diff --git a/qapi/machine-target.json b/qapi/machine-target.json
index 541f93eeb78..37e75520094 100644
--- a/qapi/machine-target.json
+++ b/qapi/machine-target.json
@@ -391,8 +391,6 @@
 #
 # Return a list of supported virtual CPU definitions
 #
-# Returns: a list of CpuDefinitionInfo
-#
 # Since: 1.2
 ##
 { 'command': 'query-cpu-definitions', 'returns': ['CpuDefinitionInfo'],
diff --git a/qapi/machine.json b/qapi/machine.json
index a6b8795b09e..415d39a1d68 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -101,8 +101,6 @@
 #
 # Returns information about all virtual CPUs.
 #
-# Returns: list of @CpuInfoFast
-#
 # Since: 2.12
 #
 # .. qmp-example::
@@ -218,8 +216,6 @@
 #
 # @unstable: Argument @compat-props is experimental.
 #
-# Returns: a list of MachineInfo
-#
 # Since: 1.2
 #
 # .. qmp-example::
@@ -268,8 +264,6 @@
 #
 # Return information on the current virtual machine.
 #
-# Returns: CurrentMachineParams
-#
 # Since: 4.0
 ##
 { 'command': 'query-current-machine', 'returns': 'CurrentMachineParams' }
@@ -291,8 +285,6 @@
 #
 # Return information about the target for this QEMU
 #
-# Returns: TargetInfo
-#
 # Since: 1.2
 ##
 { 'command': 'query-target', 'returns': 'TargetInfo' }
@@ -316,8 +308,6 @@
 #
 # Query the guest UUID information.
 #
-# Returns: The @UuidInfo for the guest
-#
 # Since: 0.14
 #
 # .. qmp-example::
@@ -469,8 +459,6 @@
 #
 # Returns information about KVM acceleration
 #
-# Returns: @KvmInfo
-#
 # Since: 0.14
 #
 # .. qmp-example::
@@ -932,8 +920,6 @@
 #
 # Returns information for all memory backends.
 #
-# Returns: a list of @Memdev.
-#
 # Since: 2.1
 #
 # .. qmp-example::
@@ -1049,8 +1035,6 @@
 #
 # TODO: Better documentation; currently there is none.
 #
-# Returns: a list of HotpluggableCPU objects.
-#
 # Since: 2.7
 #
 # .. qmp-example::
@@ -1172,9 +1156,6 @@
 #
 # Return information about the balloon device.
 #
-# Returns:
-#     @BalloonInfo
-#
 # Errors:
 #     - If the balloon driver is enabled but not functional because
 #       the KVM kernel module cannot support it, KVMMissingCap
@@ -1238,9 +1219,6 @@
 # Returns the hv-balloon driver data contained in the last received
 # "STATUS" message from the guest.
 #
-# Returns:
-#     @HvBalloonInfo
-#
 # Errors:
 #     - If no hv-balloon device is present, guest memory status
 #       reporting is not enabled or no guest memory status report
diff --git a/qapi/migration.json b/qapi/migration.json
index 8b9c53595c4..b0e9452e7fa 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -286,8 +286,6 @@
 # is active there will be another json-object with RAM migration
 # status.
 #
-# Returns: @MigrationInfo
-#
 # Since: 0.14
 #
 # .. qmp-example::
@@ -537,8 +535,6 @@
 #
 # Returns information about the current migration capabilities status
 #
-# Returns: @MigrationCapabilityStatus
-#
 # Since: 1.2
 #
 # .. qmp-example::
@@ -1322,8 +1318,6 @@
 #
 # Returns information about the current migration parameters
 #
-# Returns: @MigrationParameters
-#
 # Since: 2.4
 #
 # .. qmp-example::
@@ -1904,8 +1898,6 @@
 #
 # Query replication status while the vm is running.
 #
-# Returns: A @ReplicationStatus object showing the status.
-#
 # .. qmp-example::
 #
 #     -> { "execute": "query-xen-replication-status" }
@@ -1958,8 +1950,6 @@
 #
 # Query COLO status while the vm is running.
 #
-# Returns: A @COLOStatus object showing the status.
-#
 # .. qmp-example::
 #
 #     -> { "execute": "query-colo-status" }
@@ -2333,8 +2323,6 @@
 #
 # @deprecated: This command is deprecated with no replacement yet.
 #
-# Returns: @MigrationThreadInfo
-#
 # Since: 7.2
 ##
 { 'command': 'query-migrationthreads',
diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index 8d70bd24d8c..59a8f5b2bed 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -129,8 +129,6 @@
 #
 # Returns information about SEV
 #
-# Returns: @SevInfo
-#
 # Since: 2.12
 #
 # .. qmp-example::
@@ -205,8 +203,6 @@
 # This command is used to get the SEV capabilities, and is supported
 # on AMD X86 platforms only.
 #
-# Returns: SevCapability objects.
-#
 # Since: 2.12
 #
 # .. qmp-example::
@@ -259,8 +255,6 @@
 # @mnonce: a random 16 bytes value encoded in base64 (it will be
 #     included in report)
 #
-# Returns: SevAttestationReport objects.
-#
 # Since: 6.1
 #
 # .. qmp-example::
@@ -324,8 +318,6 @@
 # This command is ARM-only.  It will return a list of GICCapability
 # objects that describe its capability bits.
 #
-# Returns: a list of GICCapability objects.
-#
 # Since: 2.6
 #
 # .. qmp-example::
@@ -382,8 +374,6 @@
 #
 # Returns information about SGX
 #
-# Returns: @SGXInfo
-#
 # Since: 6.2
 #
 # .. qmp-example::
@@ -401,8 +391,6 @@
 #
 # Returns information from host SGX capabilities
 #
-# Returns: @SGXInfo
-#
 # Since: 6.2
 #
 # .. qmp-example::
diff --git a/qapi/misc.json b/qapi/misc.json
index 559b66f2017..de5dd531071 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -56,8 +56,6 @@
 #
 # Return the name information of a guest.
 #
-# Returns: @NameInfo of the guest
-#
 # Since: 0.14
 #
 # .. qmp-example::
@@ -332,9 +330,6 @@
 #
 # @opaque: A free-form string that can be used to describe the fd.
 #
-# Returns:
-#     @AddfdInfo
-#
 # Errors:
 #     - If file descriptor was not received, GenericError
 #     - If @fdset-id is a negative value, GenericError
@@ -415,8 +410,6 @@
 #
 # Return information describing all fd sets.
 #
-# Returns: A list of @FdsetInfo
-#
 # Since: 1.2
 #
 # .. note:: The list of fd sets is shared by all monitor connections.
diff --git a/qapi/rocker.json b/qapi/rocker.json
index 51aa5b49307..d1714a08d71 100644
--- a/qapi/rocker.json
+++ b/qapi/rocker.json
@@ -28,8 +28,6 @@
 #
 # @name: switch name
 #
-# Returns: @Rocker information
-#
 # Since: 2.4
 #
 # .. qmp-example::
@@ -98,8 +96,6 @@
 #
 # @name: port name
 #
-# Returns: a list of @RockerPort information
-#
 # Since: 2.4
 #
 # .. qmp-example::
diff --git a/qapi/run-state.json b/qapi/run-state.json
index ce95cfa46b7..ff2d694ee2f 100644
--- a/qapi/run-state.json
+++ b/qapi/run-state.json
@@ -119,8 +119,6 @@
 #
 # Query the run status of the VM
 #
-# Returns: @StatusInfo reflecting the VM
-#
 # Since: 0.14
 #
 # .. qmp-example::
diff --git a/qapi/tpm.json b/qapi/tpm.json
index a16a72edb98..f749e6869df 100644
--- a/qapi/tpm.json
+++ b/qapi/tpm.json
@@ -27,8 +27,6 @@
 #
 # Return a list of supported TPM models
 #
-# Returns: a list of TpmModel
-#
 # Since: 1.5
 #
 # .. qmp-example::
@@ -58,8 +56,6 @@
 #
 # Return a list of supported TPM types
 #
-# Returns: a list of TpmType
-#
 # Since: 1.5
 #
 # .. qmp-example::
diff --git a/qapi/ui.json b/qapi/ui.json
index c536d4e5241..46843bdbefa 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -325,8 +325,6 @@
 #
 # Returns information about the current SPICE server
 #
-# Returns: @SpiceInfo
-#
 # Since: 0.14
 #
 # .. qmp-example::
@@ -656,8 +654,6 @@
 #
 # Returns information about the current VNC server
 #
-# Returns: @VncInfo
-#
 # Since: 0.14
 #
 # .. qmp-example::
@@ -687,8 +683,6 @@
 #
 # Returns a list of vnc servers.  The list can be empty.
 #
-# Returns: a list of @VncInfo2
-#
 # Since: 2.3
 ##
 { 'command': 'query-vnc-servers', 'returns': ['VncInfo2'],
@@ -1564,8 +1558,6 @@
 #
 # Returns information about display configuration
 #
-# Returns: @DisplayOptions
-#
 # Since: 3.1
 ##
 { 'command': 'query-display-options',
diff --git a/qapi/virtio.json b/qapi/virtio.json
index d351d2166ef..93c576a21da 100644
--- a/qapi/virtio.json
+++ b/qapi/virtio.json
@@ -847,8 +847,6 @@
 #
 # @unstable: This command is meant for debugging.
 #
-# Returns: VirtioQueueElement information
-#
 # Since: 7.2
 #
 # .. qmp-example::
diff --git a/qapi/yank.json b/qapi/yank.json
index 30f46c97c98..9bd8ecce27f 100644
--- a/qapi/yank.json
+++ b/qapi/yank.json
@@ -102,7 +102,6 @@
 #
 # Query yank instances.  See @YankInstance for more information.
 #
-# Returns: list of @YankInstance
 #
 # .. qmp-example::
 #
-- 
2.48.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v2 4/4] qapi: rephrase return docs to avoid type name
  2025-03-26 19:57 [PATCH v2 0/4] qapi: add auto-generated return docs John Snow
                   ` (2 preceding siblings ...)
  2025-03-26 19:57 ` [PATCH v2 3/4] qapi: remove trivial "Returns:" sections John Snow
@ 2025-03-26 19:57 ` John Snow
  2025-03-28  8:36   ` Markus Armbruster
  3 siblings, 1 reply; 12+ messages in thread
From: John Snow @ 2025-03-26 19:57 UTC (permalink / raw)
  To: qemu-devel
  Cc: Jason Wang, Zhao Liu, Fabiano Rosas, Paolo Bonzini, John Snow,
	Mads Ynddal, Hanna Reitz, Eduardo Habkost, Michael S. Tsirkin,
	qemu-trivial, Markus Armbruster, Marc-André Lureau,
	Yanan Wang, qemu-block, Lukas Straub, Jiri Pirko, Stefan Berger,
	Gerd Hoffmann, Peter Maydell, Philippe Mathieu-Daudé,
	Michael Tokarev, Daniel P. Berrangé, Laurent Vivier,
	Zhenwei Pi, Eric Blake, Peter Xu, Ani Sinha, Marcel Apfelbaum,
	Vladimir Sementsov-Ogievskiy, Kevin Wolf, Michael Roth,
	Stefan Hajnoczi, Gonglei (Arei)

Well, I tried. Maybe not very hard. Sorry!

Signed-off-by: John Snow <jsnow@redhat.com>
---
 qapi/block-core.json     | 6 +++---
 qapi/block-export.json   | 2 +-
 qapi/block.json          | 2 +-
 qapi/control.json        | 5 ++---
 qapi/dump.json           | 5 ++---
 qapi/introspect.json     | 6 +++---
 qapi/job.json            | 2 +-
 qapi/machine-target.json | 7 +++----
 qapi/misc-target.json    | 2 +-
 qapi/misc.json           | 5 ++---
 qapi/net.json            | 2 +-
 qapi/pci.json            | 2 +-
 qapi/qdev.json           | 3 +--
 qapi/qom.json            | 8 +++-----
 qapi/stats.json          | 2 +-
 qapi/trace.json          | 2 +-
 qapi/ui.json             | 2 +-
 qapi/virtio.json         | 6 +++---
 18 files changed, 31 insertions(+), 38 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 92b2e368b72..eb97b70cd80 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -763,7 +763,7 @@
 #
 # Get a list of BlockInfo for all virtual block devices.
 #
-# Returns: a list of @BlockInfo describing each virtual block device.
+# Returns: a list describing each virtual block device.
 #     Filter nodes that were created implicitly are skipped over.
 #
 # Since: 0.14
@@ -1168,7 +1168,7 @@
 #     nodes that were created implicitly are skipped over in this
 #     mode.  (Since 2.3)
 #
-# Returns: A list of @BlockStats for each virtual block devices.
+# Returns: A list of statistics for each virtual block device.
 #
 # Since: 0.14
 #
@@ -1440,7 +1440,7 @@
 #
 # Return information about long-running block device operations.
 #
-# Returns: a list of @BlockJobInfo for each active block job
+# Returns: a list of job info for each active block job
 #
 # Since: 1.1
 ##
diff --git a/qapi/block-export.json b/qapi/block-export.json
index c783e01a532..84852606e52 100644
--- a/qapi/block-export.json
+++ b/qapi/block-export.json
@@ -472,7 +472,7 @@
 ##
 # @query-block-exports:
 #
-# Returns: A list of BlockExportInfo describing all block exports
+# Returns: A list describing all block exports
 #
 # Since: 5.2
 ##
diff --git a/qapi/block.json b/qapi/block.json
index e66666f5c64..bdbbe78854f 100644
--- a/qapi/block.json
+++ b/qapi/block.json
@@ -86,7 +86,7 @@
 # Returns a list of information about each persistent reservation
 # manager.
 #
-# Returns: a list of @PRManagerInfo for each persistent reservation
+# Returns: a list of manager info for each persistent reservation
 #     manager
 #
 # Since: 3.0
diff --git a/qapi/control.json b/qapi/control.json
index 336386f79e1..2e45bf25df8 100644
--- a/qapi/control.json
+++ b/qapi/control.json
@@ -93,8 +93,7 @@
 #
 # Returns the current version of QEMU.
 #
-# Returns: A @VersionInfo object describing the current version of
-#     QEMU.
+# Returns: An object describing the current version of QEMU.
 #
 # Since: 0.14
 #
@@ -131,7 +130,7 @@
 #
 # Return a list of supported QMP commands by this server
 #
-# Returns: A list of @CommandInfo for all supported commands
+# Returns: A list of all supported commands
 #
 # Since: 0.14
 #
diff --git a/qapi/dump.json b/qapi/dump.json
index d7826c0e323..1bd6bacc5ce 100644
--- a/qapi/dump.json
+++ b/qapi/dump.json
@@ -146,7 +146,7 @@
 #
 # Query latest dump status.
 #
-# Returns: A @DumpStatus object showing the dump status.
+# Returns: An object showing the dump status.
 #
 # Since: 2.6
 #
@@ -197,8 +197,7 @@
 #
 # Returns the available formats for dump-guest-memory
 #
-# Returns: A @DumpGuestMemoryCapability object listing available
-#     formats for dump-guest-memory
+# Returns: An object listing available formats for dump-guest-memory
 #
 # Since: 2.0
 #
diff --git a/qapi/introspect.json b/qapi/introspect.json
index 01bb242947c..7daec5045fb 100644
--- a/qapi/introspect.json
+++ b/qapi/introspect.json
@@ -34,10 +34,10 @@
 # string into a specific enum or from one specific type into an
 # alternate that includes the original type alongside something else.
 #
-# Returns: array of @SchemaInfo, where each element describes an
-#     entity in the ABI: command, event, type, ...
+# Returns: an array where each element describes an entity in the ABI:
+#     command, event, type, ...
 #
-#     The order of the various SchemaInfo is unspecified; however, all
+#     The order of the various elements is unspecified; however, all
 #     names are guaranteed to be unique (no name will be duplicated
 #     with different meta-types).
 #
diff --git a/qapi/job.json b/qapi/job.json
index cfc3beedd21..856dd688f95 100644
--- a/qapi/job.json
+++ b/qapi/job.json
@@ -269,7 +269,7 @@
 #
 # Return information about jobs.
 #
-# Returns: a list with a @JobInfo for each active job
+# Returns: a list with info for each active job
 #
 # Since: 3.0
 ##
diff --git a/qapi/machine-target.json b/qapi/machine-target.json
index 37e75520094..6d8a6e53436 100644
--- a/qapi/machine-target.json
+++ b/qapi/machine-target.json
@@ -162,8 +162,7 @@
 # @modelb: description of the second CPU model to compare, referred to
 #     as "model B" in CpuModelCompareResult
 #
-# Returns: a CpuModelCompareInfo describing how both CPU models
-#     compare
+# Returns: An object describing how both CPU models compare
 #
 # Errors:
 #     - if comparing CPU models is not supported
@@ -218,7 +217,7 @@
 #
 # @modelb: description of the second CPU model to baseline
 #
-# Returns: a CpuModelBaselineInfo describing the baselined CPU model
+# Returns: An object describing the baselined CPU model
 #
 # Errors:
 #     - if baselining CPU models is not supported
@@ -296,7 +295,7 @@
 #
 # @type: expansion type, specifying how to expand the CPU model
 #
-# Returns: a CpuModelExpansionInfo describing the expanded CPU model
+# Returns: An object describing the expanded CPU model
 #
 # Errors:
 #     - if expanding CPU models is not supported
diff --git a/qapi/misc-target.json b/qapi/misc-target.json
index 59a8f5b2bed..295d63df76b 100644
--- a/qapi/misc-target.json
+++ b/qapi/misc-target.json
@@ -158,7 +158,7 @@
 #
 # Query the SEV guest launch information.
 #
-# Returns: The @SevLaunchMeasureInfo for the guest
+# Returns: The guest's SEV guest launch measurement info
 #
 # Since: 2.12
 #
diff --git a/qapi/misc.json b/qapi/misc.json
index de5dd531071..3d10aeb215c 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -105,7 +105,7 @@
 #    declared using the ``-object iothread`` command-line option.  It
 #    is always the main thread of the process.
 #
-# Returns: a list of @IOThreadInfo for each iothread
+# Returns: a list of info for each iothread
 #
 # Since: 2.0
 #
@@ -509,8 +509,7 @@
 #
 # @option: option name
 #
-# Returns: list of @CommandLineOptionInfo for all options (or for the
-#     given @option).
+# Returns: list of objects for all options (or for the given @option).
 #
 # Errors:
 #     - if the given @option doesn't exist
diff --git a/qapi/net.json b/qapi/net.json
index 310cc4fd190..43739fd0259 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -845,7 +845,7 @@
 #
 # @name: net client name
 #
-# Returns: list of @RxFilterInfo for all NICs (or for the given NIC).
+# Returns: list of info for all NICs (or for the given NIC).
 #
 # Errors:
 #     - if the given @name doesn't exist
diff --git a/qapi/pci.json b/qapi/pci.json
index dc85a41d28b..29549d94551 100644
--- a/qapi/pci.json
+++ b/qapi/pci.json
@@ -175,7 +175,7 @@
 #
 # Return information about the PCI bus topology of the guest.
 #
-# Returns: a list of @PciInfo for each PCI bus.  Each bus is
+# Returns: a list of info for each PCI bus.  Each bus is
 #     represented by a json-object, which has a key with a json-array
 #     of all PCI devices attached to it.  Each device is represented
 #     by a json-object.
diff --git a/qapi/qdev.json b/qapi/qdev.json
index 25cbcf977b4..55a509071e9 100644
--- a/qapi/qdev.json
+++ b/qapi/qdev.json
@@ -17,8 +17,7 @@
 #
 # @typename: the type name of a device
 #
-# Returns: a list of ObjectPropertyInfo describing a devices
-#     properties
+# Returns: a list describing a devices properties
 #
 # .. note:: Objects can create properties at runtime, for example to
 #    describe links between different devices and/or objects.  These
diff --git a/qapi/qom.json b/qapi/qom.json
index 28ce24cd8d0..b053e8bf0c7 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -54,8 +54,7 @@
 # @path: the path within the object model.  See @qom-get for a
 #     description of this parameter.
 #
-# Returns: a list of @ObjectPropertyInfo that describe the properties
-#     of the object.
+# Returns: a list that describe the properties of the object.
 #
 # Since: 1.2
 #
@@ -178,8 +177,7 @@
 #
 # @abstract: if true, include abstract types in the results
 #
-# Returns: a list of @ObjectTypeInfo or an empty list if no results
-#     are found
+# Returns: a list of types, or an empty list if no results are found
 #
 # Since: 1.1
 ##
@@ -199,7 +197,7 @@
 #    describe links between different devices and/or objects.  These
 #    properties are not included in the output of this command.
 #
-# Returns: a list of ObjectPropertyInfo describing object properties
+# Returns: a list describing object properties
 #
 # Since: 2.12
 ##
diff --git a/qapi/stats.json b/qapi/stats.json
index 8902ef94e08..7e7f1dabbc3 100644
--- a/qapi/stats.json
+++ b/qapi/stats.json
@@ -186,7 +186,7 @@
 # The arguments are a StatsFilter and specify the provider and objects
 # to return statistics about.
 #
-# Returns: a list of StatsResult, one for each provider and object
+# Returns: a list of statistics, one for each provider and object
 #     (e.g., for each vCPU).
 #
 # Since: 7.1
diff --git a/qapi/trace.json b/qapi/trace.json
index eb5f63f5135..11f0b5c3427 100644
--- a/qapi/trace.json
+++ b/qapi/trace.json
@@ -47,7 +47,7 @@
 #
 # @name: Event name pattern (case-sensitive glob).
 #
-# Returns: a list of @TraceEventInfo for the matching events
+# Returns: a list of info for the matching events
 #
 # Since: 2.2
 #
diff --git a/qapi/ui.json b/qapi/ui.json
index 46843bdbefa..a1015801b1b 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -816,7 +816,7 @@
 #
 # Returns information about each active mouse device
 #
-# Returns: a list of @MouseInfo for each device
+# Returns: a list of info for each device
 #
 # Since: 0.14
 #
diff --git a/qapi/virtio.json b/qapi/virtio.json
index 93c576a21da..cee0e100d44 100644
--- a/qapi/virtio.json
+++ b/qapi/virtio.json
@@ -199,7 +199,7 @@
 #
 # @unstable: This command is meant for debugging.
 #
-# Returns: VirtioStatus of the virtio device
+# Returns: Status of the virtio device
 #
 # Since: 7.2
 #
@@ -563,7 +563,7 @@
 #
 # @unstable: This command is meant for debugging.
 #
-# Returns: VirtQueueStatus of the VirtQueue
+# Returns: Status of the queue
 #
 # .. note:: last_avail_idx will not be displayed in the case where the
 #    selected VirtIODevice has a running vhost device and the
@@ -698,7 +698,7 @@
 #
 # @unstable: This command is meant for debugging.
 #
-# Returns: VirtVhostQueueStatus of the vhost_virtqueue
+# Returns: Status of the vhost_virtqueue
 #
 # Since: 7.2
 #
-- 
2.48.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/4] docs, qapi: generate undocumented return sections
  2025-03-26 19:57 ` [PATCH v2 2/4] docs, qapi: generate undocumented return sections John Snow
@ 2025-03-27  9:11   ` Markus Armbruster
  2025-03-31 18:30     ` John Snow
  0 siblings, 1 reply; 12+ messages in thread
From: Markus Armbruster @ 2025-03-27  9:11 UTC (permalink / raw)
  To: John Snow
  Cc: qemu-devel, Jason Wang, Zhao Liu, Fabiano Rosas, Paolo Bonzini,
	Mads Ynddal, Hanna Reitz, Eduardo Habkost, Michael S. Tsirkin,
	qemu-trivial, Marc-André Lureau, Yanan Wang, qemu-block,
	Lukas Straub, Jiri Pirko, Stefan Berger, Gerd Hoffmann,
	Peter Maydell, Philippe Mathieu-Daudé, Michael Tokarev,
	Daniel P. Berrangé, Laurent Vivier, Zhenwei Pi, Eric Blake,
	Peter Xu, Ani Sinha, Marcel Apfelbaum,
	Vladimir Sementsov-Ogievskiy, Kevin Wolf, Michael Roth,
	Stefan Hajnoczi, Gonglei (Arei)

John Snow <jsnow@redhat.com> writes:

> This patch changes the qapidoc transmogrifier to generate Return value
> documentation for any command that has a return value but hasn't
> explicitly documented that return value.
>
> Signed-off-by: John Snow <jsnow@redhat.com>

Might want to briefly explain placement of the auto-generated return
value documentation.  But before we discuss that any further, let's
review the actual changes the the generated docs.

This patch adds auto-generated return value documentation where we have
none.

The next patch replaces handwritten by auto-generated return value
documentation where these are at least as good.  Moves the return value
docs in some cases.

First the additions:

* x-debug-query-block-graph

  Title, intro, features, return

* query-tpm

  Title, intro, return, example

* query-dirty-rate

  Title, intro, arguments, return, examples

* query-vcpu-dirty-limit

  Title, intro, return, example

* query-vm-generation-id

  Title, return

* query-memory-size-summary

  Title, intro, example, return

* query-memory-devices

  Title, intro, return, example

* query-acpi-ospm-status

  Title, intro, return, example

* query-stats-schemas

  Title, intro, arguments, note, return

Undesirable:

* query-memory-size-summary has returns after the example instead of
  before.  I figure it needs the TODO hack to separate intro and example
  (see announce-self).

* query-stats-schemas has a note between arguments and return.  I think
  this demonstrates that the placement algorithm is too simplistic.

Debatable:

* x-debug-query-block-graph has returns after features.  I'd prefer
  returns before features.  No need to debate this now.

Next the movements:

* x-debug-block-dirty-bitmap-sha256

  From right before errors to right after

* blockdev-snapshot-delete-internal-sync

  From right before errors to right after

* query-xen-replication-status

  From between intro and example to the end

* query-colo-status

  From between intro and example to the end

* query-balloon

  From right before errors to right after

* query-hv-balloon-status-report

  From right before errors to right after

* query-yank

  From between intro and example to the end

* add-fd

  From between arguments and errors to between last note and example

I don't like any of these :)

Undesirable:

* query-xen-replication-status, query-yank, and query-colo-status now
  have return after the example instead of before.  I figure they now
  need the TODO hack to separate intro and example.

* add-fd now has a note between arguments and return.  Same placement
  weakness as for query-stats above.

Debatable:

* x-debug-block-dirty-bitmap-sha256,
  blockdev-snapshot-delete-internal-sync, query-colo-status, and
  query-hv-balloon-status-report now have return after errors instead of
  before.  I'd prefer before.

What's the stupidest acceptable placement algorithm?  Maybe this one:

1. If we have arguments, return goes right after them.

2. Else if we have errors, return goes right before them.

3. Else if we have features, return goes right before them.

4. Else return goes right after the intro (to make this work, we need
   a few more TODO hacks).

Would you be willing to give this a try?



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 4/4] qapi: rephrase return docs to avoid type name
  2025-03-26 19:57 ` [PATCH v2 4/4] qapi: rephrase return docs to avoid type name John Snow
@ 2025-03-28  8:36   ` Markus Armbruster
  2025-03-31 18:34     ` John Snow
  0 siblings, 1 reply; 12+ messages in thread
From: Markus Armbruster @ 2025-03-28  8:36 UTC (permalink / raw)
  To: John Snow
  Cc: qemu-devel, Jason Wang, Zhao Liu, Fabiano Rosas, Paolo Bonzini,
	Mads Ynddal, Hanna Reitz, Eduardo Habkost, Michael S. Tsirkin,
	qemu-trivial, Marc-André Lureau, Yanan Wang, qemu-block,
	Lukas Straub, Jiri Pirko, Stefan Berger, Gerd Hoffmann,
	Peter Maydell, Philippe Mathieu-Daudé, Michael Tokarev,
	Daniel P. Berrangé, Laurent Vivier, Zhenwei Pi, Eric Blake,
	Peter Xu, Ani Sinha, Marcel Apfelbaum,
	Vladimir Sementsov-Ogievskiy, Kevin Wolf, Michael Roth,
	Stefan Hajnoczi, Gonglei (Arei)

John Snow <jsnow@redhat.com> writes:

> Well, I tried. Maybe not very hard. Sorry!

No need to be sorry!  Kick-starting discussion with limited effort is
better than a big effort going into a direction that turns out to be
unwanted once we discuss it.

Instead of just rephrasing Returns descriptions, I'd like us to consider
both Returns and intro.  See below for why.

> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  qapi/block-core.json     | 6 +++---
>  qapi/block-export.json   | 2 +-
>  qapi/block.json          | 2 +-
>  qapi/control.json        | 5 ++---
>  qapi/dump.json           | 5 ++---
>  qapi/introspect.json     | 6 +++---
>  qapi/job.json            | 2 +-
>  qapi/machine-target.json | 7 +++----
>  qapi/misc-target.json    | 2 +-
>  qapi/misc.json           | 5 ++---
>  qapi/net.json            | 2 +-
>  qapi/pci.json            | 2 +-
>  qapi/qdev.json           | 3 +--
>  qapi/qom.json            | 8 +++-----
>  qapi/stats.json          | 2 +-
>  qapi/trace.json          | 2 +-
>  qapi/ui.json             | 2 +-
>  qapi/virtio.json         | 6 +++---
>  18 files changed, 31 insertions(+), 38 deletions(-)
>
> diff --git a/qapi/block-core.json b/qapi/block-core.json
> index 92b2e368b72..eb97b70cd80 100644
> --- a/qapi/block-core.json
> +++ b/qapi/block-core.json
> @@ -763,7 +763,7 @@
   ##
   # @query-block:
>  #
>  # Get a list of BlockInfo for all virtual block devices.
>  #
> -# Returns: a list of @BlockInfo describing each virtual block device.
> +# Returns: a list describing each virtual block device.
>  #     Filter nodes that were created implicitly are skipped over.
>  #
>  # Since: 0.14
> @@ -1168,7 +1168,7 @@
   ##
   # @query-blockstats:
   #
   # Query the @BlockStats for all virtual block devices.
   #
   # @query-nodes: If true, the command will query all the block nodes
   #     that have a node name, in a list which will include "parent"
   #     information, but not "backing".  If false or omitted, the
   #     behavior is as before - query all the device backends,
   #     recursively including their "parent" and "backing".  Filter
>  #     nodes that were created implicitly are skipped over in this
>  #     mode.  (Since 2.3)
>  #
> -# Returns: A list of @BlockStats for each virtual block devices.
> +# Returns: A list of statistics for each virtual block device.
>  #
>  # Since: 0.14
>  #
> @@ -1440,7 +1440,7 @@
   ##
   # @query-block-jobs:
>  #
>  # Return information about long-running block device operations.
>  #
> -# Returns: a list of @BlockJobInfo for each active block job
> +# Returns: a list of job info for each active block job
>  #
>  # Since: 1.1
>  ##

Stopping here, because I believe this is already enough to make a useful
observation.

There's overlap between query-FOO's intro and Returns.  The intro
describes the command's purpose, and the purpose of a query-FOO is to
return certain information.  Returns describes the value returned.  Both
talk about the same thing, namely the value returned.

Why two prose descriptions of the value returned?  Wouldn't one be
better?

Let's try.  We need to choose where to put it, intro or Returns.

When Returns comes right after intro, putting it into Returns can work
fine, because the command's purpose still comes first.  For instance:

   ##
   # @query-block:
   #
   # Return information about all virtual block devices.  Filter nodes
   # that were created implicitly are skipped over.

renders like

    Command query-block (Since: 0.14)

       Return information about all virtual block devices. Filter nodes
       that were created implicitly are skipped over.

       Return:
          ["BlockInfo"]

and

   ##
   # @query-block:
   #
   # Returns: Information about all virtual block devices.
   #     Filter nodes that were created implicitly are skipped over.

renders like

    Command query-block (Since: 0.14)

       Return:
          ["BlockInfo"] -- Information about all virtual block
          devices. Filter nodes that were created implicitly are skipped
          over.

Both work.

But when there's something in between, intro is preferable.  For
instance:

   ##
   # @query-blockstats:
   #
   # Return statistics for all virtual block devices.
   #
   # @query-nodes: If true, the command will query all the block nodes
   #     that have a node name, in a list which will include "parent"
   #     information, but not "backing".  If false or omitted, the
   #     behavior is as before - query all the device backends,
   #     recursively including their "parent" and "backing".  Filter
   #     nodes that were created implicitly are skipped over in this
   #     mode.  (Since 2.3)

renders like

    Command query-blockstats (Since: 0.14)

       Return statistics for all virtual block devices.

       Arguments:
          * **query-nodes** ("boolean", *optional*) -- If true, the
            command will query all the block nodes that have a node name,
            in a list which will include "parent" information, but not
            "backing".  If false or omitted, the behavior is as before -
            query all the device backends, recursively including their
            "parent" and "backing".  Filter nodes that were created
            implicitly are skipped over in this mode.  (Since 2.3)

       Return:
          ["BlockStats"]

while

   ##
   # @query-blockstats:
   #
   # @query-nodes: If true, the command will query all the block nodes
   #     that have a node name, in a list which will include "parent"
   #     information, but not "backing".  If false or omitted, the
   #     behavior is as before - query all the device backends,
   #     recursively including their "parent" and "backing".  Filter
   #     nodes that were created implicitly are skipped over in this
   #     mode.  (Since 2.3)
   #
   # Returns: Statistics for all virtual block devices

renders like

    Command query-blockstats (Since: 0.14)

       Arguments:
          * **query-nodes** ("boolean", *optional*) -- If true, the
            command will query all the block nodes that have a node name,
            in a list which will include "parent" information, but not
            "backing".  If false or omitted, the behavior is as before -
            query all the device backends, recursively including their
            "parent" and "backing".  Filter nodes that were created
            implicitly are skipped over in this mode.  (Since 2.3)

       Return:
          ["BlockStats"] -- Statistics for all virtual block devices

I much prefer the former, because it puts the command's purpose where it
belongs: first.

Perhaps we should simply use the intro always.

Thoughts?



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/4] docs, qapi: generate undocumented return sections
  2025-03-27  9:11   ` Markus Armbruster
@ 2025-03-31 18:30     ` John Snow
  2025-04-01  6:07       ` Markus Armbruster
  0 siblings, 1 reply; 12+ messages in thread
From: John Snow @ 2025-03-31 18:30 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: qemu-devel, Jason Wang, Zhao Liu, Fabiano Rosas, Paolo Bonzini,
	Mads Ynddal, Hanna Reitz, Eduardo Habkost, Michael S. Tsirkin,
	qemu-trivial, Marc-André Lureau, Yanan Wang, qemu-block,
	Lukas Straub, Jiri Pirko, Stefan Berger, Gerd Hoffmann,
	Peter Maydell, Philippe Mathieu-Daudé, Michael Tokarev,
	Daniel P. Berrangé, Laurent Vivier, Zhenwei Pi, Eric Blake,
	Peter Xu, Ani Sinha, Marcel Apfelbaum,
	Vladimir Sementsov-Ogievskiy, Kevin Wolf, Michael Roth,
	Stefan Hajnoczi, Gonglei (Arei)

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

On Thu, Mar 27, 2025 at 5:11 AM Markus Armbruster <armbru@redhat.com> wrote:

> John Snow <jsnow@redhat.com> writes:
>
> > This patch changes the qapidoc transmogrifier to generate Return value
> > documentation for any command that has a return value but hasn't
> > explicitly documented that return value.
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
>
> Might want to briefly explain placement of the auto-generated return
> value documentation.  But before we discuss that any further, let's
> review the actual changes the the generated docs.
>
> This patch adds auto-generated return value documentation where we have
> none.
>
> The next patch replaces handwritten by auto-generated return value
> documentation where these are at least as good.  Moves the return value
> docs in some cases.
>
> First the additions:
>
> * x-debug-query-block-graph
>
>   Title, intro, features, return
>
> * query-tpm
>
>   Title, intro, return, example
>
> * query-dirty-rate
>
>   Title, intro, arguments, return, examples
>
> * query-vcpu-dirty-limit
>
>   Title, intro, return, example
>
> * query-vm-generation-id
>
>   Title, return
>
> * query-memory-size-summary
>
>   Title, intro, example, return
>
> * query-memory-devices
>
>   Title, intro, return, example
>
> * query-acpi-ospm-status
>
>   Title, intro, return, example
>
> * query-stats-schemas
>
>   Title, intro, arguments, note, return
>
> Undesirable:
>
> * query-memory-size-summary has returns after the example instead of
>   before.  I figure it needs the TODO hack to separate intro and example
>   (see announce-self).
>

Yes


>
> * query-stats-schemas has a note between arguments and return.  I think
>   this demonstrates that the placement algorithm is too simplistic.
>

Yeah ... It's just that *glances at length of email* it's been a sensitive
topic without a lot of certainty in desire, so I've tried to keep things on
the stupid/simple side ...


>
> Debatable:
>
> * x-debug-query-block-graph has returns after features.  I'd prefer
>   returns before features.  No need to debate this now.
>

Willing to do this for you if you'd like to enforce it globally. I'd be
happy with any mandated order of sections, really.


>
> Next the movements:
>
> * x-debug-block-dirty-bitmap-sha256
>
>   From right before errors to right after
>
> * blockdev-snapshot-delete-internal-sync
>
>   From right before errors to right after
>
> * query-xen-replication-status
>
>   From between intro and example to the end
>
> * query-colo-status
>
>   From between intro and example to the end
>
> * query-balloon
>
>   From right before errors to right after
>
> * query-hv-balloon-status-report
>
>   From right before errors to right after
>
> * query-yank
>
>   From between intro and example to the end
>
> * add-fd
>
>   From between arguments and errors to between last note and example
>
> I don't like any of these :)
>

So it goes ...


>
> Undesirable:
>
> * query-xen-replication-status, query-yank, and query-colo-status now
>   have return after the example instead of before.  I figure they now
>   need the TODO hack to separate intro and example.
>

Yes


>
> * add-fd now has a note between arguments and return.  Same placement
>   weakness as for query-stats above.
>
> Debatable:
>
> * x-debug-block-dirty-bitmap-sha256,
>   blockdev-snapshot-delete-internal-sync, query-colo-status, and
>   query-hv-balloon-status-report now have return after errors instead of
>   before.  I'd prefer before.
>
> What's the stupidest acceptable placement algorithm?  Maybe this one:
>
> 1. If we have arguments, return goes right after them.
>
> 2. Else if we have errors, return goes right before them.
>
> 3. Else if we have features, return goes right before them.
>
> 4. Else return goes right after the intro (to make this work, we need
>    a few more TODO hacks).
>
> Would you be willing to give this a try?
>

Probably ...

So this algorithm seems to imply a preference for this ordering:

1. Intro
2. Arguments
3. Return
4. Errors
5. Features
6. Details

Do I have that correct?

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 4/4] qapi: rephrase return docs to avoid type name
  2025-03-28  8:36   ` Markus Armbruster
@ 2025-03-31 18:34     ` John Snow
  2025-04-01  6:10       ` Markus Armbruster
  0 siblings, 1 reply; 12+ messages in thread
From: John Snow @ 2025-03-31 18:34 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: qemu-devel, Jason Wang, Zhao Liu, Fabiano Rosas, Paolo Bonzini,
	Mads Ynddal, Hanna Reitz, Eduardo Habkost, Michael S. Tsirkin,
	qemu-trivial, Marc-André Lureau, Yanan Wang, qemu-block,
	Lukas Straub, Jiri Pirko, Stefan Berger, Gerd Hoffmann,
	Peter Maydell, Philippe Mathieu-Daudé, Michael Tokarev,
	Daniel P. Berrangé, Laurent Vivier, Zhenwei Pi, Eric Blake,
	Peter Xu, Ani Sinha, Marcel Apfelbaum,
	Vladimir Sementsov-Ogievskiy, Kevin Wolf, Michael Roth,
	Stefan Hajnoczi, Gonglei (Arei)

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

On Fri, Mar 28, 2025 at 4:36 AM Markus Armbruster <armbru@redhat.com> wrote:

> John Snow <jsnow@redhat.com> writes:
>
> > Well, I tried. Maybe not very hard. Sorry!
>
> No need to be sorry!  Kick-starting discussion with limited effort is
> better than a big effort going into a direction that turns out to be
> unwanted once we discuss it.
>
> Instead of just rephrasing Returns descriptions, I'd like us to consider
> both Returns and intro.  See below for why.
>
> > Signed-off-by: John Snow <jsnow@redhat.com>
> > ---
> >  qapi/block-core.json     | 6 +++---
> >  qapi/block-export.json   | 2 +-
> >  qapi/block.json          | 2 +-
> >  qapi/control.json        | 5 ++---
> >  qapi/dump.json           | 5 ++---
> >  qapi/introspect.json     | 6 +++---
> >  qapi/job.json            | 2 +-
> >  qapi/machine-target.json | 7 +++----
> >  qapi/misc-target.json    | 2 +-
> >  qapi/misc.json           | 5 ++---
> >  qapi/net.json            | 2 +-
> >  qapi/pci.json            | 2 +-
> >  qapi/qdev.json           | 3 +--
> >  qapi/qom.json            | 8 +++-----
> >  qapi/stats.json          | 2 +-
> >  qapi/trace.json          | 2 +-
> >  qapi/ui.json             | 2 +-
> >  qapi/virtio.json         | 6 +++---
> >  18 files changed, 31 insertions(+), 38 deletions(-)
> >
> > diff --git a/qapi/block-core.json b/qapi/block-core.json
> > index 92b2e368b72..eb97b70cd80 100644
> > --- a/qapi/block-core.json
> > +++ b/qapi/block-core.json
> > @@ -763,7 +763,7 @@
>    ##
>    # @query-block:
> >  #
> >  # Get a list of BlockInfo for all virtual block devices.
> >  #
> > -# Returns: a list of @BlockInfo describing each virtual block device.
> > +# Returns: a list describing each virtual block device.
> >  #     Filter nodes that were created implicitly are skipped over.
> >  #
> >  # Since: 0.14
> > @@ -1168,7 +1168,7 @@
>    ##
>    # @query-blockstats:
>    #
>    # Query the @BlockStats for all virtual block devices.
>    #
>    # @query-nodes: If true, the command will query all the block nodes
>    #     that have a node name, in a list which will include "parent"
>    #     information, but not "backing".  If false or omitted, the
>    #     behavior is as before - query all the device backends,
>    #     recursively including their "parent" and "backing".  Filter
> >  #     nodes that were created implicitly are skipped over in this
> >  #     mode.  (Since 2.3)
> >  #
> > -# Returns: A list of @BlockStats for each virtual block devices.
> > +# Returns: A list of statistics for each virtual block device.
> >  #
> >  # Since: 0.14
> >  #
> > @@ -1440,7 +1440,7 @@
>    ##
>    # @query-block-jobs:
> >  #
> >  # Return information about long-running block device operations.
> >  #
> > -# Returns: a list of @BlockJobInfo for each active block job
> > +# Returns: a list of job info for each active block job
> >  #
> >  # Since: 1.1
> >  ##
>
> Stopping here, because I believe this is already enough to make a useful
> observation.
>
> There's overlap between query-FOO's intro and Returns.  The intro
> describes the command's purpose, and the purpose of a query-FOO is to
> return certain information.  Returns describes the value returned.  Both
> talk about the same thing, namely the value returned.
>
> Why two prose descriptions of the value returned?  Wouldn't one be
> better?
>
> Let's try.  We need to choose where to put it, intro or Returns.
>
> When Returns comes right after intro, putting it into Returns can work
> fine, because the command's purpose still comes first.  For instance:
>
>    ##
>    # @query-block:
>    #
>    # Return information about all virtual block devices.  Filter nodes
>    # that were created implicitly are skipped over.
>
> renders like
>
>     Command query-block (Since: 0.14)
>
>        Return information about all virtual block devices. Filter nodes
>        that were created implicitly are skipped over.
>
>        Return:
>           ["BlockInfo"]
>
> and
>
>    ##
>    # @query-block:
>    #
>    # Returns: Information about all virtual block devices.
>    #     Filter nodes that were created implicitly are skipped over.
>
> renders like
>
>     Command query-block (Since: 0.14)
>
>        Return:
>           ["BlockInfo"] -- Information about all virtual block
>           devices. Filter nodes that were created implicitly are skipped
>           over.
>
> Both work.
>
> But when there's something in between, intro is preferable.  For
> instance:
>
>    ##
>    # @query-blockstats:
>    #
>    # Return statistics for all virtual block devices.
>    #
>    # @query-nodes: If true, the command will query all the block nodes
>    #     that have a node name, in a list which will include "parent"
>    #     information, but not "backing".  If false or omitted, the
>    #     behavior is as before - query all the device backends,
>    #     recursively including their "parent" and "backing".  Filter
>    #     nodes that were created implicitly are skipped over in this
>    #     mode.  (Since 2.3)
>
> renders like
>
>     Command query-blockstats (Since: 0.14)
>
>        Return statistics for all virtual block devices.
>
>        Arguments:
>           * **query-nodes** ("boolean", *optional*) -- If true, the
>             command will query all the block nodes that have a node name,
>             in a list which will include "parent" information, but not
>             "backing".  If false or omitted, the behavior is as before -
>             query all the device backends, recursively including their
>             "parent" and "backing".  Filter nodes that were created
>             implicitly are skipped over in this mode.  (Since 2.3)
>
>        Return:
>           ["BlockStats"]
>
> while
>
>    ##
>    # @query-blockstats:
>    #
>    # @query-nodes: If true, the command will query all the block nodes
>    #     that have a node name, in a list which will include "parent"
>    #     information, but not "backing".  If false or omitted, the
>    #     behavior is as before - query all the device backends,
>    #     recursively including their "parent" and "backing".  Filter
>    #     nodes that were created implicitly are skipped over in this
>    #     mode.  (Since 2.3)
>    #
>    # Returns: Statistics for all virtual block devices
>
> renders like
>
>     Command query-blockstats (Since: 0.14)
>
>        Arguments:
>           * **query-nodes** ("boolean", *optional*) -- If true, the
>             command will query all the block nodes that have a node name,
>             in a list which will include "parent" information, but not
>             "backing".  If false or omitted, the behavior is as before -
>             query all the device backends, recursively including their
>             "parent" and "backing".  Filter nodes that were created
>             implicitly are skipped over in this mode.  (Since 2.3)
>
>        Return:
>           ["BlockStats"] -- Statistics for all virtual block devices
>
> I much prefer the former, because it puts the command's purpose where it
> belongs: first.
>
> Perhaps we should simply use the intro always.
>

I think for cases where the doc block is short and we have a desire to
merge "returns" and "intro", the intro makes the most sense if there isn't
anything of particular value assigned to the return value to begin with.

So, more or less, yeah: if semantics are partially duplicated between
intro/returns, I'm in favor of putting it all in the intro and allowing
transmogrifier generate the return type info.

I don't think there's a good case to make for a doc block with no intro but
a healthy paragraph in the returns section, that looks goofy.

Of course: I think there are still circumstances where we'll want both the
intro and the returns info explicitly labeled, when we have some
information to document about the semantics of that return value.


>
> Thoughts?
>
>

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/4] docs, qapi: generate undocumented return sections
  2025-03-31 18:30     ` John Snow
@ 2025-04-01  6:07       ` Markus Armbruster
  2025-04-03 21:05         ` John Snow
  0 siblings, 1 reply; 12+ messages in thread
From: Markus Armbruster @ 2025-04-01  6:07 UTC (permalink / raw)
  To: John Snow
  Cc: qemu-devel, Jason Wang, Zhao Liu, Fabiano Rosas, Paolo Bonzini,
	Mads Ynddal, Hanna Reitz, Eduardo Habkost, Michael S. Tsirkin,
	qemu-trivial, Marc-André Lureau, Yanan Wang, qemu-block,
	Lukas Straub, Jiri Pirko, Stefan Berger, Gerd Hoffmann,
	Peter Maydell, Philippe Mathieu-Daudé, Michael Tokarev,
	Daniel P. Berrangé, Laurent Vivier, Zhenwei Pi, Eric Blake,
	Peter Xu, Ani Sinha, Marcel Apfelbaum,
	Vladimir Sementsov-Ogievskiy, Kevin Wolf, Michael Roth,
	Stefan Hajnoczi, Gonglei (Arei)

John Snow <jsnow@redhat.com> writes:

> On Thu, Mar 27, 2025 at 5:11 AM Markus Armbruster <armbru@redhat.com> wrote:
>
>> John Snow <jsnow@redhat.com> writes:
>>
>> > This patch changes the qapidoc transmogrifier to generate Return value
>> > documentation for any command that has a return value but hasn't
>> > explicitly documented that return value.
>> >
>> > Signed-off-by: John Snow <jsnow@redhat.com>
>>
>> Might want to briefly explain placement of the auto-generated return
>> value documentation.  But before we discuss that any further, let's
>> review the actual changes the the generated docs.
>>
>> This patch adds auto-generated return value documentation where we have
>> none.
>>
>> The next patch replaces handwritten by auto-generated return value
>> documentation where these are at least as good.  Moves the return value
>> docs in some cases.
>>
>> First the additions:
>>
>> * x-debug-query-block-graph
>>
>>   Title, intro, features, return
>>
>> * query-tpm
>>
>>   Title, intro, return, example
>>
>> * query-dirty-rate
>>
>>   Title, intro, arguments, return, examples
>>
>> * query-vcpu-dirty-limit
>>
>>   Title, intro, return, example
>>
>> * query-vm-generation-id
>>
>>   Title, return
>>
>> * query-memory-size-summary
>>
>>   Title, intro, example, return
>>
>> * query-memory-devices
>>
>>   Title, intro, return, example
>>
>> * query-acpi-ospm-status
>>
>>   Title, intro, return, example
>>
>> * query-stats-schemas
>>
>>   Title, intro, arguments, note, return
>>
>> Undesirable:
>>
>> * query-memory-size-summary has returns after the example instead of
>>   before.  I figure it needs the TODO hack to separate intro and example
>>   (see announce-self).
>>
>
> Yes
>
>
>>
>> * query-stats-schemas has a note between arguments and return.  I think
>>   this demonstrates that the placement algorithm is too simplistic.
>>
>
> Yeah ... It's just that *glances at length of email* it's been a sensitive
> topic without a lot of certainty in desire, so I've tried to keep things on
> the stupid/simple side ...

When the best solution is unclear, starting discussion with a simplistic
solution is smart.  Beats starting with a complicated solution that gets
shot down.

>> Debatable:
>>
>> * x-debug-query-block-graph has returns after features.  I'd prefer
>>   returns before features.  No need to debate this now.
>>
>
> Willing to do this for you if you'd like to enforce it globally. I'd be
> happy with any mandated order of sections, really.

Could a more rigid order help the inliner, too?

>> Next the movements:
>>
>> * x-debug-block-dirty-bitmap-sha256
>>
>>   From right before errors to right after
>>
>> * blockdev-snapshot-delete-internal-sync
>>
>>   From right before errors to right after
>>
>> * query-xen-replication-status
>>
>>   From between intro and example to the end
>>
>> * query-colo-status
>>
>>   From between intro and example to the end
>>
>> * query-balloon
>>
>>   From right before errors to right after
>>
>> * query-hv-balloon-status-report
>>
>>   From right before errors to right after
>>
>> * query-yank
>>
>>   From between intro and example to the end
>>
>> * add-fd
>>
>>   From between arguments and errors to between last note and example
>>
>> I don't like any of these :)
>>
>
> So it goes ...
>
>
>>
>> Undesirable:
>>
>> * query-xen-replication-status, query-yank, and query-colo-status now
>>   have return after the example instead of before.  I figure they now
>>   need the TODO hack to separate intro and example.
>>
>
> Yes
>
>
>>
>> * add-fd now has a note between arguments and return.  Same placement
>>   weakness as for query-stats above.
>>
>> Debatable:
>>
>> * x-debug-block-dirty-bitmap-sha256,
>>   blockdev-snapshot-delete-internal-sync, query-colo-status, and
>>   query-hv-balloon-status-report now have return after errors instead of
>>   before.  I'd prefer before.
>>
>> What's the stupidest acceptable placement algorithm?  Maybe this one:
>>
>> 1. If we have arguments, return goes right after them.
>>
>> 2. Else if we have errors, return goes right before them.
>>
>> 3. Else if we have features, return goes right before them.
>>
>> 4. Else return goes right after the intro (to make this work, we need
>>    a few more TODO hacks).
>>
>> Would you be willing to give this a try?
>>
>
> Probably ...
>
> So this algorithm seems to imply a preference for this ordering:
>
> 1. Intro
> 2. Arguments
> 3. Return
> 4. Errors
> 5. Features
> 6. Details
>
> Do I have that correct?

Yes, with

  7. Since

although a case could also be made for

  1. Intro
  2. Arguments
  3. Return
  4. Errors
  5. Features
  6. Since
  7. Details



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 4/4] qapi: rephrase return docs to avoid type name
  2025-03-31 18:34     ` John Snow
@ 2025-04-01  6:10       ` Markus Armbruster
  0 siblings, 0 replies; 12+ messages in thread
From: Markus Armbruster @ 2025-04-01  6:10 UTC (permalink / raw)
  To: John Snow
  Cc: qemu-devel, Jason Wang, Zhao Liu, Fabiano Rosas, Paolo Bonzini,
	Mads Ynddal, Hanna Reitz, Eduardo Habkost, Michael S. Tsirkin,
	qemu-trivial, Marc-André Lureau, Yanan Wang, qemu-block,
	Lukas Straub, Jiri Pirko, Stefan Berger, Gerd Hoffmann,
	Peter Maydell, Philippe Mathieu-Daudé, Michael Tokarev,
	Daniel P. Berrangé, Laurent Vivier, Zhenwei Pi, Eric Blake,
	Peter Xu, Ani Sinha, Marcel Apfelbaum,
	Vladimir Sementsov-Ogievskiy, Kevin Wolf, Michael Roth,
	Stefan Hajnoczi, Gonglei (Arei)

John Snow <jsnow@redhat.com> writes:

> On Fri, Mar 28, 2025 at 4:36 AM Markus Armbruster <armbru@redhat.com> wrote:
>
>> John Snow <jsnow@redhat.com> writes:
>>
>> > Well, I tried. Maybe not very hard. Sorry!
>>
>> No need to be sorry!  Kick-starting discussion with limited effort is
>> better than a big effort going into a direction that turns out to be
>> unwanted once we discuss it.
>>
>> Instead of just rephrasing Returns descriptions, I'd like us to consider
>> both Returns and intro.  See below for why.

[...]

> I think for cases where the doc block is short and we have a desire to
> merge "returns" and "intro", the intro makes the most sense if there isn't
> anything of particular value assigned to the return value to begin with.
>
> So, more or less, yeah: if semantics are partially duplicated between
> intro/returns, I'm in favor of putting it all in the intro and allowing
> transmogrifier generate the return type info.
>
> I don't think there's a good case to make for a doc block with no intro but
> a healthy paragraph in the returns section, that looks goofy.
>
> Of course: I think there are still circumstances where we'll want both the
> intro and the returns info explicitly labeled, when we have some
> information to document about the semantics of that return value.

Agreed.

[...]



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 2/4] docs, qapi: generate undocumented return sections
  2025-04-01  6:07       ` Markus Armbruster
@ 2025-04-03 21:05         ` John Snow
  0 siblings, 0 replies; 12+ messages in thread
From: John Snow @ 2025-04-03 21:05 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: qemu-devel, Jason Wang, Zhao Liu, Fabiano Rosas, Paolo Bonzini,
	Mads Ynddal, Hanna Reitz, Eduardo Habkost, Michael S. Tsirkin,
	qemu-trivial, Marc-André Lureau, Yanan Wang, qemu-block,
	Lukas Straub, Jiri Pirko, Stefan Berger, Gerd Hoffmann,
	Peter Maydell, Philippe Mathieu-Daudé, Michael Tokarev,
	Daniel P. Berrangé, Laurent Vivier, Zhenwei Pi, Eric Blake,
	Peter Xu, Ani Sinha, Marcel Apfelbaum,
	Vladimir Sementsov-Ogievskiy, Kevin Wolf, Michael Roth,
	Stefan Hajnoczi, Gonglei (Arei)

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

On Tue, Apr 1, 2025 at 2:07 AM Markus Armbruster <armbru@redhat.com> wrote:

> John Snow <jsnow@redhat.com> writes:
>
> > On Thu, Mar 27, 2025 at 5:11 AM Markus Armbruster <armbru@redhat.com>
> wrote:
> >
> >> John Snow <jsnow@redhat.com> writes:
> >>
> >> > This patch changes the qapidoc transmogrifier to generate Return value
> >> > documentation for any command that has a return value but hasn't
> >> > explicitly documented that return value.
> >> >
> >> > Signed-off-by: John Snow <jsnow@redhat.com>
> >>
> >> Might want to briefly explain placement of the auto-generated return
> >> value documentation.  But before we discuss that any further, let's
> >> review the actual changes the the generated docs.
> >>
> >> This patch adds auto-generated return value documentation where we have
> >> none.
> >>
> >> The next patch replaces handwritten by auto-generated return value
> >> documentation where these are at least as good.  Moves the return value
> >> docs in some cases.
> >>
> >> First the additions:
> >>
> >> * x-debug-query-block-graph
> >>
> >>   Title, intro, features, return
> >>
> >> * query-tpm
> >>
> >>   Title, intro, return, example
> >>
> >> * query-dirty-rate
> >>
> >>   Title, intro, arguments, return, examples
> >>
> >> * query-vcpu-dirty-limit
> >>
> >>   Title, intro, return, example
> >>
> >> * query-vm-generation-id
> >>
> >>   Title, return
> >>
> >> * query-memory-size-summary
> >>
> >>   Title, intro, example, return
> >>
> >> * query-memory-devices
> >>
> >>   Title, intro, return, example
> >>
> >> * query-acpi-ospm-status
> >>
> >>   Title, intro, return, example
> >>
> >> * query-stats-schemas
> >>
> >>   Title, intro, arguments, note, return
> >>
> >> Undesirable:
> >>
> >> * query-memory-size-summary has returns after the example instead of
> >>   before.  I figure it needs the TODO hack to separate intro and example
> >>   (see announce-self).
> >>
> >
> > Yes
> >
> >
> >>
> >> * query-stats-schemas has a note between arguments and return.  I think
> >>   this demonstrates that the placement algorithm is too simplistic.
> >>
> >
> > Yeah ... It's just that *glances at length of email* it's been a
> sensitive
> > topic without a lot of certainty in desire, so I've tried to keep things
> on
> > the stupid/simple side ...
>
> When the best solution is unclear, starting discussion with a simplistic
> solution is smart.  Beats starting with a complicated solution that gets
> shot down.
>
> >> Debatable:
> >>
> >> * x-debug-query-block-graph has returns after features.  I'd prefer
> >>   returns before features.  No need to debate this now.
> >>
> >
> > Willing to do this for you if you'd like to enforce it globally. I'd be
> > happy with any mandated order of sections, really.
>
> Could a more rigid order help the inliner, too?
>

Oh, absolutely. My series that adds it still assumes a rigid ordering. It
just never enforced it. I don't really care what the order even is, just as
long as there is one.


>
> >> Next the movements:
> >>
> >> * x-debug-block-dirty-bitmap-sha256
> >>
> >>   From right before errors to right after
> >>
> >> * blockdev-snapshot-delete-internal-sync
> >>
> >>   From right before errors to right after
> >>
> >> * query-xen-replication-status
> >>
> >>   From between intro and example to the end
> >>
> >> * query-colo-status
> >>
> >>   From between intro and example to the end
> >>
> >> * query-balloon
> >>
> >>   From right before errors to right after
> >>
> >> * query-hv-balloon-status-report
> >>
> >>   From right before errors to right after
> >>
> >> * query-yank
> >>
> >>   From between intro and example to the end
> >>
> >> * add-fd
> >>
> >>   From between arguments and errors to between last note and example
> >>
> >> I don't like any of these :)
> >>
> >
> > So it goes ...
> >
> >
> >>
> >> Undesirable:
> >>
> >> * query-xen-replication-status, query-yank, and query-colo-status now
> >>   have return after the example instead of before.  I figure they now
> >>   need the TODO hack to separate intro and example.
> >>
> >
> > Yes
> >
> >
> >>
> >> * add-fd now has a note between arguments and return.  Same placement
> >>   weakness as for query-stats above.
> >>
> >> Debatable:
> >>
> >> * x-debug-block-dirty-bitmap-sha256,
> >>   blockdev-snapshot-delete-internal-sync, query-colo-status, and
> >>   query-hv-balloon-status-report now have return after errors instead of
> >>   before.  I'd prefer before.
> >>
> >> What's the stupidest acceptable placement algorithm?  Maybe this one:
> >>
> >> 1. If we have arguments, return goes right after them.
> >>
> >> 2. Else if we have errors, return goes right before them.
> >>
> >> 3. Else if we have features, return goes right before them.
> >>
> >> 4. Else return goes right after the intro (to make this work, we need
> >>    a few more TODO hacks).
> >>
> >> Would you be willing to give this a try?
> >>
> >
> > Probably ...
> >
> > So this algorithm seems to imply a preference for this ordering:
> >
> > 1. Intro
> > 2. Arguments
> > 3. Return
> > 4. Errors
> > 5. Features
> > 6. Details
> >
> > Do I have that correct?
>
> Yes, with
>
>   7. Since
>

Sure. I tend to exclude it because I personally remove it from the flow
anyway, so I don't actually care where it is. If you do, that's fine!


>
> although a case could also be made for
>

but you'd prefer the first one, right? I have no interest in making further
cases at the moment ;)


>
>   1. Intro
>   2. Arguments
>   3. Return
>   4. Errors
>   5. Features
>   6. Since
>   7. Details
>
>

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-04-03 21:07 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-26 19:57 [PATCH v2 0/4] qapi: add auto-generated return docs John Snow
2025-03-26 19:57 ` [PATCH v2 1/4] docs/qapi-domain: add return-nodesc John Snow
2025-03-26 19:57 ` [PATCH v2 2/4] docs, qapi: generate undocumented return sections John Snow
2025-03-27  9:11   ` Markus Armbruster
2025-03-31 18:30     ` John Snow
2025-04-01  6:07       ` Markus Armbruster
2025-04-03 21:05         ` John Snow
2025-03-26 19:57 ` [PATCH v2 3/4] qapi: remove trivial "Returns:" sections John Snow
2025-03-26 19:57 ` [PATCH v2 4/4] qapi: rephrase return docs to avoid type name John Snow
2025-03-28  8:36   ` Markus Armbruster
2025-03-31 18:34     ` John Snow
2025-04-01  6:10       ` Markus Armbruster

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).