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

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

 



On Thu, 2015-01-15 at 11:58 +0100, Michal Privoznik wrote:
> 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.

Nice catch, I just fixed that on my local branch.

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


--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]