Re: [Qemu-devel] [PATCH v2 05/11] docs: add qapi texi template

[Markus Armbruster]

Marc-André Lureau <address@hidden> writes:

> Hi
>
> ----- Original Message -----
>> Marc-André Lureau <address@hidden> writes:
>> 
>> > 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é Lureau <address@hidden>
>> 
>> I'm not exactly a Texinfo expert, but I can compare to the Texinfo
>> manual.
>> 
>> 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.

Let's touch it up once this series is in.

>> > ---
>> >  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
>> 
>> The Texinfo manual uses
>> 
>>    \input texinfo   @c -*-texinfo-*-
>> 
>> 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 
> necessary)
>> 
>> > address@hidden qemu-ga-qapi
>> 
>> Not wrong, but the Texinfo manual recommends to replace the extension
>> (here: .texi) with .info, so let's do that:
>> 
>>    @setfilename qemu-ga-qapi.info
>
> ok
>
>> 
>> > address@hidden en
>> 
>> This overrides the default en_US to just en.  Is that what we want?
>
> let's keep the default
>
>> 
>> > address@hidden 0
>> > address@hidden 0
>> > +
>> > address@hidden QEMU-GA QAPI Reference Manual
>> 
>> 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:
>> 
>>    @settitle QEMU Guest Agent Protocol Reference
>> 
>> But then the filenames are off.  Rename to qemu-ga-ref.*.
>
> fine by me
>
>> 
>> > +
>> 
>> I think we need a copyright note.  Something like:
>> 
>>    @copying
>>    This is the QEMU Guest Agent QAPI reference manual.
>> 
>>    Copyright @copyright{} 2016 The QEMU Project developers
>> 
>>    @quotation
>>    Permission is granted to ...
>>    @end quotation
>>    @end copying
>> 
>> > address@hidden
>> 
>>    @dircategory QEMU
>
> ok
>
>> Should be added to qemu-doc.texi as well.
>
> I'll leave that for another series
>
>> > address@hidden
>> > +* QEMU-GA-QAPI: (qemu-doc).    QEMU-GA QAPI Reference Manual
>> 
>> Pasto: (qemu-doc)
>> 
>> The description should start at column 32, not 31.
>> 
>> If we retitle and rename as suggested, this becomes:
>> 
>>    * QEMU-GA-Ref: (qemu-ga-ref):   QEMU Guest Agent Protocol Reference
>> 
>
> ok
>
>> > address@hidden direntry
>> > address@hidden ifinfo
>> 
>> Are you sure we need @ifinfo?
>
> Probably not, but it looks more explicit to me that this part is only for 
> .info

I don't think redundant conditionals buy us anything.

>> > +
>> > address@hidden
>> > address@hidden
>> > address@hidden 7
>> > address@hidden @titlefont{{QEMU Guest Agent {version}}}
>> 
>> {version} seems to get replaced by qapi2texi.py.  Worth a comment.
>> 
>
> ok

Peeking at your v3, I see you found a better solution :)

