Re: [libvirt] [PATCH v2 3/3] virsh.pod: update and improve a attach-interface section

On 10/29/2015 04:42 AM, Pavel Hrdina wrote: > On Tue, Oct 27, 2015 at 06:53:55PM -0400, John Ferlan wrote: >> >> >> On 10/27/2015 11:01 AM, Pavel Hrdina wrote: >>> Rewrite the attach-interface section in man page to be more readable and >>> add the new hostdev functionality. >>> >>> Signed-off-by: Pavel Hrdina <phrdina(a)redhat.com&gt; >>> --- >>> tools/virsh.pod | 82 +++++++++++++++++++++++++++++++++++---------------------- >>> 1 file changed, 50 insertions(+), 32 deletions(-) >>> >> >> Why a separate patch? It's related to the previous one and if anyone >> ever (ahem) backed ported the other one, they could miss this one... No >> strong feeling either way - just curious. > > It's a rewrite of the attach-interface part of the man page, so I thought, that > would be better to do it in a separate patch. Maybe I can at first do the > rewrite without adding anything about the new feature and than have the update > of man page together with previous patch. > Sure that works too - although I would think mostly unnecessary, but I know that's the norm to separate rewrite from feature/bug fix.

>> >>> diff --git a/tools/virsh.pod b/tools/virsh.pod >>> index 0212e7a..843c293 100644 >>> --- a/tools/virsh.pod >>> +++ b/tools/virsh.pod >>> @@ -2507,51 +2507,69 @@ Likewise, I<--shareable> is an alias for I<--mode shareable>. >>> [[[I<--live>] [I<--config>] | [I<--current>]] | [I<--persistent>]] >>> [I<--target target>] [I<--mac mac>] [I<--script script>] [I<--model model>] >>> [I<--inbound average,peak,burst,floor>] [I<--outbound average,peak,burst>] >>> -[I<--print-xml>] >>> - >>> -Attach a new network interface to the domain. I<type> can be >>> -I<network> to indicate connection via a libvirt virtual network, or >>> -I<bridge> to indicate connection via a bridge device on the host, or >>> -I<direct> to indicate connection directly to one of the host's network >>> -interfaces or bridges. I<source> indicates the source of the >>> -connection (the name of a network, or of a bridge device, or the >>> -host's network interfaces or bridges). I<target> is used to specify >>> -the tap/macvtap device to be used to connect the domain to the >>> -source. Names starting with 'vnet' are considered as auto-generated >>> -and are blanked out/regenerated each time the interface is attached. >>> -I<mac> specifies the MAC address of the network interface; if a MAC >>> +[I<--managed>] [I<--print-xml>] >>> + >>> +Attach a new network interface to the domain. >>> + >>> +B<--type> can be one of the: I<network> to indicate connection via a libvirt >>> +virtual network, I<bridge> to indicate connection via a bridge device >>> +on the host, I<direct> to indicate connection directly to one of the host's >>> +network interfaces or bridges, I<hostdev> to indicate connection using a >>> +passthrough of PCI device on the host. >> >> Using a ':' I think is unnecessary unless you somehow generate a real >> list such as via "=item * xxxx" entries. In that case you'd have the >> following: >> > > I've considered this format before writing the patch and it used a lot of space. > I agree, that it would be better arranged. I'll update it. > I contend virsh.pod is a conglomeration of writing and formatting styles. I like your use of B<> instead of I<>, but that is "different". I think separating switches and better/longer descriptions are better, but that's not always the case. The whole file could use some amount of adjustment for consistency in format/style.