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@xxxxxxxxxx +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list