[PATCH 00/15] qapi: Require member documentation (with loophole)

[PATCH 00/15] qapi: Require member documentation (with loophole)

[Markus Armbruster]

The QAPI generator forces you to document your stuff.  Except for
command arguments, event data, and members of enum and object types:
these the generator silently "documents" as "Not documented".

We can't require proper documentation there without first fixing all
the offenders.  We've always had too many offenders to pull that off.
Right now, we have more than 500.  Worse, we seem to fix old ones no
faster than we add new ones: in the past year, we fixed 22 ones, but
added 26 new ones.

PATCH 01-05 are bonus fixes & cleanups.

PATCH 06 makes missing documentation an error unless the command,
type, or event is in listed in new pragma documentation-exceptions.

PATCH 07-09 improve the "QEMU Guest Agent Protocol Reference" manual:
they document eight members and arguments, reducing the number of
offending commands and types from nine to one.  The 25 members of type
GuestNVMeSmart are left undocumented.

PATCH 10-15 improve reduce the "QEMU QMP Reference Manual" manual, but
only a bit: they document 54 members and arguments, reducing number of
offending commands and types from 117 to 65.  467 members and
arguments are left undocumented.  A few of them are not actually used
in QMP, and documenting them is not worthwhile; they should be elided
from the manual instead.  Example: DummyForceArrays.

Remaining definitions with undocumented members:

    FILE
        DEFINITION #MISSING
    ----------------------------------------------
    qga/qapi-schema.json
        GuestNVMeSmart 25
    qapi/audio.json
	AudiodevDriver 12
    qapi/block-core.json
	BlkdebugEvent 43
	BlockdevDriver 39
	BlockdevQcow2EncryptionFormat 1
	BlockdevVmdkAdapterType 4
	DummyBlockCoreForceArrays 1
	ImageInfoSpecificKind 2
	IscsiHeaderDigest 4
	IscsiTransport 2
	Qcow2OverlapCheckFlags 8
	RbdAuthMode 2
	RbdImageEncryptionFormat 2
	ThrottleGroupProperties 19
	XDbgBlockGraph 2
	blockdev-reopen 1
    qapi/char.json
	ChardevBackendKind 6
    qapi/common.json
	GrabToggleKeys 6
    qapi/crypto.json
	QCryptoAkCipherKeyType 2
    qapi/cryptodev.json
	QCryptodevBackendServiceType 5
    qapi/cxl.json
	CxlCorErrorType 1
    qapi/introspect.json
	JSONType 8
    qapi/machine-common.json
	CpuS390Entitlement 4
    qapi/machine-target.json
	CpuS390Polarization 2
	query-cpu-model-baseline 2
	query-cpu-model-comparison 2
	query-cpu-model-expansion 2
    qapi/machine.json
	CpuS390State 5
	DummyForceArrays 1
	MemoryDeviceInfoKind 1
	SysEmuTarget 29
	X86CPURegister32 8
    qapi/migration.json
	MigrateSetParameters 1
    qapi/net.json
	NetClientDriver 10
	String 1
    qapi/pci.json
	PciMemoryRegion 1
    qapi/qom.json
	ObjectType 45
    qapi/rocker.json
	query-rocker 1
	query-rocker-ports 1
    qapi/run-state.json
	GuestPanicInformationHyperV 5
	watchdog-set-action 1
    qapi/stats.json
	StatsFilter 1
	StatsValue 1
	query-stats-schemas 1
    qapi/transaction.json
	AbortWrapper 1
	BlockDirtyBitmapAddWrapper 1
	BlockDirtyBitmapMergeWrapper 1
	BlockDirtyBitmapWrapper 1
	BlockdevBackupWrapper 1
	BlockdevSnapshotInternalWrapper 1
	BlockdevSnapshotSyncWrapper 1
	BlockdevSnapshotWrapper 1
	DriveBackupWrapper 1
    qapi/ui.json
	DisplayProtocol 2
	HotKeyMod 3
	InputAxis 2
	InputButton 7
	InputMultiTouchEvent 1
	InputMultiTouchType 5
	KeyValueKind 2
	QKeyCode 119
	VncPrimaryAuth 9
	VncVencryptSubAuth 9
    qapi/virtio.json
	DummyVirtioForceArrays 1
    qapi/yank.json
	YankInstanceType 3

Markus Armbruster (15):
  docs/devel/qapi-code-gen: Normalize version refs x.y.0 to just x.y
  docs/devel/qapi-code-gen: Tweak doc comment whitespace
  qapi/block-core: Fix BlockLatencyHistogramInfo doc markup
  qapi: Indent tagged doc comment sections properly
  sphinx/qapidoc: Drop code to generate doc for simple union tag
  qapi: Require member documentation (with loophole)
  qga/qapi-schema: Clean up documentation of guest-set-memory-blocks
  qga/qapi-schema: Clean up documentation of guest-set-vcpus
  qga/qapi-schema: Plug trivial documentation holes
  qapi/yank: Clean up documentaion of yank
  qapi/dump: Clean up documentation of DumpGuestMemoryCapability
  qapi: Plug trivial documentation holes around former simple unions
  qapi: Improve documentation of file descriptor socket addresses
  qapi: Move @String out of common.json to discourage reuse
  qapi: Add missing union tag documentation

 docs/devel/qapi-code-gen.rst                  | 14 ++--
 docs/sphinx/qapidoc.py                        |  6 --
 qapi/block-core.json                          | 26 ++++++-
 qapi/block-export.json                        |  2 +
 qapi/char.json                                | 28 ++++++++
 qapi/common.json                              | 11 ---
 qapi/crypto.json                              |  2 +
 qapi/dump.json                                |  2 +-
 qapi/machine.json                             | 14 ++++
 qapi/migration.json                           | 48 ++++++-------
 qapi/misc.json                                | 12 ++--
 qapi/net.json                                 | 12 +++-
 qapi/pragma.json                              | 67 +++++++++++++++++++
 qapi/qdev.json                                | 12 ++--
 qapi/sockets.json                             | 48 +++++++++----
 qapi/stats.json                               |  2 +
 qapi/tpm.json                                 |  4 ++
 qapi/transaction.json                         |  2 +
 qapi/ui.json                                  | 14 ++++
 qapi/yank.json                                |  4 +-
 qga/qapi-schema.json                          | 58 ++++++++++------
 include/hw/virtio/vhost-vsock-common.h        |  1 +
 include/net/filter.h                          |  2 +-
 chardev/char-socket.c                         |  2 +-
 util/qemu-sockets.c                           |  3 +-
 scripts/qapi/parser.py                        |  7 +-
 scripts/qapi/source.py                        |  2 +
 .../qapi-schema/doc-bad-alternate-member.json |  2 +
 tests/qapi-schema/doc-good.json               | 14 ++--
 tests/qapi-schema/doc-good.out                |  2 +-
 30 files changed, 317 insertions(+), 106 deletions(-)

-- 
2.43.0

[PATCH 01/15] docs/devel/qapi-code-gen: Normalize version refs x.y.0 to just x.y

[Markus Armbruster]

Missed in commit 9bc6e893b72 (qapi: Normalize version references x.y.0
to just x.y).

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 docs/devel/qapi-code-gen.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst
index 76be722f4c..13d38dbb09 100644
--- a/docs/devel/qapi-code-gen.rst
+++ b/docs/devel/qapi-code-gen.rst
@@ -1023,7 +1023,7 @@ For example::
  #
  # ... more members ...
  #
