Re: [Qemu-devel] [PATCH for-2.9 00/47] qapi: Put type information back into QMP documentation

[Eric Blake]

On 03/13/2017 01:18 AM, Markus Armbruster wrote:
> I'm proposing this is 2.9 because it fixes a documentation regression.
> It affects only documentation; generated C code is unchanged except
> for the removal of trailing space in PATCH 46.
> 

> Additionally, my series fixes a number of bugs and cleans up along the
> way.  In particular, it converts qapi2texi.py from parse trees to the
> visitor interface the other generators use.
> 
> Future generated documentation work includes eliding types that aren't
> visible in QMP (like introspection does), and making uses of type
> names links in HTML.

Reporting here, so it doesn't get lost in IRC.  Other potential future
work: fix this poor error message:

  GEN docs/qemu-qmp-qapi.texi
Traceback (most recent call last):
  File "/home/eblake/qemu/scripts/qapi2texi.py", line 300, in 
main(sys.argv)
  File "/home/eblake/qemu/scripts/qapi2texi.py", line 296, in main
print texi_schema(schema)
  File "/home/eblake/qemu/scripts/qapi2texi.py", line 279, in texi_schema
gen.symbol(doc, schema.lookup_entity(doc.symbol))
  File "/home/eblake/qemu/scripts/qapi2texi.py", line 263, in symbol
entity.visit(self)
  File "/home/eblake/qemu/scripts/qapi.py", line 1448, in visit
visitor.visit_event(self.name, self.info, self.arg_type, self.boxed)
  File "/home/eblake/qemu/scripts/qapi2texi.py", line 259, in visit_event
body=texi_entity(doc, 'Arguments'))
  File "/home/eblake/qemu/scripts/qapi2texi.py", line 200, in texi_entity
+ texi_sections(doc))
  File "/home/eblake/qemu/scripts/qapi2texi.py", line 160, in texi_members
items += member_func(section.member) + desc + '\n'
  File "/home/eblake/qemu/scripts/qapi2texi.py", line 138, in texi_member
typ = member.type.doc_type()
AttributeError: 'NoneType' object has no attribute 'type'
Makefile:706: recipe for target 'docs/qemu-qmp-qapi.texi' failed
make: *** [docs/qemu-qmp-qapi.texi] Error 1
make: *** Deleting file 'docs/qemu-qmp-qapi.texi'

caused by this small change (documenting an event member, but omitting
'data' for the event itself):

diff --git i/qapi/event.json w/qapi/event.json
index 6d22b02..8978b33 100644
--- i/qapi/event.json
+++ w/qapi/event.json
@@ -68,6 +68,8 @@
 #
 # Emitted when the virtual machine is stopped
 #
+# @bogus: new field
+#
 # Since: 0.12.0
 #
 # Example:


-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH for-2.9 00/47] qapi: Put type information back into QMP documentation

[Marc-André Lureau]

Hi

On Tue, Mar 14, 2017 at 8:14 PM Markus Armbruster  wrote:

> Marc-André Lureau  writes:
>
> > Hi
> >
> > On Mon, Mar 13, 2017 at 5:12 PM Markus Armbruster 
> wrote:
> >
> >> Marc-André Lureau  writes:
> >>
> >> > Hi
> >> >
> >> > On Mon, Mar 13, 2017 at 4:14 PM Markus Armbruster 
> wrote:
> >> >
> >> >> Marc-André Lureau  writes:
> >> >>
> >> >> > Hi
> >> >> >
> >> >> > On Mon, Mar 13, 2017 at 10:23 AM Markus Armbruster <
> arm...@redhat.com
> >> >
> >> >> > wrote:
> >> >> >
> >> >> >> I'm proposing this is 2.9 because it fixes a documentation
> regression.
> >> >> >> It affects only documentation; generated C code is unchanged
> except
> >> >> >> for the removal of trailing space in PATCH 46.
> >> >> >>
> >> >> >> Based on my qapi-next branch, which contains Marc-André's PATCH
> 1/2.
> >> >> >>
> >> >> >> Marc-André's work to merge qmp-commands.txt and qmp-events.txt
> into
> >> >> >> the QAPI schema and generate their replacements from the schema
> >> >> >> (commit b6af8ea..56e8bdd) was a big step forward.  As committed,
> it
> >> >> >> also was a step back: the documentation lost information on JSON
> >> >> >> types, because I didn't like Marc-André's patch to add it.  He
> >> >> >> reposted it for further review afterwards:
> >> >> >>
> >> >> >> Subject: [PATCH 0/2] qapi2texi: add type information
> >> >> >> Message-Id: <
> 20170125130308.16104-1-marcandre.lur...@redhat.com>
> >> >> >>
> https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg05432.html
> >> >> >>
> >> >> >> His PATCH 1/2 is a straightforward cleanup.  His PATCH 2/2 adds
> type
> >> >> >> descriptions in a new formal language to the generated
> documentation.
> >> >> >> Quoting the commit message:
> >> >> >>
> >> >> >> Array types have the following syntax: type[]. Ex: str[].
> >> >> >>
> >> >> >> - Struct, commands and events use the following members
> syntax:
> >> >> >>
> >> >> >>   { 'member': type, ('foo': str), ... }
> >> >> >>
> >> >> >> Optional members are under parentheses.
> >> >> >>
> >> >> >> A structure with a base type will have 'BaseStruct +'
> prepended.
> >> >> >>
> >> >> >> - Alternates use the following syntax:
> >> >> >>
> >> >> >>   [ 'foo': type, 'bar': type, ... ]
> >> >> >>
> >> >> >> - Simple unions use the following syntax:
> >> >> >>
> >> >> >>   { 'type': str, 'data': 'type' = [ 'foo': type, 'bar':
> type... ] }
> >> >> >>
> >> >> >> - Flat unions use the following syntax:
> >> >> >>
> >> >> >>   BaseStruct + 'discriminator' = [ 'foo': type, 'bar':
> type... ]
> >> >> >>
> >> >> >> End quote.  Looks like this in generated documentation:
> >> >> >>
> >> >> >>  -- Event: VNC_CONNECTED {'server': VncServerInfo, 'client':
> >> >> >>   VncBasicInfo}
> >> >> >>
> >> >> >>  Emitted when a VNC client establishes a connection
> >> >> >>  ''server''
> >> >> >>   server information
> >> >> >>  ''client''
> >> >> >>   client information
> >> >> >>
> >> >> >>  Note: This event is emitted before any authentication takes
> place,
> >> >> >>  thus the authentication ID is not provided
> >> >> >> [...]
> >> >> >>
> >> >> >>  -- Struct: VncServerInfo VncBasicInfo + {('auth': str)}
> >> >> >>
> >> >> >>  The network connection information for server
> >> >> >>  ''auth'' (optional)
> >> >> >>   authentication method used for the plain
> (non-websocket) VNC
> >> >> >>   server
> >> >> >>
> >> >> >>  Since: 2.1
> >> >> >>
> >> >> >>  -- Simple Union: SocketAddress { 'type': str, 'data': 'type' =
> ['inet':
> >> >> >>   InetSocketAddress, 'unix': UnixSocketAddress, 'vsock':
> >> >> >>   VsockSocketAddress, 'fd': String] }
> >> >> >>
> >> >> >>  Captures the address of a socket, which could also be a
> named file
> >> >> >>  descriptor
> >> >> >>
> >> >> >>  Since: 1.3
> >> >> >>
> >> >> >> Here's my counter-proposal: instead of inventing a formal
> language,
> >> >> >> fix the natural language documentation to actually mention *all*
> >> >> >> members, and add type information in a plain, easy-to-understand
> way.
> >> >> >> Looks like this:
> >> >> >>
> >> >> >>  -- Event: VNC_CONNECTED
> >> >> >>
> >> >> >>  Emitted when a VNC client establishes a connection
> >> >> >>
> >> >> >>  Arguments:
> >> >> >>  'server: VncServerInfo'
> >> >> >>   server information
> >> >> >>  'client: VncBasicInfo'
> >> >> >>   client information
> >> >> >>
> >> >> >>  Note: This event is emitted before any authentication takes
> place,
> >> >> >>  thus the authentication ID is not provided
> >> >> >> [...]
> >> >> >>
> >> >> >>  -- Object: VncServerInfo
> >> >> >>
> >> >> >>  The network connection information for server
> >> >> >>
> >> >> >>  Members:
> >> >> >>  'auth: string' (optional)
> >> >> >>  

