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

Re: [libvirt] [PATCH] Support for IPv6 / multiple ifaces in interface_conf.[ch]



On Sun, Oct 04, 2009 at 10:58:55PM -0400, Laine Stump wrote:
> This patch updates the xml parsing and formatting, and the associated
> virInterfaceDef data structure to support IPv6, along the way adding
> support for multiple protocols per interface, and multiple IP
> addresses per protocol.
> 
> Note that netcf appears to not accept defining both ipv4 and ipv6 on
> the same interface, and still can't report live config of IPv6 (or
> multiple IPv4 addresses), so the usefulness of this patch is limited
> until those items are fixed in netcf.
> 
> This patch applies on top of commit
> 1ceabc5a152efa8892954cb4f45b85553edfee9f, which I submitted on Sept 28.
> 
> ---
>  src/conf/interface_conf.c |  308 ++++++++++++++++++++++++++++++++++-----------
>  src/conf/interface_conf.h |   21 +++-
>  2 files changed, 247 insertions(+), 82 deletions(-)
> 
> diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c
> index b422464..24f111c 100644
> --- a/src/conf/interface_conf.c
> +++ b/src/conf/interface_conf.c
> @@ -53,9 +53,32 @@ void virInterfaceBareDefFree(virInterfaceBareDefPtr def) {
>      VIR_FREE(def);
>  }
>  
> +static
> +void virInterfaceIpDefFree(virInterfaceIpDefPtr def) {
> +    if (def == NULL)
> +        return;
> +    VIR_FREE(def->address);
> +    VIR_FREE(def->source);
> +}
> +
> +static
> +void virInterfaceProtocolDefFree(virInterfaceProtocolDefPtr def) {
> +    int ii;
> +
> +    if (def == NULL)
> +        return;
> +    for (ii = 0; ii < def->nips; ii++) {
> +        virInterfaceIpDefFree(def->ips[ii]);
> +    }
> +    VIR_FREE(def->ips);
> +    VIR_FREE(def->family);
> +    VIR_FREE(def->gateway);
> +    VIR_FREE(def);
> +}
> +
>  void virInterfaceDefFree(virInterfaceDefPtr def)
>  {
> -    int i;
> +    int i, pp;
>  
>      if (def == NULL)
>          return;
> @@ -89,11 +112,11 @@ void virInterfaceDefFree(virInterfaceDefPtr def)
>              break;
>      }
>  
> -    VIR_FREE(def->proto.family);
> -    VIR_FREE(def->proto.address);
> -    VIR_FREE(def->proto.gateway);
> -    VIR_FREE(def->proto.source);
> -
> +    /* free all protos */
> +    for (pp = 0; pp < def->nprotos; pp++) {
> +        virInterfaceProtocolDefFree(def->protos[pp]);
> +    }
> +    VIR_FREE(def->protos);
>      VIR_FREE(def);
>  }
>  
> @@ -226,22 +249,22 @@ virInterfaceDefParseBondArpValid(virConnectPtr conn, xmlXPathContextPtr ctxt) {
>  }
>  
>  static int
> -virInterfaceDefParseDhcp(virConnectPtr conn, virInterfaceDefPtr def,
> +virInterfaceDefParseDhcp(virConnectPtr conn, virInterfaceProtocolDefPtr def,
>                           xmlNodePtr dhcp, xmlXPathContextPtr ctxt) {
>      xmlNodePtr save;
>      char *tmp;
>      int ret = 0;
>  
> -    def->proto.dhcp = 1;
> +    def->dhcp = 1;
>      save = ctxt->node;
>      ctxt->node = dhcp;
>      /* Not much to do in the current version */
>      tmp = virXPathString(conn, "string(./@peerdns)", ctxt);
>      if (tmp) {
>          if (STREQ(tmp, "yes"))
> -            def->proto.peerdns = 1;
> +            def->peerdns = 1;
>          else if (STREQ(tmp, "no"))
> -            def->proto.peerdns = 0;
> +            def->peerdns = 0;
>          else {
>              virInterfaceReportError(conn, VIR_ERR_XML_ERROR,
>                                _("unknown dhcp peerdns value %s"), tmp);
> @@ -249,88 +272,209 @@ virInterfaceDefParseDhcp(virConnectPtr conn, virInterfaceDefPtr def,
>          }
>          VIR_FREE(tmp);
>      } else
> -        def->proto.peerdns = -1;
> +        def->peerdns = -1;
>  
>      ctxt->node = save;
>      return(ret);
>  }
>  
>  static int
> -virInterfaceDefParseIp(virConnectPtr conn, virInterfaceDefPtr def,
> -                   xmlNodePtr ip ATTRIBUTE_UNUSED, xmlXPathContextPtr ctxt) {
> +virInterfaceDefParseIp(virConnectPtr conn, virInterfaceIpDefPtr def,
> +                       xmlXPathContextPtr ctxt) {
>      int ret = 0;
>      char *tmp;
>      long l;
>  
> -    tmp = virXPathString(conn, "string(./ip[1]/@address)", ctxt);
> -    def->proto.address = tmp;
> +    tmp = virXPathString(conn, "string(./@address)", ctxt);
> +    def->address = tmp;
>      if (tmp != NULL) {
> -        ret = virXPathLong(conn, "string(./ip[1]/@prefix)", ctxt, &l);
> +        ret = virXPathLong(conn, "string(./@prefix)", ctxt, &l);
>          if (ret == 0)
> -            def->proto.prefix = (int) l;
> +            def->prefix = (int) l;
>          else if (ret == -2) {
>              virInterfaceReportError(conn, VIR_ERR_XML_ERROR,
>                                "%s", _("Invalid ip address prefix value"));
>              return(-1);
>          }
> -        tmp = virXPathString(conn, "string(./ip[1]/@source)", ctxt);
> -        def->proto.source = tmp;
> +        tmp = virXPathString(conn, "string(./@source)", ctxt);
> +        def->source = tmp;
>      }
> -    tmp = virXPathString(conn, "string(./route[1]/@gateway)", ctxt);
> -    def->proto.gateway = tmp;
>  
>      return(0);
>  }
>  
>  static int
> -virInterfaceDefParseProtoIPv4(virConnectPtr conn, virInterfaceDefPtr def,
> +virInterfaceDefParseProtoIPv4(virConnectPtr conn, virInterfaceProtocolDefPtr def,
>                                xmlXPathContextPtr ctxt) {
> -    xmlNodePtr dhcp, ip;
> -    int ret = 0;
> +    xmlNodePtr dhcp;
> +    xmlNodePtr *ipNodes = NULL;
> +    int nIpNodes, ii, ret = 0;
> +    char *tmp;
> +
> +    tmp = virXPathString(conn, "string(./route[1]/@gateway)", ctxt);
> +    def->gateway = tmp;
>  
>      dhcp = virXPathNode(conn, "./dhcp", ctxt);
>      if (dhcp != NULL)
>          ret = virInterfaceDefParseDhcp(conn, def, dhcp, ctxt);
> +    if (ret != 0)
> +        return(ret);
> +
> +    nIpNodes = virXPathNodeSet(conn, "./ip", ctxt, &ipNodes);
> +    if (ipNodes == NULL)
> +        return 0;
> +
> +    if (VIR_ALLOC_N(def->ips, nIpNodes) < 0) {
> +        virReportOOMError(conn);
> +        ret = -1;
> +        goto error;
> +    }
> +
> +    def->nips = 0;
> +    for (ii = 0; ii < nIpNodes; ii++) {
>  
> +        virInterfaceIpDefPtr ip;
> +
> +        if (VIR_ALLOC(ip) < 0) {
> +            virReportOOMError(conn);
> +            break;
> +        }
> +
> +        ctxt->node = ipNodes[ii];
> +        ret = virInterfaceDefParseIp(conn, ip, ctxt);
> +        if (ret != 0) {
> +            virInterfaceIpDefFree(ip);
> +            break;
> +        }
> +        def->ips[def->nips++] = ip;
> +    }
> +
> +error:
> +    VIR_FREE(ipNodes);
> +    return(ret);
> +}
> +
> +static int
> +virInterfaceDefParseProtoIPv6(virConnectPtr conn, virInterfaceProtocolDefPtr def,
> +                              xmlXPathContextPtr ctxt) {
> +    xmlNodePtr dhcp, autoconf;
> +    xmlNodePtr *ipNodes = NULL;
> +    int nIpNodes, ii, ret = 0;
> +    char *tmp;
> +
> +    tmp = virXPathString(conn, "string(./route[1]/@gateway)", ctxt);
> +    def->gateway = tmp;
> +
> +    autoconf = virXPathNode(conn, "./autoconf", ctxt);
> +    if (autoconf != NULL)
> +        def->autoconf = 1;
> +
> +    dhcp = virXPathNode(conn, "./dhcp", ctxt);
> +    if (dhcp != NULL)
> +        ret = virInterfaceDefParseDhcp(conn, def, dhcp, ctxt);
>      if (ret != 0)
>          return(ret);
>  
> -    ip = virXPathNode(conn, "./ip", ctxt);
> -    if (ip != NULL)
> -        ret = virInterfaceDefParseIp(conn, def, ip, ctxt);
> +    nIpNodes = virXPathNodeSet(conn, "./ip", ctxt, &ipNodes);
> +    if (ipNodes == NULL)
> +        return 0;
> +
> +    if (VIR_ALLOC_N(def->ips, nIpNodes) < 0) {
> +        virReportOOMError(conn);
> +        ret = -1;
> +        goto error;
> +    }
> +
> +    def->nips = 0;
> +    for (ii = 0; ii < nIpNodes; ii++) {
> +
> +        virInterfaceIpDefPtr ip;
> +
> +        if (VIR_ALLOC(ip) < 0) {
> +            virReportOOMError(conn);
> +            break;
> +        }
> +
> +        ctxt->node = ipNodes[ii];
> +        ret = virInterfaceDefParseIp(conn, ip, ctxt);
> +        if (ret != 0) {
> +            virInterfaceIpDefFree(ip);
> +            break;
> +        }
> +        def->ips[def->nips++] = ip;
> +    }
> +
> +error:
> +    VIR_FREE(ipNodes);
>      return(ret);
>  }
>  
>  static int
>  virInterfaceDefParseIfAdressing(virConnectPtr conn, virInterfaceDefPtr def,
>                                  xmlXPathContextPtr ctxt) {
> -    xmlNodePtr cur, save;
> -    int ret;
> +    xmlNodePtr save;
> +    xmlNodePtr *protoNodes = NULL;
> +    int nProtoNodes, pp, ret = 0;
>      char *tmp;
>  
> -    cur = virXPathNode(conn, "./protocol[1]", ctxt);
> -    if (cur == NULL)
> -        return(0);
>      save = ctxt->node;
> -    ctxt->node = cur;
> -    tmp = virXPathString(conn, "string(./@family)", ctxt);
> -    if (tmp == NULL) {
> -        virInterfaceReportError(conn, VIR_ERR_XML_ERROR,
> -                            "%s", _("protocol misses the family attribute"));
> -        ret = -1;
> -        goto done;
> +
> +    nProtoNodes = virXPathNodeSet(conn, "./protocol", ctxt, &protoNodes);
> +    if (nProtoNodes <= 0) {
> +        /* no protocols is an acceptable outcome */
> +        return 0;
>      }
> -    if (STREQ(tmp, "ipv4")) {
> -        def->proto.family = tmp;
> -        ret = virInterfaceDefParseProtoIPv4(conn, def, ctxt);
> -    } else {
> -        virInterfaceReportError(conn, VIR_ERR_XML_ERROR,
> -                            _("unsupported protocol family '%s'"), tmp);
> +
> +    if (VIR_ALLOC_N(def->protos, nProtoNodes) < 0) {
> +        virReportOOMError(conn);
>          ret = -1;
> -        VIR_FREE(tmp);
> +        goto error;
>      }
>  
> -done:
> +    def->nprotos = 0;
> +    for (pp = 0; pp < nProtoNodes; pp++) {
> +
> +        virInterfaceProtocolDefPtr proto;
> +
> +        if (VIR_ALLOC(proto) < 0) {
> +            virReportOOMError(conn);
> +            break;
> +        }
> +
> +        ctxt->node = protoNodes[pp];
> +        tmp = virXPathString(conn, "string(./@family)", ctxt);
> +        if (tmp == NULL) {
> +            virInterfaceReportError(conn, VIR_ERR_XML_ERROR,
> +                                    "%s", _("protocol misses the family attribute"));
> +            virInterfaceProtocolDefFree(proto);
> +            ret = -1;
> +            break;
> +        }
> +        proto->family = tmp;
> +        if (STREQ(tmp, "ipv4")) {
> +            ret = virInterfaceDefParseProtoIPv4(conn, proto, ctxt);
> +            if (ret != 0) {
> +                virInterfaceProtocolDefFree(proto);
> +                break;
> +            }
> +        } else if (STREQ(tmp, "ipv6")) {
> +            ret = virInterfaceDefParseProtoIPv6(conn, proto, ctxt);
> +            if (ret != 0) {
> +                virInterfaceProtocolDefFree(proto);
> +                break;
> +            }
> +        } else {
> +            virInterfaceReportError(conn, VIR_ERR_XML_ERROR,
> +                                    _("unsupported protocol family '%s'"), tmp);
> +            ret = -1;
> +            virInterfaceProtocolDefFree(proto);
> +            break;
> +        }
> +        def->protos[def->nprotos++] = proto;
> +    }
> +
> +error:
> +    VIR_FREE(protoNodes);
>      ctxt->node = save;
>      return(ret);
>  
> @@ -1005,36 +1149,48 @@ virInterfaceVlanDefFormat(virConnectPtr conn, virBufferPtr buf,
>  static int
>  virInterfaceProtocolDefFormat(virConnectPtr conn ATTRIBUTE_UNUSED,
>                                virBufferPtr buf, const virInterfaceDefPtr def) {
> -    char prefixStr[16];
> -    if (def->proto.family == NULL)
> -        return(0);
> -    virBufferVSprintf(buf, "  <protocol family='%s'>\n", def->proto.family);
> -    if (def->proto.dhcp) {
> -        if (def->proto.peerdns == 0)
> -            virBufferAddLit(buf, "    <dhcp peerdns='no'/>\n");
> -        else if (def->proto.peerdns == 1)
> -            virBufferAddLit(buf, "    <dhcp peerdns='yes'/>\n");
> -        else
> -            virBufferAddLit(buf, "    <dhcp/>\n");
> -    }
> -    if (def->proto.address != NULL) {
> -        if (def->proto.prefix != 0)
> -            snprintf(prefixStr, sizeof(prefixStr), "%d", def->proto.prefix);
> -
> -        virBufferVSprintf(buf, "    <ip address='%s'%s%s%s%s%s%s/>\n",
> -                          def->proto.address,
> -                          def->proto.prefix ? " prefix='"       : "",
> -                          def->proto.prefix ? prefixStr         : "",
> -                          def->proto.prefix ? "'"               : "",
> -                          def->proto.source ? " source='"       : "",
> -                          def->proto.source ? def->proto.source : "",
> -                          def->proto.source ? "'"               : "");
> -    }
> -    if (def->proto.gateway != NULL) {
> -        virBufferVSprintf(buf, "    <route gateway='%s'/>\n",
> -                          def->proto.gateway);
> +    int pp, ii;
> +
> +    for (pp = 0; pp < def->nprotos; pp++) {
> +
> +        virBufferVSprintf(buf, "  <protocol family='%s'>\n", def->protos[pp]->family);
> +
> +        if (def->protos[pp]->autoconf) {
> +            virBufferAddLit(buf, "    <autoconf/>\n");
> +        }
> +
> +        if (def->protos[pp]->dhcp) {
> +            if (def->protos[pp]->peerdns == 0)
> +                virBufferAddLit(buf, "    <dhcp peerdns='no'/>\n");
> +            else if (def->protos[pp]->peerdns == 1)
> +                virBufferAddLit(buf, "    <dhcp peerdns='yes'/>\n");
> +            else
> +                virBufferAddLit(buf, "    <dhcp/>\n");
> +        }
> +
> +        for (ii = 0; ii < def->protos[pp]->nips; ii++) {
> +            if (def->protos[pp]->ips[ii]->address != NULL) {
> +
> +                virBufferVSprintf(buf, "    <ip address='%s'",
> +                                  def->protos[pp]->ips[ii]->address);
> +                if (def->protos[pp]->ips[ii]->prefix != 0) {
> +                    virBufferVSprintf(buf, " prefix='%d'",
> +                                      def->protos[pp]->ips[ii]->prefix);
> +                }
> +                if (def->protos[pp]->ips[ii]->source != NULL) {
> +                    virBufferVSprintf(buf, " source='%s'",
> +                                      def->protos[pp]->ips[ii]->source);
> +                }
> +                virBufferAddLit(buf, "/>\n");
> +            }
> +        }
> +        if (def->protos[pp]->gateway != NULL) {
> +            virBufferVSprintf(buf, "    <route gateway='%s'/>\n",
> +                              def->protos[pp]->gateway);
> +        }
> +
> +        virBufferAddLit(buf, "  </protocol>\n");
>      }
> -    virBufferAddLit(buf, "  </protocol>\n");
>      return(0);
>  }
>  
> diff --git a/src/conf/interface_conf.h b/src/conf/interface_conf.h
> index df871aa..49acdd6 100644
> --- a/src/conf/interface_conf.h
> +++ b/src/conf/interface_conf.h
> @@ -123,16 +123,25 @@ struct _virInterfaceVlanDef {
>      char *devname;   /* device name for vlan */
>  };
>  
> +typedef struct _virInterfaceIpDef virInterfaceIpDef;
> +typedef virInterfaceIpDef *virInterfaceIpDefPtr;
> +struct _virInterfaceIpDef {
> +    char *address;   /* ip address */
> +    int prefix;      /* ip prefix */
> +    char *source;    /* source of address - "device" or "config" (default) */
> +};
> +
> +
>  typedef struct _virInterfaceProtocolDef virInterfaceProtocolDef;
>  typedef virInterfaceProtocolDef *virInterfaceProtocolDefPtr;
>  struct _virInterfaceProtocolDef {
> -    char *family;    /* ipv4 only right now */
> +    char *family;    /* ipv4 or ipv6 */
>      int dhcp;        /* use dhcp */
>      int peerdns;     /* dhcp peerdns ? */
> -    char *address;   /* ip address */
> -    int prefix;      /* ip prefix */
> +    int autoconf;    /* only useful if family is ipv6 */
> +    int nips;
> +    virInterfaceIpDefPtr *ips; /* ptr to array of ips[nips] */
>      char *gateway;   /* route gateway */
> -    char *source;    /* source of address - "device" or "config" (default) */
>  };
>  
>  
> @@ -152,8 +161,8 @@ struct _virInterfaceDef {
>          virInterfaceBondDef bond;
>      } data;
>  
> -    /* separated as we may allow multiple of those in the future */
> -    virInterfaceProtocolDef proto;
> +    int nprotos;
> +    virInterfaceProtocolDefPtr *protos; /* ptr to array of protos[nprotos] */
>  };
>  
>  typedef struct _virInterfaceObj virInterfaceObj;

This looks good, with exception of the 'source' attribute I mentioned
in the review of the other patch 

Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


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