On 06/22/2016 01:37 PM, Laine Stump wrote:
There are currently two places in the domain where this combination
is
used, and there is about to be another. This patch puts them together
for brevity and uniformity.
As with the newly-renamed virNetDevIPAddr and virNetDevIPRoute
objects, the new virNetDevIPInfo object will need to be accessed by a
utility function that calls low level Netlink functions (so we don't
want it to be in the conf directory) and will be called from multiple
hypervisor drivers (so it can't be in any hypervisor directory); the
most appropriate place is thus once again the util directory.
The parse and format functions are in conf/domain_conf.c because only
the domain XML (i.e. *not* the network XML) has this exact combination
of IP addresses plus routes. They will end up being static, but since
they aren't being called yet, they are declared right before their
definition to avoid a "defined but not used" compile error. That will
be changed to static once they are in use. Likewise
virDomainNetIPInfoFormat() will end up being the only caller to
virDomainNetRoutesFormat() and virDomainNetIPsFormat(), so it will
just subsume those functions, but we can't do that until they are no
longer called.
(It would have been nice to include the interface name within the
virNetDevIPInfo object (with a slight name change), but that can't
be done cleanly, because in each case the interface name is provided
in a different place in the XML relative to the routes and IP
addresses, so putting it in this object would actually make the code
more confused rather than simpler).
---
docs/schemas/domaincommon.rng | 27 +++++++++++++++++
src/conf/domain_conf.c | 68 +++++++++++++++++++++++++++++++++++++++++++
src/libvirt_private.syms | 1 +
src/util/virnetdevip.c | 16 ++++++++++
src/util/virnetdevip.h | 11 +++++++
5 files changed, 123 insertions(+)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index b81b558..ab89dab 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -2629,6 +2629,33 @@
</optional>
</interleave>
</define>
+
+ <!--
+ All ip-related info for either the host or guest side of an interface
+ -->
+ <define name="interface-ip-info">
+ <zeroOrMore>
+ <element name="ip">
+ <attribute name="address">
+ <ref name="ipAddr"/>
+ </attribute>
+ <optional>
+ <attribute name="family">
+ <ref name="addr-family"/>
+ </attribute>
+ </optional>
+ <optional>
+ <attribute name="prefix">
+ <ref name="ipPrefix"/>
+ </attribute>
+ </optional>
+ <empty/>
+ </element>
+ </zeroOrMore>
+ <zeroOrMore>
+ <ref name="route"/>
+ </zeroOrMore>
+ </define>
<!--
An emulator description is just a path to the binary used for the task
-->
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f380271..548c750 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6173,6 +6173,58 @@ virDomainNetIPParseXML(xmlNodePtr node)
return ret;
}
+
+/* fill in a virNetDevIPInfoPtr from the <route> and <ip>
+ * elements found in the given XML context.
+ *
+ * return 0 on success (including none found) and -1 on failure.
+ */
+int
+virDomainNetIPInfoParseXML(const char *source,
+ xmlXPathContextPtr ctxt,
+ virNetDevIPInfoPtr def);
Why? If it needs to be "higher" to avoid the forward reference for
future callers, then so be it.
+int
static int
+virDomainNetIPInfoParseXML(const char *source,
+ xmlXPathContextPtr ctxt,
+ virNetDevIPInfoPtr def)
+
+{
+ xmlNodePtr *nodes = NULL;
+ virNetDevIPAddrPtr ip = NULL;
+ virNetDevIPRoutePtr route = NULL;
+ int nnodes;
+ int ret = -1;
+ size_t i;
+
+ if ((nnodes = virXPathNodeSet("./ip", ctxt, &nodes)) < 0)
+ goto cleanup;
+
+ for (i = 0; i < nnodes; i++) {
+ if (!(ip = virDomainNetIPParseXML(nodes[i])) ||
+ VIR_APPEND_ELEMENT(def->ips, def->nips, ip) < 0)
+ goto cleanup;
+ }
+ VIR_FREE(nodes);
+
+ if ((nnodes = virXPathNodeSet("./route", ctxt, &nodes)) < 0)
+ goto cleanup;
+
+ for (i = 0; i < nnodes; i++) {
+ if (!(route = virNetDevIPRouteParseXML(source, nodes[i], ctxt)) ||
+ VIR_APPEND_ELEMENT(def->routes, def->nroutes, route) < 0)
+ goto cleanup;
+ }
+
+ ret = 0;
+ cleanup:
+ if (ret < 0)
+ virNetDevIPInfoClear(def);
+ VIR_FREE(ip);
Seeing just VIR_FREE(ip) made me go look at how it was allocated - guess
I was (now) concerned that something would be allocated into ip that
wasn't free'd properly (eg. no virNetDevIPFree() API ...
Anyway, the ip->address is written with the result of a getaddrinfo in
virSocketAddrParseInternal, which when free'd should be done by
freeaddrinfo, right?
I think this is existing, but fixable... at some point in time.
> + virNetDevIPRouteFree(route);
> + VIR_FREE(nodes);
> + return ret;
> +}
> +
> static int
> virDomainHostdevDefParseXMLCaps(xmlNodePtr node ATTRIBUTE_UNUSED,
> xmlXPathContextPtr ctxt,
> @@ -20311,6 +20363,22 @@ virDomainNetRoutesFormat(virBufferPtr buf,
> return 0;
> }
>
> +
+int
> +virDomainNetIPInfoFormat(virBufferPtr buf,
> + virNetDevIPInfoPtr def);
Same complaint.
+int
static int
ACK with the forward ref and static int used. I think you need a "new"
patch at some point in time to handle the getaddrinfo/freeaddrinfo...
John
+virDomainNetIPInfoFormat(virBufferPtr buf,
+ virNetDevIPInfoPtr def)
+{
+ if (virDomainNetIPsFormat(buf, def->ips, def->nips) < 0)
+ return -1;
+ if (virDomainNetRoutesFormat(buf, def->routes, def->nroutes) < 0)
+ return -1;
+ return 0;
+}
+
+
static int
virDomainHostdevDefFormatSubsys(virBufferPtr buf,
virDomainHostdevDefPtr def,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 151cf9f..f6c3d45 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1925,6 +1925,7 @@ virNetDevBridgeSetVlanFiltering;
virNetDevIPAddrAdd;
virNetDevIPAddrDel;
virNetDevIPAddrGet;
+virNetDevIPInfoClear;
virNetDevIPRouteAdd;
virNetDevIPRouteFree;
virNetDevIPRouteGetAddress;
diff --git a/src/util/virnetdevip.c b/src/util/virnetdevip.c
index 619f926..376d4ad 100644
--- a/src/util/virnetdevip.c
+++ b/src/util/virnetdevip.c
@@ -845,3 +845,19 @@ virNetDevIPRouteGetGateway(virNetDevIPRoutePtr def)
return &def->gateway;
return NULL;
}
+
+/* manipulating the virNetDevIPInfo object */
+
+void
+virNetDevIPInfoClear(virNetDevIPInfoPtr ip)
+{
+ size_t i;
+
+ for (i = 0; i < ip->nips; i++)
+ VIR_FREE(ip->ips[i]);
+ VIR_FREE(ip->ips);
+
+ for (i = 0; i < ip->nroutes; i++)
+ virNetDevIPRouteFree(ip->routes[i]);
+ VIR_FREE(ip->routes);
+}
diff --git a/src/util/virnetdevip.h b/src/util/virnetdevip.h
index 7a07b73..be41636 100644
--- a/src/util/virnetdevip.h
+++ b/src/util/virnetdevip.h
@@ -47,6 +47,14 @@ typedef struct {
virSocketAddr gateway; /* gateway IP address for ip-route */
} virNetDevIPRoute, *virNetDevIPRoutePtr;
+/* A full set of all IP config info for a network device */
+typedef struct {
+ size_t nips;
+ virNetDevIPAddrPtr *ips;
+ size_t nroutes;
+ virNetDevIPRoutePtr *routes;
+} virNetDevIPInfo, *virNetDevIPInfoPtr;
+
/* manipulating/querying the netdev */
int virNetDevIPAddrAdd(const char *ifname,
virSocketAddr *addr,
@@ -76,4 +84,7 @@ int virNetDevIPRouteGetPrefix(virNetDevIPRoutePtr def);
unsigned int virNetDevIPRouteGetMetric(virNetDevIPRoutePtr def);
virSocketAddrPtr virNetDevIPRouteGetGateway(virNetDevIPRoutePtr def);
+/* virNetDevIPInfo object */
+void virNetDevIPInfoClear(virNetDevIPInfoPtr ip);
+
#endif /* __VIR_NETDEVIP_H__ */