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