[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] docs: improve virsh man page synopses



On Thu, Jul 14, 2011 at 11:38:54 -0600, Eric Blake wrote:
> "optional" is not a very good meta-syntactic construct in our man
> page.  I scrubbed this, and additionally improved some documentation
> on mutually exclusive options.  For example,
> 
> {[--live] [--config] | --current}
> 
> implies that the set must be satisfied ({}), and within the set, you
> either have a mandatory --current, or an optional combination of 0,
> 1, or both --live and --config.

Hmm, why not [[--live] [--config] | --current] to make it more obvious that
none of the options is in fact required?

Otherwise I like it.

> * tools/virsh.pod: Use "[name]" rather than "optional name" for
> optional arguments.
> ---
> 
> I finally did something to address a pet peeve of mine.
> 
>  tools/virsh.pod |  199 ++++++++++++++++++++++++++++++-------------------------
>  1 files changed, 108 insertions(+), 91 deletions(-)
> 
> diff --git a/tools/virsh.pod b/tools/virsh.pod
> index 1a98ec1..c6549f1 100644
> --- a/tools/virsh.pod
> +++ b/tools/virsh.pod
...
> +=item B<migrate> [I<--live>] [I<--direct>] [I<--p2p> [I<--tunnelled>]]
> +[I<--persistent>] [I<--undefinesource>] [I<--suspend>] [I<--copy-storage-all>]
> +[I<--copy-storage-inc>] [I<--verbose>] I<domain-id> I<desturi> [I<migrateuri>]
> +[I<dname>] [I<timeout>]

Shouldn't [I<timeout>] be specified as [I<--timeout> B<seconds>]?

> 
>  Migrate domain to another host.  Add I<--live> for live migration; I<--p2p>
>  for peer-2-peer migration; I<--direct> for direct migration; or I<--tunnelled>
> @@ -544,7 +545,8 @@ I<migrateuri> is the migration URI, which usually can be omitted.
>  I<dname> is used for renaming the domain to new name during migration, which
>  also usually can be omitted.
> 
> -I<--timeout> forces guest to suspend when live migration exceeds timeout, and
> +I<--timeout number> forces guest to suspend when live migration exceeds
> +I<number> seconds, and
>  then the migration will complete offline. It can only be used with I<--live>.
...
> -=item B<schedinfo> optional I<--set> B<parameter=value> I<domain-id> I<--config>
> -I<--live> I<--current>
> +=item B<schedinfo> [I<--set> B<parameter=value>] I<domain-id> {[I<--config>]
> +[I<--live>] | I<--current>}

Make it more obvious that neither or --config, --live, --current need to be
specified as mentioned above.

...
> -=item B<setmem> I<domain-id> B<kilobytes> optional I<--config> I<--live>
> -I<--current>
> +=item B<setmem> I<domain-id> B<kilobytes> {[I<--config>] [I<--live>] |
> +I<--current>}

The same here.

...
> -=item B<setmaxmem> I<domain-id> B<kilobytes> optional I<--config> I<--live>
> -I<--current>
> +=item B<setmaxmem> I<domain-id> B<kilobytes> {[I<--config>] [I<--live>] |
> +I<--current>}

And here.

...
> -=item B<vcpucount> I<domain-id>  optional I<--maximum> I<--current>
> -I<--config> I<--live>
> +=item B<vcpucount> I<domain-id>  [I<--maximum>] {I<--current> |
> +[I<--config>] [I<--live>]}

And here.

>  Print information about the virtual cpu counts of the given
>  I<domain-id>.  If no flags are specified, all possible counts are
> @@ -830,8 +832,8 @@ values; these two flags cannot both be specified.
>  Returns basic information about the domain virtual CPUs, like the number of
>  vCPUs, the running time, the affinity to physical processors.
> 
> -=item B<vcpupin> I<domain-id> optional I<vcpu> I<cpulist> I<--live> I<--config>
> -I<--current>
> +=item B<vcpupin> I<domain-id> [I<vcpu>] [I<cpulist>] {[I<--live>]
> +[I<--config>] | I<--current>}

Ditto.

...
>  Query or change the pinning of domain VCPUs to host physical CPUs.  To
>  pin a single I<vcpu>, specify I<cpulist>; otherwise, you can query one
> @@ -876,9 +878,9 @@ See the documentation to learn about libvirt XML format for a device.
>  For cdrom and floppy devices, this command only replaces the media within
>  the single existing device; consider using B<update-device> for this usage.
> 
> -=item B<attach-disk> I<domain-id> I<source> I<target> optional
> -I<--driver driver> I<--subdriver subdriver> I<--type type>
> -I<--mode mode> I<--persistent> I<--sourcetype soucetype>
> +=item B<attach-disk> I<domain-id> I<source> I<target>
> +[I<--driver driver>] [I<--subdriver subdriver>] [I<--type type>]
> +[I<--mode mode>] [I<--persistent>] [I<--sourcetype soucetype>]

Probably [I<--driver> B<driver>] instead of [I<--driver driver>] to be more
consistent with with other places, but this would probably better fit in a
follow-up patch.

ACK with those nits fixed.

Jirka


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]