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

Re: [libvirt] [PATCH 10/10] docs: Describe the <dhcp> 'enable' and 'relay' attributes

On 02/27/2013 07:57 PM, TJ wrote:
> From: TJ <linux iam tj>
> Signed-off-by: TJ <linux iam tj>
> ---
>  docs/formatnetwork.html.in | 22 +++++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> diff --git a/docs/formatnetwork.html.in b/docs/formatnetwork.html.in
> index 41a83fa..c4c4def 100644
> --- a/docs/formatnetwork.html.in
> +++ b/docs/formatnetwork.html.in

Yay - your series added documentation!  But it didn't add any unit tests
(a new .xml file somewhere under tests/networkxml2argvdata or
networkxml2xml{in,out} to verify that we can round-trip the new XML and
generate the expected command line), and it failed to add the RelaxNG
grammar specification under docs/schemas/network.rng, so there is still
more to be added.

Also, I like to put documentation FIRST in the series, as it then sets
the stage for what the reviewer will be expecting in the rest of the series.

> @@ -650,12 +650,20 @@
>            <dt><code>dhcp</code></dt>
>            <dd>Also within the <code>ip</code> element there is an
>              optional <code>dhcp</code> element. The presence of this element
> -            enables DHCP services on the virtual network. It will further
> +            enables DHCP services on the virtual network. It can further
>              contain one or more <code>range</code> elements. The
>              <code>dhcp</code> element supported for both
>              IPv4 <span class="since">Since 0.3.0</span>
>              and IPv6 <span class="since">Since 1.0.1</span>, but
>              only for one IP address of each type per network.
> +            Since $TODO.$FIXME it can optionally contain a boolean <code>enable</code> attribute

Instead of using $TODO.$FIXME, just use <span class="since">Since
1.0.4</span>.  We can later touch that up as part of merging it in if
you miss the 1.0.4 release window.

> +            ('yes' or 'no') where the value defaults to 'yes', and a boolean
> +            <code>relay</code> attribute where the value defaults to 'no'.
> +            When <code>relay='yes'</code> any settings within the <code>dhcp</code> block
> +            are ignored and a DHCP relay agent is started instead of a local DHCP server.
> +            The DHCP relay daemon will listen on the network's bridge interface for
> +            DHCP/BOOTP traffic and relay it via broadcast from the first interface declared
> +            in the <code>forward</code> element.
>              <dl>
>                <dt><code>range</code></dt>
>                <dd>The <code>start</code> and <code>end</code> attributes on the
> @@ -779,6 +787,18 @@
>          &lt;ip family="ipv6" address="2001:db8:ca2:2::1" prefix="64" /&gt;
>        &lt;/network&gt;</pre>
> +    <p>Here is the same configuration using a DHCP relay agent instead of a local DHCP server.</p>
> +    <pre>
> +      &lt;network&gt;
> +        &lt;name&gt;local&lt;/name&gt;
> +        &lt;bridge name="virbr1" /&gt;
> +        &lt;forward mode="route" dev="eth1"/&gt;
> +        &lt;ip address="" netmask=""&gt;
> +          &lt;dhcp relay='yes'/&gt;
> +        &lt;/ip&gt;
> +        &lt;ip family="ipv6" address="2001:db8:ca2:2::1" prefix="64" /&gt;
> +      &lt;/network&gt;</pre>
> + 
>      <p>
>        Below is another IPv6 varition.  Instead of a dhcp range being

Not your fault, but made obvious by your patch, so I'll fix this typo in
'variation' independently.

Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

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