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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list