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