Re: [libvirt] [PATCH V14 5/5] nwfilter: Display detected IP address in domain XML
On 05/25/2012 05:56 AM, Stefan Berger wrote:
Display detected IP addresses in the domain XML using the
IP_LEASE variable name. This variable name now becomes
a reserved variable name that can be read only but not set
by the user.
The format of the value is: <ip addresss>,<lease timeout in seconds>
s/addresss/address/
An example of a displayed XML may then be:
<interface type='bridge'>
<mac address='52:54:00:68:e3:90'/>
<source bridge='virbr0'/>
<target dev='vnet1'/>
<model type='virtio'/>
<filterref filter='clean-traffic'>
<parameter name='CTRL_IP_LEARNING' value='dhcp'/>
<parameter name='IP_LEASE' value='192.168.122.210,100'/>
It's a shame that we are making the user parse the single XML entity to
get two pieces of information. I wonder if it would be better to have:
<parameter name='IP_LEASE_ADDR' value='192.168.122.210'/>
<parameter name='IP_LEASE_TIMEOUT' value='100'/>
and reserve two variable names rather than one.
I'll review the code as is, but I would get a second opinion from one of
the Dan's on whether we should change the XML to make xpath queries
easier (basically, XML that crams 2 pieces of information into one
attributes is always harder to use).
# define NWFILTER_VARNAME_IP "IP"
# define NWFILTER_VARNAME_MAC "MAC"
# define NWFILTER_VARNAME_CTRL_IP_LEARNING "CTRL_IP_LEARNING"
# define NWFILTER_VARNAME_DHCPSERVER "DHCPSERVER"
+# define NWFILTER_VARNAME_IP_LEASE "IP_LEASE"
+# define NWFILTER_VARNAME_IPV6_LEASE "IPV6_LEASE" /* future */
This list may need to change based on the above outcome.
@@ -764,7 +771,6 @@ err_exit:
return -1;
}
-
static bool
Whitespace churn.
+++ libvirt-acl/docs/formatnwfilter.html.in
@@ -446,6 +446,22 @@
</interface>
</pre>
+ <p>
+ Once an IP address has been detected, the domain's interface XML
+ will display the detected IP address and its lease expiration time
+ in seconds. Note that the <code>IP_LEASE</code> variable is
read-only
+ and cannot be set by the user.
+ </p>
+<pre>
+ <interface type='bridge'>
+ <source bridge='virbr0'/>
+ <filterref filter='clean-traffic'>
+ <parameter name='CTRL_IP_LEARNING' value='dhcp'/>
+ <parameter name='IP_LEASE'
value='192.168.122.100,200'/>
+ </filterref>
+ </interface>
Also, if you split into two parameters (addr vs. lease expiration), then
you can omit the expiration instead of doing an output of -1 for the
cases where an expiration is not known.
+</pre>
+
<h3><a name="nwfelemsReservedVars">Reserved
Variables</a></h3>
<p>
The following table lists reserved variables in use by libvirt.
@@ -481,6 +497,17 @@
<td> CTRL_IP_LEARNING </td>
<td> The choice of the IP address detection mode </td>
</tr>
+ <tr>
+ <td> IP_LEASE </td>
+ <td> Read-only variable displaying the detected IP lease in the
+ format IP address,lease expiration time in seconds </td>
Missing a "since 0.9.13" designation.
Weak ack to the code, modulo the missing 'since' tag; but like I said
earlier, I'm leaning towards a v15 that splits things into two variables.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 620 bytes)