Re: [PATCH] qemu-nbd: Convert invocation documentation to rST

[Eric Blake]

On 1/13/20 12:08 PM, Peter Maydell wrote:
> The qemu-nbd documentation is currently in qemu-nbd.texi in Texinfo
> format, which we present to the user as:
>   * a qemu-nbd manpage
>   * a section of the main qemu-doc HTML documentation
>
> Convert the documentation to rST format, and present it to the user as:
>   * a qemu-nbd manpage
>   * part of the interop/ Sphinx manual
>
> This follows the same pattern as commit 27a296fce982 did for the
> qemu-ga manpage.

I'm not an rST expert, but trust that you compared the resulting 
renderings.  Is there a quick recipe for doing the build locally so I 
can easily inspect local artifacts myself?

> All the content of the old manpage is retained, except that I have
> dropped the "This is free software; see the source for copying
> conditions.  There is NO warranty..." text that was in the old AUTHOR
> section; Sphinx's manpage builder doesn't expect that much text in
> the AUTHOR section, and since none of our other manpages have it it
> seems easiest to delete it rather than try to figure out where else
> in the manpage to put it.

Reasonable.

> The only other textual change is that I have had to give the
> --nocache option its own description ("Equivalent to --cache=none")
> because Sphinx doesn't have an equivalent of using item/itemx
> to share a description between two options.
>
> Some minor aspects of the formatting have changed, to suit what is
> easiest for Sphinx to output. (The most notable is that Sphinx
> option section option syntax doesn't support '--option foo=bar'
> with bar underlined rather than bold, so we have to switch to
> '--option foo=BAR' instead.)

All fine by me.  If the resulting docs are more consistent between tools 
as a result of normalizations like this, it is a win.

> The contents of qemu-option-trace.texi are now duplicated in
> docs/interop/qemu-option-trace.rst.inc, until such time as we complete
> the conversion of the other files which use it; since it has had only
> 3 changes in 3 years, this shouldn't be too awkward a burden.
> (We use .rst.inc because if this file fragment has a .rst extension
> then Sphinx complains about not seeing it in a toctree.)

Yes, that's fair for the short-term.

> Signed-off-by: Peter Maydell <address@hidden>
> ---
> Another step forward for https://wiki.qemu.org/Features/Documentation
> (this is part of step 3 in the 'transition plan').
> [A
> ---

> +++ b/Makefile
> @@ -338,7 +338,7 @@ MANUAL_BUILDDIR := docs
>   endif
>   
>   ifdef BUILD_DOCS
> -DOCS=qemu-doc.html qemu-doc.txt qemu.1 qemu-img.1 qemu-nbd.8 
> $(MANUAL_BUILDDIR)/interop/qemu-ga.8
> +DOCS=qemu-doc.html qemu-doc.txt qemu.1 qemu-img.1 
> $(MANUAL_BUILDDIR)/interop/qemu-nbd.8 $(MANUAL_BUILDDIR)/interop/qemu-ga.8

Worth splitting this long line, either with \-newline, or

>   DOCS+=docs/interop/qemu-qmp-ref.html docs/interop/qemu-qmp-ref.txt 
> docs/interop/qemu-qmp-ref.7
>   DOCS+=docs/interop/qemu-ga-ref.html docs/interop/qemu-ga-ref.txt 
> docs/interop/qemu-ga-ref.7

with additional DOCS+= lines?


> @@ -1005,7 +1005,7 @@ sphinxdocs: $(MANUAL_BUILDDIR)/devel/index.html 
> $(MANUAL_BUILDDIR)/interop/index
>   # a single doctree: https://github.com/sphinx-doc/sphinx/issues/2946
>   build-manual = $(call quiet-command,CONFDIR="$(qemu_confdir)" sphinx-build $(if $(V),,-q) -W -b $2 -D 
> version=$(VERSION) -D release="$(FULL_VERSION)" -d .doctrees/$1-$2 $(SRC_PATH)/docs/$1 $(MANUAL_BUILDDIR)/$1 
> ,"SPHINX","$(MANUAL_BUILDDIR)/$1")
>   # We assume all RST files in the manual's directory are used in it
> -manual-deps = $(wildcard $(SRC_PATH)/docs/$1/*.rst) 
> $(SRC_PATH)/docs/$1/conf.py $(SRC_PATH)/docs/conf.py
> +manual-deps = $(wildcard $(SRC_PATH)/docs/$1/*.rst) $(wildcard 
> $(SRC_PATH)/docs/$1/*.rst.inc) $(SRC_PATH)/docs/$1/conf.py 
> $(SRC_PATH)/docs/conf.py

Another long line.

> +++ b/MAINTAINERS
> @@ -2503,6 +2503,7 @@ F: include/block/nbd*
>   F: qemu-nbd.*
>   F: blockdev-nbd.c
>   F: docs/interop/nbd.txt
> +F: docs/interop/qemu-nbd.rst
>   T: git https://repo.or.cz/qemu/ericb.git nbd

Would I be taking this patch through my NBD tree, or would you be 
bundling it with other doc patches?

> +++ b/docs/interop/qemu-nbd.rst
> @@ -0,0 +1,263 @@

> +.. option:: -s, --snapshot
> +
> +  Use *filename* as an external snapshot, create a temporary
> +  file with ``backing_file=``\ *filename*, redirect the write to
> +  the temporary one.

Pre-existing poor grammar. Better might be:

Use *filename* as an external snapshot, by creating a temporary file 
with ``backing_file=``\ *filename*, and redirecting writes to the 
temporary file.

But that could also be done as a separate patch, to keep this one as 
mechanical as possible on the conversion.

> +
> +.. option:: -l, --load-snapshot=SNAPSHOT_PARAM
> +
> +  Load an internal snapshot inside *filename* and export it
> +  as an read-only device, SNAPSHOT_PARAM format is

Similarly, s/an/a/


> +
> +Serve a read-only copy of just the first MBR partition of a guest
> +image over a Unix socket with as many as 5 simultaneous readers, with
> +a persistent process forked as a daemon:
> +
> +::
> +
> +  qemu-nbd --fork --persistent --shared=5 --socket=/path/to/sock \
> +    --partition=1 --read-only --format=qcow2 file.qcow2

Hmm - this example uses the deprecated --partition. And it was 
deprecated in 4.0.0. So 5.0 is a reasonable time to just nuke that code, 
and therefore this example - but as a separate patch (rather than 
touching up this example to use the spelling suggested in 
qemu-deprecated.texi).

> +
> +Expose the guest-visible contents of a qcow2 file via a block device
> +/dev/nbd0 (and possibly creating /dev/nbd0p1 and friends for
> +partitions found within), then disconnect the device when done.
> +Access to bind qemu-nbd to an /dev/nbd device generally requires root

s/an/a/

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