Re: [Qemu-devel] [PATCH for-2.9 00/47] qapi: Put type information back into QMP documentation

[Markus Armbruster]

Marc-André Lureau  writes:

> Hi
>
> On Mon, Mar 13, 2017 at 10:23 AM Markus Armbruster 
> wrote:
>
>> I'm proposing this is 2.9 because it fixes a documentation regression.
>> It affects only documentation; generated C code is unchanged except
>> for the removal of trailing space in PATCH 46.
[...]
> Except the few comments and questions I left, the series looks good to me.
> I don't think we need to rush it in the 2.9 release though, but I will let
> the maintainers decide how to deal with the planning and rules.

Letting documentation quality regress in 2.9 wouldn't be the end of the
world, but this series is as safe as they get, despite its size.

Thanks for your review!

Re: [Qemu-devel] [PATCH for-2.9 00/47] qapi: Put type information back into QMP documentation

[Markus Armbruster]

Marc-André Lureau  writes:

> Hi
>
> On Mon, Mar 13, 2017 at 5:12 PM Markus Armbruster  wrote:
>
>> Marc-André Lureau  writes:
>>
>> > Hi
>> >
>> > On Mon, Mar 13, 2017 at 4:14 PM Markus Armbruster  
>> > wrote:
>> >
>> >> Marc-André Lureau  writes:
>> >>
>> >> > Hi
>> >> >
>> >> > On Mon, Mar 13, 2017 at 10:23 AM Markus Armbruster > >
>> >> > wrote:
>> >> >
>> >> >> I'm proposing this is 2.9 because it fixes a documentation regression.
>> >> >> It affects only documentation; generated C code is unchanged except
>> >> >> for the removal of trailing space in PATCH 46.
>> >> >>
>> >> >> Based on my qapi-next branch, which contains Marc-André's PATCH 1/2.
>> >> >>
>> >> >> Marc-André's work to merge qmp-commands.txt and qmp-events.txt into
>> >> >> the QAPI schema and generate their replacements from the schema
>> >> >> (commit b6af8ea..56e8bdd) was a big step forward.  As committed, it
>> >> >> also was a step back: the documentation lost information on JSON
>> >> >> types, because I didn't like Marc-André's patch to add it.  He
>> >> >> reposted it for further review afterwards:
>> >> >>
>> >> >> Subject: [PATCH 0/2] qapi2texi: add type information
>> >> >> Message-Id: <20170125130308.16104-1-marcandre.lur...@redhat.com>
>> >> >> https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg05432.html
>> >> >>
>> >> >> His PATCH 1/2 is a straightforward cleanup.  His PATCH 2/2 adds type
>> >> >> descriptions in a new formal language to the generated documentation.
>> >> >> Quoting the commit message:
>> >> >>
>> >> >> Array types have the following syntax: type[]. Ex: str[].
>> >> >>
>> >> >> - Struct, commands and events use the following members syntax:
>> >> >>
>> >> >>   { 'member': type, ('foo': str), ... }
>> >> >>
>> >> >> Optional members are under parentheses.
>> >> >>
>> >> >> A structure with a base type will have 'BaseStruct +' prepended.
>> >> >>
>> >> >> - Alternates use the following syntax:
>> >> >>
>> >> >>   [ 'foo': type, 'bar': type, ... ]
>> >> >>
>> >> >> - Simple unions use the following syntax:
>> >> >>
>> >> >>   { 'type': str, 'data': 'type' = [ 'foo': type, 'bar': type... ] }
>> >> >>
>> >> >> - Flat unions use the following syntax:
>> >> >>
>> >> >>   BaseStruct + 'discriminator' = [ 'foo': type, 'bar': type... ]
>> >> >>
>> >> >> End quote.  Looks like this in generated documentation:
>> >> >>
>> >> >>  -- Event: VNC_CONNECTED {'server': VncServerInfo, 'client':
>> >> >>   VncBasicInfo}
>> >> >>
>> >> >>  Emitted when a VNC client establishes a connection
>> >> >>  ''server''
>> >> >>   server information
>> >> >>  ''client''
>> >> >>   client information
>> >> >>
>> >> >>  Note: This event is emitted before any authentication takes place,
>> >> >>  thus the authentication ID is not provided
>> >> >> [...]
>> >> >>
>> >> >>  -- Struct: VncServerInfo VncBasicInfo + {('auth': str)}
>> >> >>
>> >> >>  The network connection information for server
>> >> >>  ''auth'' (optional)
>> >> >>   authentication method used for the plain (non-websocket) VNC
>> >> >>   server
>> >> >>
>> >> >>  Since: 2.1
>> >> >>
>> >> >>  -- Simple Union: SocketAddress { 'type': str, 'data': 'type' = 
>> >> >> ['inet':
>> >> >>   InetSocketAddress, 'unix': UnixSocketAddress, 'vsock':
>> >> >>   VsockSocketAddress, 'fd': String] }
>> >> >>
>> >> >>  Captures the address of a socket, which could also be a named file
>> >> >>  descriptor
>> >> >>
>> >> >>  Since: 1.3
>> >> >>
>> >> >> Here's my counter-proposal: instead of inventing a formal language,
>> >> >> fix the natural language documentation to actually mention *all*
>> >> >> members, and add type information in a plain, easy-to-understand way.
>> >> >> Looks like this:
>> >> >>
>> >> >>  -- Event: VNC_CONNECTED
>> >> >>
>> >> >>  Emitted when a VNC client establishes a connection
>> >> >>
>> >> >>  Arguments:
>> >> >>  'server: VncServerInfo'
>> >> >>   server information
>> >> >>  'client: VncBasicInfo'
>> >> >>   client information
>> >> >>
>> >> >>  Note: This event is emitted before any authentication takes place,
>> >> >>  thus the authentication ID is not provided
>> >> >> [...]
>> >> >>
>> >> >>  -- Object: VncServerInfo
>> >> >>
>> >> >>  The network connection information for server
>> >> >>
>> >> >>  Members:
>> >> >>  'auth: string' (optional)
>> >> >>   authentication method used for the plain (non-websocket) VNC
>> >> >>   server
>> >> >>  The members of 'VncBasicInfo'
>> >> >>
>> >> >>  Since: 2.1
>> >> >>
>> >> >>  -- Object: SocketAddress
>> >> >>
>> >> >>  Captures the address of a socket, which could also be a named file
>> >> >>  descriptor
>> >> >>
>> >> >>  

