On 07/18/2011 01:38 AM, Jiri Denemark wrote:
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?

That works for me as well, so I'll go ahead and make that change.

...
> -=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.


vcpucount is the oddball; our current semantics are either no flags, or exactly two flags (one from the set of maximum/current, and one from the set config/live). See more here:
https://www.redhat.com/archives/libvir-list/2011-July/msg01030.html

For now, I'm representing that as:
 [{I<--maximum> | I<--current>} {I<--config> | I<--live>}]
but I have done some additional thinking on whether it might be possible to introduce a new flag --active, as well as some backwards compatibility modes where the existing valid use cases are still honored but where the new preferred way would treat --current more like other APIs - more thoughts in reply to that mail, since it can come as a later patch.

-=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.

Indeed, there's quite a few places where we could spell things out more consistently, but I agree with saving them for a separate patch.


ACK with those nits fixed.

Here's what I squashed in before pushing.

diff --git i/tools/virsh.pod w/tools/virsh.pod
index 7f035ee..5afa1f2 100644
--- i/tools/virsh.pod
+++ w/tools/virsh.pod
@@ -528,7 +528,7 @@ type attribute for the <domain> element of XML.
 =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>]
+[I<dname>] [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> @@ -545,8 +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 number> forces guest to suspend when live migration exceeds
-I<number> seconds, and
+I<--timeout> B<seconds> forces guest to suspend when live migration exceeds
+that many seconds, and
then the migration will complete offline. It can only be used with I<--live>.

B<Note>: The I<desturi> parameter for normal migration and peer2peer migration @@ -609,8 +609,8 @@ between the creation and restore point. For a more complete system
 restore point, where the disk state is saved alongside the memory
 state, see the B<snapshot> family of commands.

-=item B<schedinfo> [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>]]

 =item B<schedinfo> [I<--weight> B<number>] [I<--cap> B<number>]
 I<domain-id>
@@ -644,8 +644,8 @@ of screen. In case of multiple graphics cards, heads are enumerated before
 devices, e.g. having two graphics cards, both with four heads, screen ID 5
 addresses the second head on the second card.

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

 Change the memory allocation for a guest domain.
 If I<--live> is specified, perform a memory balloon of a running guest.
@@ -663,8 +663,8 @@ rounds the parameter up unless the kB argument is evenly divisible by 1024 For Xen, you can only adjust the memory of a running domain if the domain is
 paravirtualized or running the PV balloon driver.

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

 Change the maximum memory allocation limit for a guest domain.
 If I<--live> is specified, affect a running guest.
@@ -685,7 +685,7 @@ of 4MB, while 266240 (260MB) is valid without rounding.

 =item B<memtune> I<domain-id> [I<--hard-limit> B<kilobytes>]
 [I<--soft-limit> B<kilobytes>] [I<--swap-hard-limit> B<kilobytes>]
-[I<--min-guarantee> B<kilobytes>]
+[I<--min-guarantee> B<kilobytes>] [[I<--config>] [I<--live>] | [I<--current>]]

 Allows you to display or set the domain memory parameters. Without
 flags, the current settings are displayed; with a flag, the
@@ -729,7 +729,8 @@ value are kilobytes (i.e. blocks of 1024 bytes).

 =back

-=item B<blkiotune> I<domain-id> [I<--weight> B<weight>]
+=item B<blkiotune> I<domain-id> [I<--weight> B<weight>] [[I<--config>]
+[I<--live] | [I<--current>]]

 Display or set the blkio parameters. QEMU/KVM supports I<--weight>.
 I<--weight> is in range [100, 1000].
@@ -741,8 +742,8 @@ Both I<--live> and I<--current> flags may be given, but I<--current> is
 exclusive. If no flag is specified, behavior is different depending
 on hypervisor.

-=item B<setvcpus> I<domain-id> I<count> [I<--maximum>] {[I<--config>]
-[I<--live>] | I<--current}
+=item B<setvcpus> I<domain-id> I<count> [I<--maximum>] [[I<--config>]
+[I<--live>] | [I<--current]]

 Change the number of virtual CPUs active in a guest domain.  By default,
 this command works on active guest domains.  To change the settings for an
@@ -814,8 +815,8 @@ is not available the processes will provide an exit code of 1.
 Undefine the configuration for an inactive domain. Since it's not running
 the domain name or UUID must be used as the I<domain-id>.

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

 Print information about the virtual cpu counts of the given
 I<domain-id>.  If no flags are specified, all possible counts are
@@ -834,8 +835,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> [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>]]

 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


--
Eric Blake   ebl...@redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Reply via email to