Re: [Qemu-devel] [PATCH v4 1/3] Add manpage for QEMU Backup Tool

[John Snow]

On 09/08/2017 12:41 PM, Ishani Chugh wrote:
> 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. This commit is an
> initial implementation of manpage listing the commands which the
> backup tool will support. The manpage will be built along with other
> docs when configure is provided with --enable-docs flag in the
> location contrib/backup in build directory.
> 
> Signed-off-by: Ishani Chugh <address@hidden>
> ---
>  Makefile                        |  14 ++--
>  contrib/backup/qemu-backup.texi | 142 
> ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 152 insertions(+), 4 deletions(-)
>  create mode 100644 contrib/backup/qemu-backup.texi
> 
> diff --git a/Makefile b/Makefile
> index 337a1f6..794cac5 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -209,6 +209,7 @@ ifdef BUILD_DOCS
>  DOCS=qemu-doc.html qemu-doc.txt qemu.1 qemu-img.1 qemu-nbd.8 qemu-ga.8
>  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
> +DOCS+=contrib/backup/qemu-backup.html contrib/backup/qemu-backup.txt
>  ifdef CONFIG_VIRTFS
>  DOCS+=fsdev/virtfs-proxy-helper.1
>  endif
> @@ -517,6 +518,8 @@ VERSION ?= $(shell cat VERSION)
> 
>  dist: qemu-$(VERSION).tar.bz2
> 
> +qemu-backup.8: contrib/backup/qemu-backup.texi
> +
>  qemu-%.tar.bz2:
>       $(SRC_PATH)/scripts/make-release "$(SRC_PATH)" "$(patsubst 
> qemu-%.tar.bz2,%,$@)"
> 
> @@ -728,16 +731,19 @@ fsdev/virtfs-proxy-helper.1: 
> fsdev/virtfs-proxy-helper.texi
>  qemu-nbd.8: qemu-nbd.texi qemu-option-trace.texi
>  qemu-ga.8: qemu-ga.texi
> 
> -html: qemu-doc.html docs/interop/qemu-qmp-ref.html 
> docs/interop/qemu-ga-ref.html
> -info: qemu-doc.info docs/interop/qemu-qmp-ref.info 
> docs/interop/qemu-ga-ref.info
> -pdf: qemu-doc.pdf docs/interop/qemu-qmp-ref.pdf docs/interop/qemu-ga-ref.pdf
> -txt: qemu-doc.txt docs/interop/qemu-qmp-ref.txt docs/interop/qemu-ga-ref.txt
> +html: qemu-doc.html docs/interop/qemu-qmp-ref.html 
> docs/interop/qemu-ga-ref.html contrib/backup/qemu-backup.html
> +info: qemu-doc.info docs/interop/qemu-qmp-ref.info 
> docs/interop/qemu-ga-ref.info contrib/backup/qemu-backup.info
> +pdf: qemu-doc.pdf docs/interop/qemu-qmp-ref.pdf docs/interop/qemu-ga-ref.pdf 
> contrib/backup/qemu-backup.pdf
> +txt: qemu-doc.txt docs/interop/qemu-qmp-ref.txt docs/interop/qemu-ga-ref.txt 
> contrib/backup/qemu-backup.txt
> 
>  qemu-doc.html qemu-doc.info qemu-doc.pdf qemu-doc.txt: \
>       qemu-img.texi qemu-nbd.texi qemu-options.texi qemu-option-trace.texi \
>       qemu-monitor.texi qemu-img-cmds.texi qemu-ga.texi \
>       qemu-monitor-info.texi
> 
> +contrib/backup/qemu-backup.html contrib/backup/qemu-backup.pdf 
> contrib/backup/qemu-backup.txt contrib/backup/qemu-backup.info: \
> +     contrib/backup/qemu-backup.texi
> +
>  docs/interop/qemu-ga-ref.dvi docs/interop/qemu-ga-ref.html \
>      docs/interop/qemu-ga-ref.info docs/interop/qemu-ga-ref.pdf \
>      docs/interop/qemu-ga-ref.txt docs/interop/qemu-ga-ref.7: \
> diff --git a/contrib/backup/qemu-backup.texi b/contrib/backup/qemu-backup.texi
> new file mode 100644
> index 0000000..7ad266c
> --- /dev/null
> +++ b/contrib/backup/qemu-backup.texi
> @@ -0,0 +1,142 @@
> +\input texinfo
> address@hidden qemu-backup
> +
> address@hidden en
> address@hidden UTF-8
> +
> address@hidden QEMU Backup Tool
> address@hidden
> +
> +Copyright @copyright{} 2017 The QEMU Project developers
> address@hidden copying
> address@hidden
> address@hidden
> +* QEMU: (QEMU-backup).    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 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
> address@hidden qemu-backup guest add --guest guestname --qmp socketpath
> address@hidden qemu-backup guest list
> address@hidden qemu-backup drive add --id driveid --guest guestname --target 
> target
> address@hidden qemu-backup drive add --all --guest guestname --target target
> address@hidden qemu-backup drive list --guest guestname
> address@hidden qemu-backup restore --guest guestname
> address@hidden qemu-backup guest remove --guest guestname
> address@hidden qemu-backup drive remove --guest guestname --id driveid
> address@hidden itemize
> address@hidden  Command Parameters
> address@hidden  Command Parameters
> address@hidden
> address@hidden --all: Add all the drives present in a guest which are 
> suitable for backup.
> address@hidden --guest: Name of the guest.
> address@hidden --id: id of guest or drive.
> address@hidden --qmp: Path of qmp socket.
> address@hidden --target: Destination path on which you want your backup to be 
> made.
> address@hidden itemize
> +
> address@hidden  Command Descriptions
> address@hidden  Command Descriptions
> address@hidden
> address@hidden qemu-backup guest add --guest guestname --qmp socketpath
> +This command adds a guest to the configuration file given its path to qmp 
> socket.

