[PATCH 0/5] qapi: Doc comment cleanups

[PATCH 0/5] qapi: Doc comment cleanups

[Markus Armbruster]

Markus Armbruster (5):
  qapi/pci: Clean up documentation around PciDeviceClass
  qapi/machine: Clean up documentation around CpuInstanceProperties
  qapi/machine: Clarify query-uuid value when none has been specified
  qapi/sockets: Move deprecation note out of SocketAddress doc comment
  qapi/ui: Drop note on naming of SpiceQueryMouseMode

 qapi/machine.json | 18 ++++++++----------
 qapi/pci.json     |  6 ++----
 qapi/sockets.json |  7 +++----
 qapi/ui.json      |  2 --
 4 files changed, 13 insertions(+), 20 deletions(-)

-- 
2.45.0

[PATCH 1/5] qapi/pci: Clean up documentation around PciDeviceClass

[Markus Armbruster]

PciDeviceInfo's doc comment has a note on PciDeviceClass member @desc.
Since the note applies always, not just within PciDeviceInfo, merge it
into PciDeviceClass's description of member @desc.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qapi/pci.json | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/qapi/pci.json b/qapi/pci.json
index 8287d15dd0..97179920fb 100644
--- a/qapi/pci.json
+++ b/qapi/pci.json
@@ -93,7 +93,8 @@
 #
 # Information about the Class of a PCI device
 #
-# @desc: a string description of the device's class
+# @desc: a string description of the device's class (not stable, and
+#     should only be treated as informational)
 #
 # @class: the class code of the device
 #
@@ -146,9 +147,6 @@
 #
 # @regions: a list of the PCI I/O regions associated with the device
 #
