conf: Increase virNetDevBandwidthParse intelligence

There's this function virNetDevBandwidthParse which parses the
bandwidth XML snippet. But it's not clever much. For the
following XML it allocates the virNetDevBandwidth structure even
though it's completely empty:

    <bandwidth>
    </bandwidth>

Later in the code there are some places where we check if
bandwidth was set or not. And since we obtained pointer from the
parsing function we think that it is when in fact it isn't.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
This commit is contained in:
Michal Privoznik 2015-01-07 17:53:04 +01:00
parent 0ecd685109
commit a605025c21
5 changed files with 42 additions and 32 deletions

View File

@ -7325,8 +7325,9 @@ virDomainActualNetDefParseXML(xmlNodePtr node,
bandwidth_node = virXPathNode("./bandwidth", ctxt);
if (bandwidth_node &&
!(actual->bandwidth = virNetDevBandwidthParse(bandwidth_node,
actual->type)))
virNetDevBandwidthParse(&actual->bandwidth,
bandwidth_node,
actual->type) < 0)
goto error;
vlanNode = virXPathNode("./vlan", ctxt);
@ -7583,8 +7584,9 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt,
goto error;
}
} else if (xmlStrEqual(cur->name, BAD_CAST "bandwidth")) {
if (!(def->bandwidth = virNetDevBandwidthParse(cur,
def->type)))
if (virNetDevBandwidthParse(&def->bandwidth,
cur,
def->type) < 0)
goto error;
} else if (xmlStrEqual(cur->name, BAD_CAST "vlan")) {
if (virNetDevVlanParse(cur, ctxt, &def->vlan) < 0)

View File

@ -102,6 +102,7 @@ virNetDevBandwidthParseRate(xmlNodePtr node, virNetDevBandwidthRatePtr rate)
/**
* virNetDevBandwidthParse:
* @bandwidth: parsed bandwidth
* @node: XML node
* @net_type: one of virDomainNetType
*
@ -111,21 +112,23 @@ virNetDevBandwidthParseRate(xmlNodePtr node, virNetDevBandwidthRatePtr rate)
*
* Returns !NULL on success, NULL on error.
*/
virNetDevBandwidthPtr
virNetDevBandwidthParse(xmlNodePtr node,
int
virNetDevBandwidthParse(virNetDevBandwidthPtr *bandwidth,
xmlNodePtr node,
int net_type)
{
int ret = -1;
virNetDevBandwidthPtr def = NULL;
xmlNodePtr cur;
xmlNodePtr in = NULL, out = NULL;
if (VIR_ALLOC(def) < 0)
return NULL;
return ret;
if (!node || !xmlStrEqual(node->name, BAD_CAST "bandwidth")) {
virReportError(VIR_ERR_INVALID_ARG, "%s",
_("invalid argument supplied"));
goto error;
goto cleanup;
}
cur = node->children;
@ -137,7 +140,7 @@ virNetDevBandwidthParse(xmlNodePtr node,
virReportError(VIR_ERR_XML_DETAIL, "%s",
_("Only one child <inbound> "
"element allowed"));
goto error;
goto cleanup;
}
in = cur;
} else if (xmlStrEqual(cur->name, BAD_CAST "outbound")) {
@ -145,7 +148,7 @@ virNetDevBandwidthParse(xmlNodePtr node,
virReportError(VIR_ERR_XML_DETAIL, "%s",
_("Only one child <outbound> "
"element allowed"));
goto error;
goto cleanup;
}
out = cur;
}
@ -156,11 +159,11 @@ virNetDevBandwidthParse(xmlNodePtr node,
if (in) {
if (VIR_ALLOC(def->in) < 0)
goto error;
goto cleanup;
if (virNetDevBandwidthParseRate(in, def->in) < 0) {
/* helper reported error for us */
goto error;
goto cleanup;
}
if (def->in->floor && net_type != VIR_DOMAIN_NET_TYPE_NETWORK) {
@ -174,32 +177,37 @@ virNetDevBandwidthParse(xmlNodePtr node,
_("floor attribute is supported only for "
"interfaces of type network"));
}
goto error;
goto cleanup;
}
}
if (out) {
if (VIR_ALLOC(def->out) < 0)
goto error;
goto cleanup;
if (virNetDevBandwidthParseRate(out, def->out) < 0) {
/* helper reported error for us */
goto error;
goto cleanup;
}
if (def->out->floor) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("'floor' attribute allowed "
"only in <inbound> element"));
goto error;
goto cleanup;
}
}
return def;
if (!def->in && !def->out)
VIR_FREE(def);
error:
*bandwidth = def;
def = NULL;
ret = 0;
cleanup:
virNetDevBandwidthFree(def);
return NULL;
return ret;
}
static int

View File

@ -29,9 +29,10 @@
# include "virxml.h"
# include "domain_conf.h"
virNetDevBandwidthPtr virNetDevBandwidthParse(xmlNodePtr node,
int net_type)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
int virNetDevBandwidthParse(virNetDevBandwidthPtr *bandwidth,
xmlNodePtr node,
int net_type)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
int virNetDevBandwidthFormat(virNetDevBandwidthPtr def,
virBufferPtr buf)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);

View File

@ -1649,9 +1649,8 @@ virNetworkPortGroupParseXML(virPortGroupDefPtr def,
bandwidth_node = virXPathNode("./bandwidth", ctxt);
if (bandwidth_node &&
!(def->bandwidth = virNetDevBandwidthParse(bandwidth_node, -1))) {
virNetDevBandwidthParse(&def->bandwidth, bandwidth_node, -1) < 0)
goto cleanup;
}
vlanNode = virXPathNode("./vlan", ctxt);
if (vlanNode && virNetDevVlanParse(vlanNode, ctxt, &def->vlan) < 0)
@ -2088,8 +2087,8 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
/* Parse network domain information */
def->domain = virXPathString("string(./domain[1]/@name)", ctxt);
if ((bandwidthNode = virXPathNode("./bandwidth", ctxt)) != NULL &&
(def->bandwidth = virNetDevBandwidthParse(bandwidthNode, -1)) == NULL)
if ((bandwidthNode = virXPathNode("./bandwidth", ctxt)) &&
virNetDevBandwidthParse(&def->bandwidth, bandwidthNode, -1) < 0)
goto error;
vlanNode = virXPathNode("./vlan", ctxt);

View File

@ -43,6 +43,7 @@ struct testSetStruct {
#define PARSE(xml, var) \
do { \
int rc; \
xmlDocPtr doc; \
xmlXPathContextPtr ctxt = NULL; \
\
@ -54,11 +55,12 @@ struct testSetStruct {
&ctxt))) \
goto cleanup; \
\
(var) = virNetDevBandwidthParse(ctxt->node, \
VIR_DOMAIN_NET_TYPE_NETWORK); \
rc = virNetDevBandwidthParse(&(var), \
ctxt->node, \
VIR_DOMAIN_NET_TYPE_NETWORK); \
xmlFreeDoc(doc); \
xmlXPathFreeContext(ctxt); \
if (!(var)) \
if (rc < 0) \
goto cleanup; \
} while (0)
@ -127,9 +129,7 @@ mymain(void)
DO_TEST_SET(NULL, NULL);
DO_TEST_SET(("<bandwidth/>"),
(TC " qdisc del dev eth0 root\n"
TC " qdisc del dev eth0 ingress\n"));
DO_TEST_SET("<bandwidth/>", NULL);
DO_TEST_SET(("<bandwidth>"
" <inbound average='1024'/>"