On 07/18/2011 04:05 PM, Michal Privoznik wrote:
These functions parse given XML node and store the output at given
pointer. Unknown elements are silently ignored. Attributes must be
integer and must fit in unsigned long.
---
src/conf/domain_conf.c | 3 +
src/conf/domain_conf.h | 1 +
src/conf/network_conf.c | 5 ++
src/conf/network_conf.h | 1 +
src/libvirt_private.syms | 1 +
src/util/network.c | 112 ++++++++++++++++++++++++++++++++++++++++++++++
src/util/network.h | 2 +
7 files changed, 125 insertions(+), 0 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8c3e44e..0d8c7e7 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2854,6 +2854,9 @@ virDomainNetDefParseXML(virCapsPtr caps,
if (virDomainDeviceBootParseXML(cur,&def->bootIndex,
bootMap))
goto error;
+ } else if (xmlStrEqual(cur->name, BAD_CAST "bandwidth")) {
+ if (virBandwidthDefParseNode(cur,&def->bandwidth)< 0)
+ goto error;
}
}
cur = cur->next;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 172d3c2..06cea02 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -398,6 +398,7 @@ struct _virDomainNetDef {
virDomainDeviceInfo info;
char *filter;
virNWFilterHashTablePtr filterparams;
+ virBandwidth bandwidth;
};
enum virDomainChrDeviceType {
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index ae479bf..c9929e2 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -742,6 +742,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
char *tmp;
xmlNodePtr *ipNodes = NULL;
xmlNodePtr dnsNode = NULL;
+ xmlNodePtr bandwidthNode = NULL;
int nIps;
if (VIR_ALLOC(def)< 0) {
@@ -777,6 +778,10 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
/* Parse network domain information */
def->domain = virXPathString("string(./domain[1]/@name)", ctxt);
+ if ((bandwidthNode = virXPathNode("./bandwidth", ctxt)) != NULL&&
+ virBandwidthDefParseNode(bandwidthNode,&def->bandwidth)< 0)
+ goto error;
+
/* Parse bridge information */
def->bridge = virXPathString("string(./bridge[1]/@name)", ctxt);
tmp = virXPathString("string(./bridge[1]/@stp)", ctxt);
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 5edcf27..565a464 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -127,6 +127,7 @@ struct _virNetworkDef {
virNetworkIpDefPtr ips; /* ptr to array of IP addresses on this network */
virNetworkDNSDefPtr dns; /* ptr to dns related configuration */
+ virBandwidth bandwidth;
};
Having this contained in the parent object rather than pointed to by it
makes memory management easier, but it means that there needs to be some
"special" value of the data that means "not present". Currently you're
checking for bandwidth.in.average != 0 and bandwidth.out.average != 0.
AS long as those would never be acceptable values (for example - is it
possible that average would be 0 and burst would be something
non-default? I don't even know, just asking).
Additionally, once you can have bandwidth information in a portgroup
(which is stored in the <network> config, but used by the domain's
<interface> config), you'll need to be either allocating a virBandwidth
and passing it back from some network/portgroup helper function, or
you'll need to have the helper function fill in an existing one that you
pass to it.
Anyway, having this contained in rather than pointing to will work for
now, but you may want to change it in the future.
typedef struct _virNetworkObj virNetworkObj;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 3e3b1dd..12db3d7 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -700,6 +700,7 @@ nlComm;
# network.h
+virBandwidthDefParseNode;
virSocketAddrBroadcast;
virSocketAddrBroadcastByPrefix;
virSocketAddrIsNetmask;
diff --git a/src/util/network.c b/src/util/network.c
index eb16e0c..ce949c7 100644
--- a/src/util/network.c
+++ b/src/util/network.c
@@ -21,6 +21,9 @@
virReportErrorHelper(VIR_FROM_THIS, code, __FILE__, \
__FUNCTION__, __LINE__, __VA_ARGS__)
+#define virBandwidthError(code, ...) \
+ virReportErrorHelper(VIR_FROM_THIS, code, __FILE__, \
+ __FUNCTION__, __LINE__, __VA_ARGS__)
Is having this separate macro defined really necessary? It's identical
to virSocketError() which is already defined in network.c.
/*
* Helpers to extract the IP arrays from the virSocketAddrPtr
* That part is the less portable of the module
@@ -674,3 +677,112 @@ virSocketAddrPrefixToNetmask(unsigned int prefix,
error:
return result;
}
+
+static int
+virBandwidthParseChildDefNode(xmlNodePtr node, virRatePtr rate)
+{
+ int ret = -1;
+ char *average = NULL;
+ char *peak = NULL;
+ char *burst = NULL;
+
+ if (!node || !rate)
+ return -1;
You're returning an error indication without any log message. virsh will
show this as "unknown error", which is very difficult to diagnose.
+
+ average = virXMLPropString(node, "average");
+ peak = virXMLPropString(node, "peak");
+ burst = virXMLPropString(node, "burst");
+
+ if (average) {
+ if (virStrToLong_ul(average, NULL, 10,&rate->average)< 0) {
+ virBandwidthError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("could not convert %s"),
+ average);
+ goto cleanup;
+ }
+ } else {
+ virBandwidthError(VIR_ERR_XML_DETAIL, "%s",
+ _("Missing mandatory average attribute"));
+ goto cleanup;
+ }
+
+ if (peak&& virStrToLong_ul(peak, NULL, 10,&rate->peak)< 0) {
+ virBandwidthError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("could not convert %s"),
+ peak);
+ goto cleanup;
+ }
+
+ if (burst&& virStrToLong_ul(burst, NULL, 10,&rate->burst)< 0) {
+ virBandwidthError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("could not convert %s"),
+ burst);
+ goto cleanup;
+ }
+
+ ret = 0;
+
+cleanup:
+ VIR_FREE(average);
+ VIR_FREE(peak);
+ VIR_FREE(burst);
+
+ return ret;
+}
+
+/**
+ * virBandwidthDefParseNode:
+ * @node: XML node
+ * @def: where to store the parsed result
+ *
+ * Parse bandwidth XML and store into given pointer
+ *
+ * Returns 0 on success, -1 on error.
+ */
+int
+virBandwidthDefParseNode(xmlNodePtr node, virBandwidthPtr def)
+{
+ int ret = -1;
+ xmlNodePtr cur = node->children;
+ xmlNodePtr in = NULL, out = NULL;
+
+ if (!node || !def ||
+ !xmlStrEqual(node->name, BAD_CAST "bandwidth"))
+ return -1;
Again returning an error without logging it.
+
+ memset(def, 0, sizeof(*def));
+ while (cur) {
+ if (cur->type == XML_ELEMENT_NODE) {
+ if (xmlStrEqual(cur->name, BAD_CAST "inbound")) {
+ if (in) {
+ virBandwidthError(VIR_ERR_XML_DETAIL, "%s",
+ _("Only one child<inbound> "
+ "element allowed"));
+ goto cleanup;
+ }
+ in = cur;
+ } else if (xmlStrEqual(cur->name, BAD_CAST "outbound")) {
+ if (out) {
+ virBandwidthError(VIR_ERR_XML_DETAIL, "%s",
+ _("Only one child<outbound> "
+ "element allowed"));
+ goto cleanup;
+ }
+ out = cur;
+ }
+ /* Silently ignore unknown elements */
+ }
+ cur = cur->next;
+ }
+
+ if (in&& virBandwidthParseChildDefNode(in,&def->in)< 0)
+ goto cleanup;
+
+ if (out&& virBandwidthParseChildDefNode(out,&def->out)< 0)
+ goto cleanup;
+
+ ret = 0;
+
+cleanup:
+ return ret;
+}
diff --git a/src/util/network.h b/src/util/network.h
index af32558..54f7aad 100644
--- a/src/util/network.h
+++ b/src/util/network.h
@@ -20,6 +20,7 @@
# endif
# include<netdb.h>
# include<netinet/in.h>
+# include "xml.h"
typedef struct {
union {
@@ -104,4 +105,5 @@ int virSocketAddrPrefixToNetmask(unsigned int prefix,
virSocketAddrPtr netmask,
int family);
+int virBandwidthDefParseNode(xmlNodePtr node, virBandwidthPtr def);
#endif /* __VIR_NETWORK_H__ */
You need to log an error message in the two indicated places. I would
also suggest using the existing virSocketError() macro rather than
defining a new one, and you may want to think about making the bandwidth
in the virNetworkDef into a virBandwidthPtr (although I'm really fine
with it either way).
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list