From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42948) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1c2eX5-00061T-80 for qemu-devel@nongnu.org; Fri, 04 Nov 2016 09:27:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1c2eX1-00029i-9z for qemu-devel@nongnu.org; Fri, 04 Nov 2016 09:27:47 -0400 Received: from mx6-phx2.redhat.com ([209.132.183.39]:57630) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1c2eX1-000294-03 for qemu-devel@nongnu.org; Fri, 04 Nov 2016 09:27:43 -0400 Date: Fri, 4 Nov 2016 09:27:41 -0400 (EDT) From: =?utf-8?Q?Marc-Andr=C3=A9?= Lureau Message-ID: <851199757.3843695.1478266061834.JavaMail.zimbra@redhat.com> In-Reply-To: <877f8tg7tf.fsf@dusky.pond.sub.org> References: <20160925181836.18293-1-marcandre.lureau@redhat.com> <20160925181836.18293-6-marcandre.lureau@redhat.com> <877f8tg7tf.fsf@dusky.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2 05/11] docs: add qapi texi template List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: =?utf-8?Q?Marc-Andr=C3=A9?= Lureau , qemu-devel@nongnu.org Hi ----- Original Message ----- > Marc-Andr=C3=A9 Lureau writes: >=20 > > The qapi2texi scripts uses a template for the texi file. Since we are > > going to generate the documentation in multiple formats, move qmp-intro > > to qemu-qapi template. (it would be nice to write something similar for > > qemu-ga, but this is left for a future patch) > > > > Signed-off-by: Marc-Andr=C3=A9 Lureau >=20 > I'm not exactly a Texinfo expert, but I can compare to the Texinfo > manual. >=20 > Lots of comments below. Please don't let them discourage you! Your two > manuals are pretty slick already, and a most welcome step forward. I based it on some older version of qemu-doc.texi. Many of your remarks are= also valid for it. >=20 > > --- > > docs/qemu-ga-qapi.template.texi | 58 ++++++++++++++++ > > docs/qemu-qapi.template.texi | 148 > > ++++++++++++++++++++++++++++++++++++++++ > > docs/qmp-intro.txt | 87 ----------------------- > > 3 files changed, 206 insertions(+), 87 deletions(-) > > create mode 100644 docs/qemu-ga-qapi.template.texi > > create mode 100644 docs/qemu-qapi.template.texi > > delete mode 100644 docs/qmp-intro.txt > > > > diff --git a/docs/qemu-ga-qapi.template.texi > > b/docs/qemu-ga-qapi.template.texi > > new file mode 100644 > > index 0000000..3ddbf56 > > --- /dev/null > > +++ b/docs/qemu-ga-qapi.template.texi > > @@ -0,0 +1,58 @@ > > +\input texinfo >=20 > The Texinfo manual uses >=20 > \input texinfo @c -*-texinfo-*- >=20 > but my version of Emacs seems to be fine without this. Shouldn't be necessary imho (in general, I am not fond of modeline unless n= ecessary) >=20 > > +@setfilename qemu-ga-qapi >=20 > Not wrong, but the Texinfo manual recommends to replace the extension > (here: .texi) with .info, so let's do that: >=20 > @setfilename qemu-ga-qapi.info ok >=20 > > +@documentlanguage en >=20 > This overrides the default en_US to just en. Is that what we want? let's keep the default >=20 > > +@exampleindent 0 > > +@paragraphindent 0 > > + > > +@settitle QEMU-GA QAPI Reference Manual >=20 > What is "QAPI", and why would the reader care? I think the manual is > about the QEMU Guest Agent Protocol. The fact that its implementation > relies on QAPI is immaterial here. What about: >=20 > @settitle QEMU Guest Agent Protocol Reference >=20 > But then the filenames are off. Rename to qemu-ga-ref.*. fine by me >=20 > > + >=20 > I think we need a copyright note. Something like: >=20 > @copying > This is the QEMU Guest Agent QAPI reference manual. >=20 > Copyright @copyright{} 2016 The QEMU Project developers >=20 > @quotation > Permission is granted to ... > @end quotation > @end copying >=20 > > +@ifinfo >=20 > @dircategory QEMU ok > Should be added to qemu-doc.texi as well. I'll leave that for another series >=20 > > +@direntry > > +* QEMU-GA-QAPI: (qemu-doc). QEMU-GA QAPI Reference Manual >=20 > Pasto: (qemu-doc) >=20 > The description should start at column 32, not 31. >=20 > If we retitle and rename as suggested, this becomes: >=20 > * QEMU-GA-Ref: (qemu-ga-ref): QEMU Guest Agent Protocol Reference >=20 ok > > +@end direntry > > +@end ifinfo >=20 > Are you sure we need @ifinfo? Probably not, but it looks more explicit to me that this part is only for .= info > > + > > +@iftex > > +@titlepage > > +@sp 7 > > +@center @titlefont{{QEMU Guest Agent {version}}} >=20 > {version} seems to get replaced by qapi2texi.py. Worth a comment. >=20 ok > > +@sp 1 > > +@center @titlefont{{QAPI Reference Manual}} >=20 > Protocol Reference Manual >=20 > > +@sp 3 >=20 > Isn't @sp right before @end titlepage redundant? ok >=20 > We need to include the copyright notice: >=20 > @page > @vskip 0pt plus 1filll > @insertcopying >=20 ok > > +@end titlepage > > +@end iftex >=20 > Are you sure we need @iftex? Same as qemu-doc.texi, I suppose it looks better with info. >=20 > We can also let Texinfo do the spacing, like this: >=20 > @titlepage > @title QEMU Guest Agent {version} > @subtitle Protocol Reference Manual > @page > @vskip 0pt plus 1filll > @insertcopying > @end titlepage >=20 That ends up with a different results than qapi-doc, but I also prefer it > The @title isn't really the title, though. Could reshuffle things a > bit, e.g. >=20 > @title QEMU Guest Agent Protocol Reference Manual > @subtitle for QEMU {version} >=20 > Your choice. Yep, looks good, altough doesn't fit on a single line, I am dropping the le= ading QEMU > The examples in Texinfo manual Appendix C "Sample Texinfo Files" have > @contents right here, after the title page. >=20 Ok > > + > > +@ifnottex > > +@node Top > > +@top >=20 > @top QEMU Guest Agent QAPI reference >=20 > > + > > +This is the QEMU Guest Agent QAPI reference for QEMU {version}. >=20 > "protocol reference manual for" >=20 > According to the Texinfo manual's examples, the @end ifnottex goes > here... Removing it, now redundant with @copying text >=20 > > + > > +@menu > > +* API Reference:: > > +* Commands and Events Index:: > > +* Data Types Index:: > > +@end menu > > + > > +@end ifnottex >=20 > ... and not here. >=20 ok > > + > > +@contents >=20 > Suggest to move this up, as mentioned above. >=20 ok > > + > > +@node API Reference > > +@chapter API Reference > > + > > +@c man begin DESCRIPTION >=20 > What does this @c man magic do? Suggest to explain in a comment. It's for texi2pod, I'll add a comment >=20 > > +{qapi} >=20 > This seems to get replaced with the generated reference documentation by > qapi2texi.py. Worth a comment. ok >=20 > > +@c man end > > + > > +@c man begin SEEALSO > > +The HTML documentation of QEMU for more precise information. > > +@c man end >=20 > More @c man magic. >=20 > > + > > +@node Commands and Events Index > > +@unnumbered Commands and Events Index > > +@printindex fn >=20 > Blank line here, please. >=20 ok > > +@node Data Types Index > > +@unnumbered Data Types Index > > +@printindex tp >=20 > And here. ok >=20 > > +@bye > > diff --git a/docs/qemu-qapi.template.texi b/docs/qemu-qapi.template.tex= i > > new file mode 100644 > > index 0000000..102c8d9 > > --- /dev/null > > +++ b/docs/qemu-qapi.template.texi >=20 > The comments above largely apply; I won't repeat them. >=20 > > @@ -0,0 +1,148 @@ > > +\input texinfo > > +@setfilename qemu-qapi > > +@documentlanguage en > > +@exampleindent 0 > > +@paragraphindent 0 > > + > > +@settitle QEMU QAPI Reference Manual >=20 > Again, QAPI is detail; it's the QEMU QMP reference manual. Except it > has more than just QMP, because we choose to use qapi-schema.json for > purely internal types in addition to QMP. >=20 > Options: >=20 > * Claim this is the QMP reference manual, include the internal types > anyway. >=20 > * Filter out the internal types automatically, similar to > qmp-introspect.py. >=20 > * Filter out the internal types manually, by annotating them in the > schema. Feels error-prone. >=20 > * Split the QAPI schema. >=20 > * Reflect the curious mix of QMP protocol and internal data type > reference in the title. >=20 > We don't need a perfect solution to commit this, but an understanding of > what we want to do would be nice. What are the internal types? How is it filtered? >=20 > > + > > +@ifinfo > > +@direntry > > +* QEMU: (qemu-doc). QEMU QAPI Reference Manual > > +@end direntry > > +@end ifinfo > > + > > +@iftex > > +@titlepage > > +@sp 7 > > +@center @titlefont{{QEMU Emulator {version}}} > > +@sp 1 > > +@center @titlefont{{QAPI Reference Manual}} > > +@sp 3 > > +@end titlepage > > +@end iftex > > + > > +@ifnottex > > +@node Top > > +@top > > + > > +This is the QMP QAPI reference for QEMU {version}. > > + > > +@menu > > +* Introduction:: > > +* API Reference:: > > +* Commands and Events Index:: > > +* Data Types Index:: > > +@end menu > > + > > +@end ifnottex > > + > > +@contents > > + > > +@node Introduction > > +@chapter Introduction > > + > > +The QEMU Machine Protocol (@acronym{{QMP}}) allows applications to > > +operate a QEMU instance. > > + > > +QMP is @uref{{http://www.json.org, JSON}} based and features the > > +following: > > + > > +@itemize @minus >=20 > @bullet is more common. Matter of taste. ok >=20 > > +@item > > +Lightweight, text-based, easy to parse data format >=20 > Suggest "plain text" instead of "text-based". ok >=20 > JSON is "easy to parse" until you hit the potholes in its specification. > See "Parsing JSON is a Minefield" . >=20 > QMP provides the following features: >=20 > @itemize @bullet > @item > Simple @uref{{http://www.json.org, JSON}} syntax >=20 > > +@item > > +Asynchronous messages support (ie. events) >=20 > i.e. >=20 > But I'd say >=20 > @item > Synchronous commands and replies > @item > Asynchronous messages ("events") >=20 > > +@item > > +Capabilities Negotiation >=20 > I'd add >=20 > @item > Introspection >=20 > > +@end itemize > > + > > +For detailed information on QEMU Machine Protocol, the specification > > +is in @file{{qmp-spec.txt}}. > > + > > +@section Usage > > + > > +You can use the @option{{-qmp}} option to enable QMP. For example, the > > +following makes QMP available on localhost port 4444: > > + > > +@example > > +$ qemu [...] -qmp tcp:localhost:4444,server,nowait > > +@end example > > + > > +However, for more flexibility and to make use of more options, the > > +@option{{-mon}} command-line option should be used. For instance, the > > +following example creates one HMP instance (human monitor) on stdio > > +and one QMP instance on localhost port 4444: >=20 > This sounds a bit like we don't want people to use -qmp. What about >=20 > However, for more flexibility and to make use of more options, the > @option{{-mon}} command-line option should be used. For instance, the > following example creates one HMP instance (human monitor) on stdio > and one QMP instance on localhost port 4444: > =20 >=20 > > + > > +@example > > +$ qemu [...] -chardev stdio,id=3Dmon0 -mon chardev=3Dmon0,mode=3Dreadl= ine \ > > + -chardev > > socket,id=3Dmon1,host=3Dlocalhost,port=3D4444,server,nowait \ > > + -mon chardev=3Dmon1,mode=3Dcontrol,pretty=3Don > > +@end example >=20 > Not sure we want to drag in HMP here. >=20 > > + > > +Please, refer to QEMU's manpage for more information. >=20 > Drop the comma. >=20 > Hrmmm, I just realized this is merely moved from qmp-intro.txt. I guess > I should read your commit message before your patch %-) >=20 > I'm not sure a *reference* manual is a good home for an *introduction* > to use. It's certainly not where I'd look first. >=20 > We can decide this isn't a reference manual after all, and change title, > file name and so forth accordingly. >=20 > Or we can stick to the reference manual idea, and include qmp-intro.txt > by reference. Imho it would be nice to include qmp-intro in the document, has there are m= ore chances it gets read, be it online in html/pdf for, or with the info/ma= n pages. Any suggestion for the the naming? (tbh, I don't mind it being called refer= ence manual or not, even if it includes that text). thanks >=20 > > + > > +@section Simple testing > > + > > +To manually test QMP one can connect with telnet and issue commands by > > +hand: > > + > > +@example > > +$ telnet localhost 4444 > > +Trying 127.0.0.1... > > +Connected to localhost. > > +Escape character is '^]'. > > +@{{ > > + "QMP": @{{ > > + "version": @{{ > > + "qemu": @{{ > > + "micro": 50, > > + "minor": 6, > > + "major": 1 > > + @}}, > > + "package": "" > > + @}}, > > + "capabilities": [ > > + ] > > + @}} > > +@}} > > + > > +@{{ "execute": "qmp_capabilities" @}} > > +@{{ > > + "return": @{{ > > + @}} > > +@}} > > + > > +@{{ "execute": "query-status" @}} > > +@{{ > > + "return": @{{ > > + "status": "prelaunch", > > + "singlestep": false, > > + "running": false > > + @}} > > +@}} > > +@end example > > + > > +@section Wiki > > + > > +Please refer to the @uref{{http://wiki.qemu-project.org/QMP, QMP QEMU > > + wiki page}} for more details on QMP. > > + > > +@node API Reference > > +@chapter API Reference > > + > > +@c man begin DESCRIPTION > > +{qapi} > > +@c man end > > + > > +@c man begin SEEALSO > > +The HTML documentation of QEMU for more precise information. > > +@c man end > > + > > +@node Commands and Events Index > > +@unnumbered Commands and Events Index > > +@printindex fn > > +@node Data Types Index > > +@unnumbered Data Types Index > > +@printindex tp > > +@bye > > diff --git a/docs/qmp-intro.txt b/docs/qmp-intro.txt > > deleted file mode 100644 > > index f6a3a03..0000000 > > --- a/docs/qmp-intro.txt > > +++ /dev/null > > @@ -1,87 +0,0 @@ > > - QEMU Machine Protocol > > - =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D > > - > > -Introduction > > ------------- > > - > > -The QEMU Machine Protocol (QMP) allows applications to operate a > > -QEMU instance. > > - > > -QMP is JSON[1] based and features the following: > > - > > -- Lightweight, text-based, easy to parse data format > > -- Asynchronous messages support (ie. events) > > -- Capabilities Negotiation > > - > > -For detailed information on QMP's usage, please, refer to the followin= g > > files: > > - > > -o qmp-spec.txt QEMU Machine Protocol current specification > > -o qmp-commands.txt QMP supported commands (auto-generated at build-ti= me) > > -o qmp-events.txt List of available asynchronous events > > - > > -[1] http://www.json.org > > - > > -Usage > > ------ > > - > > -You can use the -qmp option to enable QMP. For example, the following > > -makes QMP available on localhost port 4444: > > - > > -$ qemu [...] -qmp tcp:localhost:4444,server,nowait > > - > > -However, for more flexibility and to make use of more options, the -mo= n > > -command-line option should be used. For instance, the following exampl= e > > -creates one HMP instance (human monitor) on stdio and one QMP instance > > -on localhost port 4444: > > - > > -$ qemu [...] -chardev stdio,id=3Dmon0 -mon chardev=3Dmon0,mode=3Dreadl= ine \ > > - -chardev > > socket,id=3Dmon1,host=3Dlocalhost,port=3D4444,server,nowait \ > > - -mon chardev=3Dmon1,mode=3Dcontrol,pretty=3Don > > - > > -Please, refer to QEMU's manpage for more information. > > - > > -Simple Testing > > --------------- > > - > > -To manually test QMP one can connect with telnet and issue commands by > > hand: > > - > > -$ telnet localhost 4444 > > -Trying 127.0.0.1... > > -Connected to localhost. > > -Escape character is '^]'. > > -{ > > - "QMP": { > > - "version": { > > - "qemu": { > > - "micro": 50, > > - "minor": 6, > > - "major": 1 > > - }, > > - "package": "" > > - }, > > - "capabilities": [ > > - ] > > - } > > -} > > - > > -{ "execute": "qmp_capabilities" } > > -{ > > - "return": { > > - } > > -} > > - > > -{ "execute": "query-status" } > > -{ > > - "return": { > > - "status": "prelaunch", > > - "singlestep": false, > > - "running": false > > - } > > -} > > - > > -Please, refer to the qapi-schema.json file for a complete command > > reference. > > - > > -QMP wiki page > > -------------- > > - > > -http://wiki.qemu-project.org/QMP >=20