>> > address@hidden 1
>> > address@hidden @titlefont{{QAPI Reference Manual}}
>> 
>> Protocol Reference Manual
>> 
>> > address@hidden 3
>> 
>> Isn't @sp right before @end titlepage redundant?
>
> ok
>
>> 
>> We need to include the copyright notice:
>> 
>>    @page
>>    @vskip 0pt plus 1filll
>>    @insertcopying
>> 
>
> ok
>
>> > address@hidden titlepage
>> > address@hidden iftex
>> 
>> Are you sure we need @iftex?
>
> Same as qemu-doc.texi, I suppose it looks better with info.
>
>> 
>> We can also let Texinfo do the spacing, like this:
>> 
>>    @titlepage
>>    @title QEMU Guest Agent {version}
>>    @subtitle Protocol Reference Manual
>>    @page
>>    @vskip 0pt plus 1filll
>>    @insertcopying
>>    @end titlepage
>> 
>
> 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.
>> 
>>    @title QEMU Guest Agent Protocol Reference Manual
>>    @subtitle for QEMU {version}
>> 
>> Your choice.
>
> Yep, looks good, altough doesn't fit on a single line, I am dropping the 
> leading QEMU
>
>> The examples in Texinfo manual Appendix C "Sample Texinfo Files" have
>> @contents right here, after the title page.
>> 
>
> Ok
>
>> > +
>> > address@hidden
>> > address@hidden Top
>> > address@hidden
>> 
>>    @top QEMU Guest Agent QAPI reference
>> 
>> > +
>> > +This is the QEMU Guest Agent QAPI reference for QEMU {version}.
>> 
>> "protocol reference manual for"
>> 
>> According to the Texinfo manual's examples, the @end ifnottex goes
>> here...
>
> Removing it, now redundant with @copying text
>
>> 
>> > +
>> > address@hidden
>> > +* API Reference::
>> > +* Commands and Events Index::
>> > +* Data Types Index::
>> > address@hidden menu
>> > +
>> > address@hidden ifnottex
>> 
>> ... and not here.
>> 
>
> ok
>
>> > +
>> > address@hidden
>> 
>> Suggest to move this up, as mentioned above.
>> 
>
> ok
>
>> > +
>> > address@hidden API Reference
>> > address@hidden API Reference
>> > +
>> > address@hidden man begin DESCRIPTION
>> 
>> What does this @c man magic do?  Suggest to explain in a comment.
>
> It's for texi2pod, I'll add a comment
>> 
>> > +{qapi}
>> 
>> This seems to get replaced with the generated reference documentation by
>> qapi2texi.py.  Worth a comment.
>
> ok
>
>> 
>> > address@hidden man end
>> > +
>> > address@hidden man begin SEEALSO
>> > +The HTML documentation of QEMU for more precise information.
>> > address@hidden man end
>> 
>> More @c man magic.
>> 
>> > +
>> > address@hidden Commands and Events Index
>> > address@hidden Commands and Events Index
>> > address@hidden fn
>> 
>> Blank line here, please.
>> 
>
> ok
>
>> > address@hidden Data Types Index
>> > address@hidden Data Types Index
>> > address@hidden tp
>> 
>> And here.
>
> ok
>
>> 
>> > address@hidden
>> > diff --git a/docs/qemu-qapi.template.texi b/docs/qemu-qapi.template.texi
>> > new file mode 100644
>> > index 0000000..102c8d9
>> > --- /dev/null
>> > +++ b/docs/qemu-qapi.template.texi
>> 
>> The comments above largely apply; I won't repeat them.
>> 
>> > @@ -0,0 +1,148 @@
>> > +\input texinfo
>> > address@hidden qemu-qapi
>> > address@hidden en
>> > address@hidden 0
>> > address@hidden 0
>> > +
>> > address@hidden QEMU QAPI Reference Manual
>> 
>> 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.
>> 
>> Options:
>> 
>> * Claim this is the QMP reference manual, include the internal types
>>   anyway.
>> 
>> * Filter out the internal types automatically, similar to
>>   qmp-introspect.py.
>> 
>> * Filter out the internal types manually, by annotating them in the
>>   schema.  Feels error-prone.
>> 
>> * Split the QAPI schema.
>> 
>> * Reflect the curious mix of QMP protocol and internal data type
>>   reference in the title.
>> 
>> 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?

We define several types in the QAPI schema that are used only
internally.  Useful trick to see what's external:

    $ python -B scripts/qapi-introspect.py -cu -p scratch- qapi-schema.json

This generates scratch-qmp-introspect.c describing the external QMP
interface with unmasked type names (-u).  Anything not mentioned there
is not externally visible.

To see how qapi-introspect.py finds the externally visible part of the
schema, search it for _use_type.