Re: [Qemu-devel] [PATCH for-2.9 00/47] qapi: Put type information back into QMP documentation

[Marc-André Lureau]

Hi

On Mon, Mar 13, 2017 at 10:23 AM Markus Armbruster 
wrote:

> I'm proposing this is 2.9 because it fixes a documentation regression.
> It affects only documentation; generated C code is unchanged except
> for the removal of trailing space in PATCH 46.
>
> Based on my qapi-next branch, which contains Marc-André's PATCH 1/2.
>
> Marc-André's work to merge qmp-commands.txt and qmp-events.txt into
> the QAPI schema and generate their replacements from the schema
> (commit b6af8ea..56e8bdd) was a big step forward.  As committed, it
> also was a step back: the documentation lost information on JSON
> types, because I didn't like Marc-André's patch to add it.  He
> reposted it for further review afterwards:
>
> Subject: [PATCH 0/2] qapi2texi: add type information
> Message-Id: <20170125130308.16104-1-marcandre.lur...@redhat.com>
> https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg05432.html
>
> His PATCH 1/2 is a straightforward cleanup.  His PATCH 2/2 adds type
> descriptions in a new formal language to the generated documentation.
> Quoting the commit message:
>
> Array types have the following syntax: type[]. Ex: str[].
>
> - Struct, commands and events use the following members syntax:
>
>   { 'member': type, ('foo': str), ... }
>
> Optional members are under parentheses.
>
> A structure with a base type will have 'BaseStruct +' prepended.
>
> - Alternates use the following syntax:
>
>   [ 'foo': type, 'bar': type, ... ]
>
> - Simple unions use the following syntax:
>
>   { 'type': str, 'data': 'type' = [ 'foo': type, 'bar': type... ] }
>
> - Flat unions use the following syntax:
>
>   BaseStruct + 'discriminator' = [ 'foo': type, 'bar': type... ]
>
> End quote.  Looks like this in generated documentation:
>
>  -- Event: VNC_CONNECTED {'server': VncServerInfo, 'client':
>   VncBasicInfo}
>
>  Emitted when a VNC client establishes a connection
>  ''server''
>   server information
>  ''client''
>   client information
>
>  Note: This event is emitted before any authentication takes place,
>  thus the authentication ID is not provided
> [...]
>
>  -- Struct: VncServerInfo VncBasicInfo + {('auth': str)}
>
>  The network connection information for server
>  ''auth'' (optional)
>   authentication method used for the plain (non-websocket) VNC
>   server
>
>  Since: 2.1
>
>  -- Simple Union: SocketAddress { 'type': str, 'data': 'type' = ['inet':
>   InetSocketAddress, 'unix': UnixSocketAddress, 'vsock':
>   VsockSocketAddress, 'fd': String] }
>
>  Captures the address of a socket, which could also be a named file
>  descriptor
>
>  Since: 1.3
>
> Here's my counter-proposal: instead of inventing a formal language,
> fix the natural language documentation to actually mention *all*
> members, and add type information in a plain, easy-to-understand way.
> Looks like this:
>
>  -- Event: VNC_CONNECTED
>
>  Emitted when a VNC client establishes a connection
>
>  Arguments:
>  'server: VncServerInfo'
>   server information
>  'client: VncBasicInfo'
>   client information
>
>  Note: This event is emitted before any authentication takes place,
>  thus the authentication ID is not provided
> [...]
>
>  -- Object: VncServerInfo
>
>  The network connection information for server
>
>  Members:
>  'auth: string' (optional)
>   authentication method used for the plain (non-websocket) VNC
>   server
>  The members of 'VncBasicInfo'
>
>  Since: 2.1
>
>  -- Object: SocketAddress
>
>  Captures the address of a socket, which could also be a named file
>  descriptor
>
>  Members:
>  'type'
>   One of "inet", "unix", "vsock", "fd"
>  'data: InetSocketAddress' when 'type' is "inet"
>  'data: UnixSocketAddress' when 'type' is "unix"
>  'data: VsockSocketAddress' when 'type' is "vsock"
>  'data: String' when 'type' is "fd"
>
>  Since: 1.3
>
> Additionally, my series fixes a number of bugs and cleans up along the
> way.  In particular, it converts qapi2texi.py from parse trees to the
> visitor interface the other generators use.
>
> Future generated documentation work includes eliding types that aren't
> visible in QMP (like introspection does), and making uses of type
> names links in HTML.
>
> Markus Armbruster (47):
>   qapi: Factor QAPISchemaParser._include() out of .__init__()
>   qapi: Make doc comments optional where we don't need them
>   qapi: Back out doc comments added just to please qapi.py
>   docs/qapi-code-gen.txt: Drop confusing reference to 'gen'
>   qapi: Have each QAPI schema declare its returns white-list
>   qapi: Have each QAPI schema declare its name rule violations
>   qapi: Clean up build of generated documentation
>   tests/qapi-schema: Cover empty union base
>   qapi: Fix to reject empty union base gracefully
>   