- # Since: 0.14.0
+ # Since: 0.14
  ##
  { 'struct': 'BlockStats',
    'data': {'*device': 'str', '*node-name': 'str',
@@ -1039,7 +1039,7 @@ For example::
  #
  # Returns: A list of @BlockStats for each virtual block devices.
  #
- # Since: 0.14.0
+ # Since: 0.14
  #
  # Example:
  #
-- 
2.43.0

Re: [PATCH 01/15] docs/devel/qapi-code-gen: Normalize version refs x.y.0 to just x.y

[Daniel P. Berrangé]

On Mon, Feb 05, 2024 at 08:46:55AM +0100, Markus Armbruster wrote:
> Missed in commit 9bc6e893b72 (qapi: Normalize version references x.y.0
> to just x.y).
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  docs/devel/qapi-code-gen.rst | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[PATCH 02/15] docs/devel/qapi-code-gen: Tweak doc comment whitespace

[Markus Armbruster]

Missed in commit a937b6aa739 (qapi: Reformat doc comments to conform
to current conventions).

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 docs/devel/qapi-code-gen.rst | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst
index 13d38dbb09..69c8a1e8bd 100644
--- a/docs/devel/qapi-code-gen.rst
+++ b/docs/devel/qapi-code-gen.rst
@@ -1019,7 +1019,7 @@ For example::
  # @device: If the stats are for a virtual block device, the name
  #     corresponding to the virtual block device.
  #
- # @node-name: The node name of the device. (since 2.3)
+ # @node-name: The node name of the device.  (Since 2.3)
  #
  # ... more members ...
  #
@@ -1035,7 +1035,8 @@ For example::
  # Query the @BlockStats for all virtual block devices.
  #
  # @query-nodes: If true, the command will query all the block nodes
- #     ... explain, explain ...  (since 2.3)
+ #     ... explain, explain ...
+ #     (Since 2.3)
  #
  # Returns: A list of @BlockStats for each virtual block devices.
  #
-- 
2.43.0

Re: [PATCH 02/15] docs/devel/qapi-code-gen: Tweak doc comment whitespace

[Daniel P. Berrangé]

On Mon, Feb 05, 2024 at 08:46:56AM +0100, Markus Armbruster wrote:
> Missed in commit a937b6aa739 (qapi: Reformat doc comments to conform
> to current conventions).
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  docs/devel/qapi-code-gen.rst | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[PATCH 03/15] qapi/block-core: Fix BlockLatencyHistogramInfo doc markup

[Markus Armbruster]

The description of @bins ends with a literal block:

    # @bins: list of io request counts corresponding to histogram
    #     intervals, one more element than @boundaries has.  For the
    #     example above, @bins may be something like [3, 1, 5, 2], and
    #     corresponding histogram looks like:
    #
    # ::
    #
    #        5|           *

Except it actually ends *before* the block: the unindented '::' line
starts a new section.  Makes no sense.

We could fix this by indenting the '::' line.  Instead, double the
colon at the end of the preceding paragraph, and drop the '::' line.

This shifts the box for the literal block right in generated
documentation, so it lines up with the description.

Fixes: commit a0fcff383b34 (qapi: Use rST markup for literal blocks)
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qapi/block-core.json | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 781c9bd03e..80ed4122f2 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -656,9 +656,7 @@
 # @bins: list of io request counts corresponding to histogram
 #     intervals, one more element than @boundaries has.  For the
 #     example above, @bins may be something like [3, 1, 5, 2], and
-#     corresponding histogram looks like:
-#
-# ::
+#     corresponding histogram looks like::
 #
 #        5|           *
 #        4|           *
-- 
2.43.0

Re: [PATCH 03/15] qapi/block-core: Fix BlockLatencyHistogramInfo doc markup

[Daniel P. Berrangé]

On Mon, Feb 05, 2024 at 08:46:57AM +0100, Markus Armbruster wrote:
> The description of @bins ends with a literal block:
> 
>     # @bins: list of io request counts corresponding to histogram
>     #     intervals, one more element than @boundaries has.  For the
>     #     example above, @bins may be something like [3, 1, 5, 2], and
>     #     corresponding histogram looks like:
>     #
>     # ::
>     #
>     #        5|           *
> 
> Except it actually ends *before* the block: the unindented '::' line
> starts a new section.  Makes no sense.
> 
> We could fix this by indenting the '::' line.  Instead, double the
> colon at the end of the preceding paragraph, and drop the '::' line.
> 
> This shifts the box for the literal block right in generated
> documentation, so it lines up with the description.
> 
> Fixes: commit a0fcff383b34 (qapi: Use rST markup for literal blocks)
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  qapi/block-core.json | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[PATCH 04/15] qapi: Indent tagged doc comment sections properly

[Markus Armbruster]

docs/devel/qapi-code-gen demands that the "second and subsequent lines
of sections other than "Example"/"Examples" should be indented".
Commit a937b6aa739q (qapi: Reformat doc comments to conform to current
conventions) missed a few instances, and messed up a few others.
Clean that up.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qapi/migration.json             | 46 ++++++++++++++++-----------------
 qapi/misc.json                  | 12 +++++----
 qapi/qdev.json                  | 12 ++++-----
 tests/qapi-schema/doc-good.json | 10 +++----
 tests/qapi-schema/doc-good.out  |  2 +-
 5 files changed, 42 insertions(+), 40 deletions(-)

diff --git a/qapi/migration.json b/qapi/migration.json
index 819708321d..bf89765a26 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -1699,24 +1699,24 @@
 #
 # Notes:
 #
-# 1. The 'query-migrate' command should be used to check migration's
-#    progress and final result (this information is provided by the
-#    'status' member)
+#     1. The 'query-migrate' command should be used to check
+#        migration's progress and final result (this information is
+#        provided by the 'status' member)
 #
-# 2. All boolean arguments default to false
+#     2. All boolean arguments default to false
 #
-# 3. The user Monitor's "detach" argument is invalid in QMP and should
-#    not be used
+#     3. The user Monitor's "detach" argument is invalid in QMP and
+#        should not be used
 #
-# 4. The uri argument should have the Uniform Resource Identifier of
-#    default destination VM. This connection will be bound to default
-#    network.
+#     4. The uri argument should have the Uniform Resource Identifier
+#        of default destination VM. This connection will be bound to
+#        default network.
 #
-# 5. For now, number of migration streams is restricted to one, i.e
-#    number of items in 'channels' list is just 1.
+#     5. For now, number of migration streams is restricted to one,
+#        i.e number of items in 'channels' list is just 1.
 #
-# 6. The 'uri' and 'channels' arguments are mutually exclusive;
-#    exactly one of the two should be present.
+#     6. The 'uri' and 'channels' arguments are mutually exclusive;
+#        exactly one of the two should be present.
 #
 # Example:
 #
@@ -1781,20 +1781,20 @@
 #
 # Notes:
 #
-# 1. It's a bad idea to use a string for the uri, but it needs
-#    to stay compatible with -incoming and the format of the uri
-#    is already exposed above libvirt.
+#     1. It's a bad idea to use a string for the uri, but it needs to
+#        stay compatible with -incoming and the format of the uri is
+#        already exposed above libvirt.
 #
-# 2. QEMU must be started with -incoming defer to allow
-#    migrate-incoming to be used.
+#     2. QEMU must be started with -incoming defer to allow
+#        migrate-incoming to be used.
 #
-# 3. The uri format is the same as for -incoming
+#     3. The uri format is the same as for -incoming
 #
-# 5. For now, number of migration streams is restricted to one, i.e
-#    number of items in 'channels' list is just 1.
+#     5. For now, number of migration streams is restricted to one,
+#        i.e number of items in 'channels' list is just 1.
 #
-# 4. The 'uri' and 'channels' arguments are mutually exclusive;
-#    exactly one of the two should be present.
+#     4. The 'uri' and 'channels' arguments are mutually exclusive;
+#        exactly one of the two should be present.
 #
 # Example:
 #
diff --git a/qapi/misc.json b/qapi/misc.json
index 2ca8c39874..4108a0c951 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -348,9 +348,10 @@
 #     - If file descriptor was not received, GenericError
 #     - If @fdset-id is a negative value, GenericError
 #
-# Notes: The list of fd sets is shared by all monitor connections.
+# Notes:
+#     The list of fd sets is shared by all monitor connections.
 #
-# If @fdset-id is not specified, a new fd set will be created.
+#     If @fdset-id is not specified, a new fd set will be created.
 #
 # Since: 1.2
 #
@@ -379,10 +380,11 @@
 #
 # Since: 1.2
 #
-# Notes: The list of fd sets is shared by all monitor connections.
+# Notes:
+#     The list of fd sets is shared by all monitor connections.
 #
-# If @fd is not specified, all file descriptors in @fdset-id will be
-# removed.
+#     If @fd is not specified, all file descriptors in @fdset-id will
+#     be removed.
 #
 # Example:
 #
diff --git a/qapi/qdev.json b/qapi/qdev.json
index 25bac5e611..3b3ccfa413 100644
--- a/qapi/qdev.json
+++ b/qapi/qdev.json
@@ -53,14 +53,14 @@
 #
 # Notes:
 #
-# 1. Additional arguments depend on the type.
+#     1. Additional arguments depend on the type.
 #
-# 2. For detailed information about this command, please refer to the
-#    'docs/qdev-device-use.txt' file.
+#     2. For detailed information about this command, please refer to
+#        the 'docs/qdev-device-use.txt' file.
 #
-# 3. It's possible to list device properties by running QEMU with the
-#    "-device DEVICE,help" command-line argument, where DEVICE is the
-#    device's name
+#     3. It's possible to list device properties by running QEMU with
+#        the "-device DEVICE,help" command-line argument, where DEVICE
+#        is the device's name
 #
 # Example:
 #
diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json
index 354dfdf461..976f9e1aaa 100644
--- a/tests/qapi-schema/doc-good.json
+++ b/tests/qapi-schema/doc-good.json
@@ -73,8 +73,8 @@
 # @Base:
 #
 # @base1:
-# description starts on a new line,
-# not indented
+#  description starts on a new line,
+#  minimally indented
 ##
 { 'struct': 'Base', 'data': { 'base1': 'Enum' },
   'if': { 'all': ['IFALL1', 'IFALL2'] } }
@@ -155,10 +155,10 @@
 # TODO: frobnicate
 # Notes:
 #
-# - Lorem ipsum dolor sit amet
-# - Ut enim ad minim veniam
+#  - Lorem ipsum dolor sit amet
+#  - Ut enim ad minim veniam
 #
-# Duis aute irure dolor
+#  Duis aute irure dolor
 # Example:
 #
 # -> in
diff --git a/tests/qapi-schema/doc-good.out b/tests/qapi-schema/doc-good.out
index 24d9ea954d..34ee74af4b 100644
--- a/tests/qapi-schema/doc-good.out
+++ b/tests/qapi-schema/doc-good.out
@@ -118,7 +118,7 @@ doc symbol=Base
 
     arg=base1
 description starts on a new line,
-not indented
+minimally indented
 doc symbol=Variant1
     body=
 A paragraph
-- 
2.43.0

Re: [PATCH 04/15] qapi: Indent tagged doc comment sections properly

[Daniel P. Berrangé]

On Mon, Feb 05, 2024 at 08:46:58AM +0100, Markus Armbruster wrote:
> docs/devel/qapi-code-gen demands that the "second and subsequent lines
> of sections other than "Example"/"Examples" should be indented".
> Commit a937b6aa739q (qapi: Reformat doc comments to conform to current
> conventions) missed a few instances, and messed up a few others.
> Clean that up.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  qapi/migration.json             | 46 ++++++++++++++++-----------------
>  qapi/misc.json                  | 12 +++++----
>  qapi/qdev.json                  | 12 ++++-----
>  tests/qapi-schema/doc-good.json | 10 +++----
>  tests/qapi-schema/doc-good.out  |  2 +-
>  5 files changed, 42 insertions(+), 40 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[PATCH 05/15] sphinx/qapidoc: Drop code to generate doc for simple union tag

[Markus Armbruster]

QAPISchemaGenRSTVisitor._nodes_for_members() has a special case to
auto-generate documentation for a union tag member of implicit (enum)
type that lacks documentation.

This was useful for simple unions, where the tag member's type was
implicitly.  The only implicit enum type left today is 'QType'.  Not
worth a special case.  Drop.  No change to generated documentation.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 docs/sphinx/qapidoc.py | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
index 658c288f8f..05b809af27 100644
--- a/docs/sphinx/qapidoc.py
+++ b/docs/sphinx/qapidoc.py
@@ -168,12 +168,6 @@ def _nodes_for_members(self, doc, what, base=None, variants=None):
             # TODO drop fallbacks when undocumented members are outlawed
             if section.text:
                 defn = section.text
-            elif (variants and variants.tag_member == section.member
-                  and not section.member.type.doc_type()):
-                values = section.member.type.member_names()
-                defn = [nodes.Text('One of ')]
-                defn.extend(intersperse([nodes.literal('', v) for v in values],
-                                        nodes.Text(', ')))
             else:
                 defn = [nodes.Text('Not documented')]
 
-- 
2.43.0

Re: [PATCH 05/15] sphinx/qapidoc: Drop code to generate doc for simple union tag

[Daniel P. Berrangé]

On Mon, Feb 05, 2024 at 08:46:59AM +0100, Markus Armbruster wrote:
> QAPISchemaGenRSTVisitor._nodes_for_members() has a special case to
> auto-generate documentation for a union tag member of implicit (enum)
> type that lacks documentation.
> 
> This was useful for simple unions, where the tag member's type was
> implicitly.  The only implicit enum type left today is 'QType'.  Not
> worth a special case.  Drop.  No change to generated documentation.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  docs/sphinx/qapidoc.py | 6 ------
>  1 file changed, 6 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[PATCH 06/15] qapi: Require member documentation (with loophole)

[Markus Armbruster]

The QAPI generator forces you to document your stuff.  Except for
command arguments, event data, and members of enum and object types:
these the generator silently "documents" as "Not documented".

We can't require proper documentation there without first fixing all
the offenders.  We've always had too many offenders to pull that off.
Right now, we have more than 500.  Worse, we seem to fix old ones no
faster than we add new ones: in the past year, we fixed 22 ones, but
added 26 new ones.

To help arrest the backsliding, make missing documentation an error
unless the command, type, or event is in listed in new pragma
documentation-exceptions.

List all the current offenders: 117 commands and types in qapi/, and 9
in qga/.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 docs/devel/qapi-code-gen.rst                  |   5 +
 qapi/pragma.json                              | 119 ++++++++++++++++++
 qga/qapi-schema.json                          |  13 +-
 scripts/qapi/parser.py                        |   7 +-
 scripts/qapi/source.py                        |   2 +
 .../qapi-schema/doc-bad-alternate-member.json |   2 +
 tests/qapi-schema/doc-good.json               |   4 +-
 7 files changed, 149 insertions(+), 3 deletions(-)

diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst
index 69c8a1e8bd..756adc187e 100644
--- a/docs/devel/qapi-code-gen.rst
+++ b/docs/devel/qapi-code-gen.rst
@@ -167,6 +167,7 @@ Syntax::
                    '*doc-required': BOOL,
                    '*command-name-exceptions': [ STRING, ... ],
                    '*command-returns-exceptions': [ STRING, ... ],
+                   '*documentation-exceptions': [ STRING, ... ],
                    '*member-name-exceptions': [ STRING, ... ] } }
 
 The pragma directive lets you control optional generator behavior.
