[libvirt] [PATCH] Convert virNetwork to use virSocketAddr everywhere

[Eric Blake]

On 10/21/2010 12:21 PM, Daniel P. Berrange wrote:
> Instead of storing the IP address string in virNetwork related
> structs, store the parsed virSocketAddr. This will make it
> easier to add IPv6 support in the future, by letting driver
> code directly check what address family is present
>
> * src/conf/network_conf.c, src/conf/network_conf.h,
>    src/network/bridge_driver.c: Convert to use virSocketAddr
>    in virNetwork, instead of char *.
> * src/util/bridge.c, src/util/bridge.h,
>    src/util/dnsmasq.c, src/util/dnsmasq.h,
>    src/util/iptables.c, src/util/iptables.h: Convert to
>    take a virSocketAddr instead of char * for any IP
>    address parameters
> * src/util/network.h: Add macros to determine if an address
>    is set, and what address family is set.
> ---
>   src/conf/network_conf.c     |  121 +++++++++++++----------
>   src/conf/network_conf.h     |   16 ++--
>   src/network/bridge_driver.c |  162 +++++++++++++++++-------------
>   src/util/bridge.c           |   14 +--
>   src/util/bridge.h           |    5 +-
>   src/util/dnsmasq.c          |   17 ++-
>   src/util/dnsmasq.h          |    4 +-
>   src/util/iptables.c         |  230 +++++++++++++++++++++++++++----------------
>   src/util/iptables.h         |   18 ++--
>   src/util/network.h          |    6 +
>   10 files changed, 353 insertions(+), 240 deletions(-)

Big change, but mostly good.

> @@ -1112,12 +1134,10 @@ static int networkCheckRouteCollision(virNetworkObjPtr network)
>           addr_val&= mask_val;
>
>           if ((net_dest == addr_val)&&
> -            (innetmask.data.inet4.sin_addr.s_addr == mask_val)) {
> +            (network->def->netmask.data.inet4.sin_addr.s_addr == mask_val)) {
>               networkReportError(VIR_ERR_INTERNAL_ERROR,
> -                              _("Network %s/%s is already in use by "
> -                                "interface %s"),
> -                                network->def->ipAddress,
> -                                network->def->netmask, iface);
> +                               _("Network is already in use by interface %s"),
> +                               iface);

This one loses some information in the error message; is that okay?

> +++ b/src/util/dnsmasq.c
> @@ -76,23 +76,28 @@ hostsfileFree(dnsmasqHostsfile *hostsfile)
>   static int
>   hostsfileAdd(dnsmasqHostsfile *hostsfile,
>                const char *mac,
> -             const char *ip,
> +             virSocketAddr *ip,
>                const char *name)
>   {
> +    char *ipstr;
>       if (VIR_REALLOC_N(hostsfile->hosts, hostsfile->nhosts + 1)<  0)
>           goto alloc_error;
>
...
> @@ -100,7 +105,7 @@ hostsfileAdd(dnsmasqHostsfile *hostsfile,
>
>    alloc_error:
>       virReportOOMError();
> -
> +    VIR_FREE(ipstr);

Ouch - freeing uninitialized memory.

>       return -1;
>   }
>
> @@ -279,7 +284,7 @@ dnsmasqContextFree(dnsmasqContext *ctx)
>    * dnsmasqAddDhcpHost:
>    * @ctx: pointer to the dnsmasq context for each network
>    * @mac: pointer to the string contains mac address of the host
> - * @ip: pointer to the string contains ip address of the host
> + * @ip: pointer to the socket address contains ip of the host

s/contains/containing/

>    * @name: pointer to the string contains hostname of the host or NULL

here too

ACK with those items fixed.

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org