In the generated output (contrib/backup/qemu-backup.html) this doesn't
actually create a newline, and the resultant text looks like this:

qemu-backup guest add –guest guestname –qmp socketpath This command adds
a guest to the configuration file given its path to qmp socket.

Looking at some other texi docs, @item appears by itself on a newline:

(from qemu-doc.texi:)

@item
QEMU can optionally use an in-kernel accelerator, like kvm. The
accelerators
execute most of the guest code natively, while
continuing to emulate the rest of the machine.

If your intention was to create a heading and a paragraph, you may need
additional markup to accomplish this. The resulting documentation is a
little difficult to read otherwise.

> +
> +example:

Perhaps "examples" as there are two that follow.

> +qemu-backup guest add --id=fedora --qmp=unix:/var/run/qemu/fedora.sock
> +
> +qemu-backup guest add --id=fedora --qmp=tcp:localhost:4444
> +
> address@hidden qemu-backup guest list
> +This commands lists the names of guests which are added to configuration 
> file.

"to the configuration file"

> +
> address@hidden qemu-backup drive add --guest guestname --id driveid --target 
> target
> +This command adds different drives for backup in a particular guest by 
> giving the name of drive to be backed up and target imagefile in which we 
> want to store the drive backup.

"of the drive",
"and a target image file where the drive backup is to be stored."

> +
> +example: qemu-backup drive add --guest=fedora --id=root 
> --target=/backup/root.img
> +
> address@hidden qemu-backup drive add --all --guest guestname --destination 
> destination
> +This command adds all the drives of the guest for backup other than CDROM 
> drive and read-only drives. Here all the backup drives will have the same 
> names as original drives and target will be the destination folder.
> +
> +example: qemu-backup drive add --all --guest fedora --destination 
> =/backup/fedora/
> +

extra space between destination and =/backup/fedora

> address@hidden qemu-backup drive list --guest guestname
> +This commands gives the names of the drive present in a guest which are 
> added for backup.
> +

"of the drives", and perhaps "which have been configured for backup"

> +example: qemu-backup drive list --guest=fedora
> +
> address@hidden qemu-backup backup --guest guestname
> +
> +This command makes the backup of the drives, in their respective given 
> destinations. The ids of drive and their destinations are taken from the 
> configuration file.
> +
> +example: qemu-backup backup --guest=fedora
> +
> address@hidden qemu-backup restore --guest guestname
> +This command is needed if we want to restore the backup. It will list the 
> commands to be run for performing the same but will not perform any action.
> +

Might be worthwhile to explain why it won't automatically perform these
actions for you, i.e. any one of these reasons:

- To allow you to safely shut down your VM to preserve data integrity
for drives that are (perhaps) not being restored
- To allow the administrator a chance to move backups that were moved
off-site back onto the server
- To allow the administrator a chance to move backups in a more
efficient way if possible/desired (a python utility may not always have
the best information for how to do an efficient restoration, for
instance...)

> +example: qemu-backup restore --guest=fedora
> +
> address@hidden qemu-backup guest remove --guest guestname
> +This command removes the guest from the configuration file.
> +
> +example: qemu-backup guest remove --guest=fedora
> +
> address@hidden qemu-backup drive remove --guest guestname --id driveid
> +This command helps remove a drive which is set for backup in configuration 
> of given host.
> +

"helps remove" should just be "removes"

> +example: drive remove --guest=fedora --id=root
> +
> address@hidden A full backup can be performed by following the given steps:
> +
> +Perform a full backup of 'vm001', which has one drive:
> +
> +qemu-backup guest add --guest vm001 --qmp /path/to/vm001.sock
> +
> +qemu-backup add --id drive0 --guest vm001 --target /backups/vm001-drive0.img
> +
> +qemu-backup backup --guest vm001
> +
> +
> address@hidden itemize
> +
> address@hidden License
> address@hidden License
> +QEMU is a trademark of Fabrice Bellard.
> +QEMU is released under the
> address@hidden://www.gnu.org/licenses/gpl-2.0.txt,GNU General Public License},
> +version 2. Parts of QEMU have specific licenses, see file
> address@hidden://git.qemu.org/?p=qemu.git;a=blob_plain;f=LICENSE,LICENSE}.
> address@hidden
> --
> 2.7.4
> 

Looks good so far, thank you for your work this summer :)

Since Stefan gave his Reviewed-by for the whole series, I'll leave it up
to you as to whether or not you want to continue sending revisions. If
you'd like to move on and work on something else, I can always send some
fixup patches myself.

Let me know!

In the event that you'd like to press on:

Reviewed-by: John Snow <address@hidden>