Re: [Qemu-devel] [PATCH for-2.9 00/47] qapi: Put type information back into QMP documentation

[Marc-André Lureau]

Hi

On Mon, Mar 13, 2017 at 5:12 PM Markus Armbruster  wrote:

> Marc-André Lureau  writes:
>
> > Hi
> >
> > On Mon, Mar 13, 2017 at 4:14 PM Markus Armbruster 
> wrote:
> >
> >> Marc-André Lureau  writes:
> >>
> >> > Hi
> >> >
> >> > On Mon, Mar 13, 2017 at 10:23 AM Markus Armbruster  >
> >> > wrote:
> >> >
> >> >> I'm proposing this is 2.9 because it fixes a documentation
> regression.
> >> >> It affects only documentation; generated C code is unchanged except
> >> >> for the removal of trailing space in PATCH 46.
> >> >>
> >> >> Based on my qapi-next branch, which contains Marc-André's PATCH 1/2.
> >> >>
> >> >> Marc-André's work to merge qmp-commands.txt and qmp-events.txt into
> >> >> the QAPI schema and generate their replacements from the schema
> >> >> (commit b6af8ea..56e8bdd) was a big step forward.  As committed, it
> >> >> also was a step back: the documentation lost information on JSON
> >> >> types, because I didn't like Marc-André's patch to add it.  He
> >> >> reposted it for further review afterwards:
> >> >>
> >> >> Subject: [PATCH 0/2] qapi2texi: add type information
> >> >> Message-Id: <20170125130308.16104-1-marcandre.lur...@redhat.com>
> >> >>
> https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg05432.html
> >> >>
> >> >> His PATCH 1/2 is a straightforward cleanup.  His PATCH 2/2 adds type
> >> >> descriptions in a new formal language to the generated documentation.
> >> >> Quoting the commit message:
> >> >>
> >> >> Array types have the following syntax: type[]. Ex: str[].
> >> >>
> >> >> - Struct, commands and events use the following members syntax:
> >> >>
> >> >>   { 'member': type, ('foo': str), ... }
> >> >>
> >> >> Optional members are under parentheses.
> >> >>
> >> >> A structure with a base type will have 'BaseStruct +' prepended.
> >> >>
> >> >> - Alternates use the following syntax:
> >> >>
> >> >>   [ 'foo': type, 'bar': type, ... ]
> >> >>
> >> >> - Simple unions use the following syntax:
> >> >>
> >> >>   { 'type': str, 'data': 'type' = [ 'foo': type, 'bar': type...
> ] }
> >> >>
> >> >> - Flat unions use the following syntax:
> >> >>
> >> >>   BaseStruct + 'discriminator' = [ 'foo': type, 'bar': type... ]
> >> >>
> >> >> End quote.  Looks like this in generated documentation:
> >> >>
> >> >>  -- Event: VNC_CONNECTED {'server': VncServerInfo, 'client':
> >> >>   VncBasicInfo}
> >> >>
> >> >>  Emitted when a VNC client establishes a connection
> >> >>  ''server''
> >> >>   server information
> >> >>  ''client''
> >> >>   client information
> >> >>
> >> >>  Note: This event is emitted before any authentication takes
> place,
> >> >>  thus the authentication ID is not provided
> >> >> [...]
> >> >>
> >> >>  -- Struct: VncServerInfo VncBasicInfo + {('auth': str)}
> >> >>
> >> >>  The network connection information for server
> >> >>  ''auth'' (optional)
> >> >>   authentication method used for the plain (non-websocket)
> VNC
> >> >>   server
> >> >>
> >> >>  Since: 2.1
> >> >>
> >> >>  -- Simple Union: SocketAddress { 'type': str, 'data': 'type' =
> ['inet':
> >> >>   InetSocketAddress, 'unix': UnixSocketAddress, 'vsock':
> >> >>   VsockSocketAddress, 'fd': String] }
> >> >>
> >> >>  Captures the address of a socket, which could also be a named
> file
> >> >>  descriptor
> >> >>
> >> >>  Since: 1.3
> >> >>
> >> >> Here's my counter-proposal: instead of inventing a formal language,
> >> >> fix the natural language documentation to actually mention *all*
> >> >> members, and add type information in a plain, easy-to-understand way.
> >> >> Looks like this:
> >> >>
> >> >>  -- Event: VNC_CONNECTED
> >> >>
> >> >>  Emitted when a VNC client establishes a connection
> >> >>
> >> >>  Arguments:
> >> >>  'server: VncServerInfo'
> >> >>   server information
> >> >>  'client: VncBasicInfo'
> >> >>   client information
> >> >>
> >> >>  Note: This event is emitted before any authentication takes
> place,
> >> >>  thus the authentication ID is not provided
> >> >> [...]
> >> >>
> >> >>  -- Object: VncServerInfo
> >> >>
> >> >>  The network connection information for server
> >> >>
> >> >>  Members:
> >> >>  'auth: string' (optional)
> >> >>   authentication method used for the plain (non-websocket)
> VNC
> >> >>   server
> >> >>  The members of 'VncBasicInfo'
> >> >>
> >> >>  Since: 2.1
> >> >>
> >> >>  -- Object: SocketAddress
> >> >>
> >> >>  Captures the address of a socket, which could also be a named
> file
> >> >>  descriptor
> >> >>
> >> >>  Members:
> >> >>  'type'
> >> >>   One of "inet", "unix", "vsock", "fd"
> >> >>  'data: InetSocketAddress' when 'type' is "inet"
> >> >>  'data: UnixSocketAddress' 