-# .. note:: The contents of @class_info.desc are not stable and should
-#    only be treated as informational.
-#
 # Since: 0.14
 ##
 { 'struct': 'PciDeviceInfo',
-- 
2.45.0

Re: [PATCH 1/5] qapi/pci: Clean up documentation around PciDeviceClass

[John Snow]

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

On Thu, Jul 11, 2024, 7:24 AM Markus Armbruster <armbru@redhat.com> wrote:

> PciDeviceInfo's doc comment has a note on PciDeviceClass member @desc.
> Since the note applies always, not just within PciDeviceInfo, merge it
> into PciDeviceClass's description of member @desc.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>

Reviewed-by: John Snow <jsnow@redhat.com>

---
>  qapi/pci.json | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/qapi/pci.json b/qapi/pci.json
> index 8287d15dd0..97179920fb 100644
> --- a/qapi/pci.json
> +++ b/qapi/pci.json
> @@ -93,7 +93,8 @@
>  #
>  # Information about the Class of a PCI device
>  #
> -# @desc: a string description of the device's class
> +# @desc: a string description of the device's class (not stable, and
> +#     should only be treated as informational)
>  #
>  # @class: the class code of the device
>  #
> @@ -146,9 +147,6 @@
>  #
>  # @regions: a list of the PCI I/O regions associated with the device
>  #
> -# .. note:: The contents of @class_info.desc are not stable and should
> -#    only be treated as informational.
> -#
>  # Since: 0.14
>  ##
>  { 'struct': 'PciDeviceInfo',
> --
> 2.45.0
>
>
>

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

[PATCH 2/5] qapi/machine: Clean up documentation around CpuInstanceProperties

[Markus Armbruster]

CpuInstanceProperties' doc comment describes its members as properties
to be passed to device_add when hot-plugging a CPU.

This was in fact the initial use of this type, with
query-hotpluggable-cpus: letting management applications find out what
properties need to be passed with device_add to hot-plug a CPU.

We've since added other uses: set-numa-node (commit 419fcdec3c1 and
f3be67812c2), and query-cpus-fast (commit ce74ee3dea6).  These are not
about device-add.

query-hotpluggable-cpus uses CpuInstanceProperties within
HotpluggableCPU.  Lift the documentation related to device-add from
CpuInstanceProperties to HotpluggableCPU.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qapi/machine.json | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/qapi/machine.json b/qapi/machine.json
index f15ad1b43e..50ff102d56 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -960,9 +960,7 @@
 ##
 # @CpuInstanceProperties:
 #
-# List of properties to be used for hotplugging a CPU instance, it
-# should be passed by management with device_add command when a CPU is
-# being hotplugged.
+# Properties identifying a CPU.
 #
 # Which members are optional and which mandatory depends on the
 # architecture and board.
@@ -996,9 +994,6 @@
 #
 # @thread-id: thread number within the core the CPU  belongs to
 #
-# .. note:: Management should be prepared to pass through additional
-#    properties with device_add.
-#
 # Since: 2.7
 ##
 { 'struct': 'CpuInstanceProperties',
@@ -1020,7 +1015,8 @@
 #
 # @type: CPU object type for usage with device_add command
 #
-# @props: list of properties to be used for hotplugging CPU
+# @props: list of properties to pass for hotplugging a CPU with
+#     device_add
 #
 # @vcpus-count: number of logical VCPU threads @HotpluggableCPU
 #     provides
@@ -1028,6 +1024,9 @@
 # @qom-path: link to existing CPU object if CPU is present or omitted
 #     if CPU is not present.
 #
+# .. note:: Management should be prepared to pass through additional
+#    properties with device_add.
+#
 # Since: 2.7
 ##
 { 'struct': 'HotpluggableCPU',
-- 
2.45.0

Re: [PATCH 2/5] qapi/machine: Clean up documentation around CpuInstanceProperties

[John Snow]

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

On Thu, Jul 11, 2024, 7:22 AM Markus Armbruster <armbru@redhat.com> wrote:

> CpuInstanceProperties' doc comment describes its members as properties
> to be passed to device_add when hot-plugging a CPU.
>
> This was in fact the initial use of this type, with
> query-hotpluggable-cpus: letting management applications find out what
> properties need to be passed with device_add to hot-plug a CPU.
>
> We've since added other uses: set-numa-node (commit 419fcdec3c1 and
> f3be67812c2), and query-cpus-fast (commit ce74ee3dea6).  These are not
> about device-add.
>
> query-hotpluggable-cpus uses CpuInstanceProperties within
> HotpluggableCPU.  Lift the documentation related to device-add from
> CpuInstanceProperties to HotpluggableCPU.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>

Reviewed-by: John Snow <jsnow@redhat.com>

---
>  qapi/machine.json | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/qapi/machine.json b/qapi/machine.json
> index f15ad1b43e..50ff102d56 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -960,9 +960,7 @@
>  ##
>  # @CpuInstanceProperties:
>  #
> -# List of properties to be used for hotplugging a CPU instance, it
> -# should be passed by management with device_add command when a CPU is
> -# being hotplugged.
> +# Properties identifying a CPU.
>  #
>  # Which members are optional and which mandatory depends on the
>  # architecture and board.
> @@ -996,9 +994,6 @@
>  #
>  # @thread-id: thread number within the core the CPU  belongs to
>  #
> -# .. note:: Management should be prepared to pass through additional
> -#    properties with device_add.
> -#
>  # Since: 2.7
>  ##
>  { 'struct': 'CpuInstanceProperties',
> @@ -1020,7 +1015,8 @@
>  #
>  # @type: CPU object type for usage with device_add command
>  #
> -# @props: list of properties to be used for hotplugging CPU
> +# @props: list of properties to pass for hotplugging a CPU with
> +#     device_add
>  #
>  # @vcpus-count: number of logical VCPU threads @HotpluggableCPU
>  #     provides
> @@ -1028,6 +1024,9 @@
>  # @qom-path: link to existing CPU object if CPU is present or omitted
>  #     if CPU is not present.
>  #
> +# .. note:: Management should be prepared to pass through additional
> +#    properties with device_add.
> +#
>  # Since: 2.7
>  ##
>  { 'struct': 'HotpluggableCPU',
> --
> 2.45.0
>
>

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

[PATCH 3/5] qapi/machine: Clarify query-uuid value when none has been specified

[Markus Armbruster]

When no UUID has been specified, query-uuid returns

    {"UUID": "00000000-0000-0000-0000-000000000000"}

The doc comment calls this "a null UUID", which I find less than
clear.  Change it to "an all-zero UUID".

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

diff --git a/qapi/machine.json b/qapi/machine.json
index 50ff102d56..f40427f21a 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -305,9 +305,8 @@
 #
 # Since: 0.14
 #
-# .. note:: If no UUID was specified for the guest, a null UUID is
-#    returned.
-#
+# .. note:: If no UUID was specified for the guest, an all-zero UUID
+#    is returned.
 ##
 { 'struct': 'UuidInfo', 'data': {'UUID': 'str'} }
 
-- 
2.45.0

Re: [PATCH 3/5] qapi/machine: Clarify query-uuid value when none has been specified

[John Snow]

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

On Thu, Jul 11, 2024, 7:22 AM Markus Armbruster <armbru@redhat.com> wrote:

> When no UUID has been specified, query-uuid returns
>
>     {"UUID": "00000000-0000-0000-0000-000000000000"}
>
> The doc comment calls this "a null UUID", which I find less than
> clear.  Change it to "an all-zero UUID".
>

Technically it's a "nil UUID";
https://datatracker.ietf.org/doc/html/rfc9562#name-nil-uuid

If you wanted to be pedantic, you could say "the nil UUID (all zeroes) is
returned"

but your rephrasing is clear even w/o using the standard name, so I'm fine
either way.


> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>

Reviewed-by: John Snow <jsnow@redhat.com>

---
>  qapi/machine.json | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/qapi/machine.json b/qapi/machine.json
> index 50ff102d56..f40427f21a 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -305,9 +305,8 @@
>  #
>  # Since: 0.14
>  #
> -# .. note:: If no UUID was specified for the guest, a null UUID is
> -#    returned.
> -#
> +# .. note:: If no UUID was specified for the guest, an all-zero UUID
> +#    is returned.
>  ##
>  { 'struct': 'UuidInfo', 'data': {'UUID': 'str'} }
>
> --
> 2.45.0
>
>

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

Re: [PATCH 3/5] qapi/machine: Clarify query-uuid value when none has been specified

[Markus Armbruster]

John Snow <jsnow@redhat.com> writes:

> On Thu, Jul 11, 2024, 7:22 AM Markus Armbruster <armbru@redhat.com> wrote:
>
>> When no UUID has been specified, query-uuid returns
>>
>>     {"UUID": "00000000-0000-0000-0000-000000000000"}
>>
>> The doc comment calls this "a null UUID", which I find less than
>> clear.  Change it to "an all-zero UUID".
>>
>
> Technically it's a "nil UUID";
> https://datatracker.ietf.org/doc/html/rfc9562#name-nil-uuid
>
> If you wanted to be pedantic, you could say "the nil UUID (all zeroes) is
> returned"

Sold!

> but your rephrasing is clear even w/o using the standard name, so I'm fine
> either way.
>
>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>
>
> Reviewed-by: John Snow <jsnow@redhat.com>

Thank you!

Re: [PATCH 3/5] qapi/machine: Clarify query-uuid value when none has been specified

[Philippe Mathieu-Daudé]

On 16/7/24 20:12, Markus Armbruster wrote:
> John Snow <jsnow@redhat.com> writes:
> 
>> On Thu, Jul 11, 2024, 7:22 AM Markus Armbruster <armbru@redhat.com> wrote:
>>
>>> When no UUID has been specified, query-uuid returns
>>>
>>>      {"UUID": "00000000-0000-0000-0000-000000000000"}
>>>
>>> The doc comment calls this "a null UUID", which I find less than
>>> clear.  Change it to "an all-zero UUID".
>>>
>>
>> Technically it's a "nil UUID";
>> https://datatracker.ietf.org/doc/html/rfc9562#name-nil-uuid
>>
>> If you wanted to be pedantic, you could say "the nil UUID (all zeroes) is
>> returned"

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH 4/5] qapi/sockets: Move deprecation note out of SocketAddress doc comment

[Markus Armbruster]

Doc comments are reference documentation for users of QMP.
SocketAddress's doc comment contains a deprecation note advising
developers to use SocketAddress for new code.  Irrelevant for users of
QMP.  Move the note out of the doc comment.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qapi/sockets.json | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/qapi/sockets.json b/qapi/sockets.json
index 4d78d2ccb7..e76fdb9925 100644
--- a/qapi/sockets.json
+++ b/qapi/sockets.json
@@ -179,10 +179,6 @@
 #
 # @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.
-#
 # Since: 1.3
 ##
 { 'union': 'SocketAddressLegacy',
@@ -193,6 +189,9 @@
     'unix': 'UnixSocketAddressWrapper',
     'vsock': 'VsockSocketAddressWrapper',
     'fd': 'FdSocketAddressWrapper' } }
+# Note: This type is deprecated in favor of SocketAddress.  The
+# difference between SocketAddressLegacy and SocketAddress is that the
+# latter has fewer ``{}`` on the wire.
 
 ##
 # @SocketAddressType:
-- 
2.45.0

Re: [PATCH 4/5] qapi/sockets: Move deprecation note out of SocketAddress doc comment

[John Snow]

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

On Thu, Jul 11, 2024, 7:22 AM Markus Armbruster <armbru@redhat.com> wrote:

> Doc comments are reference documentation for users of QMP.
> SocketAddress's doc comment contains a deprecation note advising
> developers to use SocketAddress for new code.  Irrelevant for users of
> QMP.  Move the note out of the doc comment.
>

Sure.


> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>

Reviewed-by: John Snow <jsnow@redhat.com>


---
>  qapi/sockets.json | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/qapi/sockets.json b/qapi/sockets.json
> index 4d78d2ccb7..e76fdb9925 100644
> --- a/qapi/sockets.json
> +++ b/qapi/sockets.json
> @@ -179,10 +179,6 @@
>  #
>  # @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.
> -#
>  # Since: 1.3
>  ##
>  { 'union': 'SocketAddressLegacy',
> @@ -193,6 +189,9 @@
>      'unix': 'UnixSocketAddressWrapper',
>      'vsock': 'VsockSocketAddressWrapper',
>      'fd': 'FdSocketAddressWrapper' } }
> +# Note: This type is deprecated in favor of SocketAddress.  The
> +# difference between SocketAddressLegacy and SocketAddress is that the
> +# latter has fewer ``{}`` on the wire.
>
>  ##
>  # @SocketAddressType:
> --
> 2.45.0
>
>

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

Re: [PATCH 4/5] qapi/sockets: Move deprecation note out of SocketAddress doc comment

[Philippe Mathieu-Daudé]

On 11/7/24 13:22, Markus Armbruster wrote:
> Doc comments are reference documentation for users of QMP.
> SocketAddress's doc comment contains a deprecation note advising
> developers to use SocketAddress for new code.  Irrelevant for users of
> QMP.  Move the note out of the doc comment.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   qapi/sockets.json | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH 5/5] qapi/ui: Drop note on naming of SpiceQueryMouseMode

