[libvirt] [PATCH 0/4] generate SYNTAX section of "virsh help CMD" output

This started when I noticed that a lot of virsh help output was out of date. Dan Berrange suggested to generate the SYNTAX line automatically based on existing option descriptions, since they tell us what arguments/types each command accepts and can (usually) tell us when options or arguments are optional. Doing this exposed several inaccurate SYNTAX lines, as well as some inaccurate option descriptions. But since the option description structs are currently used only to generate the text of ./virsh help's OPTIONS section, it's not terribly important. Other than changes to help output, this change induces some minor user-visible changes: It is currently undocumented that virsh accepts "-h C1 C2 C3..." and treats it like "virsh help C1", ignoring C2 C3... This change disables that feature (to clean up a few interfaces and to make automatic help SYNTAX generation simpler) and makes virsh diagnose any unused arguments with --help (-h), so now, this command fails rather than trying to interpret "A" as a command: $ virsh -h -c test:///default A virsh: error: extra argument 'A'. See --help. To view the other changes induced by this patch, I've run the following commands: v='virsh -c test:///default' cmd=$(eval $v help|sed -n 's/^ \([^ ][^ ]*\) .*/\1/p') for i in $(echo $cmd); do diff -ubBw -L $i.orig -L $i.new <(eval $v help $i) <(eval ./$v help $i) done They use diff to compare e.g., the output of my just-built ./virsh and the installed-in-PATH "virsh" program. Note that there is currently no way via options to indicate that one of two or more options must be selected, so the automatically generated syntax string list both as independent options. Here are the induced help changes: [code diffs coming in separate messages] --- attach-disk.orig +++ attach-disk.new @@ -2,7 +2,7 @@ attach-disk - attach disk device SYNOPSIS - attach-disk <domain> <source> <target> [--driver <driver>] [--subdriver <subdriver>] [--type <type>] [--mode <mode>] + attach-disk <domain> <source> <target> [--driver <string>] [--subdriver <string>] [--type <string>] [--mode <string>] DESCRIPTION Attach new disk device. @@ -11,9 +11,9 @@ <domain> domain name, id or uuid <source> source of disk device <target> target of disk device - <driver> driver of disk device - <subdriver> subdriver of disk device - <type> target device type - <mode> mode of device reading and writing + --driver <string> driver of disk device + --subdriver <string> subdriver of disk device + --type <string> target device type + --mode <string> mode of device reading and writing --- attach-interface.orig +++ attach-interface.new @@ -2,7 +2,7 @@ attach-interface - attach network interface SYNOPSIS - attach-interface <domain> <type> <source> [--target <target>] [--mac <mac>] [--script <script>] + attach-interface <domain> <type> <source> [<target>] [<mac>] [<script>] DESCRIPTION Attach new network interface. --- autostart.orig +++ autostart.new @@ -2,7 +2,7 @@ autostart - autostart a domain SYNOPSIS - autostart [--disable] <domain> + autostart <domain> [--disable] DESCRIPTION Configure a domain to be automatically started at boot. --- connect.orig +++ connect.new @@ -2,7 +2,7 @@ connect - (re)connect to hypervisor SYNOPSIS - connect [name] [--readonly] + connect [<name>] [--readonly] DESCRIPTION Connect to local hypervisor. This is built-in command after shell start up. --- create.orig +++ create.new @@ -2,7 +2,7 @@ create - create a domain from an XML file SYNOPSIS - create a domain from an XML <file> + create <file> DESCRIPTION Create a domain. --- start.orig +++ start.new @@ -8,6 +8,6 @@ Start a domain. OPTIONS - <name> name of the inactive domain + <domain> name of the inactive domain --- detach-interface.orig +++ detach-interface.new @@ -2,7 +2,7 @@ detach-interface - detach network interface SYNOPSIS - detach-interface <domain> <type> [--mac <mac>] + detach-interface <domain> <type> [--mac <string>] DESCRIPTION Detach network interface. @@ -10,6 +10,6 @@ OPTIONS <domain> domain name, id or uuid <type> network interface type - <mac> MAC address + --mac <string> MAC address --- define.orig +++ define.new @@ -2,7 +2,7 @@ define - define (but don't start) a domain from an XML file SYNOPSIS - define a domain from an XML <file> + define <file> DESCRIPTION Define a domain. --- domblkstat.orig +++ domblkstat.new @@ -2,7 +2,7 @@ domblkstat - get device block stats for a domain SYNOPSIS - domblkstat <domain> <dev> + domblkstat <domain> <device> DESCRIPTION Get device block stats for a running domain. --- domifstat.orig +++ domifstat.new @@ -2,7 +2,7 @@ domifstat - get network interface stats for a domain SYNOPSIS - domifstat <domain> <dev> + domifstat <domain> <interface> DESCRIPTION Get network interface stats for a running domain. --- find-storage-pool-sources.orig +++ find-storage-pool-sources.new @@ -2,7 +2,7 @@ find-storage-pool-sources - discover potential storage pool sources SYNOPSIS - find-storage-pool-sources <type> [srcSpec] + find-storage-pool-sources <type> [<srcSpec>] DESCRIPTION Returns XML <sources> document. --- find-storage-pool-sources-as.orig +++ find-storage-pool-sources-as.new @@ -2,7 +2,7 @@ find-storage-pool-sources-as - find potential storage pool sources SYNOPSIS - find-storage-pool-sources-as <type> [options] + find-storage-pool-sources-as <type> [<host>] [<port>] DESCRIPTION Returns XML <sources> document. --- list.orig +++ list.new @@ -2,7 +2,7 @@ list - list domains SYNOPSIS - list [--inactive | --all] + list [--inactive] [--all] DESCRIPTION Returns list of domains. --- migrate.orig +++ migrate.new @@ -2,7 +2,7 @@ migrate - migrate domain to another host SYNOPSIS - migrate [--live] <domain> <desturi> [<migrateuri>] + migrate [--live] <domain> <desturi> [<migrateuri>] [<dname>] DESCRIPTION Migrate domain to another host. Add --live for live migration. --- net-autostart.orig +++ net-autostart.new @@ -2,7 +2,7 @@ net-autostart - autostart a network SYNOPSIS - net-autostart [--disable] <network> + net-autostart <network> [--disable] DESCRIPTION Configure a network to be automatically started at boot. --- net-list.orig +++ net-list.new @@ -2,7 +2,7 @@ net-list - list networks SYNOPSIS - net-list [ --inactive | --all ] + net-list [--inactive] [--all] DESCRIPTION Returns list of networks. --- net-start.orig +++ net-start.new @@ -2,7 +2,7 @@ net-start - start a (previously defined) inactive network SYNOPSIS - net-start <network> + net-start <name> DESCRIPTION Start a network. --- nodedev-list.orig +++ nodedev-list.new @@ -2,7 +2,7 @@ nodedev-list - enumerate devices on this host SYNOPSIS - nodedev-list [--cap <capability>] + nodedev-list [--cap <string>] OPTIONS --cap <string> capability name --- pool-autostart.orig +++ pool-autostart.new @@ -2,7 +2,7 @@ pool-autostart - autostart a pool SYNOPSIS - pool-autostart [--disable] <pool> + pool-autostart <pool> [--disable] DESCRIPTION Configure a pool to be automatically started at boot. --- pool-create-as.orig +++ pool-create-as.new @@ -2,7 +2,7 @@ pool-create-as - create a pool from a set of args SYNOPSIS - pool-create-as <name> <type> + pool-create-as <name> <type> [<source-host>] [<source-path>] [<source-dev>] [<target>] DESCRIPTION Create a pool. --- pool-define-as.orig +++ pool-define-as.new @@ -2,7 +2,7 @@ pool-define-as - define a pool from a set of args SYNOPSIS - pool-define-as <name> <type> + pool-define-as <name> <type> [<source-host>] [<source-path>] [<source-dev>] [<source-name>] [<target>] DESCRIPTION Define a pool. --- pool-edit.orig +++ pool-edit.new @@ -2,7 +2,7 @@ pool-edit - edit XML configuration for a storage pool SYNOPSIS - pool-edit <domain> + pool-edit <pool> DESCRIPTION Edit the XML configuration for a storage pool. --- pool-list.orig +++ pool-list.new @@ -2,7 +2,7 @@ pool-list - list pools SYNOPSIS - pool-list [ --inactive | --all ] + pool-list [--inactive] [--all] DESCRIPTION Returns list of pools. --- pool-start.orig +++ pool-start.new @@ -8,6 +8,6 @@ Start a pool. OPTIONS - <name> name of the inactive pool + <pool> name of the inactive pool --- restore.orig +++ restore.new @@ -2,7 +2,7 @@ restore - restore a domain from a saved state in a file SYNOPSIS - restore a domain from <file> + restore <file> DESCRIPTION Restore a domain. --- schedinfo.orig +++ schedinfo.new @@ -2,7 +2,7 @@ schedinfo - show/set scheduler parameters SYNOPSIS - schedinfo <domain> + schedinfo <domain> [--set <string>] [--weight <number>] [--cap <number>] DESCRIPTION Show/Set scheduler parameters. --- vol-create.orig +++ vol-create.new @@ -2,7 +2,7 @@ vol-create - create a vol from an XML file SYNOPSIS - vol-create <file> + vol-create <pool> <file> DESCRIPTION Create a vol. --- vol-create-as.orig +++ vol-create-as.new @@ -2,7 +2,7 @@ vol-create-as - create a volume from a set of args SYNOPSIS - vol-create-as <pool> <name> <capacity> + vol-create-as <pool> <name> <capacity> [--allocation <string>] [--format <string>] DESCRIPTION Create a vol. @@ -11,7 +11,7 @@ <pool> pool name <name> name of the volume <capacity> size of the vol with optional k,M,G,T suffix - <allocation> initial allocation size with optional k,M,G,T suffix - <format> file format type raw,bochs,qcow,qcow2,vmdk + --allocation <string> initial allocation size with optional k,M,G,T suffix + --format <string> file format type raw,bochs,qcow,qcow2,vmdk --- vol-delete.orig +++ vol-delete.new @@ -2,7 +2,7 @@ vol-delete - delete a vol SYNOPSIS - vol-delete <vol> + vol-delete [--pool <string>] <vol> DESCRIPTION Delete a given vol. --- vol-dumpxml.orig +++ vol-dumpxml.new @@ -2,7 +2,7 @@ vol-dumpxml - vol information in XML SYNOPSIS - vol-dumpxml <vol> + vol-dumpxml [--pool <string>] <vol> DESCRIPTION Output the vol information as an XML dump to stdout. --- vol-info.orig +++ vol-info.new @@ -2,7 +2,7 @@ vol-info - storage vol information SYNOPSIS - vol-info <vol> + vol-info [--pool <string>] <vol> DESCRIPTION Returns basic information about the storage vol. --- vol-path.orig +++ vol-path.new @@ -2,7 +2,7 @@ vol-path - convert a vol UUID to vol path SYNOPSIS - vol-path <pool> <vol> + vol-path [--pool <string>] <vol> OPTIONS --pool <string> pool name or uuid