Re: [Qemu-devel] [PATCH for-2.9 00/47] qapi: Put type information back into QMP documentation

[Markus Armbruster]

Marc-André Lureau  writes:

> Hi
>
> On Mon, Mar 13, 2017 at 4:14 PM Markus Armbruster  wrote:
>
>> Marc-André Lureau  writes:
>>
>> > Hi
>> >
>> > On Mon, Mar 13, 2017 at 10:23 AM Markus Armbruster 
>> > wrote:
>> >
>> >> I'm proposing this is 2.9 because it fixes a documentation regression.
>> >> It affects only documentation; generated C code is unchanged except
>> >> for the removal of trailing space in PATCH 46.
>> >>
>> >> Based on my qapi-next branch, which contains Marc-André's PATCH 1/2.
>> >>
>> >> Marc-André's work to merge qmp-commands.txt and qmp-events.txt into
>> >> the QAPI schema and generate their replacements from the schema
>> >> (commit b6af8ea..56e8bdd) was a big step forward.  As committed, it
>> >> also was a step back: the documentation lost information on JSON
>> >> types, because I didn't like Marc-André's patch to add it.  He
>> >> reposted it for further review afterwards:
>> >>
>> >> Subject: [PATCH 0/2] qapi2texi: add type information
>> >> Message-Id: <20170125130308.16104-1-marcandre.lur...@redhat.com>
>> >> https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg05432.html
>> >>
>> >> His PATCH 1/2 is a straightforward cleanup.  His PATCH 2/2 adds type
>> >> descriptions in a new formal language to the generated documentation.
>> >> Quoting the commit message:
>> >>
>> >> Array types have the following syntax: type[]. Ex: str[].
>> >>
>> >> - Struct, commands and events use the following members syntax:
>> >>
>> >>   { 'member': type, ('foo': str), ... }
>> >>
>> >> Optional members are under parentheses.
>> >>
>> >> A structure with a base type will have 'BaseStruct +' prepended.
>> >>
>> >> - Alternates use the following syntax:
>> >>
>> >>   [ 'foo': type, 'bar': type, ... ]
>> >>
>> >> - Simple unions use the following syntax:
>> >>
>> >>   { 'type': str, 'data': 'type' = [ 'foo': type, 'bar': type... ] }
>> >>
>> >> - Flat unions use the following syntax:
>> >>
>> >>   BaseStruct + 'discriminator' = [ 'foo': type, 'bar': type... ]
>> >>
>> >> End quote.  Looks like this in generated documentation:
>> >>
>> >>  -- Event: VNC_CONNECTED {'server': VncServerInfo, 'client':
>> >>   VncBasicInfo}
>> >>
>> >>  Emitted when a VNC client establishes a connection
>> >>  ''server''
>> >>   server information
>> >>  ''client''
>> >>   client information
>> >>
>> >>  Note: This event is emitted before any authentication takes place,
>> >>  thus the authentication ID is not provided
>> >> [...]
>> >>
>> >>  -- Struct: VncServerInfo VncBasicInfo + {('auth': str)}
>> >>
>> >>  The network connection information for server
>> >>  ''auth'' (optional)
>> >>   authentication method used for the plain (non-websocket) VNC
>> >>   server
>> >>
>> >>  Since: 2.1
>> >>
>> >>  -- Simple Union: SocketAddress { 'type': str, 'data': 'type' = ['inet':
>> >>   InetSocketAddress, 'unix': UnixSocketAddress, 'vsock':
>> >>   VsockSocketAddress, 'fd': String] }
>> >>
>> >>  Captures the address of a socket, which could also be a named file
>> >>  descriptor
>> >>
>> >>  Since: 1.3
>> >>
>> >> Here's my counter-proposal: instead of inventing a formal language,
>> >> fix the natural language documentation to actually mention *all*
>> >> members, and add type information in a plain, easy-to-understand way.
>> >> Looks like this:
>> >>
>> >>  -- Event: VNC_CONNECTED
>> >>
>> >>  Emitted when a VNC client establishes a connection
>> >>
>> >>  Arguments:
>> >>  'server: VncServerInfo'
>> >>   server information
>> >>  'client: VncBasicInfo'
>> >>   client information
>> >>
>> >>  Note: This event is emitted before any authentication takes place,
>> >>  thus the authentication ID is not provided
>> >> [...]
>> >>
>> >>  -- Object: VncServerInfo
>> >>
>> >>  The network connection information for server
>> >>
>> >>  Members:
>> >>  'auth: string' (optional)
>> >>   authentication method used for the plain (non-websocket) VNC
>> >>   server
>> >>  The members of 'VncBasicInfo'
>> >>
>> >>  Since: 2.1
>> >>
>> >>  -- Object: SocketAddress
>> >>
>> >>  Captures the address of a socket, which could also be a named file
>> >>  descriptor
>> >>
>> >>  Members:
>> >>  'type'
>> >>   One of "inet", "unix", "vsock", "fd"
>> >>  'data: InetSocketAddress' when 'type' is "inet"
>> >>  'data: UnixSocketAddress' when 'type' is "unix"
>> >>  'data: VsockSocketAddress' when 'type' is "vsock"
>> >>  'data: String' when 'type' is "fd"
>> >>
>> >>  Since: 1.3
>> >>
>> >>
>> > I like both, to me they serve different purposes. I like to have a short
>> > overview / signature and then a more detailed documentation for each
>> field.
>>
>> I sympathize with the 

