Re: [libvirt] [PATCH V14 3/5] nwfilter: move code for IP address map into separate file
On 05/25/2012 05:56 AM, Stefan Berger wrote:
The goal of this patch is to prepare for support for multiple IP
addresses per interface in the DHCP snooping code.
Move the code for the IP address map that maps interface names to
IP addresses into their own file. Rename the functions on the way
but otherwise leave the code as-is. Initialize this new layer
separately before dependent layers (iplearning, dhcpsnooping)
and shut it down after them.
---
src/Makefile.am | 4
src/conf/nwfilter_ipaddrmap.c | 167 +++++++++++++++++++++++++++++++++
src/conf/nwfilter_ipaddrmap.h | 37 +++++++
src/libvirt_private.syms | 8 +
src/nwfilter/nwfilter_driver.c | 11 +-
src/nwfilter/nwfilter_gentech_driver.c | 5
src/nwfilter/nwfilter_learnipaddr.c | 126 ------------------------
src/nwfilter/nwfilter_learnipaddr.h | 3
8 files changed, 229 insertions(+), 132 deletions(-)
@@ -67,10 +68,12 @@ static int
nwfilterDriverStartup(int privileged) {
char *base = NULL;
- if (virNWFilterDHCPSnoopInit() < 0)
+ if (virNWFilterIPAddrMapInit() < 0)
return -1;
if (virNWFilterLearnInit() < 0)
- return -1;
+ goto err_exit_ipaddrmapshutdown;
+ if (virNWFilterDHCPSnoopInit() < 0)
+ goto err_exit_learnshutdown;
You know, if you would make the shutdown functions a no-op if they were
not previously init'ed, then a single conf_init_err label would suffice
for all cleanup, rather than having to have one label per cleanup. But
that's not a showstopper for this patch.
+
+#define VIR_FROM_THIS VIR_FROM_NWFILTER
+
+static virMutex ipAddressMapLock;
+static virNWFilterHashTablePtr ipAddressMap;
Is it really appropriate to have a single map shared across all
hypervisors? I think we will eventually need a followup patch that
creates a virIPAddrMap object, and allocates the object during each call
to the init function, so that multiple hypervisors can manage identical
IP addresses across independent virtualization technologies without
colliding into the same map all by having each hypervisor driver call an
init function for its own map. But that can be a followup, I'm okay
with this patch going in without changing it right now.
ACK.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 620 bytes)