On Fri, Dec 05, 2008 at 07:24:31PM +0100, Jim Meyering wrote:
This started when I noticed that a lot of virsh help output was out of date. Dan Berrange suggested to generate the SYNTAX line automatically based on existing option descriptions, since they tell us what arguments/types each command accepts and can (usually) tell us when options or arguments are optional.
Doing this exposed several inaccurate SYNTAX lines, as well as some inaccurate option descriptions. But since the option description structs are currently used only to generate the text of ./virsh help's OPTIONS section, it's not terribly important.
Other than changes to help output, this change induces some minor user-visible changes:
It is currently undocumented that virsh accepts "-h C1 C2 C3..." and treats it like "virsh help C1", ignoring C2 C3...
This change disables that feature (to clean up a few interfaces and to make automatic help SYNTAX generation simpler) and makes virsh diagnose any unused arguments with --help (-h), so now, this command fails rather than trying to interpret "A" as a command:
$ virsh -h -c test:///default A virsh: error: extra argument 'A'. See --help.
To view the other changes induced by this patch, I've run the following commands:
v='virsh -c test:///default' cmd=$(eval $v help|sed -n 's/^ \([^ ][^ ]*\) .*/\1/p') for i in $(echo $cmd); do diff -ubBw -L $i.orig -L $i.new <(eval $v help $i) <(eval ./$v help $i) done
They use diff to compare e.g., the output of my just-built ./virsh and the installed-in-PATH "virsh" program.
Note that there is currently no way via options to indicate that one of two or more options must be selected, so the automatically generated syntax string list both as independent options.
Here are the induced help changes: [code diffs coming in separate messages]
Looks fine to me, all those diffs seems to fix errors in synopsis or rename the arguments to be more coherent. I don't think breaking the unspecified -h behaviour is a problem. So +1 from me ! thanks ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