Re: [Qemu-devel] [PATCH for-2.9 00/47] qapi: Put type information back into QMP documentation

[Marc-André Lureau]

Hi

On Mon, Mar 13, 2017 at 4:14 PM Markus Armbruster  wrote:

> Marc-André Lureau  writes:
>
> > Hi
> >
> > On Mon, Mar 13, 2017 at 10:23 AM Markus Armbruster 
> > wrote:
> >
> >> I'm proposing this is 2.9 because it fixes a documentation regression.
> >> It affects only documentation; generated C code is unchanged except
> >> for the removal of trailing space in PATCH 46.
> >>
> >> Based on my qapi-next branch, which contains Marc-André's PATCH 1/2.
> >>
> >> Marc-André's work to merge qmp-commands.txt and qmp-events.txt into
> >> the QAPI schema and generate their replacements from the schema
> >> (commit b6af8ea..56e8bdd) was a big step forward.  As committed, it
> >> also was a step back: the documentation lost information on JSON
> >> types, because I didn't like Marc-André's patch to add it.  He
> >> reposted it for further review afterwards:
> >>
> >> Subject: [PATCH 0/2] qapi2texi: add type information
> >> Message-Id: <20170125130308.16104-1-marcandre.lur...@redhat.com>
> >> https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg05432.html
> >>
> >> His PATCH 1/2 is a straightforward cleanup.  His PATCH 2/2 adds type
> >> descriptions in a new formal language to the generated documentation.
> >> Quoting the commit message:
> >>
> >> Array types have the following syntax: type[]. Ex: str[].
> >>
> >> - Struct, commands and events use the following members syntax:
> >>
> >>   { 'member': type, ('foo': str), ... }
> >>
> >> Optional members are under parentheses.
> >>
> >> A structure with a base type will have 'BaseStruct +' prepended.
> >>
> >> - Alternates use the following syntax:
> >>
> >>   [ 'foo': type, 'bar': type, ... ]
> >>
> >> - Simple unions use the following syntax:
> >>
> >>   { 'type': str, 'data': 'type' = [ 'foo': type, 'bar': type... ] }
> >>
> >> - Flat unions use the following syntax:
> >>
> >>   BaseStruct + 'discriminator' = [ 'foo': type, 'bar': type... ]
> >>
> >> End quote.  Looks like this in generated documentation:
> >>
> >>  -- Event: VNC_CONNECTED {'server': VncServerInfo, 'client':
> >>   VncBasicInfo}
> >>
> >>  Emitted when a VNC client establishes a connection
> >>  ''server''
> >>   server information
> >>  ''client''
> >>   client information
> >>
> >>  Note: This event is emitted before any authentication takes place,
> >>  thus the authentication ID is not provided
> >> [...]
> >>
> >>  -- Struct: VncServerInfo VncBasicInfo + {('auth': str)}
> >>
> >>  The network connection information for server
> >>  ''auth'' (optional)
> >>   authentication method used for the plain (non-websocket) VNC
> >>   server
> >>
> >>  Since: 2.1
> >>
> >>  -- Simple Union: SocketAddress { 'type': str, 'data': 'type' = ['inet':
> >>   InetSocketAddress, 'unix': UnixSocketAddress, 'vsock':
> >>   VsockSocketAddress, 'fd': String] }
> >>
> >>  Captures the address of a socket, which could also be a named file
> >>  descriptor
> >>
> >>  Since: 1.3
> >>
> >> Here's my counter-proposal: instead of inventing a formal language,
> >> fix the natural language documentation to actually mention *all*
> >> members, and add type information in a plain, easy-to-understand way.
> >> Looks like this:
> >>
> >>  -- Event: VNC_CONNECTED
> >>
> >>  Emitted when a VNC client establishes a connection
> >>
> >>  Arguments:
> >>  'server: VncServerInfo'
> >>   server information
> >>  'client: VncBasicInfo'
> >>   client information
> >>
> >>  Note: This event is emitted before any authentication takes place,
> >>  thus the authentication ID is not provided
> >> [...]
> >>
> >>  -- Object: VncServerInfo
> >>
> >>  The network connection information for server
> >>
> >>  Members:
> >>  'auth: string' (optional)
> >>   authentication method used for the plain (non-websocket) VNC
> >>   server
> >>  The members of 'VncBasicInfo'
> >>
> >>  Since: 2.1
> >>
> >>  -- Object: SocketAddress
> >>
> >>  Captures the address of a socket, which could also be a named file
> >>  descriptor
> >>
> >>  Members:
> >>  'type'
> >>   One of "inet", "unix", "vsock", "fd"
> >>  'data: InetSocketAddress' when 'type' is "inet"
> >>  'data: UnixSocketAddress' when 'type' is "unix"
> >>  'data: VsockSocketAddress' when 'type' is "vsock"
> >>  'data: String' when 'type' is "fd"
> >>
> >>  Since: 1.3
> >>
> >>
> > I like both, to me they serve different purposes. I like to have a short
> > overview / signature and then a more detailed documentation for each
> field.
>
> I sympathize with the argument.  Unfortunately, the "short" signatures
> are anything but for real-world QAPI:
>

