Re: [libvirt] [PATCH v2 3/7] bandwidth: Add format parsing functions

2011-07-18 Thread Jiri Denemark
On Tue, Jul 12, 2011 at 13:57:09 +0200, Michal Privoznik wrote:
 These functions take on input decimal numbers optionally followed
 by unit. Units are exactly the same as 'tc' accepts.
 ---
  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   |  203 
 ++
  src/util/network.h   |2 +
  7 files changed, 216 insertions(+), 0 deletions(-)

This can be greatly simplified once we forbid using units in bandwidth
attributes.

...
 diff --git a/src/util/network.c b/src/util/network.c
 index eb16e0c..476ecde 100644
 --- a/src/util/network.c
 +++ b/src/util/network.c
 @@ -10,6 +10,7 @@
  
  #include config.h
  #include arpa/inet.h
 +#include math.h

BTW, why did you need to include math.h? Getting rid of it is another good
reason for removing units.

  
  #include memory.h
  #include network.h
 @@ -21,6 +22,9 @@
...
 +/**
 + * virBandwidthParseXML:

The name here doesn't match the function this is supposed to document.

 + * @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;
 +
 +memset(def, 0, sizeof(virBandwidth));

Using sizeof(*def) is better.

 +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;
 +} else {
 +virBandwidthError(VIR_ERR_CONFIG_UNSUPPORTED,
 +  _(Unknown element %s),
 +  cur-name);
 +goto cleanup;

AFAIK we ignore unknown XML elements in other XML parsing code, shouldn't we
do the same here?

...

Jirka

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH v2 3/7] bandwidth: Add format parsing functions

2011-07-12 Thread Michal Privoznik
These functions take on input decimal numbers optionally followed
by unit. Units are exactly the same as 'tc' accepts.
---
 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   |  203 ++
 src/util/network.h   |2 +
 7 files changed, 216 insertions(+), 0 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index bf2eadf..c109153 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2853,6 +2853,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 8262d25..84ae171 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;
 };
 
 typedef struct _virNetworkObj virNetworkObj;
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 1112398..c0e1bfa 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -682,6 +682,7 @@ nlComm;
 
 
 # network.h
+virBandwidthDefParseNode;
 virSocketAddrBroadcast;
 virSocketAddrBroadcastByPrefix;
 virSocketAddrIsNetmask;
diff --git a/src/util/network.c b/src/util/network.c
index eb16e0c..476ecde 100644
--- a/src/util/network.c
+++ b/src/util/network.c
@@ -10,6 +10,7 @@
 
 #include config.h
 #include arpa/inet.h
+#include math.h
 
 #include memory.h
 #include network.h
@@ -21,6 +22,9 @@
 virReportErrorHelper(VIR_FROM_THIS, code, __FILE__, \
  __FUNCTION__, __LINE__, __VA_ARGS__)
 
+#define virBandwidthError(code, ...)\
+virReportErrorHelper(VIR_FROM_THIS, code, __FILE__, \
+ __FUNCTION__, __LINE__, __VA_ARGS__)
 /*
  * Helpers to extract the IP arrays from the virSocketAddrPtr
  * That part is the less portable of the module
@@ -674,3 +678,202 @@ virSocketAddrPrefixToNetmask(unsigned int prefix,
 error:
 return result;
 }
+
+static const struct rate_suffix {
+const char *name;
+double scale;
+} suffixes[] = {
+{ bit,1. },
+{ Kibit,  1024. },
+{ kbit,   1000. },
+{ mibit,  1024.*1024. },
+{ mbit,   1000.*1000. },
+{ gibit,  1024.*1024.*1024. },
+{ gbit,   1000.*1000.*1000. },
+{ tibit,  1024.*1024.*1024.*1024. },
+{ tbit,   1000.*1000.*1000.*1000. },
+{ Bps,8. },
+{ KiBps,  8.*1024. },
+{ KBps,   8.*1000. },
+{ MiBps,  8.*1024.*1024. },
+{ MBps,   8.*1000.*1000. },
+{ GiBps,  8.*1024.*1024.*1024. },
+{ GBps,   8.*1000.*1000.*1000. },
+{ TiBps,  8.*1024.*1024.*1024.*1024. },
+{ TBps,   8.*1000.*1000.*1000.*1000. },
+{ NULL, 0 }
+};
+
+static int
+virRateToBps(const char *str, unsigned long *rate)
+{
+char *p;
+double bps;
+const struct rate_suffix *s;
+
+if (virStrToDouble(str, p, bps)  0)
+return -1;
+
+if (p == str)
+return -1;
+
+if (*p == '\0') {
+*rate = bps / 8.;   /* assume bytes/sec */
+return 0;
+}
+
+for (s = suffixes; s-name; ++s) {
+if