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

[Pavel Hrdina]

On Thu, Oct 29, 2015 at 10:52:00AM -0400, John Ferlan wrote:
> 
> 
> 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 at redhat.com>
> >>> ---
> >>>  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.

It's not that hard to split the patch, so I'll do it.

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

I agree, the man page is a mess and really hard to read and understand.

Pavel