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

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