5:01 p.m.
For background, see
https://listman.redhat.com/archives/libvir-list/2021-April/msg00668.html
Tim Wiederhake (10):
virNodeDeviceDefParseXML: Use g_auto*
virDomainNumatuneNodeParseXML: Use virXMLProp*
virDomainNumatuneNodeParseXML: Use g_autofree
virDomainNumaDefNodeDistanceParseXML: Use virXMLProp*
virDomainNumaDefParseXML: Use virXMLProp*
virDomainNumaDefParseXML: Use g_autofree
virStorageAdapterParseXMLFCHost: Use virXMLProp*
virStoragePoolDefParseSource: Use virXMLProp*
virStoragePoolDefParseSource: Use VIR_XPATH_NODE_AUTORESTORE
virStoragePoolDefParseXML: Use virXMLProp*
src/conf/node_device_conf.c | 34 +--
src/conf/numa_conf.c | 285 +++++-------------
src/conf/storage_adapter_conf.c | 17 +-
src/conf/storage_conf.c | 83 ++---
.../hugepages-memaccess-invalid.err | 2 +-
5 files changed, 125 insertions(+), 296 deletions(-)
--
2.26.3
5:01 p.m.
New subject: [libvirt PATCH 01/10] virNodeDeviceDefParseXML: Use g_auto*
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/conf/node_device_conf.c | 34 +++++++++++++---------------------
1 file changed, 13 insertions(+), 21 deletions(-)
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 4477a8d9d2..5ac046f768 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -2059,21 +2059,19 @@ 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 *nodes = 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");
@@ -2083,7 +2081,7 @@ virNodeDeviceDefParseXML(xmlXPathContextPtr ctxt,
/* Parse devnodes */
if ((n = virXPathNodeSet("./devnode", ctxt, &nodes)) < 0)
- goto error;
+ return NULL;
def->devlinks = g_new0(char *, n + 1);
@@ -2093,16 +2091,16 @@ virNodeDeviceDefParseXML(xmlXPathContextPtr ctxt,
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 +2116,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,7 +2124,7 @@ 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);
@@ -2134,13 +2132,13 @@ virNodeDeviceDefParseXML(xmlXPathContextPtr ctxt,
/* Parse device capabilities */
VIR_FREE(nodes);
if ((n = virXPathNodeSet("./capability", ctxt, &nodes)) < 0)
- goto error;
+ 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;
@@ -2150,18 +2148,12 @@ virNodeDeviceDefParseXML(xmlXPathContextPtr ctxt,
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
6:06 p.m.
New subject: [libvirt PATCH 01/10] virNodeDeviceDefParseXML: Use g_auto*
On 5/11/21 11:01 AM, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/conf/node_device_conf.c | 34 +++++++++++++---------------------
1 file changed, 13 insertions(+), 21 deletions(-)
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 4477a8d9d2..5ac046f768 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -2059,21 +2059,19 @@ 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 *nodes = 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");
@@ -2083,7 +2081,7 @@ virNodeDeviceDefParseXML(xmlXPathContextPtr ctxt,
/* Parse devnodes */
if ((n = virXPathNodeSet("./devnode", ctxt, &nodes)) < 0)
- goto error;
+ return NULL;
def->devlinks = g_new0(char *, n + 1);
@@ -2093,16 +2091,16 @@ virNodeDeviceDefParseXML(xmlXPathContextPtr ctxt,
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 +2116,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,7 +2124,7 @@ 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);
@@ -2134,13 +2132,13 @@ virNodeDeviceDefParseXML(xmlXPathContextPtr ctxt,
/* Parse device capabilities */
VIR_FREE(nodes);
The presence of a VIR_FREE() of a pointer that was declared g_autofree
points out that it's being re-used, which we had decided is a bad idea
(unless more discussion has happened on the topic that I missed). You
should instead define two separate g_autofree pointers, one for each use.
if ((n = virXPathNodeSet("./capability", ctxt,
&nodes)) < 0)
- goto error;
+ 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;
@@ -2150,18 +2148,12 @@ virNodeDeviceDefParseXML(xmlXPathContextPtr ctxt,
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);
}
5:01 p.m.
New subject: [libvirt PATCH 02/10] 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
5:01 p.m.
New subject: [libvirt PATCH 03/10] virDomainNumatuneNodeParseXML: Use g_autofree
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/conf/numa_conf.c | 33 ++++++++++++++-------------------
1 file changed, 14 insertions(+), 19 deletions(-)
diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index bae59ac7b8..531bdc6eba 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -155,16 +155,15 @@ static int
virDomainNumatuneNodeParseXML(virDomainNuma *numa,
xmlXPathContextPtr ctxt)
{
- char *tmp = NULL;
+ g_autofree 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,14 +174,14 @@ 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++) {
@@ -192,13 +191,13 @@ virDomainNumatuneNodeParseXML(virDomainNuma *numa,
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,20 @@ 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
6:11 p.m.
New subject: [libvirt PATCH 03/10] virDomainNumatuneNodeParseXML: Use g_autofree
On 5/11/21 11:01 AM, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/conf/numa_conf.c | 33 ++++++++++++++-------------------
1 file changed, 14 insertions(+), 19 deletions(-)
diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index bae59ac7b8..531bdc6eba 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -155,16 +155,15 @@ static int
virDomainNumatuneNodeParseXML(virDomainNuma *numa,
xmlXPathContextPtr ctxt)
{
- char *tmp = NULL;
+ g_autofree char *tmp = NULL;
tmp should be defined inside the for loop so that it's autofreed after
each iteration.
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,14 +174,14 @@ 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++) {
@@ -192,13 +191,13 @@ virDomainNumatuneNodeParseXML(virDomainNuma *numa,
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,20 @@ 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
5:01 p.m.
New subject: [libvirt PATCH 04/10] 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 531bdc6eba..e631bfa341 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -743,7 +743,6 @@ virDomainNumaDefNodeDistanceParseXML(virDomainNuma *def,
{
int ret = -1;
int sibling;
- char *tmp = NULL;
xmlNodePtr *nodes = NULL;
size_t i, ndistances = def->nmem_nodes;
@@ -765,24 +764,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) {
@@ -793,26 +777,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.
@@ -867,7 +834,6 @@ virDomainNumaDefNodeDistanceParseXML(virDomainNuma *def,
def->mem_nodes[i].ndistances = 0;
}
VIR_FREE(nodes);
- VIR_FREE(tmp);
return ret;
}
--
2.26.3
6:15 p.m.
New subject: [libvirt PATCH 04/10] virDomainNumaDefNodeDistanceParseXML: Use virXMLProp*
On 5/11/21 11:01 AM, Tim Wiederhake wrote:
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 531bdc6eba..e631bfa341 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -743,7 +743,6 @@ virDomainNumaDefNodeDistanceParseXML(virDomainNuma *def,
{
int ret = -1;
int sibling;
- char *tmp = NULL;
xmlNodePtr *nodes = NULL;
(Not about *this* patch, but rather the absence of a related patch) The
other "Use virXMLProp*" patches have a companion patch that makes
"nodes" a g_autofree, but you haven't included that patch for this
function.
size_t i, ndistances = def->nmem_nodes;
@@ -765,24 +764,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) {
@@ -793,26 +777,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.
@@ -867,7 +834,6 @@ virDomainNumaDefNodeDistanceParseXML(virDomainNuma *def,
def->mem_nodes[i].ndistances = 0;
}
VIR_FREE(nodes);
- VIR_FREE(tmp);
return ret;
}
5:32 p.m.
New subject: [libvirt PATCH 04/10] virDomainNumaDefNodeDistanceParseXML: Use virXMLProp*
On Tue, 2021-05-11 at 12:15 -0400, Laine Stump wrote:
On 5/11/21 11:01 AM, Tim Wiederhake wrote:
> 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 531bdc6eba..e631bfa341 100644
> --- a/src/conf/numa_conf.c
> +++ b/src/conf/numa_conf.c
> @@ -743,7 +743,6 @@
> virDomainNumaDefNodeDistanceParseXML(virDomainNuma *def,
> {
> int ret = -1;
> int sibling;
> - char *tmp = NULL;
> xmlNodePtr *nodes = NULL;
(Not about *this* patch, but rather the absence of a related patch)
The
other "Use virXMLProp*" patches have a companion patch that makes
"nodes" a g_autofree, but you haven't included that patch for this
function.
g-autofree-ing this function is not trivial (see label "cleanup"), like
it was with other function. The topic of this series-of-series is
replacing virXMLPropString + virStrTo* combos with
virXMLProp{enum,int,tristate,etc}, which is why at this time, this
function stays un-g-autofree-d. I will propably have a look at it at
some point in the future though.
Cheers,
Tim
> size_t i, ndistances = def->nmem_nodes;
>
> @@ -765,24 +764,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) {
> @@ -793,26 +777,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.
> @@ -867,7 +834,6 @@
> virDomainNumaDefNodeDistanceParseXML(virDomainNuma *def,
> def->mem_nodes[i].ndistances = 0;
> }
> VIR_FREE(nodes);
> - VIR_FREE(tmp);
>
> return ret;
> }
>
5:01 p.m.
New subject: [libvirt PATCH 05/10] 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 e631bfa341..27aac87a8d 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -955,26 +955,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,
@@ -1014,29 +1011,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)
@@ -1058,24 +1041,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;
@@ -1089,56 +1063,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
5:01 p.m.
New subject: [libvirt PATCH 06/10] virDomainNumaDefParseXML: Use g_autofree
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/conf/numa_conf.c | 49 +++++++++++++++++++-------------------------
1 file changed, 21 insertions(+), 28 deletions(-)
diff --git a/src/conf/numa_conf.c b/src/conf/numa_conf.c
index 27aac87a8d..abb39a36ac 100644
--- a/src/conf/numa_conf.c
+++ b/src/conf/numa_conf.c
@@ -933,11 +933,9 @@ int
virDomainNumaDefParseXML(virDomainNuma *def,
xmlXPathContextPtr ctxt)
{
- xmlNodePtr *nodes = NULL;
- char *tmp = NULL;
+ g_autofree xmlNodePtr *nodes = NULL;
int n;
size_t i, j;
- int ret = -1;
/* check if NUMA definition is present */
if (!virXPathNode("./cpu/numa[1]", ctxt))
@@ -946,7 +944,7 @@ virDomainNumaDefParseXML(virDomainNuma *def,
if ((n = virXPathNodeSet("./cpu/numa[1]/cell", ctxt, &nodes)) <= 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);
@@ -954,12 +952,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,
&cur_cell)) < 0)
- goto cleanup;
+ return -1;
if (rc == 0)
cur_cell = i;
@@ -970,25 +969,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"))) {
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++) {
@@ -1002,38 +1000,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];
if (virDomainParseMemory("./@memory", "./@unit", ctxt,
&def->mem_nodes[cur_cell].mem, true, false) <
0)
- goto cleanup;
+ return -1;
if (virXMLPropEnum(nodes[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,
&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;
+ return -1;
def->interconnects = g_new0(virDomainNumaInterconnect, n);
for (i = 0; i < n; i++) {
@@ -1049,7 +1047,7 @@ virDomainNumaDefParseXML(virDomainNuma *def,
if (virXMLPropULongLong(nodes[i], "value", 10,
VIR_XML_PROP_REQUIRED, &value) < 0)
- goto cleanup;
+ return -1;
} else if (virXMLNodeNameEqual(nodes[i], "bandwidth")) {
VIR_XPATH_NODE_AUTORESTORE(ctxt)
type = VIR_DOMAIN_NUMA_INTERCONNECT_TYPE_BANDWIDTH;
@@ -1057,7 +1055,7 @@ virDomainNumaDefParseXML(virDomainNuma *def,
ctxt->node = nodes[i];
if (virDomainParseMemory("./@value", "./@unit", ctxt,
&value, true, false) < 0)
- goto cleanup;
+ return -1;
} else {
/* Ignore yet unknown child elements. */
continue;
@@ -1065,32 +1063,27 @@ virDomainNumaDefParseXML(virDomainNuma *def,
if (virXMLPropUInt(nodes[i], "initiator", 10, VIR_XML_PROP_REQUIRED,
&initiator) < 0)
- goto cleanup;
+ return -1;
if (virXMLPropUInt(nodes[i], "target", 10, VIR_XML_PROP_REQUIRED,
&target) < 0)
- goto cleanup;
+ return -1;
if (virXMLPropUInt(nodes[i], "cache", 10, VIR_XML_PROP_NONE,
&cache) < 0)
- goto cleanup;
+ return -1;
if (virXMLPropEnum(nodes[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
7:06 p.m.
New subject: [libvirt PATCH 06/10] virDomainNumaDefParseXML: Use g_autofree
On 5/11/21 11:01 AM, Tim Wiederhake wrote:
}
VIR_FREE(nodes);
if ((n = virXPathNodeSet("./cpu/numa[1]/interconnects[1]/latency|"
"./cpu/numa[1]/interconnects[1]/bandwidth",
ctxt, &nodes)) <
Again, the VIR_FREE(nodes) points out that you're re-using a g_autofree
pointer, and should instead use two separate pointers.
5:01 p.m.
New subject: [libvirt PATCH 07/10] virStorageAdapterParseXMLFCHost: Use virXMLProp*
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/conf/storage_adapter_conf.c | 17 +++--------------
1 file changed, 3 insertions(+), 14 deletions(-)
diff --git a/src/conf/storage_adapter_conf.c b/src/conf/storage_adapter_conf.c
index 142489f6cd..6b5a58e1e7 100644
--- a/src/conf/storage_adapter_conf.c
+++ b/src/conf/storage_adapter_conf.c
@@ -64,28 +64,17 @@ static int
virStorageAdapterParseXMLFCHost(xmlNodePtr node,
virStorageAdapterFCHost *fchost)
{
- char *managed = NULL;
+ if (virXMLPropTristateBool(node, "managed", VIR_XML_PROP_NONE,
+ &fchost->managed) < 0)
+ return -1;
fchost->parent = virXMLPropString(node, "parent");
- if ((managed = virXMLPropString(node, "managed"))) {
- int value;
- if ((value = virTristateBoolTypeFromString(managed)) < 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unknown fc_host managed setting '%s'"),
- managed);
- VIR_FREE(managed);
- return -1;
- }
- fchost->managed = value;
- }
-
fchost->parent_wwnn = virXMLPropString(node, "parent_wwnn");
fchost->parent_wwpn = virXMLPropString(node, "parent_wwpn");
fchost->parent_fabric_wwn = virXMLPropString(node,
"parent_fabric_wwn");
fchost->wwpn = virXMLPropString(node, "wwpn");
fchost->wwnn = virXMLPropString(node, "wwnn");
- VIR_FREE(managed);
return 0;
}
--
2.26.3
7:08 p.m.
New subject: [libvirt PATCH 07/10] virStorageAdapterParseXMLFCHost: Use virXMLProp*
On 5/11/21 11:01 AM, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
Reviewed-by: Laine Stump <laine(a)redhat.com>
---
src/conf/storage_adapter_conf.c | 17 +++--------------
1 file changed, 3 insertions(+), 14 deletions(-)
diff --git a/src/conf/storage_adapter_conf.c b/src/conf/storage_adapter_conf.c
index 142489f6cd..6b5a58e1e7 100644
--- a/src/conf/storage_adapter_conf.c
+++ b/src/conf/storage_adapter_conf.c
@@ -64,28 +64,17 @@ static int
virStorageAdapterParseXMLFCHost(xmlNodePtr node,
virStorageAdapterFCHost *fchost)
{
- char *managed = NULL;
+ if (virXMLPropTristateBool(node, "managed", VIR_XML_PROP_NONE,
+ &fchost->managed) < 0)
+ return -1;
fchost->parent = virXMLPropString(node, "parent");
- if ((managed = virXMLPropString(node, "managed"))) {
- int value;
- if ((value = virTristateBoolTypeFromString(managed)) < 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unknown fc_host managed setting '%s'"),
- managed);
- VIR_FREE(managed);
- return -1;
- }
- fchost->managed = value;
- }
-
fchost->parent_wwnn = virXMLPropString(node, "parent_wwnn");
fchost->parent_wwpn = virXMLPropString(node, "parent_wwpn");
fchost->parent_fabric_wwn = virXMLPropString(node,
"parent_fabric_wwn");
fchost->wwpn = virXMLPropString(node, "wwpn");
fchost->wwnn = virXMLPropString(node, "wwnn");
- VIR_FREE(managed);
return 0;
}
5:01 p.m.
New subject: [libvirt PATCH 08/10] virStoragePoolDefParseSource: Use virXMLProp*
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/conf/storage_conf.c | 31 ++++++++-----------------------
1 file changed, 8 insertions(+), 23 deletions(-)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 328650bd6a..435a029b4e 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -531,7 +531,6 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
virStoragePoolOptions *options;
int n;
g_autoptr(virStorageAuthDef) authdef = NULL;
- g_autofree char *port = NULL;
g_autofree char *ver = NULL;
g_autofree xmlNodePtr *nodeset = NULL;
g_autofree char *sourcedir = NULL;
@@ -580,16 +579,9 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
goto cleanup;
}
- port = virXMLPropString(nodeset[i], "port");
- if (port) {
- if (virStrToLong_i(port, NULL, 10, &source->hosts[i].port) < 0)
{
- virReportError(VIR_ERR_XML_ERROR,
- _("Invalid port number: %s"),
- port);
- goto cleanup;
- }
- }
- VIR_FREE(port);
+ if (virXMLPropInt(nodeset[i], "port", 10, VIR_XML_PROP_NONE,
+ &source->hosts[i].port, 0) < 0)
+ goto cleanup;
}
}
@@ -602,7 +594,6 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
goto cleanup;
for (i = 0; i < nsource; i++) {
- g_autofree char *partsep = NULL;
virStoragePoolSourceDevice dev = { .path = NULL };
dev.path = virXMLPropString(nodeset[i], "path");
@@ -612,17 +603,11 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
goto cleanup;
}
- partsep = virXMLPropString(nodeset[i], "part_separator");
- if (partsep) {
- int value = virTristateBoolTypeFromString(partsep);
- if (value <= 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("invalid part_separator setting
'%s'"),
- partsep);
- virStoragePoolSourceDeviceClear(&dev);
- goto cleanup;
- }
- dev.part_separator = value;
+ if (virXMLPropTristateBool(nodeset[i], "part_separator",
+ VIR_XML_PROP_NONE,
+ &dev.part_separator) < 0) {
+ virStoragePoolSourceDeviceClear(&dev);
+ goto cleanup;
}
if (VIR_APPEND_ELEMENT(source->devices, source->ndevice, dev) < 0) {
--
2.26.3
7:20 p.m.
New subject: [libvirt PATCH 08/10] virStoragePoolDefParseSource: Use virXMLProp*
On 5/11/21 11:01 AM, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
Reviewed-by: Laine Stump <laine(a)redhat.com>
---
src/conf/storage_conf.c | 31 ++++++++-----------------------
1 file changed, 8 insertions(+), 23 deletions(-)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 328650bd6a..435a029b4e 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -531,7 +531,6 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
virStoragePoolOptions *options;
int n;
g_autoptr(virStorageAuthDef) authdef = NULL;
- g_autofree char *port = NULL;
g_autofree char *ver = NULL;
g_autofree xmlNodePtr *nodeset = NULL;
g_autofree char *sourcedir = NULL;
@@ -580,16 +579,9 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
goto cleanup;
}
- port = virXMLPropString(nodeset[i], "port");
- if (port) {
- if (virStrToLong_i(port, NULL, 10, &source->hosts[i].port) <
0) {
- virReportError(VIR_ERR_XML_ERROR,
- _("Invalid port number: %s"),
- port);
- goto cleanup;
- }
- }
- VIR_FREE(port);
+ if (virXMLPropInt(nodeset[i], "port", 10, VIR_XML_PROP_NONE,
+ &source->hosts[i].port, 0) < 0)
+ goto cleanup;
}
}
@@ -602,7 +594,6 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
goto cleanup;
for (i = 0; i < nsource; i++) {
- g_autofree char *partsep = NULL;
virStoragePoolSourceDevice dev = { .path = NULL };
dev.path = virXMLPropString(nodeset[i], "path");
@@ -612,17 +603,11 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
goto cleanup;
}
- partsep = virXMLPropString(nodeset[i], "part_separator");
- if (partsep) {
- int value = virTristateBoolTypeFromString(partsep);
- if (value <= 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("invalid part_separator setting
'%s'"),
- partsep);
- virStoragePoolSourceDeviceClear(&dev);
- goto cleanup;
- }
- dev.part_separator = value;
+ if (virXMLPropTristateBool(nodeset[i], "part_separator",
+ VIR_XML_PROP_NONE,
+ &dev.part_separator) < 0) {
+ virStoragePoolSourceDeviceClear(&dev);
+ goto cleanup;
}
if (VIR_APPEND_ELEMENT(source->devices, source->ndevice, dev) < 0) {
5:01 p.m.
New subject: [libvirt PATCH 09/10] virStoragePoolDefParseSource: Use VIR_XPATH_NODE_AUTORESTORE
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/conf/storage_conf.c | 41 ++++++++++++++++++-----------------------
1 file changed, 18 insertions(+), 23 deletions(-)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 435a029b4e..10b46ac368 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -523,8 +523,7 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
int pool_type,
xmlNodePtr node)
{
- int ret = -1;
- xmlNodePtr relnode, authnode;
+ xmlNodePtr authnode;
xmlNodePtr adapternode;
int nsource;
size_t i;
@@ -534,18 +533,18 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
g_autofree char *ver = NULL;
g_autofree xmlNodePtr *nodeset = NULL;
g_autofree char *sourcedir = NULL;
+ VIR_XPATH_NODE_AUTORESTORE(ctxt)
- relnode = ctxt->node;
ctxt->node = node;
if ((options = virStoragePoolOptionsForPoolType(pool_type)) == NULL)
- goto cleanup;
+ return -1;
source->name = virXPathString("string(./name)", ctxt);
if (pool_type == VIR_STORAGE_POOL_RBD && source->name == NULL) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("element 'name' is mandatory for RBD pool"));
- goto cleanup;
+ return -1;
}
if (options->formatFromString) {
@@ -560,12 +559,12 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
if (source->format < 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("unknown pool format type %s"), format);
- goto cleanup;
+ return -1;
}
}
if ((n = virXPathNodeSet("./host", ctxt, &nodeset)) < 0)
- goto cleanup;
+ return -1;
if (n) {
source->hosts = g_new0(virStoragePoolSourceHost, n);
@@ -576,12 +575,12 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
if (!source->hosts[i].name) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("missing storage pool host name"));
- goto cleanup;
+ return -1;
}
if (virXMLPropInt(nodeset[i], "port", 10, VIR_XML_PROP_NONE,
&source->hosts[i].port, 0) < 0)
- goto cleanup;
+ return -1;
}
}
@@ -591,7 +590,7 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
nsource = virXPathNodeSet("./device", ctxt, &nodeset);
if (nsource < 0)
- goto cleanup;
+ return -1;
for (i = 0; i < nsource; i++) {
virStoragePoolSourceDevice dev = { .path = NULL };
@@ -600,19 +599,19 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
if (dev.path == NULL) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("missing storage pool source device path"));
- goto cleanup;
+ return -1;
}
if (virXMLPropTristateBool(nodeset[i], "part_separator",
VIR_XML_PROP_NONE,
&dev.part_separator) < 0) {
virStoragePoolSourceDeviceClear(&dev);
- goto cleanup;
+ return -1;
}
if (VIR_APPEND_ELEMENT(source->devices, source->ndevice, dev) < 0) {
virStoragePoolSourceDeviceClear(&dev);
- goto cleanup;
+ return -1;
}
}
@@ -626,17 +625,17 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
if ((adapternode = virXPathNode("./adapter", ctxt))) {
if (virStorageAdapterParseXML(&source->adapter, adapternode, ctxt) <
0)
- goto cleanup;
+ return -1;
}
if ((authnode = virXPathNode("./auth", ctxt))) {
if (!(authdef = virStorageAuthDefParse(authnode, ctxt)))
- goto cleanup;
+ return -1;
if (authdef->authType == VIR_STORAGE_AUTH_TYPE_NONE) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("storage pool missing auth type"));
- goto cleanup;
+ return -1;
}
source->auth = g_steal_pointer(&authdef);
@@ -650,24 +649,20 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
_("storage pool protocol ver unsupported for "
"pool type '%s'"),
virStoragePoolFormatFileSystemNetTypeToString(source->format));
- goto cleanup;
+ return -1;
}
if (virStrToLong_uip(ver, NULL, 0, &source->protocolVer) < 0) {
virReportError(VIR_ERR_XML_ERROR,
_("storage pool protocol ver '%s' is
malformed"),
ver);
- goto cleanup;
+ return -1;
}
}
source->vendor = virXPathString("string(./vendor/@name)", ctxt);
source->product = virXPathString("string(./product/@name)", ctxt);
- ret = 0;
- cleanup:
- ctxt->node = relnode;
-
- return ret;
+ return 0;
}
--
2.26.3
5:01 p.m.
New subject: [libvirt PATCH 10/10] virStoragePoolDefParseXML: Use virXMLProp*
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/conf/storage_conf.c | 15 ++++-----------
1 file changed, 4 insertions(+), 11 deletions(-)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 10b46ac368..e481cac75c 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -841,24 +841,17 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt)
virStoragePoolOptions *options;
xmlNodePtr source_node;
g_autoptr(virStoragePoolDef) def = NULL;
- g_autofree char *type = NULL;
+ virStoragePoolType type;
g_autofree char *uuid = NULL;
g_autofree char *target_path = NULL;
def = g_new0(virStoragePoolDef, 1);
- type = virXPathString("string(./@type)", ctxt);
- if (type == NULL) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("storage pool missing type attribute"));
+ if (virXMLPropEnum(ctxt->node, "type", virStoragePoolTypeFromString,
+ VIR_XML_PROP_REQUIRED, &type) < 0)
return NULL;
- }
- if ((def->type = virStoragePoolTypeFromString(type)) < 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unknown storage pool type %s"), type);
- return NULL;
- }
+ def->type = type;
if ((options = virStoragePoolOptionsForPoolType(def->type)) == NULL)
return NULL;
--
2.26.3
7:21 p.m.
New subject: [libvirt PATCH 10/10] virStoragePoolDefParseXML: Use virXMLProp*
On 5/11/21 11:01 AM, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
Reviewed-by: Laine Stump <laine(a)redhat.com>
---
src/conf/storage_conf.c | 15 ++++-----------
1 file changed, 4 insertions(+), 11 deletions(-)
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 10b46ac368..e481cac75c 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -841,24 +841,17 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt)
virStoragePoolOptions *options;
xmlNodePtr source_node;
g_autoptr(virStoragePoolDef) def = NULL;
- g_autofree char *type = NULL;
+ virStoragePoolType type;
g_autofree char *uuid = NULL;
g_autofree char *target_path = NULL;
def = g_new0(virStoragePoolDef, 1);
- type = virXPathString("string(./@type)", ctxt);
- if (type == NULL) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("storage pool missing type attribute"));
+ if (virXMLPropEnum(ctxt->node, "type", virStoragePoolTypeFromString,
+ VIR_XML_PROP_REQUIRED, &type) < 0)
return NULL;
- }
- if ((def->type = virStoragePoolTypeFromString(type)) < 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unknown storage pool type %s"), type);
- return NULL;
- }
+ def->type = type;
if ((options = virStoragePoolOptionsForPoolType(def->type)) == NULL)
return NULL;
9:26 a.m.
New subject: [libvirt PATCH 00/10] Refactor more XML parsing boilerplate code, part X
On 5/11/21 5:01 PM, Tim Wiederhake wrote:
For background, see
https://listman.redhat.com/archives/libvir-list/2021-April/msg00668.html
Tim Wiederhake (10):
virNodeDeviceDefParseXML: Use g_auto*
virDomainNumatuneNodeParseXML: Use virXMLProp*
virDomainNumatuneNodeParseXML: Use g_autofree
virDomainNumaDefNodeDistanceParseXML: Use virXMLProp*
virDomainNumaDefParseXML: Use virXMLProp*
virDomainNumaDefParseXML: Use g_autofree
virStorageAdapterParseXMLFCHost: Use virXMLProp*
virStoragePoolDefParseSource: Use virXMLProp*
virStoragePoolDefParseSource: Use VIR_XPATH_NODE_AUTORESTORE
virStoragePoolDefParseXML: Use virXMLProp*
src/conf/node_device_conf.c | 34 +--
src/conf/numa_conf.c | 285 +++++-------------
src/conf/storage_adapter_conf.c | 17 +-
src/conf/storage_conf.c | 83 ++---
.../hugepages-memaccess-invalid.err | 2 +-
5 files changed, 125 insertions(+), 296 deletions(-)
I've pushed reviewed patches (07-10/10).
Michal
1225
days inactive
1227
days old
20 comments
3 participants
participants (3)
-
Laine Stump -
Michal Prívozník -
Tim Wiederhake