That's a worse case, a regular case is more readable. And it is still
useful anyway since the common members 

Re: [Qemu-devel] [PATCH for-2.9 00/47] qapi: Put type information back into QMP documentation

[Markus Armbruster]

Marc-André Lureau  writes:

> Hi
>
> On Mon, Mar 13, 2017 at 10:23 AM Markus Armbruster 
> wrote:
>
>> I'm proposing this is 2.9 because it fixes a documentation regression.
>> It affects only documentation; generated C code is unchanged except
>> for the removal of trailing space in PATCH 46.
>>
>> Based on my qapi-next branch, which contains Marc-André's PATCH 1/2.
>>
>> Marc-André's work to merge qmp-commands.txt and qmp-events.txt into
>> the QAPI schema and generate their replacements from the schema
>> (commit b6af8ea..56e8bdd) was a big step forward.  As committed, it
>> also was a step back: the documentation lost information on JSON
>> types, because I didn't like Marc-André's patch to add it.  He
>> reposted it for further review afterwards:
>>
>> Subject: [PATCH 0/2] qapi2texi: add type information
>> Message-Id: <20170125130308.16104-1-marcandre.lur...@redhat.com>
>> https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg05432.html
>>
>> His PATCH 1/2 is a straightforward cleanup.  His PATCH 2/2 adds type
>> descriptions in a new formal language to the generated documentation.
>> Quoting the commit message:
>>
>> Array types have the following syntax: type[]. Ex: str[].
>>
>> - Struct, commands and events use the following members syntax:
>>
>>   { 'member': type, ('foo': str), ... }
>>
>> Optional members are under parentheses.
>>
>> A structure with a base type will have 'BaseStruct +' prepended.
>>
>> - Alternates use the following syntax:
>>
>>   [ 'foo': type, 'bar': type, ... ]
>>
>> - Simple unions use the following syntax:
>>
>>   { 'type': str, 'data': 'type' = [ 'foo': type, 'bar': type... ] }
>>
>> - Flat unions use the following syntax:
>>
>>   BaseStruct + 'discriminator' = [ 'foo': type, 'bar': type... ]
>>
>> End quote.  Looks like this in generated documentation:
>>
>>  -- Event: VNC_CONNECTED {'server': VncServerInfo, 'client':
>>   VncBasicInfo}
>>
>>  Emitted when a VNC client establishes a connection
>>  ''server''
>>   server information
>>  ''client''
>>   client information
>>
>>  Note: This event is emitted before any authentication takes place,
>>  thus the authentication ID is not provided
>> [...]
>>
>>  -- Struct: VncServerInfo VncBasicInfo + {('auth': str)}
>>
>>  The network connection information for server
>>  ''auth'' (optional)
>>   authentication method used for the plain (non-websocket) VNC
>>   server
>>
>>  Since: 2.1
>>
>>  -- Simple Union: SocketAddress { 'type': str, 'data': 'type' = ['inet':
>>   InetSocketAddress, 'unix': UnixSocketAddress, 'vsock':
>>   VsockSocketAddress, 'fd': String] }
>>
>>  Captures the address of a socket, which could also be a named file
>>  descriptor
>>
>>  Since: 1.3
>>
>> Here's my counter-proposal: instead of inventing a formal language,
>> fix the natural language documentation to actually mention *all*
>> members, and add type information in a plain, easy-to-understand way.
>> Looks like this:
>>
>>  -- Event: VNC_CONNECTED
>>
>>  Emitted when a VNC client establishes a connection
>>
>>  Arguments:
>>  'server: VncServerInfo'
>>   server information
>>  'client: VncBasicInfo'
>>   client information
>>
>>  Note: This event is emitted before any authentication takes place,
>>  thus the authentication ID is not provided
>> [...]
>>
>>  -- Object: VncServerInfo
>>
>>  The network connection information for server
>>
>>  Members:
>>  'auth: string' (optional)
>>   authentication method used for the plain (non-websocket) VNC
>>   server
>>  The members of 'VncBasicInfo'
>>
>>  Since: 2.1
>>
>>  -- Object: SocketAddress
>>
>>  Captures the address of a socket, which could also be a named file
>>  descriptor
>>
>>  Members:
>>  'type'
>>   One of "inet", "unix", "vsock", "fd"
>>  'data: InetSocketAddress' when 'type' is "inet"
>>  'data: UnixSocketAddress' when 'type' is "unix"
>>  'data: VsockSocketAddress' when 'type' is "vsock"
>>  'data: String' when 'type' is "fd"
>>
>>  Since: 1.3
>>
>>
> I like both, to me they serve different purposes. I like to have a short
> overview / signature and then a more detailed documentation for each field.