@@ -183,6 +184,10 @@ may contain ``"_"`` instead of ``"-"``.  Default is none.
 Pragma 'command-returns-exceptions' takes a list of commands that may
 violate the rules on permitted return types.  Default is none.
 
+Pragma 'documentation-exceptions' takes a list of types, commands, and
+events whose members / arguments need not be documented.  Default is
+none.
+
 Pragma 'member-name-exceptions' takes a list of types whose member
 names may contain uppercase letters, and ``"_"`` instead of ``"-"``.
 Default is none.
diff --git a/qapi/pragma.json b/qapi/pragma.json
index 0aa4eeddd3..0fa64742b5 100644
--- a/qapi/pragma.json
+++ b/qapi/pragma.json
@@ -31,6 +31,125 @@
         'query-tpm-models',
         'query-tpm-types',
         'ringbuf-read' ],
+    # Types, commands, and events with undocumented members / arguments:
+    'documentation-exceptions': [
+        'AbortWrapper',
+        'AudiodevDriver',
+        'BlkdebugEvent',
+        'BlockDirtyBitmapAddWrapper',
+        'BlockDirtyBitmapMergeWrapper',
+        'BlockDirtyBitmapWrapper',
+        'BlockExportOptions',
+        'BlockStatsSpecific',
+        'BlockdevBackupWrapper',
+        'BlockdevDriver',
+        'BlockdevQcow2Encryption',
+        'BlockdevQcow2EncryptionFormat',
+        'BlockdevQcowEncryption',
+        'BlockdevSnapshotInternalWrapper',
+        'BlockdevSnapshotSyncWrapper',
+        'BlockdevSnapshotWrapper',
+        'BlockdevVmdkAdapterType',
+        'ChardevBackend',
+        'ChardevBackendKind',
+        'ChardevCommonWrapper',
+        'ChardevDBusWrapper',
+        'ChardevFileWrapper',
+        'ChardevHostdevWrapper',
+        'ChardevMuxWrapper',
+        'ChardevQemuVDAgentWrapper',
+        'ChardevRingbufWrapper',
+        'ChardevSocketWrapper',
+        'ChardevSpiceChannelWrapper',
+        'ChardevSpicePortWrapper',
+        'ChardevStdioWrapper',
+        'ChardevUdpWrapper',
+        'ChardevVCWrapper',
+        'CpuS390Entitlement',
+        'CpuS390Polarization',
+        'CpuS390State',
+        'CxlCorErrorType',
+        'DisplayProtocol',
+        'DriveBackupWrapper',
+        'DummyBlockCoreForceArrays',
+        'DummyForceArrays',
+        'DummyVirtioForceArrays',
+        'DumpGuestMemoryCapability',
+        'GrabToggleKeys',
+        'GuestPanicInformationHyperV',
+        'HotKeyMod',
+        'HvBalloonDeviceInfoWrapper',
+        'ImageInfoSpecific',
+        'ImageInfoSpecificFileWrapper',
+        'ImageInfoSpecificKind',
+        'ImageInfoSpecificLUKSWrapper',
+        'ImageInfoSpecificQCow2Wrapper',
+        'ImageInfoSpecificRbdWrapper',
+        'ImageInfoSpecificVmdkWrapper',
+        'InetSocketAddressWrapper',
+        'InputAxis',
+        'InputBtnEventWrapper',
+        'InputButton',
+        'InputKeyEventWrapper',
+        'InputMoveEventWrapper',
+        'InputMultiTouchEvent',
+        'InputMultiTouchEventWrapper',
+        'InputMultiTouchType',
+        'IntWrapper',
+        'IscsiHeaderDigest',
+        'IscsiTransport',
+        'JSONType',
+        'KeyValue',
+        'KeyValueKind',
+        'MemoryDeviceInfo',
+        'MemoryDeviceInfoKind',
+        'MigrateSetParameters',
+        'MigrationAddress',
+        'NetClientDriver',
+        'NumaOptions',
+        'ObjectType',
+        'PCDIMMDeviceInfoWrapper',
+        'PciMemoryRegion',
+        'QCryptoAkCipherKeyType',
+        'QCryptoAkCipherOptions',
+        'QCryptodevBackendServiceType',
+        'QKeyCode',
+        'QKeyCodeWrapper',
+        'Qcow2OverlapCheckFlags',
+        'RbdAuthMode',
+        'RbdEncryptionCreateOptions',
+        'RbdImageEncryptionFormat',
+        'SgxEPCDeviceInfoWrapper',
+        'SocketAddressLegacy',
+        'SshHostKeyCheck',
+        'StatsFilter',
+        'StatsValue',
+        'String',
+        'StringWrapper',
+        'SysEmuTarget',
+        'TPMEmulatorOptionsWrapper',
+        'TPMPassthroughOptionsWrapper',
+        'ThrottleGroupProperties',
+        'TransactionAction',
+        'UnixSocketAddressWrapper',
+        'VirtioMEMDeviceInfoWrapper',
+        'VirtioPMEMDeviceInfoWrapper',
+        'VncPrimaryAuth',
+        'VncVencryptSubAuth',
+        'VsockSocketAddressWrapper',
+        'X86CPURegister32',
+        'XDbgBlockGraph',
+        'YankInstance',
+        'YankInstanceType',
+        'blockdev-reopen',
+        'query-cpu-model-baseline',
+        'query-cpu-model-comparison',
+        'query-cpu-model-expansion',
+        'query-rocker',
+        'query-rocker-ports',
+        'query-stats-schemas',
+        'watchdog-set-action',
+        'yank' ],
     # Externally visible types whose member names may use uppercase
     'member-name-exceptions': [     # visible in:
         'ACPISlotType',             # query-acpi-ospm-status
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 50b0a558c7..b9501c8c81 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -33,7 +33,18 @@
         'guest-get-time',
         'guest-set-vcpus',
         'guest-sync',
-        'guest-sync-delimited' ] } }
+        'guest-sync-delimited' ],
+    # Types and commands with undocumented members:
+    'documentation-exceptions': [
+        'GuestCpuStats',
+        'GuestCpuStatsType',
+        'GuestDeviceId',
+        'GuestDeviceType',
+        'GuestDiskSmart',
+        'GuestDiskStatsInfo',
+        'GuestNVMeSmart',
+        'guest-set-memory-blocks',
+        'guest-set-vcpus' ] } }
 
 ##
 # @guest-sync-delimited:
diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index 48cd55a38c..88221b3c64 100644
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -238,6 +238,8 @@ def check_list_str(name: str, value: object) -> List[str]:
             pragma.command_name_exceptions = check_list_str(name, value)
         elif name == 'command-returns-exceptions':
             pragma.command_returns_exceptions = check_list_str(name, value)
+        elif name == 'documentation-exceptions':
+            pragma.documentation_exceptions = check_list_str(name, value)
         elif name == 'member-name-exceptions':
             pragma.member_name_exceptions = check_list_str(name, value)
         else:
@@ -739,7 +741,10 @@ def _append_freeform(self, line: str) -> None:
 
     def connect_member(self, member: 'QAPISchemaMember') -> None:
         if member.name not in self.args:
-            # Undocumented TODO outlaw
+            if self.symbol not in member.info.pragma.documentation_exceptions:
+                raise QAPISemError(member.info,
+                                   "%s '%s' lacks documentation"
+                                   % (member.role, member.name))
             self.args[member.name] = QAPIDoc.ArgSection(self._parser,
                                                         member.name)
         self.args[member.name].connect(member)
diff --git a/scripts/qapi/source.py b/scripts/qapi/source.py
index 04193cc964..7b379fdc92 100644
--- a/scripts/qapi/source.py
+++ b/scripts/qapi/source.py
@@ -24,6 +24,8 @@ def __init__(self) -> None:
         self.command_name_exceptions: List[str] = []
         # Commands allowed to return a non-dictionary
         self.command_returns_exceptions: List[str] = []
+        # Types, commands, and events with undocumented members
+        self.documentation_exceptions: List[str] = []
         # Types whose member names may violate case conventions
         self.member_name_exceptions: List[str] = []
 
diff --git a/tests/qapi-schema/doc-bad-alternate-member.json b/tests/qapi-schema/doc-bad-alternate-member.json
index fa4143da4c..37593b6698 100644
--- a/tests/qapi-schema/doc-bad-alternate-member.json
+++ b/tests/qapi-schema/doc-bad-alternate-member.json
@@ -2,6 +2,8 @@
 
 ##
 # @AorB:
+# @a: a
+# @b: b
 # @aa: a
 # @bb: b
 ##
diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json
index 976f9e1aaa..24a84fe6d7 100644
--- a/tests/qapi-schema/doc-good.json
+++ b/tests/qapi-schema/doc-good.json
@@ -3,7 +3,9 @@
 #
 # Positive QAPI doc comment tests
 
