[libvirt] [PATCH v4 3/7] bandwidth: Add parsing and free functions
Laine Stump
laine at laine.org
Fri Jul 22 18:38:58 UTC 2011
On 07/22/2011 10:07 AM, Michal Privoznik wrote:
> These functions parse given XML node and return pointer to the
> output. Unknown elements are silently ignored. Attributes must
> be integer and must fit in unsigned long long.
>
> Free function frees elements of virBandwidth structure.
> ---
> src/conf/domain_conf.c | 5 ++
> src/conf/domain_conf.h | 1 +
> src/conf/network_conf.c | 7 ++
> src/conf/network_conf.h | 1 +
> src/libvirt_private.syms | 2 +
> src/util/network.c | 147 ++++++++++++++++++++++++++++++++++++++++++++++
> src/util/network.h | 2 +
> 7 files changed, 165 insertions(+), 0 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 919a75a..e11ad98 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -810,6 +810,8 @@ void virDomainNetDefFree(virDomainNetDefPtr def)
> VIR_FREE(def->filter);
> virNWFilterHashTableFree(def->filterparams);
>
> + virBandwidthDefFree(def->bandwidth);
> +
> VIR_FREE(def);
> }
>
> @@ -2823,6 +2825,9 @@ virDomainNetDefParseXML(virCapsPtr caps,
> xmlStrEqual(cur->name, BAD_CAST "actual")) {
> if (virDomainActualNetDefParseXML(cur, ctxt,&actual)< 0)
> goto error;
> + } else if (xmlStrEqual(cur->name, BAD_CAST "bandwidth")) {
> + if (!(def->bandwidth = virBandwidthDefParseNode(cur)))
> + goto error;
> }
> }
> cur = cur->next;
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 551946b..212f0c5 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -425,6 +425,7 @@ struct _virDomainNetDef {
> virDomainDeviceInfo info;
> char *filter;
> virNWFilterHashTablePtr filterparams;
> + virBandwidthPtr bandwidth;
> };
>
> enum virDomainChrDeviceType {
> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
> index 860eea3..01c094c 100644
> --- a/src/conf/network_conf.c
> +++ b/src/conf/network_conf.c
> @@ -167,6 +167,8 @@ void virNetworkDefFree(virNetworkDefPtr def)
>
> virNetworkDNSDefFree(def->dns);
>
> + virBandwidthDefFree(def->bandwidth);
> +
> VIR_FREE(def);
> }
>
> @@ -814,6 +816,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
> int nIps, nPortGroups, nForwardIfs;
> char *forwardDev = NULL;
> xmlNodePtr save = ctxt->node;
> + xmlNodePtr bandwidthNode = NULL;
>
> if (VIR_ALLOC(def)< 0) {
> virReportOOMError();
> @@ -848,6 +851,10 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
> /* Parse network domain information */
> def->domain = virXPathString("string(./domain[1]/@name)", ctxt);
>
> + if ((bandwidthNode = virXPathNode("./bandwidth", ctxt)) != NULL&&
> + (def->bandwidth = virBandwidthDefParseNode(bandwidthNode)) == NULL)
> + goto error;
Heh. Having the domain and network versions of adding bandwidth together
in the same patch points out just how inconsistent the style is in
various parts of our XML parsing - this one uses virXPathNode, and the
domain version uses a loop through all the elements with a bunch of
xmlStrEqual comparisons against each element's name. Almost perl-like in
its quest to do the same thing in as many different ways as possible :-P
(BTW, I still would have preferred to have the patches organized as 1)
all levels of change for bandwidth object, 2) all levels of change for
domain parse/format changes (RNG, data, parse, format, test cases,
docs), 3) all levels of change for network parse/format changes, 4)
implement the actual functionality. However, the end result is the same,
the patches provided pass make check at each step (so bisecting isn't
broken, and the end result is the same, so I'm fine with it going in
like this.)
> +
> /* Parse bridge information */
> def->bridge = virXPathString("string(./bridge[1]/@name)", ctxt);
> stp = virXPathString("string(./bridge[1]/@stp)", ctxt);
> diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
> index 92cc03e..6d0845e 100644
> --- a/src/conf/network_conf.h
> +++ b/src/conf/network_conf.h
> @@ -154,6 +154,7 @@ struct _virNetworkDef {
>
> size_t nPortGroups;
> virPortGroupDefPtr portGroups;
> + virBandwidthPtr bandwidth;
Changed into a pointer to the object, so we check for presence of the
element by looking for a NULL pointer instead of a special value of one
of the attributes.
> };
>
> typedef struct _virNetworkObj virNetworkObj;
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index acf7bb1..832c8a6 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -707,6 +707,8 @@ nlComm;
>
>
> # network.h
> +virBandwidthDefFree;
> +virBandwidthDefParseNode;
> virSocketAddrBroadcast;
> virSocketAddrBroadcastByPrefix;
> virSocketAddrIsNetmask;
> diff --git a/src/util/network.c b/src/util/network.c
> index 64f5645..2ba6f76 100644
> --- a/src/util/network.c
> +++ b/src/util/network.c
> @@ -883,3 +883,150 @@ virVirtualPortProfileFormat(virBufferPtr buf,
>
> virBufferAsprintf(buf, "%s</virtualport>\n", indent);
> }
> +
> +static int
> +virBandwidthParseChildDefNode(xmlNodePtr node, virRatePtr rate)
> +{
> + int ret = -1;
> + char *average = NULL;
> + char *peak = NULL;
> + char *burst = NULL;
> +
> + if (!node || !rate) {
> + virSocketError(VIR_ERR_INVALID_ARG, "%s",
> + _("invalid argument supplied"));
> + return -1;
> + }
> +
> + average = virXMLPropString(node, "average");
> + peak = virXMLPropString(node, "peak");
> + burst = virXMLPropString(node, "burst");
> +
> + if (average) {
> + if (virStrToLong_ull(average, NULL, 10,&rate->average)< 0) {
> + virSocketError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("could not convert %s"),
> + average);
> + goto cleanup;
> + }
> + } else {
> + virSocketError(VIR_ERR_XML_DETAIL, "%s",
> + _("Missing mandatory average attribute"));
> + goto cleanup;
> + }
> +
> + if (peak&& virStrToLong_ull(peak, NULL, 10,&rate->peak)< 0) {
> + virSocketError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("could not convert %s"),
> + peak);
> + goto cleanup;
> + }
> +
> + if (burst&& virStrToLong_ull(burst, NULL, 10,&rate->burst)< 0) {
> + virSocketError(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
> + *
> + * Parse bandwidth XML and return pointer to structure
> + *
> + * Returns !NULL on success, NULL on error.
> + */
> +virBandwidthPtr
> +virBandwidthDefParseNode(xmlNodePtr node)
> +{
> + virBandwidthPtr def = NULL;
> + xmlNodePtr cur = node->children;
> + xmlNodePtr in = NULL, out = NULL;
> +
> + if (VIR_ALLOC(def)< 0) {
> + virReportOOMError();
> + return NULL;
> + }
> +
> + if (!node || !xmlStrEqual(node->name, BAD_CAST "bandwidth")) {
> + virSocketError(VIR_ERR_INVALID_ARG, "%s",
> + _("invalid argument supplied"));
> + goto error;
> + }
> +
> + while (cur) {
> + if (cur->type == XML_ELEMENT_NODE) {
> + if (xmlStrEqual(cur->name, BAD_CAST "inbound")) {
> + if (in) {
> + virSocketError(VIR_ERR_XML_DETAIL, "%s",
> + _("Only one child<inbound> "
> + "element allowed"));
> + goto error;
> + }
> + in = cur;
> + } else if (xmlStrEqual(cur->name, BAD_CAST "outbound")) {
> + if (out) {
> + virSocketError(VIR_ERR_XML_DETAIL, "%s",
> + _("Only one child<outbound> "
> + "element allowed"));
> + goto error;
> + }
> + out = cur;
> + }
> + /* Silently ignore unknown elements */
> + }
> + cur = cur->next;
> + }
> +
> + if (in) {
> + if (VIR_ALLOC(def->in)< 0) {
> + virReportOOMError();
> + goto error;
> + }
> +
> + if (virBandwidthParseChildDefNode(in, def->in)< 0) {
> + /* helper reported error for us */
> + goto error;
> + }
> + }
> +
> + if (out) {
> + if (VIR_ALLOC(def->out)< 0) {
> + virReportOOMError();
> + goto error;
> + }
> +
> + if (virBandwidthParseChildDefNode(out, def->out)< 0) {
> + /* helper reported error for us */
> + goto error;
> + }
> + }
> +
> + return def;
> +
> +error:
> + virBandwidthDefFree(def);
> + return NULL;
> +}
> +
> +void
> +virBandwidthDefFree(virBandwidthPtr def)
> +{
> + if (!def)
> + return;
> +
> + VIR_FREE(def->in);
> + VIR_FREE(def->out);
> + VIR_FREE(def);
> +}
virBandwidthFree() needs to be added to the list of "VIR_FREE()-like"
functions in cfg.mk (be sure to keep the list alphabetical!)
ACK with that one nit fixed.
> diff --git a/src/util/network.h b/src/util/network.h
> index 3c090d8..4d35388 100644
> --- a/src/util/network.h
> +++ b/src/util/network.h
> @@ -150,4 +150,6 @@ virVirtualPortProfileFormat(virBufferPtr buf,
> virVirtualPortProfileParamsPtr virtPort,
> const char *indent);
>
> +virBandwidthPtr virBandwidthDefParseNode(xmlNodePtr node);
> +void virBandwidthDefFree(virBandwidthPtr def);
> #endif /* __VIR_NETWORK_H__ */
More information about the libvir-list
mailing list