10:31 a.m.
For background, see
https://listman.redhat.com/archives/libvir-list/2021-April/msg00668.html
Changes since V1:
* Split up VIR_FREE'd and reused ´g_autofree xmlNodePtr *´ variables.
Tim Wiederhake (6):
virNodeDeviceDefParseXML: Use g_auto*
virDomainNumatuneNodeParseXML: Use virXMLProp*
virDomainNumatuneNodeParseXML: Use g_autofree
virDomainNumaDefNodeDistanceParseXML: Use virXMLProp*
virDomainNumaDefParseXML: Use virXMLProp*
virDomainNumaDefParseXML: Use g_autofree
src/conf/node_device_conf.c | 44 ++-
src/conf/numa_conf.c | 305 ++++++------------
.../hugepages-memaccess-invalid.err | 2 +-
3 files changed, 110 insertions(+), 241 deletions(-)
--
2.26.3
10:31 a.m.
New subject: [libvirt PATCH v2 1/6] virNodeDeviceDefParseXML: Use g_auto*
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/conf/node_device_conf.c | 44 +++++++++++++++----------------------
1 file changed, 18 insertions(+), 26 deletions(-)
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 4477a8d9d2..861f43f6c4 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -2059,21 +2059,20 @@ virNodeDeviceDefParseXML(xmlXPathContextPtr ctxt,
int create,
const char *virt_type)
{
- virNodeDeviceDef *def;
+ g_autoptr(virNodeDeviceDef) def = g_new0(virNodeDeviceDef, 1);
virNodeDevCapsDef **next_cap;
- xmlNodePtr *nodes = NULL;
+ g_autofree xmlNodePtr *devnode = NULL;
+ g_autofree xmlNodePtr *capability = NULL;
int n, m;
size_t i;
- def = g_new0(virNodeDeviceDef, 1);
-
/* Extract device name */
if (create == EXISTING_DEVICE) {
def->name = virXPathString("string(./name[1])", ctxt);
if (!def->name) {
virReportError(VIR_ERR_NO_NAME, NULL);
- goto error;
+ return NULL;
}
} else {
def->name = g_strdup("new device");
@@ -2082,27 +2081,27 @@ virNodeDeviceDefParseXML(xmlXPathContextPtr ctxt,
def->sysfs_path = virXPathString("string(./path[1])", ctxt);
/* Parse devnodes */
- if ((n = virXPathNodeSet("./devnode", ctxt, &nodes)) < 0)
- goto error;
+ if ((n = virXPathNodeSet("./devnode", ctxt, &devnode)) < 0)
+ return NULL;
def->devlinks = g_new0(char *, n + 1);
for (i = 0, m = 0; i < n; i++) {
- xmlNodePtr node = nodes[i];
+ xmlNodePtr node = devnode[i];
virNodeDevDevnodeType val;
if (virXMLPropEnum(node, "type", virNodeDevDevnodeTypeFromString,
VIR_XML_PROP_REQUIRED, &val) < 0)
- goto error;
+ return NULL;
switch (val) {
case VIR_NODE_DEV_DEVNODE_DEV:
if (!(def->devnode = virXMLNodeContentString(node)))
- goto error;
+ return NULL;
break;
case VIR_NODE_DEV_DEVNODE_LINK:
if (!(def->devlinks[m++] = virXMLNodeContentString(node)))
- goto error;
+ return NULL;
break;
case VIR_NODE_DEV_DEVNODE_LAST:
break;
@@ -2118,7 +2117,7 @@ virNodeDeviceDefParseXML(xmlXPathContextPtr ctxt,
_("when providing parent wwnn='%s', the "
"wwpn must also be provided"),
def->parent_wwnn);
- goto error;
+ return NULL;
}
if (!def->parent_wwnn && def->parent_wwpn) {
@@ -2126,42 +2125,35 @@ virNodeDeviceDefParseXML(xmlXPathContextPtr ctxt,
_("when providing parent wwpn='%s', the "
"wwnn must also be provided"),
def->parent_wwpn);
- goto error;
+ return NULL;
}
def->parent_fabric_wwn =
virXPathString("string(./parent[1]/@fabric_wwn)",
ctxt);
/* Parse device capabilities */
- VIR_FREE(nodes);
- if ((n = virXPathNodeSet("./capability", ctxt, &nodes)) < 0)
- goto error;
+ if ((n = virXPathNodeSet("./capability", ctxt, &capability)) < 0)
+ return NULL;
if (n == 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("no device capabilities for '%s'"),
def->name);
- goto error;
+ return NULL;
}
next_cap = &def->caps;
for (i = 0; i < n; i++) {
*next_cap = virNodeDevCapsDefParseXML(ctxt, def,
- nodes[i],
+ capability[i],
create,
virt_type);
if (!*next_cap)
- goto error;
+ return NULL;
next_cap = &(*next_cap)->next;
}
- VIR_FREE(nodes);
-
- return def;
- error:
- virNodeDeviceDefFree(def);
- VIR_FREE(nodes);
- return NULL;
+ return g_steal_pointer(&def);
}
--
2.26.3
10:31 a.m.
New subject: [libvirt PATCH v2 2/6] virDomainNumatuneNodeParseXML: Use virXMLProp*
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/conf/numa_conf.c | 45 +++++++++++++-------------------------------
1 file changed, 13 insertions(+), 32 deletions(-)
diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index 932af4a185..bae59ac7b8 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -186,25 +186,13 @@ virDomainNumatuneNodeParseXML(virDomainNuma *numa,
}
for (i = 0; i < n; i++) {
- int mode = 0;
unsigned int cellid = 0;
virDomainNumaNode *mem_node = NULL;
xmlNodePtr cur_node = nodes[i];
- tmp = virXMLPropString(cur_node, "cellid");
- if (!tmp) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("Missing required cellid attribute "
- "in memnode element"));
+ if (virXMLPropUInt(cur_node, "cellid", 10, VIR_XML_PROP_REQUIRED,
+ &cellid) < 0)
goto cleanup;
- }
- if (virStrToLong_uip(tmp, NULL, 10, &cellid) < 0) {
- virReportError(VIR_ERR_XML_ERROR,
- _("Invalid cellid attribute in memnode element:
%s"),
- tmp);
- goto cleanup;
- }
- VIR_FREE(tmp);
if (cellid >= numa->nmem_nodes) {
virReportError(VIR_ERR_XML_ERROR, "%s",
@@ -222,25 +210,18 @@ virDomainNumatuneNodeParseXML(virDomainNuma *numa,
goto cleanup;
}
- tmp = virXMLPropString(cur_node, "mode");
- if (!tmp) {
- mem_node->mode = VIR_DOMAIN_NUMATUNE_MEM_STRICT;
- } else {
- if ((mode = virDomainNumatuneMemModeTypeFromString(tmp)) < 0) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("Invalid mode attribute in memnode
element"));
- goto cleanup;
- }
+ if (virXMLPropEnumDefault(cur_node, "mode",
+ virDomainNumatuneMemModeTypeFromString,
+ VIR_XML_PROP_NONE, &mem_node->mode,
+ VIR_DOMAIN_NUMATUNE_MEM_STRICT) < 0)
+ goto cleanup;
- if (numa->memory.mode == VIR_DOMAIN_NUMATUNE_MEM_RESTRICTIVE &&
- mode != VIR_DOMAIN_NUMATUNE_MEM_RESTRICTIVE) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("'restrictive' mode is required in memnode
element "
- "when mode is 'restrictive' in memory
element"));
- goto cleanup;
- }
- VIR_FREE(tmp);
- mem_node->mode = mode;
+ if (numa->memory.mode == VIR_DOMAIN_NUMATUNE_MEM_RESTRICTIVE &&
+ mem_node->mode != VIR_DOMAIN_NUMATUNE_MEM_RESTRICTIVE) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("'restrictive' mode is required in memnode
element "
+ "when mode is 'restrictive' in memory
element"));
+ goto cleanup;
}
tmp = virXMLPropString(cur_node, "nodeset");
--
2.26.3
10:31 a.m.
New subject: [libvirt PATCH v2 3/6] virDomainNumatuneNodeParseXML: Use g_autofree
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/conf/numa_conf.c | 34 ++++++++++++++--------------------
1 file changed, 14 insertions(+), 20 deletions(-)
diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index bae59ac7b8..15bfda3aa1 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -155,16 +155,14 @@ static int
virDomainNumatuneNodeParseXML(virDomainNuma *numa,
xmlXPathContextPtr ctxt)
{
- char *tmp = NULL;
int n = 0;
- int ret = -1;
size_t i = 0;
- xmlNodePtr *nodes = NULL;
+ g_autofree xmlNodePtr *nodes = NULL;
if ((n = virXPathNodeSet("./numatune/memnode", ctxt, &nodes)) < 0)
{
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Cannot extract memnode nodes"));
- goto cleanup;
+ return -1;
}
if (!n)
@@ -175,30 +173,31 @@ virDomainNumatuneNodeParseXML(virDomainNuma *numa,
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("Per-node binding is not compatible with "
"automatic NUMA placement."));
- goto cleanup;
+ return -1;
}
if (!numa->nmem_nodes) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("Element 'memnode' is invalid without "
"any guest NUMA cells"));
- goto cleanup;
+ return -1;
}
for (i = 0; i < n; i++) {
unsigned int cellid = 0;
virDomainNumaNode *mem_node = NULL;
xmlNodePtr cur_node = nodes[i];
+ g_autofree char *tmp = NULL;
if (virXMLPropUInt(cur_node, "cellid", 10, VIR_XML_PROP_REQUIRED,
&cellid) < 0)
- goto cleanup;
+ return -1;
if (cellid >= numa->nmem_nodes) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("Argument 'cellid' in memnode element must
"
"correspond to existing guest's NUMA cell"));
- goto cleanup;
+ return -1;
}
mem_node = &numa->mem_nodes[cellid];
@@ -207,21 +206,21 @@ virDomainNumatuneNodeParseXML(virDomainNuma *numa,
virReportError(VIR_ERR_XML_ERROR,
_("Multiple memnode elements with cellid %u"),
cellid);
- goto cleanup;
+ return -1;
}
if (virXMLPropEnumDefault(cur_node, "mode",
virDomainNumatuneMemModeTypeFromString,
VIR_XML_PROP_NONE, &mem_node->mode,
VIR_DOMAIN_NUMATUNE_MEM_STRICT) < 0)
- goto cleanup;
+ return -1;
if (numa->memory.mode == VIR_DOMAIN_NUMATUNE_MEM_RESTRICTIVE &&
mem_node->mode != VIR_DOMAIN_NUMATUNE_MEM_RESTRICTIVE) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("'restrictive' mode is required in memnode
element "
"when mode is 'restrictive' in memory
element"));
- goto cleanup;
+ return -1;
}
tmp = virXMLPropString(cur_node, "nodeset");
@@ -229,24 +228,19 @@ virDomainNumatuneNodeParseXML(virDomainNuma *numa,
virReportError(VIR_ERR_XML_ERROR, "%s",
_("Missing required nodeset attribute "
"in memnode element"));
- goto cleanup;
+ return -1;
}
if (virBitmapParse(tmp, &mem_node->nodeset, VIR_DOMAIN_CPUMASK_LEN) <
0)
- goto cleanup;
+ return -1;
if (virBitmapIsAllClear(mem_node->nodeset)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Invalid value of 'nodeset': %s"), tmp);
- goto cleanup;
+ return -1;
}
- VIR_FREE(tmp);
}
- ret = 0;
- cleanup:
- VIR_FREE(nodes);
- VIR_FREE(tmp);
- return ret;
+ return 0;
}
int
--
2.26.3
10:31 a.m.
New subject: [libvirt PATCH v2 4/6] virDomainNumaDefNodeDistanceParseXML: Use virXMLProp*
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/conf/numa_conf.c | 42 ++++--------------------------------------
1 file changed, 4 insertions(+), 38 deletions(-)
diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index 15bfda3aa1..60cd7767ef 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -742,7 +742,6 @@ virDomainNumaDefNodeDistanceParseXML(virDomainNuma *def,
{
int ret = -1;
int sibling;
- char *tmp = NULL;
xmlNodePtr *nodes = NULL;
size_t i, ndistances = def->nmem_nodes;
@@ -764,24 +763,9 @@ virDomainNumaDefNodeDistanceParseXML(virDomainNuma *def,
virDomainNumaDistance *rdist;
unsigned int sibling_id, sibling_value;
- /* siblings are in order of parsing or explicitly numbered */
- if (!(tmp = virXMLPropString(nodes[i], "id"))) {
- virReportError(VIR_ERR_XML_ERROR,
- _("Missing 'id' attribute in NUMA "
- "distances under 'cell id %d'"),
- cur_cell);
- goto cleanup;
- }
-
- /* The "id" needs to be applicable */
- if (virStrToLong_uip(tmp, NULL, 10, &sibling_id) < 0) {
- virReportError(VIR_ERR_XML_ERROR,
- _("Invalid 'id' attribute in NUMA "
- "distances for sibling: '%s'"),
- tmp);
+ if (virXMLPropUInt(nodes[i], "id", 10, VIR_XML_PROP_REQUIRED,
+ &sibling_id) < 0)
goto cleanup;
- }
- VIR_FREE(tmp);
/* The "id" needs to be within numa/cell range */
if (sibling_id >= ndistances) {
@@ -792,26 +776,9 @@ virDomainNumaDefNodeDistanceParseXML(virDomainNuma *def,
goto cleanup;
}
- /* We need a locality value. Check and correct
- * distance to local and distance to remote node.
- */
- if (!(tmp = virXMLPropString(nodes[i], "value"))) {
- virReportError(VIR_ERR_XML_ERROR,
- _("Missing 'value' attribute in NUMA distances
"
- "under 'cell id %d' for 'sibling id
%d'"),
- cur_cell, sibling_id);
- goto cleanup;
- }
-
- /* The "value" needs to be applicable */
- if (virStrToLong_uip(tmp, NULL, 10, &sibling_value) < 0) {
- virReportError(VIR_ERR_XML_ERROR,
- _("'value %s' is invalid for "
- "'sibling id %d' under NUMA 'cell id
%d'"),
- tmp, sibling_id, cur_cell);
+ if (virXMLPropUInt(nodes[i], "value", 10, VIR_XML_PROP_REQUIRED,
+ &sibling_value) < 0)
goto cleanup;
- }
- VIR_FREE(tmp);
/* Assure LOCAL_DISTANCE <= "value" <= UNREACHABLE
* and correct LOCAL_DISTANCE setting if such applies.
@@ -866,7 +833,6 @@ virDomainNumaDefNodeDistanceParseXML(virDomainNuma *def,
def->mem_nodes[i].ndistances = 0;
}
VIR_FREE(nodes);
- VIR_FREE(tmp);
return ret;
}
--
2.26.3
10:32 a.m.
New subject: [libvirt PATCH v2 5/6] virDomainNumaDefParseXML: Use virXMLProp*
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/conf/numa_conf.c | 128 +++++-------------
.../hugepages-memaccess-invalid.err | 2 +-
2 files changed, 35 insertions(+), 95 deletions(-)
diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index 60cd7767ef..9fe4998951 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -954,26 +954,23 @@ virDomainNumaDefParseXML(virDomainNuma *def,
for (i = 0; i < n; i++) {
VIR_XPATH_NODE_AUTORESTORE(ctxt)
int rc;
- unsigned int cur_cell = i;
+ unsigned int cur_cell;
- /* cells are in order of parsing or explicitly numbered */
- if ((tmp = virXMLPropString(nodes[i], "id"))) {
- if (virStrToLong_uip(tmp, NULL, 10, &cur_cell) < 0) {
- virReportError(VIR_ERR_XML_ERROR,
- _("Invalid 'id' attribute in NUMA cell:
'%s'"),
- tmp);
- goto cleanup;
- }
+ if ((rc = virXMLPropUInt(nodes[i], "id", 10, VIR_XML_PROP_NONE,
+ &cur_cell)) < 0)
+ goto cleanup;
- if (cur_cell >= n) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("Exactly one 'cell' element per guest
"
- "NUMA cell allowed, non-contiguous ranges or
"
- "ranges not starting from 0 are not
allowed"));
- goto cleanup;
- }
+ if (rc == 0)
+ cur_cell = i;
+
+ /* cells are in order of parsing or explicitly numbered */
+ if (cur_cell >= n) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("Exactly one 'cell' element per guest "
+ "NUMA cell allowed, non-contiguous ranges or "
+ "ranges not starting from 0 are not allowed"));
+ goto cleanup;
}
- VIR_FREE(tmp);
if (def->mem_nodes[cur_cell].mem) {
virReportError(VIR_ERR_XML_ERROR,
@@ -1013,29 +1010,15 @@ virDomainNumaDefParseXML(virDomainNuma *def,
&def->mem_nodes[cur_cell].mem, true, false) <
0)
goto cleanup;
- if ((tmp = virXMLPropString(nodes[i], "memAccess"))) {
- if ((rc = virDomainMemoryAccessTypeFromString(tmp)) <= 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Invalid 'memAccess' attribute value
'%s'"),
- tmp);
- goto cleanup;
- }
-
- def->mem_nodes[cur_cell].memAccess = rc;
- VIR_FREE(tmp);
- }
-
- if ((tmp = virXMLPropString(nodes[i], "discard"))) {
- if ((rc = virTristateBoolTypeFromString(tmp)) <= 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Invalid 'discard' attribute value
'%s'"),
- tmp);
- goto cleanup;
- }
+ if (virXMLPropEnum(nodes[i], "memAccess",
+ virDomainMemoryAccessTypeFromString,
+ VIR_XML_PROP_NONZERO,
+ &def->mem_nodes[cur_cell].memAccess) < 0)
+ goto cleanup;
- def->mem_nodes[cur_cell].discard = rc;
- VIR_FREE(tmp);
- }
+ if (virXMLPropTristateBool(nodes[i], "discard", VIR_XML_PROP_NONE,
+ &def->mem_nodes[cur_cell].discard) < 0)
+ goto cleanup;
/* Parse NUMA distances info */
if (virDomainNumaDefNodeDistanceParseXML(def, ctxt, cur_cell) < 0)
@@ -1057,24 +1040,15 @@ virDomainNumaDefParseXML(virDomainNuma *def,
unsigned int initiator;
unsigned int target;
unsigned int cache = 0;
- int accessType;
+ virDomainMemoryLatency accessType;
unsigned long long value;
if (virXMLNodeNameEqual(nodes[i], "latency")) {
type = VIR_DOMAIN_NUMA_INTERCONNECT_TYPE_LATENCY;
- if (!(tmp = virXMLPropString(nodes[i], "value"))) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("Missing 'value' attribute in NUMA
interconnects"));
- goto cleanup;
- }
-
- if (virStrToLong_ullp(tmp, NULL, 10, &value) < 0) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("Invalid 'value' attribute in NUMA
interconnects"));
+ if (virXMLPropULongLong(nodes[i], "value", 10,
+ VIR_XML_PROP_REQUIRED, &value) < 0)
goto cleanup;
- }
- VIR_FREE(tmp);
} else if (virXMLNodeNameEqual(nodes[i], "bandwidth")) {
VIR_XPATH_NODE_AUTORESTORE(ctxt)
type = VIR_DOMAIN_NUMA_INTERCONNECT_TYPE_BANDWIDTH;
@@ -1088,56 +1062,22 @@ virDomainNumaDefParseXML(virDomainNuma *def,
continue;
}
- if (!(tmp = virXMLPropString(nodes[i], "initiator"))) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("Missing 'initiator' attribute in NUMA
interconnects"));
+ if (virXMLPropUInt(nodes[i], "initiator", 10, VIR_XML_PROP_REQUIRED,
+ &initiator) < 0)
goto cleanup;
- }
-
- if (virStrToLong_uip(tmp, NULL, 10, &initiator) < 0) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("Invalid 'initiator' attribute in NUMA
interconnects"));
- goto cleanup;
- }
- VIR_FREE(tmp);
-
- if (!(tmp = virXMLPropString(nodes[i], "target"))) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("Missing 'target' attribute in NUMA
interconnects"));
- goto cleanup;
- }
- if (virStrToLong_uip(tmp, NULL, 10, &target) < 0) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("Invalid 'target' attribute in NUMA
interconnects"));
+ if (virXMLPropUInt(nodes[i], "target", 10, VIR_XML_PROP_REQUIRED,
+ &target) < 0)
goto cleanup;
- }
- VIR_FREE(tmp);
-
- /* cache attribute is optional */
- if ((tmp = virXMLPropString(nodes[i], "cache"))) {
- if (virStrToLong_uip(tmp, NULL, 10, &cache) < 0 ||
- cache == 0) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("Invalid 'cache' attribute in NUMA
interconnects"));
- goto cleanup;
- }
- }
- VIR_FREE(tmp);
-
- if (!(tmp = virXMLPropString(nodes[i], "type"))) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("Missing 'type' attribute in NUMA
interconnects"));
+ if (virXMLPropUInt(nodes[i], "cache", 10, VIR_XML_PROP_NONE,
+ &cache) < 0)
goto cleanup;
- }
- if ((accessType = virDomainMemoryLatencyTypeFromString(tmp)) <= 0) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("Invalid 'type' attribute in NUMA
interconnects"));
+ if (virXMLPropEnum(nodes[i], "type",
virDomainMemoryLatencyTypeFromString,
+ VIR_XML_PROP_REQUIRED | VIR_XML_PROP_NONZERO,
+ &accessType) < 0)
goto cleanup;
- }
- VIR_FREE(tmp);
def->interconnects[i] = (virDomainNumaInterconnect) {type, initiator, target,
cache, accessType, value};
diff --git a/tests/qemuxml2argvdata/hugepages-memaccess-invalid.err
b/tests/qemuxml2argvdata/hugepages-memaccess-invalid.err
index 75ddd3e282..a1f07fb04d 100644
--- a/tests/qemuxml2argvdata/hugepages-memaccess-invalid.err
+++ b/tests/qemuxml2argvdata/hugepages-memaccess-invalid.err
@@ -1 +1 @@
-unsupported configuration: Invalid 'memAccess' attribute value 'invalid'
+XML error: Invalid value for attribute 'memAccess' in element 'cell':
'invalid'.
--
2.26.3
10:32 a.m.
New subject: [libvirt PATCH v2 6/6] virDomainNumaDefParseXML: Use g_autofree
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/conf/numa_conf.c | 84 +++++++++++++++++++++-----------------------
1 file changed, 40 insertions(+), 44 deletions(-)
diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index 9fe4998951..525bc28962 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -932,20 +932,20 @@ int
virDomainNumaDefParseXML(virDomainNuma *def,
xmlXPathContextPtr ctxt)
{
- xmlNodePtr *nodes = NULL;
- char *tmp = NULL;
+ g_autofree xmlNodePtr *cell = NULL;
+ g_autofree xmlNodePtr *interconnect = NULL;
+
int n;
size_t i, j;
- int ret = -1;
/* check if NUMA definition is present */
if (!virXPathNode("./cpu/numa[1]", ctxt))
return 0;
- if ((n = virXPathNodeSet("./cpu/numa[1]/cell", ctxt, &nodes)) <= 0)
{
+ if ((n = virXPathNodeSet("./cpu/numa[1]/cell", ctxt, &cell)) <= 0)
{
virReportError(VIR_ERR_XML_ERROR, "%s",
_("NUMA topology defined without NUMA cells"));
- goto cleanup;
+ return -1;
}
def->mem_nodes = g_new0(struct _virDomainNumaNode, n);
@@ -953,12 +953,13 @@ virDomainNumaDefParseXML(virDomainNuma *def,
for (i = 0; i < n; i++) {
VIR_XPATH_NODE_AUTORESTORE(ctxt)
+ g_autofree char *tmp = NULL;
int rc;
unsigned int cur_cell;
- if ((rc = virXMLPropUInt(nodes[i], "id", 10, VIR_XML_PROP_NONE,
+ if ((rc = virXMLPropUInt(cell[i], "id", 10, VIR_XML_PROP_NONE,
&cur_cell)) < 0)
- goto cleanup;
+ return -1;
if (rc == 0)
cur_cell = i;
@@ -969,25 +970,24 @@ virDomainNumaDefParseXML(virDomainNuma *def,
_("Exactly one 'cell' element per guest "
"NUMA cell allowed, non-contiguous ranges or "
"ranges not starting from 0 are not allowed"));
- goto cleanup;
+ return -1;
}
if (def->mem_nodes[cur_cell].mem) {
virReportError(VIR_ERR_XML_ERROR,
_("Duplicate NUMA cell info for cell id
'%u'"),
cur_cell);
- goto cleanup;
+ return -1;
}
- if ((tmp = virXMLPropString(nodes[i], "cpus"))) {
+ if ((tmp = virXMLPropString(cell[i], "cpus"))) {
g_autoptr(virBitmap) cpumask = NULL;
if (virBitmapParse(tmp, &cpumask, VIR_DOMAIN_CPUMASK_LEN) < 0)
- goto cleanup;
+ return -1;
if (!virBitmapIsAllClear(cpumask))
def->mem_nodes[cur_cell].cpumask = g_steal_pointer(&cpumask);
- VIR_FREE(tmp);
}
for (j = 0; j < n; j++) {
@@ -1001,38 +1001,38 @@ virDomainNumaDefParseXML(virDomainNuma *def,
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("NUMA cells %u and %zu have overlapping vCPU
ids"),
cur_cell, j);
- goto cleanup;
+ return -1;
}
}
- ctxt->node = nodes[i];
+ ctxt->node = cell[i];
if (virDomainParseMemory("./@memory", "./@unit", ctxt,
&def->mem_nodes[cur_cell].mem, true, false) <
0)
- goto cleanup;
+ return -1;
- if (virXMLPropEnum(nodes[i], "memAccess",
+ if (virXMLPropEnum(cell[i], "memAccess",
virDomainMemoryAccessTypeFromString,
VIR_XML_PROP_NONZERO,
&def->mem_nodes[cur_cell].memAccess) < 0)
- goto cleanup;
+ return -1;
- if (virXMLPropTristateBool(nodes[i], "discard", VIR_XML_PROP_NONE,
+ if (virXMLPropTristateBool(cell[i], "discard", VIR_XML_PROP_NONE,
&def->mem_nodes[cur_cell].discard) < 0)
- goto cleanup;
+ return -1;
/* Parse NUMA distances info */
if (virDomainNumaDefNodeDistanceParseXML(def, ctxt, cur_cell) < 0)
- goto cleanup;
+ return -1;
/* Parse cache info */
if (virDomainNumaDefNodeCacheParseXML(def, ctxt, cur_cell) < 0)
- goto cleanup;
+ return -1;
}
- VIR_FREE(nodes);
if ((n = virXPathNodeSet("./cpu/numa[1]/interconnects[1]/latency|"
- "./cpu/numa[1]/interconnects[1]/bandwidth", ctxt,
&nodes)) < 0)
- goto cleanup;
+ "./cpu/numa[1]/interconnects[1]/bandwidth", ctxt,
+ &interconnect)) < 0)
+ return -1;
def->interconnects = g_new0(virDomainNumaInterconnect, n);
for (i = 0; i < n; i++) {
@@ -1043,53 +1043,49 @@ virDomainNumaDefParseXML(virDomainNuma *def,
virDomainMemoryLatency accessType;
unsigned long long value;
- if (virXMLNodeNameEqual(nodes[i], "latency")) {
+ if (virXMLNodeNameEqual(interconnect[i], "latency")) {
type = VIR_DOMAIN_NUMA_INTERCONNECT_TYPE_LATENCY;
- if (virXMLPropULongLong(nodes[i], "value", 10,
+ if (virXMLPropULongLong(interconnect[i], "value", 10,
VIR_XML_PROP_REQUIRED, &value) < 0)
- goto cleanup;
- } else if (virXMLNodeNameEqual(nodes[i], "bandwidth")) {
+ return -1;
+ } else if (virXMLNodeNameEqual(interconnect[i], "bandwidth")) {
VIR_XPATH_NODE_AUTORESTORE(ctxt)
type = VIR_DOMAIN_NUMA_INTERCONNECT_TYPE_BANDWIDTH;
- ctxt->node = nodes[i];
+ ctxt->node = interconnect[i];
if (virDomainParseMemory("./@value", "./@unit", ctxt,
&value, true, false) < 0)
- goto cleanup;
+ return -1;
} else {
/* Ignore yet unknown child elements. */
continue;
}
- if (virXMLPropUInt(nodes[i], "initiator", 10, VIR_XML_PROP_REQUIRED,
+ if (virXMLPropUInt(interconnect[i], "initiator", 10,
VIR_XML_PROP_REQUIRED,
&initiator) < 0)
- goto cleanup;
+ return -1;
- if (virXMLPropUInt(nodes[i], "target", 10, VIR_XML_PROP_REQUIRED,
+ if (virXMLPropUInt(interconnect[i], "target", 10,
VIR_XML_PROP_REQUIRED,
&target) < 0)
- goto cleanup;
+ return -1;
- if (virXMLPropUInt(nodes[i], "cache", 10, VIR_XML_PROP_NONE,
+ if (virXMLPropUInt(interconnect[i], "cache", 10, VIR_XML_PROP_NONE,
&cache) < 0)
- goto cleanup;
+ return -1;
- if (virXMLPropEnum(nodes[i], "type",
virDomainMemoryLatencyTypeFromString,
+ if (virXMLPropEnum(interconnect[i], "type",
+ virDomainMemoryLatencyTypeFromString,
VIR_XML_PROP_REQUIRED | VIR_XML_PROP_NONZERO,
&accessType) < 0)
- goto cleanup;
+ return -1;
def->interconnects[i] = (virDomainNumaInterconnect) {type, initiator, target,
cache, accessType, value};
def->ninterconnects++;
}
- ret = 0;
-
- cleanup:
- VIR_FREE(nodes);
- VIR_FREE(tmp);
- return ret;
+ return 0;
}
--
2.26.3
10:12 a.m.
On 5/13/21 11:31 AM, Tim Wiederhake wrote:
For background, see
https://listman.redhat.com/archives/libvir-list/2021-April/msg00668.html
Changes since V1:
* Split up VIR_FREE'd and reused ´g_autofree xmlNodePtr *´ variables.
Tim Wiederhake (6):
virNodeDeviceDefParseXML: Use g_auto*
virDomainNumatuneNodeParseXML: Use virXMLProp*
virDomainNumatuneNodeParseXML: Use g_autofree
virDomainNumaDefNodeDistanceParseXML: Use virXMLProp*
virDomainNumaDefParseXML: Use virXMLProp*
virDomainNumaDefParseXML: Use g_autofree
src/conf/node_device_conf.c | 44 ++-
src/conf/numa_conf.c | 305 ++++++------------
.../hugepages-memaccess-invalid.err | 2 +-
3 files changed, 110 insertions(+), 241 deletions(-)
Reviewed-by: Laine Stump <laine(a)redhat.com>
I'll push them as soon as simple smoke test build has completed
1324
days inactive
1325
days old
7 comments
2 participants
participants (2)
-
Laine Stump -
Tim Wiederhake