On 28/02/13 19:51, Laine Stump wrote:
On 02/27/2013 09:57 PM, TJ wrote:
From: TJ <linux@iam.tj>
Rather than iterate through virNetworkIPDef arrays multiple times use the new virNetworkDef ipv4_dhcp and ipv6_dhcp active stanza pointers.
Signed-off-by: TJ <linux@iam.tj> --- src/network/bridge_driver.c | 63 +++++++++++---------------------------------- 1 file changed, 15 insertions(+), 48 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 0932cf8..8410c93 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -810,24 +810,16 @@ networkDnsmasqConfContents(virNetworkObjPtr network, } }
- /* Find the first dhcp for both IPv4 and IPv6 */ - for (ii = 0, ipv4def = NULL, ipv6def = NULL, ipv6SLAAC = false;
Your refactoring has eliminated the initialization of ipv6SLAAC to false.
- (ipdef = virNetworkDefGetIpByIndex(network->def, AF_UNSPEC, ii)); - ii++) { - if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET)) { - if (ipdef->nranges || ipdef->nhosts) { - if (ipv4def) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("For IPv4, multiple DHCP definitions " - "cannot be specified.")); - goto cleanup; - } else { - ipv4def = ipdef; - } - } - } - if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET6)) { - if (ipdef->nranges || ipdef->nhosts) {
I'm starting to warm to the idea of a convenience pointer, but still dislike the fact that it could get out of sync due to careless programming. How about a convenience *function* instead. It would be just as inefficient, but the code would look cleaner (and the inefficiency isn't a big problem anyway, since it's very rarely done, and takes a miniscule amount of time anyway).
Actually that is where I started, see patch 06 "network: Bridge - Add support for DHCP relay agent" and static virNetworkGetActiveDhcp(). Later I wanted to eliminate the iterations over the <ip> structures that I had moved into this function since it was called from several places, and ended up replacing it with code in the XML parser that sets the ->ipv{4,6}_dhcp pointers once.