qapi: Require member documentation (with loophole)

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> Message-ID: <20240205074709.3613229-7-armbru@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
qemu · Feb 12, 2024 · 0cec501 · 0cec501
1 parent fd62bff
commit 0cec501
Show file tree

Hide file tree

Showing 7 changed files with 149 additions and 3 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -3,7 +3,9 @@
 #
 # Positive QAPI doc comment tests
 
-{ 'pragma': { 'doc-required': true } }
+{ 'pragma': {
+    'doc-required': true,
+    'documentation-exceptions': [ 'Enum', 'Variant1', 'Alternate', 'cmd' ] } }
 
 ##
 # = Section