>> > +
>> > address@hidden
>> > address@hidden
>> > +* QEMU: (qemu-doc).    QEMU QAPI Reference Manual
>> > address@hidden direntry
>> > address@hidden ifinfo
>> > +
>> > address@hidden
>> > address@hidden
>> > address@hidden 7
>> > address@hidden @titlefont{{QEMU Emulator {version}}}
>> > address@hidden 1
>> > address@hidden @titlefont{{QAPI Reference Manual}}
>> > address@hidden 3
>> > address@hidden titlepage
>> > address@hidden iftex
>> > +
>> > address@hidden
>> > address@hidden Top
>> > address@hidden
>> > +
>> > +This is the QMP QAPI reference for QEMU {version}.
>> > +
>> > address@hidden
>> > +* Introduction::
>> > +* API Reference::
>> > +* Commands and Events Index::
>> > +* Data Types Index::
>> > address@hidden menu
>> > +
>> > address@hidden ifnottex
>> > +
>> > address@hidden
>> > +
>> > address@hidden Introduction
>> > address@hidden 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:
>> > +
>> > address@hidden @minus
>> 
>> @bullet is more common.  Matter of taste.
>
> ok
>
>> 
>> > address@hidden
>> > +Lightweight, text-based, easy to parse data format
>> 
>> Suggest "plain text" instead of "text-based".
>
> ok
>
>> 
>> JSON is "easy to parse" until you hit the potholes in its specification.
>> See "Parsing JSON is a Minefield" <http://seriot.ch/parsing_json.html>.
>> 
>>    QMP provides the following features:
>> 
>>    @itemize @bullet
>>    @item
>>    Simple @uref{{http://www.json.org, JSON}} syntax
>> 
>> > address@hidden
>> > +Asynchronous messages support (ie. events)
>> 
>> i.e.
>> 
>> But I'd say
>> 
>>    @item
>>    Synchronous commands and replies
>>    @item
>>    Asynchronous messages ("events")
>> 
>> > address@hidden
>> > +Capabilities Negotiation
>> 
>> I'd add
>> 
>>    @item
>>    Introspection
>> 
>> > address@hidden itemize
>> > +
>> > +For detailed information on QEMU Machine Protocol, the specification
>> > +is in @file{{qmp-spec.txt}}.
>> > +
>> > address@hidden Usage
>> > +
>> > +You can use the @option{{-qmp}} option to enable QMP. For example, the
>> > +following makes QMP available on localhost port 4444:
>> > +
>> > address@hidden
>> > +$ qemu [...] -qmp tcp:localhost:4444,server,nowait
>> > address@hidden example
>> > +
>> > +However, for more flexibility and to make use of more options, the
>> > address@hidden 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:
>> 
>> This sounds a bit like we don't want people to use -qmp.  What about
>> 
>>    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:
>>    
>> 
>> > +
>> > address@hidden
>> > +$ qemu [...] -chardev stdio,id=mon0 -mon chardev=mon0,mode=readline \
>> > +             -chardev
>> > socket,id=mon1,host=localhost,port=4444,server,nowait \
>> > +             -mon chardev=mon1,mode=control,pretty=on
>> > address@hidden example
>> 
>> Not sure we want to drag in HMP here.
>> 
>> > +
>> > +Please, refer to QEMU's manpage for more information.
>> 
>> Drop the comma.
>> 
>> Hrmmm, I just realized this is merely moved from qmp-intro.txt.  I guess
>> I should read your commit message before your patch %-)
>> 
>> 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.
>> 
>> We can decide this isn't a reference manual after all, and change title,
>> file name and so forth accordingly.
>> 
>> 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 
> more chances it gets read, be it online in html/pdf for, or with the info/man 
> pages.

Replacing qmp-commands.txt by a QMP reference manual is a
straightforward task: we don't have to worry about scope or structure,
only about not losing valuable contents.

But as soon you draw in qmp-intro.txt, we're making a QMP manual.
That's a more complex task, posing additional questions.  For starters,
why is qmp-spec.txt not part of it?  Should the QGA manual get similar
contents?

I'm not saying that task should not be tackled.  But let's control this
series' scope, and make just a QMP reference manual.  We can expand it
into a QMP manual with a reference part later if we like.

> Any suggestion for the the naming? (tbh, I don't mind it being called 
> reference manual or not, even if it includes that text).

We could call it "QMP User Manual".