Daniel Veillard <veillard@redhat.com> wrote:
Here are the induced help changes: [code diffs coming in separate messages]
Looks fine to me, all those diffs seems to fix errors in synopsis or rename the arguments to be more coherent. I don't think breaking the unspecified -h behaviour is a problem. So +1 from me !
Thanks for the review. I've committed those changes.

On Fri, Dec 05, 2008 at 07:24:31PM +0100, Jim Meyering wrote:
This started when I noticed that a lot of virsh help output was out of date. Dan Berrange suggested to generate the SYNTAX line automatically based on existing option descriptions, since they tell us what arguments/types each command accepts and can (usually) tell us when options or arguments are optional.
ACK to all these changes since they're a significant improvement on what we have.
--- attach-disk.orig +++ attach-disk.new @@ -2,7 +2,7 @@ attach-disk - attach disk device
SYNOPSIS - attach-disk <domain> <source> <target> [--driver <driver>] [--subdriver <subdriver>] [--type <type>] [--mode <mode>] + attach-disk <domain> <source> <target> [--driver <string>] [--subdriver <string>] [--type <string>] [--mode <string>]
I'm wondering if we ought to go further and make ths SYNOPSIS follow the style used in manpages more explicitly. So leave out explicit listing of optional flags, and get rid of the angle brackets since they look rather wierd then you then combine with square brackets. To quote 'chmod(1)' SYNOPSIS chmod [OPTION]... MODE[,MODE]... FILE... chmod [OPTION]... OCTAL-MODE FILE... chmod [OPTION]... --reference=RFILE FILE... So getting a style like attach-disk [OPTIONS]... DOMAIN SOURCE TARGET
--- connect.orig +++ connect.new @@ -2,7 +2,7 @@ connect - (re)connect to hypervisor
SYNOPSIS - connect [name] [--readonly] + connect [<name>] [--readonly]
connect [OPTIONS]... [NAME]
--- create.orig +++ create.new @@ -2,7 +2,7 @@ create - create a domain from an XML file
SYNOPSIS - create a domain from an XML <file> + create <file>
create FILE Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

