[libvirt] [PATCH V14 3/5] nwfilter: move code for IP address map into separate file

[Eric Blake]

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 at 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: 620 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120601/f72f9cd3/attachment-0001.sig>