Re: [Qemu-devel] [PATCH v2] Add manpage for QEMU Backup Tool

[Stefan Hajnoczi]

On Tue, Jun 13, 2017 at 08:50:06PM +0530, Ishani Chugh wrote:

Please include a commit description.  In this case it would be
appropriate to explain that qemu-backup will be a command-line tool for
performing full and incremental disk backups on running VMs.  It is
intended as a reference implementation for management stack and backup
developers to see QEMU's backup features in action.

> Signed-off-by: Ishani Chugh <address@hidden>
> ---
>  Makefile                        |   2 +-
>  contrib/backup/qemu-backup.texi | 139 
> ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 140 insertions(+), 1 deletion(-)
>  create mode 100644 contrib/backup/qemu-backup.texi
> 
> diff --git a/Makefile b/Makefile
> index c830d7a..f42cb1d 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -504,7 +504,7 @@ clean:
>  VERSION ?= $(shell cat VERSION)
>  
>  dist: qemu-$(VERSION).tar.bz2
> -
> +qemu-backup.8: contrib/qemu-backup/qemu-backup.texi
>  qemu-%.tar.bz2:
>       $(SRC_PATH)/scripts/make-release "$(SRC_PATH)" "$(patsubst 
> qemu-%.tar.bz2,%,$@)"
>  
> diff --git a/contrib/backup/qemu-backup.texi b/contrib/backup/qemu-backup.texi
> new file mode 100644
> index 0000000..1db63c0
> --- /dev/null
> +++ b/contrib/backup/qemu-backup.texi
> @@ -0,0 +1,139 @@
> address@hidden qemu-backup-tool-manpage

I think just "qemu-backup" is fine, judging by other uses of
@setfilename.

> +
> address@hidden en
> address@hidden UTF-8
> +
> +\input texinfo
> address@hidden Manpage_backup_tool 1.0

All other uses of @settitle have a humand readable title like "QEMU
Guest Agent".  Please use "QEMU Backup Tool" or something similar.

> address@hidden
> +This is a manpage for qemu_backup_tool, version 1.0.

Please drop this line.  Looking at man pages for ls(1), wget(1), etc
they tend to give the copyright and documentation license terms.

Please don't hardcode a version number for now.

> +
> +Copyright @copyright{} 2016 Free Software Foundation, Inc.

I suggest:

Copyright @copyright{} 2017 The QEMU Project developers

> address@hidden copying
> address@hidden
> address@hidden
> +* QEMU: (qemu-backup-tool-manpage).    Man page for QEMU backup tool.
> address@hidden direntry
> address@hidden ifinfo
> address@hidden
> address@hidden
> address@hidden 7
> address@hidden @titlefont{QEMU_backup_tool}
> address@hidden 1
> address@hidden @titlefont{Man Page}
> address@hidden 3
> address@hidden titlepage
> address@hidden iftex
> address@hidden
> address@hidden Top
> address@hidden Short Sample
> +
> address@hidden
> +* Name::
> +* Synopsis::
> +* list of Commands::
> +* Command Parameters::
> +* Command Descriptions::
> +* License::
> address@hidden menu
> +
> address@hidden ifnottex
> +
> address@hidden Name
> address@hidden Name
> +
> +QEMU disk backup tool.
> +
> address@hidden Synopsis
> address@hidden Synopsis
> +
> +qemu-backup command [ command options].
> +
> address@hidden  list of Commands
> address@hidden  list of Commands
> address@hidden chapter, first dummy
> address@hidden
> address@hidden qemu-backup guest add [--id id] [--qmp socketpath]

Both --id and --qmp are required options, please drop the block quotes
('[', ']').  The same applies to most options below, too.

> address@hidden qemu-backup guest list
> address@hidden qemu-backup drive add [--guest guestname] [--id driveid] 
> [--target target]
> address@hidden qemu-backup drive list [--guest guestname]
> address@hidden qemu-backup backup [--guest guestname]
> address@hidden qemu-backup restore [--guest guestname]
> address@hidden qemu-backup drive-remove [--guest guestname] [--id driveid]

It's more consistent to call it "drive remove" instead of
"drive-remove" since its dual is "drive add".

> address@hidden qemu-backup remove [--guest guestname]

"guest add" <-> "guest remove"

> address@hidden qemu-backup drive add [--all] [--guest guestname] [--target 
> target]

Please move this below the other "drive add" definition.

> address@hidden qemu-backup backup [--inc] [--guest guestname]

Please move this below the other "backup" definition.

> address@hidden itemize
> address@hidden  Command Parameters
> address@hidden  Command Parameters
> address@hidden chapter, first dummy
> address@hidden
> address@hidden --guest: Name of the guest.

"id of guest" would make it clearer that --guest ID is the same value as given
by --id ID.  Otherwise the reader might wonder what the difference
between a "name" and an "id" is.

> address@hidden --id: id of guest or drive.
> address@hidden --target: Destination on which you want your backup to be made.

Adding the word "path" may help make it clear that this is a file or
directory name.

> address@hidden --all: Add all the drives present in a guest for backup except 
> cd-rom.

There may be other conditions besides excluding CD-ROM drives (e.g.
other read-only drives).  I suggest simply saying "Automatically add all
drives that are suitable for backups".

> address@hidden License
> address@hidden License
> +
> +QEMU is a trademark of Fabrice Bellard.
> +
> +QEMU is released under the GNU General Public License (TODO: add link).
> +Parts of QEMU have specific licenses, see file LICENSE.
> +
> +TODO (refer to file LICENSE, include it, include the GPL?)

Most documentation in docs/ is licensed under the GPL.  I recommend
following that.

signature.asc
Description: PGP signature