"Daniel P. Berrange" <berrange@redhat.com> wrote:
On Fri, Dec 05, 2008 at 07:24:31PM +0100, Jim Meyering wrote:
This started when I noticed that a lot of virsh help output was out of date. Dan Berrange suggested to generate the SYNTAX line automatically based on existing option descriptions, since they tell us what arguments/types each command accepts and can (usually) tell us when options or arguments are optional.
ACK to all these changes since they're a significant improvement on what we have.
--- attach-disk.orig +++ attach-disk.new @@ -2,7 +2,7 @@ attach-disk - attach disk device
SYNOPSIS - attach-disk <domain> <source> <target> [--driver <driver>] [--subdriver <subdriver>] [--type <type>] [--mode <mode>] + attach-disk <domain> <source> <target> [--driver <string>] [--subdriver <string>] [--type <string>] [--mode <string>]
I'm wondering if we ought to go further and make ths SYNOPSIS follow the style used in manpages more explicitly. So leave out explicit listing of optional flags, and get rid of the angle brackets since they look rather wierd then you then combine with square brackets.
To quote 'chmod(1)'
SYNOPSIS chmod [OPTION]... MODE[,MODE]... FILE... chmod [OPTION]... OCTAL-MODE FILE... chmod [OPTION]... --reference=RFILE FILE...
So getting a style like
attach-disk [OPTIONS]... DOMAIN SOURCE TARGET
--- connect.orig +++ connect.new @@ -2,7 +2,7 @@ connect - (re)connect to hypervisor
SYNOPSIS - connect [name] [--readonly] + connect [<name>] [--readonly]
connect [OPTIONS]... [NAME]
--- create.orig +++ create.new @@ -2,7 +2,7 @@ create - create a domain from an XML file
SYNOPSIS - create a domain from an XML <file> + create <file>
create FILE
I agree: that'd be a fine improvement (but I'm biased ;-) I'll do it if no one objects.
participants (3)
-
Daniel P. Berrange
-
Daniel Veillard
-
Jim Meyering