-{ 'pragma': { 'doc-required': true } }
+{ 'pragma': {
+    'doc-required': true,
+    'documentation-exceptions': [ 'Enum', 'Variant1', 'Alternate', 'cmd' ] } }
 
 ##
 # = Section
-- 
2.43.0

Re: [PATCH 06/15] qapi: Require member documentation (with loophole)

[Daniel P. Berrangé]

On Mon, Feb 05, 2024 at 08:47:00AM +0100, Markus Armbruster wrote:
> The QAPI generator forces you to document your stuff.  Except for
> command arguments, event data, and members of enum and object types:
> these the generator silently "documents" as "Not documented".
> 
> We can't require proper documentation there without first fixing all
> the offenders.  We've always had too many offenders to pull that off.
> Right now, we have more than 500.  Worse, we seem to fix old ones no
> faster than we add new ones: in the past year, we fixed 22 ones, but
> added 26 new ones.
> 
> To help arrest the backsliding, make missing documentation an error
> unless the command, type, or event is in listed in new pragma
> documentation-exceptions.

If we want to really annoy people we could print a warning to
stderr during docs generation, for each undocumented field.
The many pages  of warnings would likely trigger a much quicker
series to patches to fix it to eliminate the annoying warnings :-)

> 
> List all the current offenders: 117 commands and types in qapi/, and 9
> in qga/.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  docs/devel/qapi-code-gen.rst                  |   5 +
>  qapi/pragma.json                              | 119 ++++++++++++++++++
>  qga/qapi-schema.json                          |  13 +-
>  scripts/qapi/parser.py                        |   7 +-
>  scripts/qapi/source.py                        |   2 +
>  .../qapi-schema/doc-bad-alternate-member.json |   2 +
>  tests/qapi-schema/doc-good.json               |   4 +-
>  7 files changed, 149 insertions(+), 3 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 06/15] qapi: Require member documentation (with loophole)

[Markus Armbruster]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Mon, Feb 05, 2024 at 08:47:00AM +0100, Markus Armbruster wrote:
>> The QAPI generator forces you to document your stuff.  Except for
>> command arguments, event data, and members of enum and object types:
>> these the generator silently "documents" as "Not documented".
>> 
>> We can't require proper documentation there without first fixing all
>> the offenders.  We've always had too many offenders to pull that off.
>> Right now, we have more than 500.  Worse, we seem to fix old ones no
>> faster than we add new ones: in the past year, we fixed 22 ones, but
>> added 26 new ones.
>> 
>> To help arrest the backsliding, make missing documentation an error
>> unless the command, type, or event is in listed in new pragma
>> documentation-exceptions.
>
> If we want to really annoy people we could print a warning to
> stderr during docs generation, for each undocumented field.
> The many pages  of warnings would likely trigger a much quicker
> series to patches to fix it to eliminate the annoying warnings :-)

Heh.

Let's give not annoying people that much a try.  Can always escalate
later :)

[...]

[PATCH 07/15] qga/qapi-schema: Clean up documentation of guest-set-memory-blocks

[Markus Armbruster]

The command's doc comment describes the argument, but it's not marked
up as such.  Easy enough to fix.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qga/qapi-schema.json | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index b9501c8c81..35bde36a1f 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -43,7 +43,6 @@
         'GuestDiskSmart',
         'GuestDiskStatsInfo',
         'GuestNVMeSmart',
-        'guest-set-memory-blocks',
         'guest-set-vcpus' ] } }
 
 ##
@@ -1174,14 +1173,16 @@
 # Attempt to reconfigure (currently: enable/disable) state of memory
 # blocks inside the guest.
 #
-# The input list is processed node by node in order.  In each node
-# @phys-index is used to look up the guest MEMORY BLOCK, for which
-# @online specifies the requested state.  The set of distinct
-# @phys-index's is only required to be a subset of the guest-supported
-# identifiers.  There's no restriction on list length or on repeating
-# the same @phys-index (with possibly different @online field).
-# Preferably the input list should describe a modified subset of
-# @guest-get-memory-blocks' return value.
+# @mem-blks: The memory blocks to be reconfigured.  This list is
+#     processed node by node in order.  In each node @phys-index is
+#     used to look up the guest MEMORY BLOCK, for which @online
+#     specifies the requested state.  The set of distinct
+#     @phys-index's is only required to be a subset of the
+#     guest-supported identifiers.  There's no restriction on list
+#     length or on repeating the same @phys-index (with possibly
+#     different @online field).  Preferably the input list should
+#     describe a modified subset of @guest-get-memory-blocks' return
+#     value.
 #
 # Returns: The operation results, it is a list of
 #     @GuestMemoryBlockResponse, which is corresponding to the input
-- 
2.43.0

Re: [PATCH 07/15] qga/qapi-schema: Clean up documentation of guest-set-memory-blocks

[Daniel P. Berrangé]

On Mon, Feb 05, 2024 at 08:47:01AM +0100, Markus Armbruster wrote:
> The command's doc comment describes the argument, but it's not marked
> up as such.  Easy enough to fix.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  qga/qapi-schema.json | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 07/15] qga/qapi-schema: Clean up documentation of guest-set-memory-blocks

[Konstantin Kostiuk]

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

Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>

On Mon, Feb 5, 2024 at 9:47 AM Markus Armbruster <armbru@redhat.com> wrote:

> The command's doc comment describes the argument, but it's not marked
> up as such.  Easy enough to fix.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  qga/qapi-schema.json | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> index b9501c8c81..35bde36a1f 100644
> --- a/qga/qapi-schema.json
> +++ b/qga/qapi-schema.json
> @@ -43,7 +43,6 @@
>          'GuestDiskSmart',
>          'GuestDiskStatsInfo',
>          'GuestNVMeSmart',
> -        'guest-set-memory-blocks',
>          'guest-set-vcpus' ] } }
>
>  ##
> @@ -1174,14 +1173,16 @@
>  # Attempt to reconfigure (currently: enable/disable) state of memory
>  # blocks inside the guest.
>  #
> -# The input list is processed node by node in order.  In each node
> -# @phys-index is used to look up the guest MEMORY BLOCK, for which
> -# @online specifies the requested state.  The set of distinct
> -# @phys-index's is only required to be a subset of the guest-supported
> -# identifiers.  There's no restriction on list length or on repeating
> -# the same @phys-index (with possibly different @online field).
> -# Preferably the input list should describe a modified subset of
> -# @guest-get-memory-blocks' return value.
> +# @mem-blks: The memory blocks to be reconfigured.  This list is
> +#     processed node by node in order.  In each node @phys-index is
> +#     used to look up the guest MEMORY BLOCK, for which @online
> +#     specifies the requested state.  The set of distinct
> +#     @phys-index's is only required to be a subset of the
> +#     guest-supported identifiers.  There's no restriction on list
> +#     length or on repeating the same @phys-index (with possibly
> +#     different @online field).  Preferably the input list should
> +#     describe a modified subset of @guest-get-memory-blocks' return
> +#     value.
>  #
>  # Returns: The operation results, it is a list of
>  #     @GuestMemoryBlockResponse, which is corresponding to the input
> --
> 2.43.0
>
>

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

[PATCH 08/15] qga/qapi-schema: Clean up documentation of guest-set-vcpus

[Markus Armbruster]

The command's doc comment describes the argument, but it's not marked
up as such.  Easy enough to fix.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qga/qapi-schema.json | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 35bde36a1f..f3d168d542 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -42,8 +42,7 @@
         'GuestDeviceType',
         'GuestDiskSmart',
         'GuestDiskStatsInfo',
-        'GuestNVMeSmart',
-        'guest-set-vcpus' ] } }
+        'GuestNVMeSmart' ] } }
 
 ##
 # @guest-sync-delimited:
@@ -786,14 +785,15 @@
 # Attempt to reconfigure (currently: enable/disable) logical
 # processors inside the guest.
 #
-# The input list is processed node by node in order.  In each node
-# @logical-id is used to look up the guest VCPU, for which @online
-# specifies the requested state.  The set of distinct @logical-id's is
-# only required to be a subset of the guest-supported identifiers.
-# There's no restriction on list length or on repeating the same
-# @logical-id (with possibly different @online field). Preferably the
-# input list should describe a modified subset of @guest-get-vcpus'
-# return value.
+# @vcpus: The logical processors to be reconfigured.  This list is
+#     processed node by node in order.  In each node @logical-id is
+#     used to look up the guest VCPU, for which @online specifies the
+#     requested state.  The set of distinct @logical-id's is only
+#     required to be a subset of the guest-supported identifiers.
+#     There's no restriction on list length or on repeating the same
+#     @logical-id (with possibly different @online field).  Preferably
+#     the input list should describe a modified subset of
+#     @guest-get-vcpus' return value.
 #
 # Returns: The length of the initial sublist that has been
 #     successfully processed.  The guest agent maximizes this value.
-- 
2.43.0

Re: [PATCH 08/15] qga/qapi-schema: Clean up documentation of guest-set-vcpus

[Daniel P. Berrangé]

On Mon, Feb 05, 2024 at 08:47:02AM +0100, Markus Armbruster wrote:
> The command's doc comment describes the argument, but it's not marked
> up as such.  Easy enough to fix.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  qga/qapi-schema.json | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 08/15] qga/qapi-schema: Clean up documentation of guest-set-vcpus

[Konstantin Kostiuk]

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

Reviewed-by: Konstantin Kostiuk <kkostiuk@redhat.com>

On Mon, Feb 5, 2024 at 9:47 AM Markus Armbruster <armbru@redhat.com> wrote:

> The command's doc comment describes the argument, but it's not marked
> up as such.  Easy enough to fix.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  qga/qapi-schema.json | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
> index 35bde36a1f..f3d168d542 100644
> --- a/qga/qapi-schema.json
> +++ b/qga/qapi-schema.json
> @@ -42,8 +42,7 @@
>          'GuestDeviceType',
>          'GuestDiskSmart',
>          'GuestDiskStatsInfo',
> -        'GuestNVMeSmart',
> -        'guest-set-vcpus' ] } }
> +        'GuestNVMeSmart' ] } }
>
>  ##
>  # @guest-sync-delimited:
> @@ -786,14 +785,15 @@
>  # Attempt to reconfigure (currently: enable/disable) logical
>  # processors inside the guest.
>  #
> -# The input list is processed node by node in order.  In each node
> -# @logical-id is used to look up the guest VCPU, for which @online
> -# specifies the requested state.  The set of distinct @logical-id's is
> -# only required to be a subset of the guest-supported identifiers.
> -# There's no restriction on list length or on repeating the same
> -# @logical-id (with possibly different @online field). Preferably the
> -# input list should describe a modified subset of @guest-get-vcpus'
> -# return value.
> +# @vcpus: The logical processors to be reconfigured.  This list is
> +#     processed node by node in order.  In each node @logical-id is
> +#     used to look up the guest VCPU, for which @online specifies the
> +#     requested state.  The set of distinct @logical-id's is only
> +#     required to be a subset of the guest-supported identifiers.
> +#     There's no restriction on list length or on repeating the same
> +#     @logical-id (with possibly different @online field).  Preferably
> +#     the input list should describe a modified subset of
> +#     @guest-get-vcpus' return value.
>  #
>  # Returns: The length of the initial sublist that has been
>  #     successfully processed.  The guest agent maximizes this value.
> --
> 2.43.0
>
>

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

