[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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 redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]