[libvirt] [PATCHv2] network: add force attribute for dhcp options

[Eric Blake]

On 02/25/2013 11:30 AM, Laine Stump wrote:
> If a dhcp option has "force='yes'", the dhcp server will send that
> option back to every client regardless of whether or not the client
> requests that option (without "force='yes'", an option will only be
> sent to those clients that ask for it in their request packet). For example:
> 
>   <option number='40' value='libvirt' force='yes'/>
> 
> This information is relayed to dnsmasq by using the
> "dhcp-option-force" option, rather than "dhcp-option".
> ---
> Changes from V1:
> * add forgotten addition to RNG
> * add forgotten addition to virNetworkIpDefFormat
> * add networkxml2xml test (which would have caught the above)

I'm sure there are useful symbolic names for the various options,
instead of just number='40'; but like you said, that can be added later
as needed.

It's borderline on whether to take this patch without support for option
names, since it was submitted prior to freeze but not reviewed until
after.  It is cleaning up the feature of <option> parsing that was just
barely applied prior to freeze, so that argues that it can be classed as
rounding out an incomplete feature.  Delaying until post-release would
let you get option names in the same release, but then we might want to
revert the <option> support that just went in.  Meanwhile, since both
this patch, and any future patch for adding option name support, deals
with just XML additions, they can easily be backported by distros
without a soname bump, regardless of whether they make this release or
not; and since it is just XML additions, I don't see how it could cause
regressions to existing domain XML.  I'm probably 60:40 in favor of
including this patch now, but if anyone else speaks clearly for or
against taking it in 1.0.3, I can be swayed.

> +              <dt><code>option</code></dt>
> +              <dd>The optional <code>option</code> element (which can
> +                be repeated for multiple DHCP options) specifies
> +                generic DHCP options (as defined in RFC 2132, or later
> +                RFCs in some cases) that will be sent by the DHCP
> +                server to requesting clients (IPv4 only).  There are
> +                three possible attributes: <code>number</code> is
> +                mandatory and gives the standard option
> +                number; <code>value</code> is optional and gives the
> +                value of that option to be provided to the client
> +                <code>force</code> is also optional, and defaults to
> +                "no"; if <code>force</code> is "yes", the option will
> +                be sent to all DHCP clients regardless of whether or
> +                not the client requests it (usually only options
> +                specifically requested by the client are sent in the
> +                DHCP response).<span class="since">Since 1.0.3</span>

We definitely want the docs for <option>, even if we defer the force=
attribute until post-release.

> @@ -811,12 +812,25 @@ virNetworkDHCPOptionDefParseXML(const char *networkName,
>                         number);
>          goto cleanup;
>      }
> +
>      option->value = virXMLPropString(node, "value");
>  
> +    if ((force = virXMLPropString(node, "force"))) {
> +       if (STRCASEEQ(force, "yes")) {

I don't know that we have done case-insensitive matching in many other
places; but being liberal in what we accept doesn't hurt my feelings.
On the other hand, the RNG rejects any uppercase variant.

ACK.

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 621 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20130225/a0611461/attachment-0001.sig>