[PATCH 09/15] qga/qapi-schema: Plug trivial documentation holes

[Markus Armbruster]

Add missing return member documentation of guest-get-disks,
guest-get-devices, guest-get-diskstats, and guest-get-cpustats.

The NVMe SMART information returned by guest-getdisks remains
undocumented.  Add a TODO there.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qga/qapi-schema.json | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index f3d168d542..b8efe31897 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -36,12 +36,6 @@
         'guest-sync-delimited' ],
     # Types and commands with undocumented members:
     'documentation-exceptions': [
-        'GuestCpuStats',
-        'GuestCpuStatsType',
-        'GuestDeviceId',
-        'GuestDeviceType',
-        'GuestDiskSmart',
-        'GuestDiskStatsInfo',
         'GuestNVMeSmart' ] } }
 
 ##
@@ -944,6 +938,8 @@
 # NVMe smart information, based on NVMe specification, section
 # <SMART / Health Information (Log Identifier 02h)>
 #
+# TODO: document members briefly
+#
 # Since: 7.1
 ##
 { 'struct': 'GuestNVMeSmart',
@@ -978,7 +974,7 @@
 #
 # Disk type related smart information.
 #
-# - @nvme: NVMe disk smart
+# @type: disk bus type
 #
 # Since: 7.1
 ##
@@ -1499,6 +1495,8 @@
 
 ##
 # @GuestDeviceType:
+#
+# @pci: PCI device
 ##
 { 'enum': 'GuestDeviceType',
   'data': [ 'pci' ] }
@@ -1518,7 +1516,9 @@
 ##
 # @GuestDeviceId:
 #
-# Id of the device - @pci: PCI ID, since: 5.2
+# Id of the device
+#
+# @type: device type
 #
 # Since: 5.2
 ##
@@ -1700,6 +1700,8 @@
 # @major: major device number of disk
 #
 # @minor: minor device number of disk
+#
+# @stats: I/O statistics
 ##
 { 'struct': 'GuestDiskStatsInfo',
   'data': {'name': 'str',
@@ -1723,7 +1725,9 @@
 ##
 # @GuestCpuStatsType:
 #
-# An enumeration of OS type
+# Guest operating systems supporting CPU statistics
+#
+# @linux: Linux
 #
 # Since: 7.1
 ##
@@ -1780,7 +1784,7 @@
 #
 # Get statistics of each CPU in millisecond.
 #
-# - @linux: Linux style CPU statistics
+# @type: guest operating system
 #
 # Since: 7.1
 ##
-- 
2.43.0

Re: [PATCH 09/15] qga/qapi-schema: Plug trivial documentation holes

[Daniel P. Berrangé]

On Mon, Feb 05, 2024 at 08:47:03AM +0100, Markus Armbruster wrote:
> Add missing return member documentation of guest-get-disks,
> guest-get-devices, guest-get-diskstats, and guest-get-cpustats.
> 
> The NVMe SMART information returned by guest-getdisks remains
> undocumented.  Add a TODO there.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  qga/qapi-schema.json | 24 ++++++++++++++----------
>  1 file changed, 14 insertions(+), 10 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

> @@ -978,7 +974,7 @@
>  #
>  # Disk type related smart information.
>  #
> -# - @nvme: NVMe disk smart
> +# @type: disk bus type
>  #
>  # Since: 7.1
>  ##

Fun, so not only were the fields that exist not documented,
but we've documented fields which don't exist.


> @@ -1780,7 +1784,7 @@
>  #
>  # Get statistics of each CPU in millisecond.
>  #
> -# - @linux: Linux style CPU statistics
> +# @type: guest operating system
>  #
>  # Since: 7.1
>  ##

And more things which don't exist !


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 09/15] qga/qapi-schema: Plug trivial documentation holes

[Markus Armbruster]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Mon, Feb 05, 2024 at 08:47:03AM +0100, Markus Armbruster wrote:
>> Add missing return member documentation of guest-get-disks,
>> guest-get-devices, guest-get-diskstats, and guest-get-cpustats.
>> 
>> The NVMe SMART information returned by guest-getdisks remains
>> undocumented.  Add a TODO there.
>> 
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  qga/qapi-schema.json | 24 ++++++++++++++----------
>>  1 file changed, 14 insertions(+), 10 deletions(-)
>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>
>> @@ -978,7 +974,7 @@
>>  #
>>  # Disk type related smart information.
>>  #
>> -# - @nvme: NVMe disk smart
>> +# @type: disk bus type
>>  #
>>  # Since: 7.1
>>  ##
>
> Fun, so not only were the fields that exist not documented,
> but we've documented fields which don't exist.

I think the @nvme line tried to document the branch.  Not useful; the
doc generator takes care of that:

    GuestDiskSmart (Object)
    -----------------------

    Disk type related smart information.

    * nvme: NVMe disk smart

    Members
    ~~~~~~~

    "type": "GuestDiskBusType"
        Not documented

--> The members of "GuestNVMeSmart" when "type" is "nvme"

>> @@ -1780,7 +1784,7 @@
>>  #
>>  # Get statistics of each CPU in millisecond.
>>  #
>> -# - @linux: Linux style CPU statistics
>> +# @type: guest operating system
>>  #
>>  # Since: 7.1
>>  ##
>
> And more things which don't exist !

Likewise.

[PATCH 10/15] qapi/yank: Clean up documentaion of yank

[Markus Armbruster]

The command's doc comment describes the argument, but it's not marked
up as such.  Easy enough to fix.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qapi/pragma.json | 3 +--
 qapi/yank.json   | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/qapi/pragma.json b/qapi/pragma.json
index 0fa64742b5..544f138afa 100644
--- a/qapi/pragma.json
+++ b/qapi/pragma.json
@@ -148,8 +148,7 @@
         'query-rocker',
         'query-rocker-ports',
         'query-stats-schemas',
-        'watchdog-set-action',
-        'yank' ],
+        'watchdog-set-action' ],
     # Externally visible types whose member names may use uppercase
     'member-name-exceptions': [     # visible in:
         'ACPISlotType',             # query-acpi-ospm-status
diff --git a/qapi/yank.json b/qapi/yank.json
index 60eda20816..bfc71a07a6 100644
--- a/qapi/yank.json
+++ b/qapi/yank.json
@@ -74,7 +74,7 @@
 # Try to recover from hanging QEMU by yanking the specified instances.
 # See @YankInstance for more information.
 #
-# Takes a list of @YankInstance as argument.
+# @instances: the instances to be yanked
 #
 # Returns:
 #     - Nothing on success
-- 
2.43.0

Re: [PATCH 10/15] qapi/yank: Clean up documentaion of yank

[Daniel P. Berrangé]

On Mon, Feb 05, 2024 at 08:47:04AM +0100, Markus Armbruster wrote:
> The command's doc comment describes the argument, but it's not marked
> up as such.  Easy enough to fix.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  qapi/pragma.json | 3 +--
>  qapi/yank.json   | 2 +-
>  2 files changed, 2 insertions(+), 3 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[PATCH 11/15] qapi/dump: Clean up documentation of DumpGuestMemoryCapability

[Markus Armbruster]

The type's doc comment describes its member, but it's not marked up as
such.  Easy enough to fix.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qapi/dump.json   | 2 +-
 qapi/pragma.json | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/qapi/dump.json b/qapi/dump.json
index 5cbc237ad9..1997c1d1d4 100644
--- a/qapi/dump.json
+++ b/qapi/dump.json
@@ -186,7 +186,7 @@
 ##
 # @DumpGuestMemoryCapability:
 #
-# A list of the available formats for dump-guest-memory
+# @formats: the available formats for dump-guest-memory
 #
 # Since: 2.0
 ##
diff --git a/qapi/pragma.json b/qapi/pragma.json
index 544f138afa..aea6384255 100644
--- a/qapi/pragma.json
+++ b/qapi/pragma.json
@@ -74,7 +74,6 @@
         'DummyBlockCoreForceArrays',
         'DummyForceArrays',
         'DummyVirtioForceArrays',
-        'DumpGuestMemoryCapability',
         'GrabToggleKeys',
         'GuestPanicInformationHyperV',
         'HotKeyMod',
-- 
2.43.0

Re: [PATCH 11/15] qapi/dump: Clean up documentation of DumpGuestMemoryCapability

[Daniel P. Berrangé]

On Mon, Feb 05, 2024 at 08:47:05AM +0100, Markus Armbruster wrote:
> The type's doc comment describes its member, but it's not marked up as
> such.  Easy enough to fix.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  qapi/dump.json   | 2 +-
>  qapi/pragma.json | 1 -
>  2 files changed, 1 insertion(+), 2 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[PATCH 12/15] qapi: Plug trivial documentation holes around former simple unions

[Markus Armbruster]

The conversion of simple to flat unions left the @data members
undocumented.  Add documentation where it's trivial.  Copy verbatim
from the wrapped type's description where possible.

Leftovers: String (to be taken care of in the next commit), and
TransActionAction (left for another day).

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qapi/block-core.json | 10 ++++++++++
 qapi/char.json       | 26 ++++++++++++++++++++++++++
 qapi/machine.json    | 10 ++++++++++
 qapi/pragma.json     | 34 ----------------------------------
 qapi/sockets.json    |  6 ++++++
 qapi/tpm.json        |  4 ++++
 qapi/ui.json         | 12 ++++++++++++
 7 files changed, 68 insertions(+), 34 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 80ed4122f2..55b583f079 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -196,6 +196,8 @@
 ##
 # @ImageInfoSpecificQCow2Wrapper:
 #
+# @data: image information specific to QCOW2
+#
 # Since: 1.7
 ##
 { 'struct': 'ImageInfoSpecificQCow2Wrapper',
@@ -204,6 +206,8 @@
 ##
 # @ImageInfoSpecificVmdkWrapper:
 #
+# @data: image information specific to VMDK
+#
 # Since: 6.1
 ##
 { 'struct': 'ImageInfoSpecificVmdkWrapper',
@@ -212,6 +216,8 @@
 ##
 # @ImageInfoSpecificLUKSWrapper:
 #
+# @data: image information specific to LUKS
+#
 # Since: 2.7
 ##
 { 'struct': 'ImageInfoSpecificLUKSWrapper',
@@ -223,6 +229,8 @@
 ##
 # @ImageInfoSpecificRbdWrapper:
 #
+# @data: image information specific to RBD
+#
 # Since: 6.1
 ##
 { 'struct': 'ImageInfoSpecificRbdWrapper',
@@ -231,6 +239,8 @@
 ##
 # @ImageInfoSpecificFileWrapper:
 #
+# @data: image information specific to files
+#
 # Since: 8.0
 ##
 { 'struct': 'ImageInfoSpecificFileWrapper',
diff --git a/qapi/char.json b/qapi/char.json
index 6c6ad3b10c..e3e1b2c9f5 100644
--- a/qapi/char.json
+++ b/qapi/char.json
@@ -498,6 +498,8 @@
 ##
 # @ChardevFileWrapper:
 #
+# @data: Configuration info for file chardevs
+#
 # Since: 1.4
 ##
 { 'struct': 'ChardevFileWrapper',
@@ -506,6 +508,8 @@
 ##
 # @ChardevHostdevWrapper:
 #
+# @data: Configuration info for device and pipe chardevs
+#
 # Since: 1.4
 ##
 { 'struct': 'ChardevHostdevWrapper',
@@ -514,6 +518,8 @@
 ##
 # @ChardevSocketWrapper:
 #
+# @data: Configuration info for (stream) socket chardevs
+#
 # Since: 1.4
 ##
 { 'struct': 'ChardevSocketWrapper',
@@ -522,6 +528,8 @@
 ##
 # @ChardevUdpWrapper:
 #
+# @data: Configuration info for datagram socket chardevs
+#
 # Since: 1.5
 ##
 { 'struct': 'ChardevUdpWrapper',
@@ -530,6 +538,8 @@
 ##
 # @ChardevCommonWrapper:
 #
+# @data: Configuration shared across all chardev backends
+#
 # Since: 2.6
 ##
 { 'struct': 'ChardevCommonWrapper',
@@ -538,6 +548,8 @@
 ##
 # @ChardevMuxWrapper:
 #
+# @data: Configuration info for mux chardevs
+#
 # Since: 1.5
 ##
 { 'struct': 'ChardevMuxWrapper',
@@ -546,6 +558,8 @@
 ##
 # @ChardevStdioWrapper:
 #
+# @data: Configuration info for stdio chardevs
+#
 # Since: 1.5
 ##
 { 'struct': 'ChardevStdioWrapper',
@@ -554,6 +568,8 @@
 ##
 # @ChardevSpiceChannelWrapper:
 #
+# @data: Configuration info for spice vm channel chardevs
+#
 # Since: 1.5
 ##
 { 'struct': 'ChardevSpiceChannelWrapper',
@@ -563,6 +579,8 @@
 ##
 # @ChardevSpicePortWrapper:
 #
+# @data: Configuration info for spice port chardevs
+#
 # Since: 1.5
 ##
 { 'struct': 'ChardevSpicePortWrapper',
@@ -572,6 +590,8 @@
 ##
 # @ChardevQemuVDAgentWrapper:
 #
+# @data: Configuration info for qemu vdagent implementation
+#
 # Since: 6.1
 ##
 { 'struct': 'ChardevQemuVDAgentWrapper',
@@ -581,6 +601,8 @@
 ##
 # @ChardevDBusWrapper:
 #
+# @data: Configuration info for DBus chardevs
+#
 # Since: 7.0
 ##
 { 'struct': 'ChardevDBusWrapper',
@@ -590,6 +612,8 @@
 ##
 # @ChardevVCWrapper:
 #
+# @data: Configuration info for virtual console chardevs
+#
 # Since: 1.5
 ##
 { 'struct': 'ChardevVCWrapper',
@@ -598,6 +622,8 @@
 ##
 # @ChardevRingbufWrapper:
 #
+# @data: Configuration info for ring buffer chardevs
+#
 # Since: 1.5
 ##
 { 'struct': 'ChardevRingbufWrapper',
diff --git a/qapi/machine.json b/qapi/machine.json
index aa99fa333f..6a25e39f44 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1396,6 +1396,8 @@
 ##
 # @PCDIMMDeviceInfoWrapper:
 #
+# @data: PCDIMMDevice state information
+#
 # Since: 2.1
 ##
 { 'struct': 'PCDIMMDeviceInfoWrapper',
@@ -1404,6 +1406,8 @@
 ##
 # @VirtioPMEMDeviceInfoWrapper:
 #
+# @data: VirtioPMEM state information
+#
 # Since: 2.1
 ##
 { 'struct': 'VirtioPMEMDeviceInfoWrapper',
@@ -1412,6 +1416,8 @@
 ##
 # @VirtioMEMDeviceInfoWrapper:
 #
+# @data: VirtioMEMDevice state information
+#
 # Since: 2.1
 ##
 { 'struct': 'VirtioMEMDeviceInfoWrapper',
@@ -1420,6 +1426,8 @@
 ##
 # @SgxEPCDeviceInfoWrapper:
 #
+# @data: Sgx EPC state information
+#
 # Since: 6.2
 ##
 { 'struct': 'SgxEPCDeviceInfoWrapper',
@@ -1428,6 +1436,8 @@
 ##
 # @HvBalloonDeviceInfoWrapper:
 #
+# @data: hv-balloon provided memory state information
+#
 # Since: 8.2
 ##
 { 'struct': 'HvBalloonDeviceInfoWrapper',
diff --git a/qapi/pragma.json b/qapi/pragma.json
index aea6384255..d5e3f6f142 100644
--- a/qapi/pragma.json
+++ b/qapi/pragma.json
@@ -52,19 +52,6 @@
         'BlockdevVmdkAdapterType',
         'ChardevBackend',
         'ChardevBackendKind',
-        'ChardevCommonWrapper',
-        'ChardevDBusWrapper',
-        'ChardevFileWrapper',
-        'ChardevHostdevWrapper',
-        'ChardevMuxWrapper',
-        'ChardevQemuVDAgentWrapper',
-        'ChardevRingbufWrapper',
-        'ChardevSocketWrapper',
-        'ChardevSpiceChannelWrapper',
-        'ChardevSpicePortWrapper',
-        'ChardevStdioWrapper',
-        'ChardevUdpWrapper',
-        'ChardevVCWrapper',
         'CpuS390Entitlement',
         'CpuS390Polarization',
         'CpuS390State',
@@ -77,24 +64,12 @@
         'GrabToggleKeys',
         'GuestPanicInformationHyperV',
         'HotKeyMod',
-        'HvBalloonDeviceInfoWrapper',
         'ImageInfoSpecific',
-        'ImageInfoSpecificFileWrapper',
         'ImageInfoSpecificKind',
-        'ImageInfoSpecificLUKSWrapper',
-        'ImageInfoSpecificQCow2Wrapper',
-        'ImageInfoSpecificRbdWrapper',
-        'ImageInfoSpecificVmdkWrapper',
-        'InetSocketAddressWrapper',
         'InputAxis',
-        'InputBtnEventWrapper',
         'InputButton',
-        'InputKeyEventWrapper',
-        'InputMoveEventWrapper',
         'InputMultiTouchEvent',
-        'InputMultiTouchEventWrapper',
         'InputMultiTouchType',
-        'IntWrapper',
         'IscsiHeaderDigest',
         'IscsiTransport',
         'JSONType',
@@ -107,18 +82,15 @@
         'NetClientDriver',
         'NumaOptions',
         'ObjectType',
-        'PCDIMMDeviceInfoWrapper',
         'PciMemoryRegion',
         'QCryptoAkCipherKeyType',
         'QCryptoAkCipherOptions',
         'QCryptodevBackendServiceType',
         'QKeyCode',
-        'QKeyCodeWrapper',
         'Qcow2OverlapCheckFlags',
         'RbdAuthMode',
         'RbdEncryptionCreateOptions',
         'RbdImageEncryptionFormat',
-        'SgxEPCDeviceInfoWrapper',
         'SocketAddressLegacy',
         'SshHostKeyCheck',
         'StatsFilter',
@@ -126,16 +98,10 @@
         'String',
         'StringWrapper',
         'SysEmuTarget',
-        'TPMEmulatorOptionsWrapper',
-        'TPMPassthroughOptionsWrapper',
         'ThrottleGroupProperties',
         'TransactionAction',
-        'UnixSocketAddressWrapper',
-        'VirtioMEMDeviceInfoWrapper',
-        'VirtioPMEMDeviceInfoWrapper',
         'VncPrimaryAuth',
         'VncVencryptSubAuth',
-        'VsockSocketAddressWrapper',
         'X86CPURegister32',
         'XDbgBlockGraph',
         'YankInstance',
diff --git a/qapi/sockets.json b/qapi/sockets.json
index 6213154525..c3b616731d 100644
--- a/qapi/sockets.json
+++ b/qapi/sockets.json
@@ -119,6 +119,8 @@
 ##
 # @InetSocketAddressWrapper:
 #
+# @data: internet domain socket address
+#
 # Since: 1.3
 ##
 { 'struct': 'InetSocketAddressWrapper',
@@ -127,6 +129,8 @@
 ##
 # @UnixSocketAddressWrapper:
 #
+# @data: UNIX domain socket address
+#
 # Since: 1.3
 ##
 { 'struct': 'UnixSocketAddressWrapper',
@@ -135,6 +139,8 @@
 ##
 # @VsockSocketAddressWrapper:
 #
+# @data: VSOCK domain socket address
+#
 # Since: 2.8
 ##
 { 'struct': 'VsockSocketAddressWrapper',
diff --git a/qapi/tpm.json b/qapi/tpm.json
index a754455ca5..f9c1e866e7 100644
--- a/qapi/tpm.json
+++ b/qapi/tpm.json
@@ -102,6 +102,8 @@
 ##
 # @TPMPassthroughOptionsWrapper:
 #
+# @data: Information about the TPM passthrough type
+#
 # Since: 1.5
 ##
 { 'struct': 'TPMPassthroughOptionsWrapper',
@@ -111,6 +113,8 @@
 ##
 # @TPMEmulatorOptionsWrapper:
 #
+# @data: Information about the TPM emulator type
+#
 # Since: 2.11
 ##
 { 'struct': 'TPMEmulatorOptionsWrapper',
diff --git a/qapi/ui.json b/qapi/ui.json
index a0158baf23..1eccad0a83 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -990,6 +990,8 @@
 ##
 # @IntWrapper:
 #
+# @data: a numeric key code
+#
 # Since: 1.3
 ##
 { 'struct': 'IntWrapper',
@@ -998,6 +1000,8 @@
 ##
 # @QKeyCodeWrapper:
 #
+# @data: An enumeration of key name
+#
 # Since: 1.3
 ##
 { 'struct': 'QKeyCodeWrapper',
@@ -1175,6 +1179,8 @@
 ##
 # @InputKeyEventWrapper:
 #
+# @data: Keyboard input event
+#
 # Since: 2.0
 ##
 { 'struct': 'InputKeyEventWrapper',
@@ -1183,6 +1189,8 @@
 ##
 # @InputBtnEventWrapper:
 #
+# @data: Pointer button input event
+#
 # Since: 2.0
 ##
 { 'struct': 'InputBtnEventWrapper',
@@ -1191,6 +1199,8 @@
 ##
 # @InputMoveEventWrapper:
 #
+# @data: Pointer motion input event
+#
 # Since: 2.0
 ##
 { 'struct': 'InputMoveEventWrapper',
@@ -1199,6 +1209,8 @@
 ##
 # @InputMultiTouchEventWrapper:
 #
+# @data: MultiTouch input event
+#
 # Since: 8.1
 ##
 { 'struct': 'InputMultiTouchEventWrapper',
-- 
2.43.0

Re: [PATCH 12/15] qapi: Plug trivial documentation holes around former simple unions

[Daniel P. Berrangé]

On Mon, Feb 05, 2024 at 08:47:06AM +0100, Markus Armbruster wrote:
> The conversion of simple to flat unions left the @data members
> undocumented.  Add documentation where it's trivial.  Copy verbatim
> from the wrapped type's description where possible.
> 
> Leftovers: String (to be taken care of in the next commit), and
> TransActionAction (left for another day).
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  qapi/block-core.json | 10 ++++++++++
>  qapi/char.json       | 26 ++++++++++++++++++++++++++
>  qapi/machine.json    | 10 ++++++++++
>  qapi/pragma.json     | 34 ----------------------------------
>  qapi/sockets.json    |  6 ++++++
>  qapi/tpm.json        |  4 ++++
>  qapi/ui.json         | 12 ++++++++++++
>  7 files changed, 68 insertions(+), 34 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[PATCH 13/15] qapi: Improve documentation of file descriptor socket addresses

[Markus Armbruster]

SocketAddress branch @fd is documented in enum SocketAddressType,
unlike the other branches.  That's because the branch's type is String
from common.json.

Use a local copy of String, so we can put the documentation in the
usual place.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qapi/sockets.json                      | 40 +++++++++++++++++---------
 include/hw/virtio/vhost-vsock-common.h |  1 +
 chardev/char-socket.c                  |  2 +-
 util/qemu-sockets.c                    |  3 +-
 4 files changed, 31 insertions(+), 15 deletions(-)

diff --git a/qapi/sockets.json b/qapi/sockets.json
index c3b616731d..5e6af5504d 100644
--- a/qapi/sockets.json
+++ b/qapi/sockets.json
@@ -5,8 +5,6 @@
 # = Socket data types
 ##
 
-{ 'include': 'common.json' }
-
 ##
 # @NetworkAddressFamily:
 #
@@ -116,6 +114,24 @@
     'cid': 'str',
     'port': 'str' } }
 
+##
+# @FdSocketAddress:
+#
+# A file descriptor name or number.
+#
+# @str: decimal is for file descriptor number, otherwise it's a file
+#     descriptor name.  Named file descriptors are permitted in
+#     monitor commands, in combination with the 'getfd' command.
+#     Decimal file descriptors are permitted at startup or other
+#     contexts where no monitor context is active.
+#
+#
+# Since: 1.2
+##
+{ 'struct': 'FdSocketAddress',
+  'data': {
+    'str': 'str' } }
+
 ##
 # @InetSocketAddressWrapper:
 #
@@ -147,12 +163,14 @@
   'data': { 'data': 'VsockSocketAddress' } }
 
 ##
-# @StringWrapper:
+# @FdSocketAddressWrapper:
+#
+# @data: file descriptor name or number
 #
 # Since: 1.3
 ##
-{ 'struct': 'StringWrapper',
-  'data': { 'data': 'String' } }
+{ 'struct': 'FdSocketAddressWrapper',
+  'data': { 'data': 'FdSocketAddress' } }
 
 ##
 # @SocketAddressLegacy:
@@ -173,7 +191,7 @@
     'inet': 'InetSocketAddressWrapper',
     'unix': 'UnixSocketAddressWrapper',
     'vsock': 'VsockSocketAddressWrapper',
-    'fd': 'StringWrapper' } }
+    'fd': 'FdSocketAddressWrapper' } }
 
 ##
 # @SocketAddressType:
@@ -186,11 +204,7 @@
 #
 # @vsock: VMCI address
 #
-# @fd: decimal is for file descriptor number, otherwise a file
-#     descriptor name.  Named file descriptors are permitted in
-#     monitor commands, in combination with the 'getfd' command.
-#     Decimal file descriptors are permitted at startup or other
-#     contexts where no monitor context is active.
+# @fd: Socket file descriptor
 #
 # Since: 2.9
 ##
@@ -200,7 +214,7 @@
 ##
 # @SocketAddress:
 #
-# Captures the address of a socket, which could also be a named file
+# Captures the address of a socket, which could also be a socket file
 # descriptor
 #
 # @type: Transport type
@@ -213,4 +227,4 @@
   'data': { 'inet': 'InetSocketAddress',
             'unix': 'UnixSocketAddress',
             'vsock': 'VsockSocketAddress',
-            'fd': 'String' } }
+            'fd': 'FdSocketAddress' } }
diff --git a/include/hw/virtio/vhost-vsock-common.h b/include/hw/virtio/vhost-vsock-common.h
index 93c782101d..75a74e8a99 100644
--- a/include/hw/virtio/vhost-vsock-common.h
+++ b/include/hw/virtio/vhost-vsock-common.h
@@ -11,6 +11,7 @@
 #ifndef QEMU_VHOST_VSOCK_COMMON_H
 #define QEMU_VHOST_VSOCK_COMMON_H
 
+#include "qapi/qapi-types-common.h"
 #include "hw/virtio/virtio.h"
 #include "hw/virtio/vhost.h"
 #include "qom/object.h"
diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 73947da188..ff8f845cca 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -1504,7 +1504,7 @@ static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend,
         };
     } else {
         addr->type = SOCKET_ADDRESS_TYPE_FD;
-        addr->u.fd.data = g_new(String, 1);
+        addr->u.fd.data = g_new(FdSocketAddress, 1);
         addr->u.fd.data->str = g_strdup(fd);
     }
     sock->addr = addr;
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 83e84b1186..60c44b2b56 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -1464,7 +1464,8 @@ SocketAddress *socket_address_flatten(SocketAddressLegacy *addr_legacy)
         break;
     case SOCKET_ADDRESS_TYPE_FD:
         addr->type = SOCKET_ADDRESS_TYPE_FD;
-        QAPI_CLONE_MEMBERS(String, &addr->u.fd, addr_legacy->u.fd.data);
+        QAPI_CLONE_MEMBERS(FdSocketAddress, &addr->u.fd,
+                           addr_legacy->u.fd.data);
         break;
     default:
         abort();
-- 
2.43.0

Re: [PATCH 13/15] qapi: Improve documentation of file descriptor socket addresses

[Daniel P. Berrangé]

On Mon, Feb 05, 2024 at 08:47:07AM +0100, Markus Armbruster wrote:
> SocketAddress branch @fd is documented in enum SocketAddressType,
> unlike the other branches.  That's because the branch's type is String
> from common.json.
> 
> Use a local copy of String, so we can put the documentation in the
> usual place.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  qapi/sockets.json                      | 40 +++++++++++++++++---------
>  include/hw/virtio/vhost-vsock-common.h |  1 +
>  chardev/char-socket.c                  |  2 +-
>  util/qemu-sockets.c                    |  3 +-
>  4 files changed, 31 insertions(+), 15 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[PATCH 14/15] qapi: Move @String out of common.json to discourage reuse

[Markus Armbruster]

Use of String is problematic, because it results in awkward interface
documentation.  The previous commit cleaned up one instance.

Move String out of common.json next to its remaining users in net.json
to discourage reuse elsewhere.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qapi/common.json     | 11 -----------
 qapi/net.json        | 12 +++++++++++-
 include/net/filter.h |  2 +-
 3 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/qapi/common.json b/qapi/common.json
index 6fed9cde1a..f1bb841951 100644
--- a/qapi/common.json
+++ b/qapi/common.json
@@ -51,17 +51,6 @@
 { 'enum': 'OnOffSplit',
   'data': [ 'on', 'off', 'split' ] }
 
-##
-# @String:
-#
-# A fat type wrapping 'str', to be embedded in lists.
-#
-# Since: 1.2
-##
-{ 'struct': 'String',
-  'data': {
-    'str': 'str' } }
-
 ##
 # @StrOrNull:
 #
diff --git a/qapi/net.json b/qapi/net.json
index 68493d6ac9..0a993e1a3d 100644
--- a/qapi/net.json
+++ b/qapi/net.json
@@ -6,7 +6,6 @@
 # = Net devices
 ##
 
-{ 'include': 'common.json' }
 { 'include': 'sockets.json' }
 
 ##
@@ -105,6 +104,17 @@
     '*addr':    'str',
     '*vectors': 'uint32' } }
 
+##
+# @String:
+#
+# A fat type wrapping 'str', to be embedded in lists.
+#
+# Since: 1.2
+##
+{ 'struct': 'String',
+  'data': {
+    'str': 'str' } }
+
 ##
 # @NetdevUserOptions:
 #
diff --git a/include/net/filter.h b/include/net/filter.h
index 27ffc630df..f15f7932b2 100644
--- a/include/net/filter.h
+++ b/include/net/filter.h
@@ -9,7 +9,7 @@
 #ifndef QEMU_NET_FILTER_H
 #define QEMU_NET_FILTER_H
 
-#include "qapi/qapi-types-net.h"
+#include "qapi/qapi-types-common.h"
 #include "qemu/queue.h"
 #include "qom/object.h"
 #include "net/queue.h"
-- 
2.43.0

Re: [PATCH 14/15] qapi: Move @String out of common.json to discourage reuse

[Daniel P. Berrangé]

On Mon, Feb 05, 2024 at 08:47:08AM +0100, Markus Armbruster wrote:
> Use of String is problematic, because it results in awkward interface
> documentation.  The previous commit cleaned up one instance.
> 
> Move String out of common.json next to its remaining users in net.json
> to discourage reuse elsewhere.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  qapi/common.json     | 11 -----------
>  qapi/net.json        | 12 +++++++++++-
>  include/net/filter.h |  2 +-
>  3 files changed, 12 insertions(+), 13 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

[PATCH 15/15] qapi: Add missing union tag documentation

[Markus Armbruster]

Low-hanging fruit, and except for StatsFilter, the only members of
these unions lacking documentation.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qapi/block-core.json   | 12 ++++++++++++
 qapi/block-export.json |  2 ++
 qapi/char.json         |  2 ++
 qapi/crypto.json       |  2 ++
 qapi/machine.json      |  4 ++++
 qapi/migration.json    |  2 ++
 qapi/pragma.json       | 16 ----------------
 qapi/sockets.json      |  2 ++
 qapi/stats.json        |  2 ++
 qapi/transaction.json  |  2 ++
 qapi/ui.json           |  2 ++
 qapi/yank.json         |  2 ++
 12 files changed, 34 insertions(+), 16 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 55b583f079..ded6437c06 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -252,6 +252,8 @@
 # A discriminated record of image format specific information
 # structures.
 #
+# @type: block driver name
+#
 # Since: 1.7
 ##
 { 'union': 'ImageInfoSpecific',
@@ -1102,6 +1104,8 @@
 #
 # Block driver specific statistics
 #
+# @driver: block driver name
+#
 # Since: 4.2
 ##
 { 'union': 'BlockStatsSpecific',
@@ -3472,6 +3476,8 @@
 ##
 # @BlockdevQcowEncryption:
 #
+# @format: encryption format
+#
 # Since: 2.10
 ##
 { 'union': 'BlockdevQcowEncryption',
@@ -3506,6 +3512,8 @@
 ##
 # @BlockdevQcow2Encryption:
 #
+# @format: encryption format
+#
 # Since: 2.10
 ##
 { 'union': 'BlockdevQcow2Encryption',
@@ -3656,6 +3664,8 @@
 ##
 # @SshHostKeyCheck:
 #
+# @mode: How to check the host key
+#
 # Since: 2.12
 ##
 { 'union': 'SshHostKeyCheck',
@@ -4225,6 +4235,8 @@
 ##
 # @RbdEncryptionCreateOptions:
 #
+# @format: Encryption format.
+#
 # Since: 6.1
 ##
 { 'union': 'RbdEncryptionCreateOptions',
diff --git a/qapi/block-export.json b/qapi/block-export.json
index e063e9255a..d9bd376b48 100644
--- a/qapi/block-export.json
+++ b/qapi/block-export.json
@@ -346,6 +346,8 @@
 # Describes a block export, i.e. how single node should be exported on
 # an external interface.
 #
+# @type: Block export type
+#
 # @id: A unique identifier for the block export (across all export
 #     types)
 #
diff --git a/qapi/char.json b/qapi/char.json
index e3e1b2c9f5..390e3ef1b9 100644
--- a/qapi/char.json
+++ b/qapi/char.json
@@ -634,6 +634,8 @@
 #
 # Configuration info for the new chardev backend.
 #
+# @type: backend type
+#
 # Since: 1.4
 ##
 { 'union': 'ChardevBackend',
diff --git a/qapi/crypto.json b/qapi/crypto.json
index fd3d46ebd1..03de66e6f6 100644
--- a/qapi/crypto.json
+++ b/qapi/crypto.json
@@ -645,6 +645,8 @@
 # The options that are available for all asymmetric key algorithms
 # when creating a new QCryptoAkCipher.
 #
+# @alg: encryption cipher algorithm
+#
 # Since: 7.1
 ##
 { 'union': 'QCryptoAkCipherOptions',
diff --git a/qapi/machine.json b/qapi/machine.json
index 6a25e39f44..d816c5c02e 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -443,6 +443,8 @@
 #
 # A discriminated record of NUMA options.  (for OptsVisitor)
 #
+# @type: NUMA option type
+#
 # Since: 2.1
 ##
 { 'union': 'NumaOptions',
@@ -1448,6 +1450,8 @@
 #
 # Union containing information about a memory device
 #
+# @type: memory device type
+#
 # Since: 2.1
 ##
 { 'union': 'MemoryDeviceInfo',
diff --git a/qapi/migration.json b/qapi/migration.json
index bf89765a26..7c8881abda 100644
--- a/qapi/migration.json
+++ b/qapi/migration.json
@@ -1630,6 +1630,8 @@
 #
 # Migration endpoint configuration.
 #
+# @transport: The migration stream transport mechanism
+#
 # Since: 8.2
 ##
 { 'union': 'MigrationAddress',
diff --git a/qapi/pragma.json b/qapi/pragma.json
index d5e3f6f142..7ac05ccc26 100644
--- a/qapi/pragma.json
+++ b/qapi/pragma.json
@@ -39,18 +39,13 @@
         'BlockDirtyBitmapAddWrapper',
         'BlockDirtyBitmapMergeWrapper',
         'BlockDirtyBitmapWrapper',
-        'BlockExportOptions',
-        'BlockStatsSpecific',
         'BlockdevBackupWrapper',
         'BlockdevDriver',
-        'BlockdevQcow2Encryption',
         'BlockdevQcow2EncryptionFormat',
-        'BlockdevQcowEncryption',
         'BlockdevSnapshotInternalWrapper',
         'BlockdevSnapshotSyncWrapper',
         'BlockdevSnapshotWrapper',
         'BlockdevVmdkAdapterType',
-        'ChardevBackend',
         'ChardevBackendKind',
         'CpuS390Entitlement',
         'CpuS390Polarization',
@@ -64,7 +59,6 @@
         'GrabToggleKeys',
         'GuestPanicInformationHyperV',
         'HotKeyMod',
-        'ImageInfoSpecific',
         'ImageInfoSpecificKind',
         'InputAxis',
         'InputButton',
@@ -73,38 +67,28 @@
         'IscsiHeaderDigest',
         'IscsiTransport',
         'JSONType',
-        'KeyValue',
         'KeyValueKind',
-        'MemoryDeviceInfo',
         'MemoryDeviceInfoKind',
         'MigrateSetParameters',
-        'MigrationAddress',
         'NetClientDriver',
-        'NumaOptions',
         'ObjectType',
         'PciMemoryRegion',
         'QCryptoAkCipherKeyType',
-        'QCryptoAkCipherOptions',
         'QCryptodevBackendServiceType',
         'QKeyCode',
         'Qcow2OverlapCheckFlags',
         'RbdAuthMode',
-        'RbdEncryptionCreateOptions',
         'RbdImageEncryptionFormat',
-        'SocketAddressLegacy',
-        'SshHostKeyCheck',
         'StatsFilter',
         'StatsValue',
         'String',
         'StringWrapper',
         'SysEmuTarget',
         'ThrottleGroupProperties',
-        'TransactionAction',
         'VncPrimaryAuth',
         'VncVencryptSubAuth',
         'X86CPURegister32',
         'XDbgBlockGraph',
-        'YankInstance',
         'YankInstanceType',
         'blockdev-reopen',
         'query-cpu-model-baseline',
diff --git a/qapi/sockets.json b/qapi/sockets.json
index 5e6af5504d..ef777928e7 100644
--- a/qapi/sockets.json
+++ b/qapi/sockets.json
@@ -178,6 +178,8 @@
 # Captures the address of a socket, which could also be a named file
 # descriptor
 #
+# @type: Transport type
+#
 # Note: This type is deprecated in favor of SocketAddress.  The
 #     difference between SocketAddressLegacy and SocketAddress is that
 #     the latter has fewer {} on the wire.
diff --git a/qapi/stats.json b/qapi/stats.json
index 01791e86d5..ce9d8161ec 100644
--- a/qapi/stats.json
+++ b/qapi/stats.json
@@ -120,6 +120,8 @@
 # - which providers to request statistics from
 # - which named values to return within each provider
 #
+# @target: the kind of objects to query
+#
 # Since: 7.1
 ##
 { 'union': 'StatsFilter',
diff --git a/qapi/transaction.json b/qapi/transaction.json
index cffee2de28..7a95c081e9 100644
--- a/qapi/transaction.json
+++ b/qapi/transaction.json
@@ -158,6 +158,8 @@
 # A discriminated record of operations that can be performed with
 # @transaction.
 #
+# @type: the operation to be performed
+#
 # Since: 1.1
 ##
 { 'union': 'TransactionAction',
diff --git a/qapi/ui.json b/qapi/ui.json
index 1eccad0a83..b6d7e142b7 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -1012,6 +1012,8 @@
 #
 # Represents a keyboard key.
 #
+# @type: key encoding
+#
 # Since: 1.3
 ##
 { 'union': 'KeyValue',
diff --git a/qapi/yank.json b/qapi/yank.json
index bfc71a07a6..ee038a11a1 100644
--- a/qapi/yank.json
+++ b/qapi/yank.json
@@ -49,6 +49,8 @@
 # A yank instance can be yanked with the @yank qmp command to recover
 # from a hanging QEMU.
 #
+# @type: yank instance type
+#
 # Currently implemented yank instances:
 #
 # - nbd block device: Yanking it will shut down the connection to the
-- 
2.43.0

Re: [PATCH 15/15] qapi: Add missing union tag documentation

[Daniel P. Berrangé]

On Mon, Feb 05, 2024 at 08:47:09AM +0100, Markus Armbruster wrote:
> Low-hanging fruit, and except for StatsFilter, the only members of
> these unions lacking documentation.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  qapi/block-core.json   | 12 ++++++++++++
>  qapi/block-export.json |  2 ++
>  qapi/char.json         |  2 ++
>  qapi/crypto.json       |  2 ++
>  qapi/machine.json      |  4 ++++
>  qapi/migration.json    |  2 ++
>  qapi/pragma.json       | 16 ----------------
>  qapi/sockets.json      |  2 ++
>  qapi/stats.json        |  2 ++
>  qapi/transaction.json  |  2 ++
>  qapi/ui.json           |  2 ++
>  qapi/yank.json         |  2 ++
>  12 files changed, 34 insertions(+), 16 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH 00/15] qapi: Require member documentation (with loophole)

[Peter Xu]

On Mon, Feb 05, 2024 at 08:46:54AM +0100, Markus Armbruster wrote:
>     qapi/migration.json
> 	MigrateSetParameters 1

It's tls-authz.  I'll send a patch for this one.

Thanks,

-- 
Peter Xu