[libvirt] [PATCH] Share the code and schemas for domain and network route definitions
Michal Privoznik
mprivozn at redhat.com
Mon Jan 12 10:47:18 UTC 2015
On 09.01.2015 17:47, Cédric Bosdonnat wrote:
> Made the network configuration schemas and codes for the route element reusable.
> Created networkcommon_conf.[ch] files containing pieces to be used in both domain
> and network configurations.
>
> Replaced the brand new domain route configuration by the network one. Note that it
> now forces the destination address to be defined, even if the user wants to define
> the default gateway.
> ---
> docs/formatdomain.html.in | 14 +-
> docs/schemas/basictypes.rng | 2 +-
> docs/schemas/domaincommon.rng | 29 +-
> docs/schemas/network.rng | 20 +-
> docs/schemas/networkcommon.rng | 22 ++
> po/POTFILES.in | 1 +
> src/Makefile.am | 3 +-
> src/conf/domain_conf.c | 117 ++------
> src/conf/domain_conf.h | 14 +-
> src/conf/network_conf.c | 284 +-----------------
> src/conf/network_conf.h | 20 +-
> src/conf/networkcommon_conf.c | 337 ++++++++++++++++++++++
> src/conf/networkcommon_conf.h | 76 +++++
> src/libvirt_private.syms | 7 +
> src/lxc/lxc_container.c | 22 +-
> src/lxc/lxc_native.c | 26 +-
> tests/lxcconf2xmldata/lxcconf2xml-physnetwork.xml | 4 +-
> tests/lxcconf2xmldata/lxcconf2xml-simple.xml | 4 +-
> tests/lxcxml2xmldata/lxc-hostdev.xml | 4 +-
> tests/lxcxml2xmldata/lxc-idmap.xml | 4 +-
> 20 files changed, 542 insertions(+), 468 deletions(-)
> create mode 100644 src/conf/networkcommon_conf.c
> create mode 100644 src/conf/networkcommon_conf.h
>
I've always felt that what's shared in RNG should share parser/formatter
in the code too.
> diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng
> index 9ddd92b..efc9da4 100644
> --- a/docs/schemas/basictypes.rng
> +++ b/docs/schemas/basictypes.rng
> @@ -174,7 +174,7 @@
> <define name="ipv6Addr">
> <data type="string">
> <!-- To understand this better, take apart the toplevel "|"s -->
> -<param name="pattern">(([0-9A-Fa-f]{1,4}:){7}[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){6}:[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){5}:([0-9A-Fa-f]{1,4}:)?[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){4}:([0-9A-Fa-f]{1,4}:){0,2}[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){3}:([0-9A-Fa-f]{1,4}:){0,3}[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){2}:([0-9A-Fa-f]{1,4}:){0,4}[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){6}(((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([1-9][0-9])|([0-9]))\.){3}((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([1-9][0-9])|([0-9])))|(([0-9A-Fa-f]{1,4}:){0,5}:(((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([1-9][0-9])|([0-9]))\.){3}((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([1-9][0-9])|([0-9])))|(::([0-9A-Fa-f]{1,4}:){0,5}(((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([1-9][0-9])|([0-9]))\.){3}((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([1-9][0-9])|([0-9])))|([0-9A-Fa-f]{1,4}::([0-9A-Fa-f]{1,4}:){0,5}[0-9A-Fa-f]{1,4})|(::([0-9A-Fa-f]{1,4}:){0,6}[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){1,7}:)</param>
> +<param name="pattern">(([0-9A-Fa-f]{1,4}:){7}[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){6}:[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){5}:([0-9A-Fa-f]{1,4}:)?[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){4}:([0-9A-Fa-f]{1,4}:){0,2}[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){3}:([0-9A-Fa-f]{1,4}:){0,3}[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){2}:([0-9A-Fa-f]{1,4}:){0,4}[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){6}(((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([1-9][0-9])|([0-9]))\.){3}((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([1-9][0-9])|([0-9])))|(([0-9A-Fa-f]{1,4}:){0,5}:(((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([1-9][0-9])|([0-9]))\.){3}((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([1-9][0-9])|([0-9])))|(::([0-9A-Fa-f]{1,4}:){0,5}(((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([1-9][0-9])|([0-9]))\.){3}((25[0-5])|(2[0-4][0-9])|(1[0-9]{2})|([1-9][0-9])|([0-9])))|([0-9A-Fa-f]{1,4}::([0-9A-Fa-f]{1,4}:){0,5}[0-9A-Fa-f]{1,4})|(::([0-9A-Fa-f]{1,4}:){0,6}[0-9A-Fa-f]{1,4})|(([0-9A-Fa-f]{1,4}:){1,7}:)|(::)</param>
My brain is just too small to go through this on Monday morning.
> </data>
> </define>
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 57e99e6..8e34fd6 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -3,6 +3,7 @@
> *
> * Copyright (C) 2006-2014 Red Hat, Inc.
> * Copyright (C) 2006-2008 Daniel P. Berrange
> + * Copyright (c) 2015 SUSE LINUX Products GmbH, Nuernberg, Germany.
> *
> * This library is free software; you can redistribute it and/or
> * modify it under the terms of the GNU Lesser General Public
> @@ -1475,11 +1476,13 @@ void virDomainNetDefFree(virDomainNetDefPtr def)
> VIR_FREE(def->ips[i]);
> VIR_FREE(def->ips);
>
> - for (i = 0; i < def->nroutes; i++)
> + for (i = 0; i < def->nroutes; i++) {
> + virNetworkRouteDefClear(def->routes[i]);
> VIR_FREE(def->routes[i]);
This code snippet repeats itself in a few places too. Looks like
virNetworkRouteDefFree() which encapsulates the two comes handy.
> + }
> VIR_FREE(def->routes);
>
> - virDomainDeviceInfoClear(&def->info);
> + virDomainDeviceInfoClear(&def->info);
>
> VIR_FREE(def->filter);
> virNWFilterHashTableFree(def->filterparams);
> static void
> virDomainNetRoutesFormat(virBufferPtr buf,
> - virDomainNetRouteDefPtr *routes,
> + virNetworkRouteDefPtr *routes,
> size_t nroutes)
> {
> size_t i;
>
> for (i = 0; i < nroutes; i++) {
> - virDomainNetRouteDefPtr route = routes[i];
> - const char *familyStr = NULL;
> - char *via = virSocketAddrFormat(&route->via);
> - char *to = NULL;
> -
> - if (VIR_SOCKET_ADDR_IS_FAMILY(&route->via, AF_INET6))
> - familyStr = "ipv6";
> - else if (VIR_SOCKET_ADDR_IS_FAMILY(&route->via, AF_INET))
> - familyStr = "ipv4";
> - virBufferAsprintf(buf, "<route family='%s' via='%s'", familyStr, via);
> -
> - if (VIR_SOCKET_ADDR_VALID(&route->to)) {
> - to = virSocketAddrFormat(&route->to);
> - virBufferAsprintf(buf, " address='%s'", to);
> - }
> -
> - if (route->prefix > 0)
> - virBufferAsprintf(buf, " prefix='%d'", route->prefix);
> -
> - virBufferAddLit(buf, "/>\n");
> + virNetworkRouteDefPtr route = routes[i];
> + virNetworkRouteDefFormat(buf, route);
Or just user routes[i] directly.
> }
> }
>
> diff --git a/src/conf/networkcommon_conf.c b/src/conf/networkcommon_conf.c
> new file mode 100644
> index 0000000..da80c0f
> --- /dev/null
> +++ b/src/conf/networkcommon_conf.c
> +int
> +virNetworkRouteDefFormat(virBufferPtr buf,
> + const virNetworkRouteDef *def)
> +{
> + int result = -1;
> +
> + virBufferAddLit(buf, "<route");
> +
> + if (def->family)
> + virBufferAsprintf(buf, " family='%s'", def->family);
> + if (VIR_SOCKET_ADDR_VALID(&def->address)) {
> + char *addr = virSocketAddrFormat(&def->address);
> +
> + if (!addr)
> + goto error;
> + virBufferAsprintf(buf, " address='%s'", addr);
> + VIR_FREE(addr);
> + }
> + if (VIR_SOCKET_ADDR_VALID(&def->netmask)) {
> + char *addr = virSocketAddrFormat(&def->netmask);
> +
> + if (!addr)
> + goto error;
> + virBufferAsprintf(buf, " netmask='%s'", addr);
> + VIR_FREE(addr);
> + }
> + if (def->has_prefix)
> + virBufferAsprintf(buf, " prefix='%u'", def->prefix);
> + if (VIR_SOCKET_ADDR_VALID(&def->gateway)) {
I know you've copied the preexisting code, but since this kind of check
is done in virNetworkRouteDefCreate() - is it needed here too? It
doesn't hurt anything (but performance, which we don't care about that
much). I'm just curious.
> + char *addr = virSocketAddrFormat(&def->gateway);
> + if (!addr)
> + goto error;
> + virBufferAsprintf(buf, " gateway='%s'", addr);
> + VIR_FREE(addr);
> + }
> + if (def->has_metric && def->metric > 0)
> + virBufferAsprintf(buf, " metric='%u'", def->metric);
> + virBufferAddLit(buf, "/>\n");
> +
> + result = 0;
> + error:
> + return result;
> +}
> diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c
> index d7cd1d5..dd99bbb 100644
> --- a/src/lxc/lxc_native.c
> +++ b/src/lxc/lxc_native.c
> @@ -2,7 +2,7 @@
> * lxc_native.c: LXC native configuration import
> *
> * Copyright (c) 2014 Red Hat, Inc.
> - * Copyright (c) 2013 SUSE LINUX Products GmbH, Nuernberg, Germany.
> + * Copyright (c) 2013-2015 SUSE LINUX Products GmbH, Nuernberg, Germany.
Technically speaking this should be 2013,2015 but who cares, right? :)
> *
> * This library is free software; you can redistribute it and/or
> * modify it under the terms of the GNU Lesser General Public
ACK
Michal
More information about the libvir-list
mailing list