[Markus Armbruster]

Doc comments are reference documentation for users of QMP.
SpiceQueryMouseMode's doc comment contains a note explaining why it's
not named SpiceMouseMode: spice/enums.h has it already.  Irrelevant
for users of QMP; delete the note.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 qapi/ui.json | 2 --
 1 file changed, 2 deletions(-)

diff --git a/qapi/ui.json b/qapi/ui.json
index 5bcccbfc93..df089869a5 100644
--- a/qapi/ui.json
+++ b/qapi/ui.json
@@ -273,8 +273,6 @@
 # @unknown: No information is available about mouse mode used by the
 #     spice server.
 #
-# .. note:: spice/enums.h has a SpiceMouseMode already, hence the name.
-#
 # Since: 1.1
 ##
 { 'enum': 'SpiceQueryMouseMode',
-- 
2.45.0

Re: [PATCH 5/5] qapi/ui: Drop note on naming of SpiceQueryMouseMode

[John Snow]

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

On Thu, Jul 11, 2024, 7:22 AM Markus Armbruster <armbru@redhat.com> wrote:

> Doc comments are reference documentation for users of QMP.
> SpiceQueryMouseMode's doc comment contains a note explaining why it's
> not named SpiceMouseMode: spice/enums.h has it already.  Irrelevant
> for users of QMP; delete the note.
>

Hmmm, yeah, alright. I feel like this one's sorta borderline but I don't
feel strongly enough about it to argue to keep it :)


> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>

Reviewed-by: John Snow <jsnow@redhat.com>

---
>  qapi/ui.json | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/qapi/ui.json b/qapi/ui.json
> index 5bcccbfc93..df089869a5 100644
> --- a/qapi/ui.json
> +++ b/qapi/ui.json
> @@ -273,8 +273,6 @@
>  # @unknown: No information is available about mouse mode used by the
>  #     spice server.
>  #
> -# .. note:: spice/enums.h has a SpiceMouseMode already, hence the name.
> -#
>  # Since: 1.1
>  ##
>  { 'enum': 'SpiceQueryMouseMode',
> --
> 2.45.0
>
>

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

Re: [PATCH 0/5] qapi: Doc comment cleanups

[Michael S. Tsirkin]

On Thu, Jul 11, 2024 at 01:22:23PM +0200, Markus Armbruster wrote:
> Markus Armbruster (5):
>   qapi/pci: Clean up documentation around PciDeviceClass
>   qapi/machine: Clean up documentation around CpuInstanceProperties
>   qapi/machine: Clarify query-uuid value when none has been specified
>   qapi/sockets: Move deprecation note out of SocketAddress doc comment
>   qapi/ui: Drop note on naming of SpiceQueryMouseMode

pci/machine bits:

Reviewed-by: Michael S. Tsirkin <mst@redhat.com>

>  qapi/machine.json | 18 ++++++++----------
>  qapi/pci.json     |  6 ++----
>  qapi/sockets.json |  7 +++----
>  qapi/ui.json      |  2 --
>  4 files changed, 13 insertions(+), 20 deletions(-)
> 
> -- 
> 2.45.0

Re: [PATCH 0/5] qapi: Doc comment cleanups

[Markus Armbruster]

Queued.