[libvirt] [PATCHv2 3/7] Move code related to network routes to networkcommon_conf.[ch]

Michal Privoznik mprivozn at redhat.com
Thu Jan 15 10:58:26 UTC 2015


On 15.01.2015 10:25, Cédric Bosdonnat wrote:
> Moving code for parsing and formatting network routes to
> networkcommon_conf helps reusing those routes for domains. The route
> definition has been hidden to help reducing the number of unnecessary
> checks in the format function.
> ---
>  po/POTFILES.in                |   1 +
>  src/Makefile.am               |   3 +-
>  src/conf/network_conf.c       | 297 ++----------------------------
>  src/conf/network_conf.h       |  22 +--
>  src/conf/networkcommon_conf.c | 414 ++++++++++++++++++++++++++++++++++++++++++
>  src/conf/networkcommon_conf.h |  72 ++++++++
>  src/libvirt_private.syms      |  11 ++
>  src/network/bridge_driver.c   |  44 ++---
>  8 files changed, 526 insertions(+), 338 deletions(-)
>  create mode 100644 src/conf/networkcommon_conf.c
>  create mode 100644 src/conf/networkcommon_conf.h
> 
> diff --git a/po/POTFILES.in b/po/POTFILES.in
> index 094c8e3..3064037 100644
> --- a/po/POTFILES.in
> +++ b/po/POTFILES.in
> @@ -25,6 +25,7 @@ src/conf/netdev_bandwidth_conf.c
>  src/conf/netdev_vlan_conf.c
>  src/conf/netdev_vport_profile_conf.c
>  src/conf/network_conf.c
> +src/conf/networkcommon_conf.c
>  src/conf/node_device_conf.c
>  src/conf/numatune_conf.c
>  src/conf/nwfilter_conf.c
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 216abac..4bba536 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -287,7 +287,8 @@ NETWORK_EVENT_SOURCES =						\
>  
>  # Network driver generic impl APIs
>  NETWORK_CONF_SOURCES =						\
> -		conf/network_conf.c conf/network_conf.h
> +		conf/network_conf.c conf/network_conf.h \
> +		conf/networkcommon_conf.c conf/networkcommon_conf.h
>  
>  # Network filter driver generic impl APIs
>  NWFILTER_PARAM_CONF_SOURCES =					\
> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
> index 26fe18d..66f9b6c 100644
> --- a/src/conf/network_conf.c
> +++ b/src/conf/network_conf.c
> @@ -163,12 +163,6 @@ virNetworkIpDefClear(virNetworkIpDefPtr def)
>  }
>  
>  static void
> -virNetworkRouteDefClear(virNetworkRouteDefPtr def)
> -{
> -    VIR_FREE(def->family);
> -}
> -
> -static void
>  virNetworkDNSTxtDefClear(virNetworkDNSTxtDefPtr def)
>  {
>      VIR_FREE(def->name);
> @@ -251,7 +245,7 @@ virNetworkDefFree(virNetworkDefPtr def)
>      VIR_FREE(def->ips);
>  
>      for (i = 0; i < def->nroutes && def->routes; i++)
> -        virNetworkRouteDefClear(&def->routes[i]);
> +        virNetworkRouteDefFree(def->routes[i]);
>      VIR_FREE(def->routes);
>  
>      for (i = 0; i < def->nPortGroups && def->portGroups; i++)
> @@ -1371,232 +1365,6 @@ virNetworkIPDefParseXML(const char *networkName,
>  }
>  
>  static int
> -virNetworkRouteDefParseXML(const char *networkName,
> -                           xmlNodePtr node,
> -                           xmlXPathContextPtr ctxt,
> -                           virNetworkRouteDefPtr def)
> -{
> -    /*
> -     * virNetworkRouteDef object is already allocated as part
> -     * of an array.  On failure clear: it out, but don't free it.
> -     */
> -
> -    xmlNodePtr save;
> -    char *address = NULL, *netmask = NULL;
> -    char *gateway = NULL;
> -    unsigned long prefix = 0, metric = 0;
> -    int result = -1;
> -    int prefixRc, metricRc;
> -    virSocketAddr testAddr;
> -
> -    save = ctxt->node;
> -    ctxt->node = node;
> -
> -    /* grab raw data from XML */
> -    def->family = virXPathString("string(./@family)", ctxt);
> -    address = virXPathString("string(./@address)", ctxt);
> -    netmask = virXPathString("string(./@netmask)", ctxt);
> -    gateway = virXPathString("string(./@gateway)", ctxt);
> -    prefixRc = virXPathULong("string(./@prefix)", ctxt, &prefix);
> -    if (prefixRc == -2) {
> -        virReportError(VIR_ERR_XML_ERROR,
> -                       _("Invalid prefix specified "
> -                         "in route definition of network '%s'"),
> -                       networkName);
> -        goto cleanup;
> -    }
> -    def->has_prefix = (prefixRc == 0);
> -    def->prefix = prefix;
> -    metricRc = virXPathULong("string(./@metric)", ctxt, &metric);
> -    if (metricRc == -2) {
> -        virReportError(VIR_ERR_XML_ERROR,
> -                       _("Invalid metric specified "
> -                         "in route definition of network '%s'"),
> -                       networkName);
> -        goto cleanup;
> -    }
> -    if (metricRc == 0) {
> -        def->has_metric = true;
> -        if (metric == 0) {
> -            virReportError(VIR_ERR_XML_ERROR,
> -                           _("Invalid metric value, must be > 0 "
> -                             "in route definition of network '%s'"),
> -                           networkName);
> -            goto cleanup;
> -        }
> -    }
> -    def->metric = metric;
> -
> -    /* Note: both network and gateway addresses must be specified */
> -
> -    if (!address) {
> -        virReportError(VIR_ERR_XML_ERROR,
> -                       _("Missing required address attribute "
> -                         "in route definition of network '%s'"),
> -                       networkName);
> -        goto cleanup;
> -    }
> -
> -    if (!gateway) {
> -        virReportError(VIR_ERR_XML_ERROR,
> -                       _("Missing required gateway attribute "
> -                         "in route definition of network '%s'"),
> -                       networkName);
> -        goto cleanup;
> -    }
> -
> -    if (virSocketAddrParse(&def->address, address, AF_UNSPEC) < 0) {
> -        virReportError(VIR_ERR_XML_ERROR,
> -                       _("Bad network address '%s' "
> -                         "in route definition of network '%s'"),
> -                       address, networkName);
> -        goto cleanup;
> -    }
> -
> -    if (virSocketAddrParse(&def->gateway, gateway, AF_UNSPEC) < 0) {
> -        virReportError(VIR_ERR_XML_ERROR,
> -                       _("Bad gateway address '%s' "
> -                         "in route definition of network '%s'"),
> -                       gateway, networkName);
> -        goto cleanup;
> -    }
> -
> -    /* validate network address, etc. for each family */
> -    if ((def->family == NULL) || (STREQ(def->family, "ipv4"))) {
> -        if (!(VIR_SOCKET_ADDR_IS_FAMILY(&def->address, AF_INET) ||
> -              VIR_SOCKET_ADDR_IS_FAMILY(&def->address, AF_UNSPEC))) {
> -            virReportError(VIR_ERR_XML_ERROR,
> -                           def->family == NULL ?
> -                           _("No family specified for non-IPv4 address '%s' "
> -                             "in route definition of network '%s'") :
> -                           _("IPv4 family specified for non-IPv4 address '%s' "
> -                             "in route definition of network '%s'"),
> -                           address, networkName);
> -            goto cleanup;
> -        }
> -        if (!VIR_SOCKET_ADDR_IS_FAMILY(&def->gateway, AF_INET)) {
> -            virReportError(VIR_ERR_XML_ERROR,
> -                           def->family == NULL ?
> -                           _("No family specified for non-IPv4 gateway '%s' "
> -                             "in route definition of network '%s'") :
> -                           _("IPv4 family specified for non-IPv4 gateway '%s' "
> -                             "in route definition of network '%s'"),
> -                           address, networkName);
> -            goto cleanup;
> -        }
> -        if (netmask) {
> -            if (virSocketAddrParse(&def->netmask, netmask, AF_UNSPEC) < 0) {
> -                virReportError(VIR_ERR_XML_ERROR,
> -                               _("Bad netmask address '%s' "
> -                                 "in route definition of network '%s'"),
> -                               netmask, networkName);
> -                goto cleanup;
> -            }
> -            if (!VIR_SOCKET_ADDR_IS_FAMILY(&def->netmask, AF_INET)) {
> -                virReportError(VIR_ERR_XML_ERROR,
> -                               _("Network '%s' has invalid netmask '%s' "
> -                                 "for address '%s' (both must be IPv4)"),
> -                               networkName, netmask, address);
> -                goto cleanup;
> -            }
> -            if (def->has_prefix) {
> -                /* can't have both netmask and prefix at the same time */
> -                virReportError(VIR_ERR_XML_ERROR,
> -                               _("Route definition '%s' cannot have both "
> -                                 "a prefix and a netmask"),
> -                               networkName);
> -                goto cleanup;
> -            }
> -        }
> -        if (def->prefix > 32) {
> -            virReportError(VIR_ERR_XML_ERROR,
> -                           _("Invalid prefix %u specified "
> -                             "in route definition of network '%s', "
> -                             "must be 0 - 32"),
> -                           def->prefix, networkName);
> -            goto cleanup;
> -        }
> -    } else if (STREQ(def->family, "ipv6")) {
> -        if (!VIR_SOCKET_ADDR_IS_FAMILY(&def->address, AF_INET6)) {
> -            virReportError(VIR_ERR_XML_ERROR,
> -                           _("ipv6 family specified for non-IPv6 address '%s' "
> -                             "in route definition of network '%s'"),
> -                           address, networkName);
> -            goto cleanup;
> -        }
> -        if (netmask) {
> -            virReportError(VIR_ERR_XML_ERROR,
> -                           _("Specifying netmask invalid for IPv6 address '%s' "
> -                             "in route definition of network '%s'"),
> -                           address, networkName);
> -            goto cleanup;
> -        }
> -        if (!VIR_SOCKET_ADDR_IS_FAMILY(&def->gateway, AF_INET6)) {
> -            virReportError(VIR_ERR_XML_ERROR,
> -                           _("ipv6 specified for non-IPv6 gateway address '%s' "
> -                             "in route definition of network '%s'"),
> -                           gateway, networkName);
> -            goto cleanup;
> -        }
> -        if (def->prefix > 128) {
> -            virReportError(VIR_ERR_XML_ERROR,
> -                           _("Invalid prefix %u specified "
> -                             "in route definition of network '%s', "
> -                             "must be 0 - 128"),
> -                           def->prefix, networkName);
> -            goto cleanup;
> -        }
> -    } else {
> -        virReportError(VIR_ERR_XML_ERROR,
> -                       _("Unrecognized family '%s' "
> -                         "in route definition of network'%s'"),
> -                       def->family, networkName);
> -        goto cleanup;
> -    }
> -
> -    /* make sure the address is a network address */
> -    if (netmask) {
> -        if (virSocketAddrMask(&def->address, &def->netmask, &testAddr) < 0) {
> -            virReportError(VIR_ERR_INTERNAL_ERROR,
> -                           _("error converting address '%s' with netmask '%s' "
> -                             "to network-address "
> -                             "in route definition of network '%s'"),
> -                           address, netmask, networkName);
> -            goto cleanup;
> -        }
> -    } else {
> -        if (virSocketAddrMaskByPrefix(&def->address,
> -                                      def->prefix, &testAddr) < 0) {
> -            virReportError(VIR_ERR_INTERNAL_ERROR,
> -                           _("error converting address '%s' with prefix %u "
> -                             "to network-address "
> -                             "in route definition of network '%s'"),
> -                           address, def->prefix, networkName);
> -            goto cleanup;
> -        }
> -    }
> -    if (!virSocketAddrEqual(&def->address, &testAddr)) {
> -        virReportError(VIR_ERR_XML_ERROR,
> -                       _("address '%s' in route definition of network '%s' "
> -                         "is not a network address"),
> -                       address, networkName);
> -        goto cleanup;
> -    }
> -
> -    result = 0;
> -
> - cleanup:
> -    if (result < 0)
> -        virNetworkRouteDefClear(def);
> -    VIR_FREE(address);
> -    VIR_FREE(netmask);
> -    VIR_FREE(gateway);
> -
> -    ctxt->node = save;
> -    return result;
> -}
> -
> -static int
>  virNetworkPortGroupParseXML(virPortGroupDefPtr def,
>                              xmlNodePtr node,
>                              xmlXPathContextPtr ctxt)
> @@ -2209,11 +1977,13 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
>              goto error;
>          /* parse each definition */
>          for (i = 0; i < nRoutes; i++) {
> -            if (virNetworkRouteDefParseXML(def->name,
> +            virNetworkRouteDefPtr route = NULL;
> +
> +            if (!(route = virNetworkRouteDefParseXML(def->name,
>                                             routeNodes[i],
> -                                           ctxt,
> -                                           &def->routes[i]) < 0)
> +                                           ctxt)))

Indentation's off.

>                  goto error;
> +            def->routes[i] = route;
>              def->nroutes++;
>          }
>  

Michal




More information about the libvir-list mailing list