I sympathize with the argument.  Unfortunately, the "short" signatures
are anything but for real-world QAPI:

 -- Flat Union: BlockdevOptions {'driver': BlockdevDriver, ('node-name':
  str), ('discard': BlockdevDiscardOptions), ('cache':
  BlockdevCacheOptions), ('read-only': bool), ('detect-zeroes':
  BlockdevDetectZeroesOptions)} + 'driver' = ['archipelago':
  BlockdevOptionsArchipelago, 'blkdebug':
  BlockdevOptionsBlkdebug, 'blkverify':
  BlockdevOptionsBlkverify, 'bochs':
  BlockdevOptionsGenericFormat, 

Re: [Qemu-devel] [PATCH for-2.9 00/47] qapi: Put type information back into QMP documentation

[Marc-André Lureau]

Hi

On Mon, Mar 13, 2017 at 10:23 AM Markus Armbruster 
wrote:

> I'm proposing this is 2.9 because it fixes a documentation regression.
> It affects only documentation; generated C code is unchanged except
> for the removal of trailing space in PATCH 46.
>
> Based on my qapi-next branch, which contains Marc-André's PATCH 1/2.
>
> Marc-André's work to merge qmp-commands.txt and qmp-events.txt into
> the QAPI schema and generate their replacements from the schema
> (commit b6af8ea..56e8bdd) was a big step forward.  As committed, it
> also was a step back: the documentation lost information on JSON
> types, because I didn't like Marc-André's patch to add it.  He
> reposted it for further review afterwards:
>
> Subject: [PATCH 0/2] qapi2texi: add type information
> Message-Id: <20170125130308.16104-1-marcandre.lur...@redhat.com>
> https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg05432.html
>
> His PATCH 1/2 is a straightforward cleanup.  His PATCH 2/2 adds type
> descriptions in a new formal language to the generated documentation.
> Quoting the commit message:
>
> Array types have the following syntax: type[]. Ex: str[].
>
> - Struct, commands and events use the following members syntax:
>
>   { 'member': type, ('foo': str), ... }
>
> Optional members are under parentheses.
>
> A structure with a base type will have 'BaseStruct +' prepended.
>
> - Alternates use the following syntax:
>
>   [ 'foo': type, 'bar': type, ... ]
>
> - Simple unions use the following syntax:
>
>   { 'type': str, 'data': 'type' = [ 'foo': type, 'bar': type... ] }
>
> - Flat unions use the following syntax:
>
>   BaseStruct + 'discriminator' = [ 'foo': type, 'bar': type... ]
>
> End quote.  Looks like this in generated documentation:
>
>  -- Event: VNC_CONNECTED {'server': VncServerInfo, 'client':
>   VncBasicInfo}
>
>  Emitted when a VNC client establishes a connection
>  ''server''
>   server information
>  ''client''
>   client information
>
>  Note: This event is emitted before any authentication takes place,
>  thus the authentication ID is not provided
> [...]
>
>  -- Struct: VncServerInfo VncBasicInfo + {('auth': str)}
>
>  The network connection information for server
>  ''auth'' (optional)
>   authentication method used for the plain (non-websocket) VNC
>   server
>
>  Since: 2.1
>
>  -- Simple Union: SocketAddress { 'type': str, 'data': 'type' = ['inet':
>   InetSocketAddress, 'unix': UnixSocketAddress, 'vsock':
>   VsockSocketAddress, 'fd': String] }
>
>  Captures the address of a socket, which could also be a named file
>  descriptor
>
>  Since: 1.3
>
> Here's my counter-proposal: instead of inventing a formal language,
> fix the natural language documentation to actually mention *all*
> members, and add type information in a plain, easy-to-understand way.
> Looks like this:
>
>  -- Event: VNC_CONNECTED
>
>  Emitted when a VNC client establishes a connection
>
>  Arguments:
>  'server: VncServerInfo'
>   server information
>  'client: VncBasicInfo'
>   client information
>
>  Note: This event is emitted before any authentication takes place,
>  thus the authentication ID is not provided
> [...]
>
>  -- Object: VncServerInfo
>
>  The network connection information for server
>
>  Members:
>  'auth: string' (optional)
>   authentication method used for the plain (non-websocket) VNC
>   server
>  The members of 'VncBasicInfo'
>
>  Since: 2.1
>
>  -- Object: SocketAddress
>
>  Captures the address of a socket, which could also be a named file
>  descriptor
>
>  Members:
>  'type'
>   One of "inet", "unix", "vsock", "fd"
>  'data: InetSocketAddress' when 'type' is "inet"
>  'data: UnixSocketAddress' when 'type' is "unix"
>  'data: VsockSocketAddress' when 'type' is "vsock"
>  'data: String' when 'type' is "fd"
>
>  Since: 1.3
>
>
I like both, to me they serve different purposes. I like to have a short
overview / signature and then a more detailed documentation for each field.

Additionally, my series fixes a number of bugs and cleans up along the
> way.  In particular, it converts qapi2texi.py from parse trees to the
> visitor interface the other generators use.
>
>
Your series failed to apply in patchew, and I can't find the branch in your
repo. Could you publish it?


> Future generated documentation work includes eliding types that aren't
> visible in QMP (like introspection does), and making uses of type
> names links in HTML.
>
>
Yes, links would be really nice.

Thanks


> Markus Armbruster (47):
>   qapi: Factor QAPISchemaParser._include() out of .__init__()
>   qapi: Make doc comments optional where we don't need them
>   qapi: Back out doc comments added just to please qapi.py
>   docs/qapi-code-gen.txt: Drop confusing