[libvirt PATCH 06/10] virDomainNumaDefParseXML: Use g_autofree
Signed-off-by: Tim Wiederhake
---
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_AU
[libvirt PATCH 07/10] virStorageAdapterParseXMLFCHost: Use virXMLProp*
Signed-off-by: Tim Wiederhake
---
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
[libvirt PATCH 08/10] virStoragePoolDefParseSource: Use virXMLProp*
Signed-off-by: Tim Wiederhake
---
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
[libvirt PATCH 00/10] Refactor more XML parsing boilerplate code, part X
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
[libvirt PATCH 02/10] virDomainNumatuneNodeParseXML: Use virXMLProp*
Signed-off-by: Tim Wiederhake
---
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
[libvirt PATCH 01/10] virNodeDeviceDefParseXML: Use g_auto*
Signed-off-by: Tim Wiederhake
---
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
[libvirt PATCH 03/10] virDomainNumatuneNodeParseXML: Use g_autofree
Signed-off-by: Tim Wiederhake
---
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
[libvirt PATCH v2 3/6] virDomainNumatuneNodeParseXML: Use g_autofree
Signed-off-by: Tim Wiederhake
---
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
[libvirt PATCH v2 0/6] Refactor more XML parsing boilerplate code, part X
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
[libvirt PATCH v2 6/6] virDomainNumaDefParseXML: Use g_autofree
Signed-off-by: Tim Wiederhake
---
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]/bandwi
[libvirt PATCH v2 1/6] virNodeDeviceDefParseXML: Use g_auto*
Signed-off-by: Tim Wiederhake
---
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
[libvirt PATCH v2 5/6] virDomainNumaDefParseXML: Use virXMLProp*
Signed-off-by: Tim Wiederhake
---
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 NUM
[libvirt PATCH v2 4/6] virDomainNumaDefNodeDistanceParseXML: Use virXMLProp*
Signed-off-by: Tim Wiederhake
---
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
[libvirt PATCH v2 2/6] virDomainNumatuneNodeParseXML: Use virXMLProp*
Signed-off-by: Tim Wiederhake
---
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
Re: [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
> > ---
> > 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;
> > }
> >
Re: [libvirt PATCH v2 0/7] Enable sanitizers
Ping. On Thu, 2021-05-06 at 17:08 +0200, Tim Wiederhake wrote: > This series enables and adds AddressSanitizer and > UndefinedBehaviorSanitizer > builds to the CI. > > See: > https://clang.llvm.org/docs/AddressSanitizer.html and > https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html > > These sanitizers already found some issues in libvirt, e.g. > 4eb7c621985dad4de911ec394ac628bd1a5b29ab, > 1294de209cee6643511265c7e2d4283c047cf652, > 8b8c91f487592c6c067847ca59dde405ca17573f, or > 1c34211c22de28127a509edbf2cf2f44cb0d891e. > > There exist two more relevant sanitizers, ThreadSanitizer and > MemorySanitizer. > Unfortunately, those two require an instrumented build of all > dependencies, > including libc, to work correctly. > > Note that clang and gcc have different implementations of these > sanitizers, > hence the introduction of two new jobs to the CI. The latter one > issues a > warning about the use of LD_PRELOAD in `virTestMain`, which in this > particular case can be safely ignored by setting `ASAN_OPTIONS` to > verify_asan_link_order=0` for the gcc build. > > Changes since V1: > > Incorporated changes suggested by Pavel, except for #6 (now #7): The > statement > in > https://listman.redhat.com/archives/libvir-list/2021-May/msg00070.html > on > the sanitizers working with Fedora 33 is wrong, I was fooled by > caching. The > bug described there is present in Fedora 33, 34, and Rawhide. > > Cheers, > Tim > > Tim Wiederhake (7): > meson: Allow larger stack frames when instrumenting > meson: Allow undefined symbols when sanitizers are enabled > tests: virfilemock: realpath: Allow non-null second parameter > openvz: Add missing symbols to libvirt_openvz.syms > tests: openvzutilstest: Remove duplicate linking with > libvirt_openvz.a > virt-aa-helper: Remove duplicate linking with src/datatypes.o > ci: Enable address and undefined behavior sanitizers > > .gitlab-ci.yml| 35 +++ > build-aux/syntax-check.mk | 2 +- > meson.build | 14 ++ > src/libvirt_openvz.syms | 2 ++ > src/security/meson.build | 1 - > tests/meson.build | 2 +- > tests/virfilemock.c | 20 > 7 files changed, 61 insertions(+), 15 deletions(-) > > -- > 2.26.3 > >
[libvirt PATCH 00/10] Refactor more XML parsing boilerplate code, part XI
For background, see https://listman.redhat.com/archives/libvir-list/2021-April/msg00668.html Tim Wiederhake (10): virDomainHostdevDef: Change type of startupPolicy to virDomainStartupPolicy virDomainHostdevSubsysUSBDefParseXML: Use virXMLProp* virDomainDeviceUSBMasterParseXML: Use virXMLProp* virDomainDiskDef: Change type of geometry.trans to virDomainDiskGeometryTrans virDomainDiskDefGeometryParse: Use virXMLProp* virDomainChrSourceReconnectDefParseXML: Use virXMLProp* virDomainChrDefParseTargetXML: Use virXMLProp* virDomainAudioCoreAudioParse: Use virXMLProp* virDomainAudioOSSParse: Use virXMLProp* virNodeDevCapPCIDevIommuGroupParseXML: Use virXMLProp* src/conf/domain_conf.c | 168 +--- src/conf/domain_conf.h | 23 +++-- src/conf/node_device_conf.c | 15 +--- 3 files changed, 52 insertions(+), 154 deletions(-) -- 2.26.3
[libvirt PATCH 02/10] virDomainHostdevSubsysUSBDefParseXML: Use virXMLProp*
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 23 +--
1 file changed, 9 insertions(+), 14 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 734fa584a4..661fa53206 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6694,28 +6694,23 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
virDomainHostdevDef *def)
{
virDomainHostdevSubsysUSB *usbsrc = &def->source.subsys.u.usb;
-g_autofree char *startupPolicy = NULL;
-g_autofree char *autoAddress = NULL;
xmlNodePtr vendorNode;
xmlNodePtr productNode;
xmlNodePtr addressNode;
+virTristateBool autoAddress;
VIR_XPATH_NODE_AUTORESTORE(ctxt)
ctxt->node = node;
-if ((startupPolicy = virXMLPropString(node, "startupPolicy"))) {
-int value = virDomainStartupPolicyTypeFromString(startupPolicy);
-if (value <= 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Unknown startup policy '%s'"),
- startupPolicy);
-return -1;
-}
-def->startupPolicy = value;
-}
+if (virXMLPropEnum(node, "startupPolicy",
+ virDomainStartupPolicyTypeFromString,
+ VIR_XML_PROP_NONZERO, &def->startupPolicy) < 0)
+return -1;
-if ((autoAddress = virXMLPropString(node, "autoAddress")))
-ignore_value(virStringParseYesNo(autoAddress, &usbsrc->autoAddress));
+if (virXMLPropTristateBool(node, "autoAddress", VIR_XML_PROP_NONE,
+ &autoAddress) < 0)
+return -1;
+usbsrc->autoAddress = autoAddress == VIR_TRISTATE_BOOL_YES;
/* Product can validly be 0, so we need some extra help to determine
* if it is uninitialized */
--
2.26.3
[libvirt PATCH 01/10] virDomainHostdevDef: Change type of startupPolicy to virDomainStartupPolicy
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 6 +++---
src/conf/domain_conf.h | 21 ++---
2 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7044701fac..734fa584a4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6704,14 +6704,14 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
ctxt->node = node;
if ((startupPolicy = virXMLPropString(node, "startupPolicy"))) {
-def->startupPolicy =
-virDomainStartupPolicyTypeFromString(startupPolicy);
-if (def->startupPolicy <= 0) {
+int value = virDomainStartupPolicyTypeFromString(startupPolicy);
+if (value <= 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Unknown startup policy '%s'"),
startupPolicy);
return -1;
}
+def->startupPolicy = value;
}
if ((autoAddress = virXMLPropString(node, "autoAddress")))
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 2d5462bb55..41e570765e 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -332,6 +332,15 @@ struct _virDomainHostdevCaps {
};
+typedef enum {
+VIR_DOMAIN_STARTUP_POLICY_DEFAULT = 0,
+VIR_DOMAIN_STARTUP_POLICY_MANDATORY,
+VIR_DOMAIN_STARTUP_POLICY_REQUISITE,
+VIR_DOMAIN_STARTUP_POLICY_OPTIONAL,
+
+VIR_DOMAIN_STARTUP_POLICY_LAST
+} virDomainStartupPolicy;
+
/* basic device for direct passthrough */
struct _virDomainHostdevDef {
/* If 'parentnet' is non-NULL it means this host dev was
@@ -343,7 +352,7 @@ struct _virDomainHostdevDef {
virDomainNetDef *parentnet;
int mode; /* enum virDomainHostdevMode */
-int startupPolicy; /* enum virDomainStartupPolicy */
+virDomainStartupPolicy startupPolicy;
bool managed;
bool missing;
bool readonly;
@@ -432,16 +441,6 @@ typedef enum {
VIR_DOMAIN_DISK_IO_LAST
} virDomainDiskIo;
-typedef enum {
-VIR_DOMAIN_STARTUP_POLICY_DEFAULT = 0,
-VIR_DOMAIN_STARTUP_POLICY_MANDATORY,
-VIR_DOMAIN_STARTUP_POLICY_REQUISITE,
-VIR_DOMAIN_STARTUP_POLICY_OPTIONAL,
-
-VIR_DOMAIN_STARTUP_POLICY_LAST
-} virDomainStartupPolicy;
-
-
typedef enum {
VIR_DOMAIN_DEVICE_SGIO_DEFAULT = 0,
VIR_DOMAIN_DEVICE_SGIO_FILTERED,
--
2.26.3
[libvirt PATCH 03/10] virDomainDeviceUSBMasterParseXML: Use virXMLProp*
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `startport`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute.
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 11 ++-
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 661fa53206..86680e0cdb 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6439,18 +6439,11 @@ static int
virDomainDeviceUSBMasterParseXML(xmlNodePtr node,
virDomainDeviceUSBMaster *master)
{
-g_autofree char *startport = NULL;
-
memset(master, 0, sizeof(*master));
-startport = virXMLPropString(node, "startport");
-
-if (startport &&
-virStrToLong_ui(startport, NULL, 10, &master->startport) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("Cannot parse 'startport' attribute"));
+if (virXMLPropUInt(node, "startport", 10, VIR_XML_PROP_NONE,
+ &master->startport) < 0)
return -1;
-}
return 0;
}
--
2.26.3
[libvirt PATCH 04/10] virDomainDiskDef: Change type of geometry.trans to virDomainDiskGeometryTrans
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 5 +++--
src/conf/domain_conf.h | 2 +-
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 86680e0cdb..f55117e849 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8845,13 +8845,14 @@ virDomainDiskDefGeometryParse(virDomainDiskDef *def,
}
if ((tmp = virXMLPropString(cur, "trans"))) {
-def->geometry.trans = virDomainDiskGeometryTransTypeFromString(tmp);
-if (def->geometry.trans <= 0) {
+int value;
+if ((value = virDomainDiskGeometryTransTypeFromString(tmp)) <= 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("invalid translation value '%s'"),
tmp);
return -1;
}
+def->geometry.trans = value;
}
return 0;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 41e570765e..cf8481f1f6 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -549,7 +549,7 @@ struct _virDomainDiskDef {
unsigned int cylinders;
unsigned int heads;
unsigned int sectors;
-int trans; /* enum virDomainDiskGeometryTrans */
+virDomainDiskGeometryTrans trans;
} geometry;
struct {
--
2.26.3
[libvirt PATCH 05/10] virDomainDiskDefGeometryParse: Use virXMLProp*
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attributes `cyls`, `heads` and `secs`.
Allowing negative numbers to be interpreted this way makes no sense for
these attributes.
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 48 +++---
1 file changed, 12 insertions(+), 36 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f55117e849..bfcc56ca9e 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8815,45 +8815,21 @@ static int
virDomainDiskDefGeometryParse(virDomainDiskDef *def,
xmlNodePtr cur)
{
-g_autofree char *tmp = NULL;
-
-if ((tmp = virXMLPropString(cur, "cyls"))) {
-if (virStrToLong_ui(tmp, NULL, 10, &def->geometry.cylinders) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("invalid geometry settings (cyls)"));
-return -1;
-}
-VIR_FREE(tmp);
-}
+if (virXMLPropUInt(cur, "cyls", 10, VIR_XML_PROP_NONE,
+ &def->geometry.cylinders) < 0)
+return -1;
-if ((tmp = virXMLPropString(cur, "heads"))) {
-if (virStrToLong_ui(tmp, NULL, 10, &def->geometry.heads) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("invalid geometry settings (heads)"));
-return -1;
-}
-VIR_FREE(tmp);
-}
+if (virXMLPropUInt(cur, "heads", 10, VIR_XML_PROP_NONE,
+ &def->geometry.heads) < 0)
+return -1;
-if ((tmp = virXMLPropString(cur, "secs"))) {
-if (virStrToLong_ui(tmp, NULL, 10, &def->geometry.sectors) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("invalid geometry settings (secs)"));
-return -1;
-}
-VIR_FREE(tmp);
-}
+if (virXMLPropUInt(cur, "secs", 10, VIR_XML_PROP_NONE,
+ &def->geometry.sectors) < 0)
+return -1;
-if ((tmp = virXMLPropString(cur, "trans"))) {
-int value;
-if ((value = virDomainDiskGeometryTransTypeFromString(tmp)) <= 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("invalid translation value '%s'"),
- tmp);
-return -1;
-}
-def->geometry.trans = value;
-}
+if (virXMLPropEnum(cur, "trans", virDomainDiskGeometryTransTypeFromString,
+ VIR_XML_PROP_NONZERO, &def->geometry.trans) < 0)
+return -1;
return 0;
}
--
2.26.3
[libvirt PATCH 08/10] virDomainAudioCoreAudioParse: Use virXMLProp*
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `bufferCount`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute.
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 10 ++
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 57a54f12ef..a46e64c64a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13046,15 +13046,9 @@ static int
virDomainAudioCoreAudioParse(virDomainAudioIOCoreAudio *def,
xmlNodePtr node)
{
-g_autofree char *bufferCount = virXMLPropString(node, "bufferCount");
-
-if (bufferCount &&
-virStrToLong_ui(bufferCount, NULL, 10,
-&def->bufferCount) < 0) {
-virReportError(VIR_ERR_XML_ERROR,
- _("cannot parse 'bufferCount' value '%s'"),
bufferCount);
+if (virXMLPropUInt(node, "bufferCount", 10, VIR_XML_PROP_NONE,
+ &def->bufferCount) < 0)
return -1;
-}
return 0;
}
--
2.26.3
[libvirt PATCH 07/10] virDomainChrDefParseTargetXML: Use virXMLProp*
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `port`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute.
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 29 +
1 file changed, 5 insertions(+), 24 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a5514660cc..57a54f12ef 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10977,7 +10977,6 @@ virDomainChrDefParseTargetXML(virDomainChrDef *def,
g_autofree char *targetModel = NULL;
g_autofree char *addrStr = NULL;
g_autofree char *portStr = NULL;
-g_autofree char *stateStr = NULL;
VIR_XPATH_NODE_AUTORESTORE(ctxt)
ctxt->node = cur;
@@ -11007,7 +11006,6 @@ virDomainChrDefParseTargetXML(virDomainChrDef *def,
switch (def->targetType) {
case VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_GUESTFWD:
addrStr = virXMLPropString(cur, "address");
-portStr = virXMLPropString(cur, "port");
def->target.addr = g_new0(virSocketAddr, 1);
@@ -11028,19 +11026,8 @@ virDomainChrDefParseTargetXML(virDomainChrDef *def,
return -1;
}
-if (portStr == NULL) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
- _("guestfwd channel does "
- "not define a target port"));
-return -1;
-}
-
-if (virStrToLong_ui(portStr, NULL, 10, &port) < 0) {
-virReportError(VIR_ERR_XML_ERROR,
- _("Invalid port number: %s"),
- portStr);
+if (virXMLPropUInt(cur, "port", 10, VIR_XML_PROP_REQUIRED, &port)
< 0)
return -1;
-}
virSocketAddrSetPort(def->target.addr, port);
break;
@@ -11050,18 +11037,12 @@ virDomainChrDefParseTargetXML(virDomainChrDef *def,
def->target.name = virXMLPropString(cur, "name");
if (def->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO &&
-!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) &&
-(stateStr = virXMLPropString(cur, "state"))) {
-int tmp;
+!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)) {
-if ((tmp = virDomainChrDeviceStateTypeFromString(stateStr)) <=
0) {
-virReportError(VIR_ERR_XML_ERROR,
- _("invalid channel state value '%s'"),
- stateStr);
+if (virXMLPropEnum(cur, "state",
+ virDomainChrDeviceStateTypeFromString,
+ VIR_XML_PROP_NONZERO, &def->state) < 0)
return -1;
-}
-
-def->state = tmp;
}
break;
}
--
2.26.3
[libvirt PATCH 09/10] virDomainAudioOSSParse: Use virXMLProp*
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `bufferCount`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute.
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 19 ---
1 file changed, 4 insertions(+), 15 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a46e64c64a..b3ed3a9c5a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13074,26 +13074,15 @@ static int
virDomainAudioOSSParse(virDomainAudioIOOSS *def,
xmlNodePtr node)
{
-g_autofree char *tryPoll = virXMLPropString(node, "tryPoll");
-g_autofree char *bufferCount = virXMLPropString(node, "bufferCount");
-
def->dev = virXMLPropString(node, "dev");
-if (tryPoll &&
-((def->tryPoll =
- virTristateBoolTypeFromString(tryPoll)) <= 0)) {
-virReportError(VIR_ERR_XML_ERROR,
- _("unknown 'tryPoll' value '%s'"), tryPoll);
+if (virXMLPropTristateBool(node, "tryPoll", VIR_XML_PROP_NONE,
+ &def->tryPoll) < 0)
return -1;
-}
-if (bufferCount &&
-virStrToLong_ui(bufferCount, NULL, 10,
-&def->bufferCount) < 0) {
-virReportError(VIR_ERR_XML_ERROR,
- _("cannot parse 'bufferCount' value '%s'"),
bufferCount);
+if (virXMLPropUInt(node, "bufferCount", 10, VIR_XML_PROP_NONE,
+ &def->bufferCount) < 0)
return -1;
-}
return 0;
}
--
2.26.3
[libvirt PATCH 06/10] virDomainChrSourceReconnectDefParseXML: Use virXMLProp*
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `timeout`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute.
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 29 +
1 file changed, 5 insertions(+), 24 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index bfcc56ca9e..a5514660cc 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10093,39 +10093,20 @@
virDomainChrSourceReconnectDefParseXML(virDomainChrSourceReconnectDef *def,
xmlNodePtr node,
xmlXPathContextPtr ctxt)
{
-int tmpVal;
VIR_XPATH_NODE_AUTORESTORE(ctxt)
xmlNodePtr cur;
-g_autofree char *tmp = NULL;
ctxt->node = node;
if ((cur = virXPathNode("./reconnect", ctxt))) {
-if ((tmp = virXMLPropString(cur, "enabled"))) {
-if ((tmpVal = virTristateBoolTypeFromString(tmp)) < 0) {
-virReportError(VIR_ERR_XML_ERROR,
- _("invalid reconnect enabled value: '%s'"),
- tmp);
-return -1;
-}
-def->enabled = tmpVal;
-VIR_FREE(tmp);
-}
+if (virXMLPropTristateBool(cur, "enabled", VIR_XML_PROP_NONE,
+ &def->enabled) < 0)
+return -1;
if (def->enabled == VIR_TRISTATE_BOOL_YES) {
-if ((tmp = virXMLPropString(cur, "timeout"))) {
-if (virStrToLong_ui(tmp, NULL, 10, &def->timeout) < 0) {
-virReportError(VIR_ERR_XML_ERROR,
- _("invalid reconnect timeout value: '%s'"),
- tmp);
-return -1;
-}
-} else {
-virReportError(VIR_ERR_XML_ERROR, "%s",
- _("missing timeout for chardev with "
- "reconnect enabled"));
+if (virXMLPropUInt(cur, "timeout", 10, VIR_XML_PROP_REQUIRED,
+ &def->timeout) < 0)
return -1;
-}
}
}
--
2.26.3
[libvirt PATCH 10/10] virNodeDevCapPCIDevIommuGroupParseXML: Use virXMLProp*
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `number`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute.
Signed-off-by: Tim Wiederhake
---
src/conf/node_device_conf.c | 15 ++-
1 file changed, 2 insertions(+), 13 deletions(-)
diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 861f43f6c4..332b12f997 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -1557,25 +1557,14 @@
virNodeDevCapPCIDevIommuGroupParseXML(xmlXPathContextPtr ctxt,
{
VIR_XPATH_NODE_AUTORESTORE(ctxt)
g_autofree xmlNodePtr *addrNodes = NULL;
-g_autofree char *numberStr = NULL;
int nAddrNodes;
size_t i;
ctxt->node = iommuGroupNode;
-numberStr = virXMLPropString(iommuGroupNode, "number");
-if (!numberStr) {
-virReportError(VIR_ERR_XML_ERROR,
- "%s", _("missing iommuGroup number attribute"));
+if (virXMLPropUInt(iommuGroupNode, "number", 10, VIR_XML_PROP_REQUIRED,
+ &pci_dev->iommuGroupNumber) < 0)
return -1;
-}
-if (virStrToLong_ui(numberStr, NULL, 10,
-&pci_dev->iommuGroupNumber) < 0) {
-virReportError(VIR_ERR_XML_ERROR,
- _("invalid iommuGroup number attribute '%s'"),
- numberStr);
-return -1;
-}
if ((nAddrNodes = virXPathNodeSet("./address", ctxt, &addrNodes)) < 0)
return -1;
--
2.26.3
[libvirt PATCH 10/10] virDomainDeviceSpaprVioAddressParseXML: Use virXMLProp*
This strictens the parser to disallow negative values (interpreted as
`ULLONG_MAX + value + 1`) for attribute `reg`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute, as it
refers to a 32 bit address space.
Signed-off-by: Tim Wiederhake
---
src/conf/device_conf.c | 14 ++
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c
index 034f072df4..e587d90c59 100644
--- a/src/conf/device_conf.c
+++ b/src/conf/device_conf.c
@@ -417,19 +417,17 @@ int
virDomainDeviceSpaprVioAddressParseXML(xmlNodePtr node,
virDomainDeviceSpaprVioAddress *addr)
{
-g_autofree char *reg = virXMLPropString(node, "reg");
+int reg;
memset(addr, 0, sizeof(*addr));
-if (reg) {
-if (virStrToLong_ull(reg, NULL, 16, &addr->reg) < 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("Cannot parse 'reg' attribute"));
-return -1;
-}
+if ((reg = virXMLPropULongLong(node, "reg", 16, VIR_XML_PROP_NONE,
+ &addr->reg)) < 0)
+return -1;
+if (reg != 0)
addr->has_reg = true;
-}
+
return 0;
}
--
2.26.3
[libvirt PATCH 07/10] virStorageAdapterParseXML: Use g_autofree
Signed-off-by: Tim Wiederhake
---
src/conf/storage_adapter_conf.c | 17 ++---
1 file changed, 6 insertions(+), 11 deletions(-)
diff --git a/src/conf/storage_adapter_conf.c b/src/conf/storage_adapter_conf.c
index 6b5a58e1e7..717d00dc4a 100644
--- a/src/conf/storage_adapter_conf.c
+++ b/src/conf/storage_adapter_conf.c
@@ -168,9 +168,8 @@ virStorageAdapterParseXML(virStorageAdapter *adapter,
xmlNodePtr node,
xmlXPathContextPtr ctxt)
{
-int ret = -1;
VIR_XPATH_NODE_AUTORESTORE(ctxt)
-char *adapter_type = NULL;
+g_autofree char *adapter_type = NULL;
ctxt->node = node;
@@ -180,27 +179,23 @@ virStorageAdapterParseXML(virStorageAdapter *adapter,
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Unknown pool adapter type '%s'"),
adapter_type);
-goto cleanup;
+return -1;
}
if ((adapter->type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) &&
(virStorageAdapterParseXMLFCHost(node, &adapter->data.fchost)) < 0)
-goto cleanup;
+return -1;
if ((adapter->type == VIR_STORAGE_ADAPTER_TYPE_SCSI_HOST) &&
(virStorageAdapterParseXMLSCSIHost(node, ctxt,
&adapter->data.scsi_host)) < 0)
-goto cleanup;
+return -1;
} else {
if (virStorageAdapterParseXMLLegacy(node, ctxt, adapter) < 0)
-goto cleanup;
+return -1;
}
-ret = 0;
-
- cleanup:
-VIR_FREE(adapter_type);
-return ret;
+return 0;
}
--
2.26.3
[libvirt PATCH 08/10] virStorageAdapterFCHost: Change type of "type" to virStorageAdapterType
Signed-off-by: Tim Wiederhake
---
src/conf/storage_adapter_conf.c | 5 +++--
src/conf/storage_adapter_conf.h | 2 +-
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/conf/storage_adapter_conf.c b/src/conf/storage_adapter_conf.c
index 717d00dc4a..a834eee81f 100644
--- a/src/conf/storage_adapter_conf.c
+++ b/src/conf/storage_adapter_conf.c
@@ -168,19 +168,20 @@ virStorageAdapterParseXML(virStorageAdapter *adapter,
xmlNodePtr node,
xmlXPathContextPtr ctxt)
{
+int type;
VIR_XPATH_NODE_AUTORESTORE(ctxt)
g_autofree char *adapter_type = NULL;
ctxt->node = node;
if ((adapter_type = virXMLPropString(node, "type"))) {
-if ((adapter->type =
- virStorageAdapterTypeFromString(adapter_type)) <= 0) {
+if ((type = virStorageAdapterTypeFromString(adapter_type)) <= 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("Unknown pool adapter type '%s'"),
adapter_type);
return -1;
}
+adapter->type = type;
if ((adapter->type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) &&
(virStorageAdapterParseXMLFCHost(node, &adapter->data.fchost)) < 0)
diff --git a/src/conf/storage_adapter_conf.h b/src/conf/storage_adapter_conf.h
index e6d9c864cd..1f0e74631e 100644
--- a/src/conf/storage_adapter_conf.h
+++ b/src/conf/storage_adapter_conf.h
@@ -54,7 +54,7 @@ struct _virStorageAdapterFCHost {
typedef struct _virStorageAdapter virStorageAdapter;
struct _virStorageAdapter {
-int type; /* virStorageAdapterType */
+virStorageAdapterType type;
union {
virStorageAdapterSCSIHost scsi_host;
--
2.26.3
[libvirt PATCH 09/10] virStorageAdapterParseXML: Use virXMLProp*
Signed-off-by: Tim Wiederhake
---
src/conf/storage_adapter_conf.c | 30 +++---
1 file changed, 11 insertions(+), 19 deletions(-)
diff --git a/src/conf/storage_adapter_conf.c b/src/conf/storage_adapter_conf.c
index a834eee81f..e93b26f06f 100644
--- a/src/conf/storage_adapter_conf.c
+++ b/src/conf/storage_adapter_conf.c
@@ -170,31 +170,23 @@ virStorageAdapterParseXML(virStorageAdapter *adapter,
{
int type;
VIR_XPATH_NODE_AUTORESTORE(ctxt)
-g_autofree char *adapter_type = NULL;
ctxt->node = node;
-if ((adapter_type = virXMLPropString(node, "type"))) {
-if ((type = virStorageAdapterTypeFromString(adapter_type)) <= 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("Unknown pool adapter type '%s'"),
- adapter_type);
+if ((type = virXMLPropEnum(node, "type", virStorageAdapterTypeFromString,
VIR_XML_PROP_NONZERO, &adapter->type)) < 0)
return -1;
-}
-adapter->type = type;
-if ((adapter->type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) &&
-(virStorageAdapterParseXMLFCHost(node, &adapter->data.fchost)) < 0)
-return -1;
+if (type == 0)
+return virStorageAdapterParseXMLLegacy(node, ctxt, adapter);
-if ((adapter->type == VIR_STORAGE_ADAPTER_TYPE_SCSI_HOST) &&
-(virStorageAdapterParseXMLSCSIHost(node, ctxt,
- &adapter->data.scsi_host)) < 0)
-return -1;
-} else {
-if (virStorageAdapterParseXMLLegacy(node, ctxt, adapter) < 0)
-return -1;
-}
+if ((adapter->type == VIR_STORAGE_ADAPTER_TYPE_FC_HOST) &&
+(virStorageAdapterParseXMLFCHost(node, &adapter->data.fchost)) < 0)
+return -1;
+
+if ((adapter->type == VIR_STORAGE_ADAPTER_TYPE_SCSI_HOST) &&
+(virStorageAdapterParseXMLSCSIHost(node, ctxt,
+ &adapter->data.scsi_host)) < 0)
+return -1;
return 0;
}
--
2.26.3
[libvirt PATCH 02/10] virDomainAudioDef: Change type of "type" to virDomainAudioType
Signed-off-by: Tim Wiederhake
---
src/bhyve/bhyve_command.c | 2 +-
src/conf/domain_conf.c| 18 ++
src/conf/domain_conf.h| 2 +-
src/qemu/qemu_command.c | 4 ++--
src/qemu/qemu_validate.c | 2 +-
5 files changed, 15 insertions(+), 13 deletions(-)
diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c
index f8e0ce5123..ab9d3026cc 100644
--- a/src/bhyve/bhyve_command.c
+++ b/src/bhyve/bhyve_command.c
@@ -524,7 +524,7 @@ bhyveBuildSoundArgStr(const virDomainDef *def G_GNUC_UNUSED,
virCommandAddArg(cmd, "-s");
if (audio) {
-switch ((virDomainAudioType) audio->type) {
+switch (audio->type) {
case VIR_DOMAIN_AUDIO_TYPE_OSS:
if (virDomainAudioIOCommonIsSet(&audio->input) ||
virDomainAudioIOCommonIsSet(&audio->output)) {
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 942d6f269a..758f699c2c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2922,7 +2922,7 @@ virDomainAudioDefFree(virDomainAudioDef *def)
if (!def)
return;
-switch ((virDomainAudioType) def->type) {
+switch (def->type) {
case VIR_DOMAIN_AUDIO_TYPE_NONE:
break;
@@ -13123,24 +13123,26 @@ virDomainAudioDefParseXML(virDomainXMLOption *xmlopt
G_GNUC_UNUSED,
virDomainAudioDef *def;
VIR_XPATH_NODE_AUTORESTORE(ctxt)
g_autofree char *tmp = NULL;
-g_autofree char *type = NULL;
+g_autofree char *typestr = NULL;
+int type;
xmlNodePtr inputNode, outputNode;
def = g_new0(virDomainAudioDef, 1);
ctxt->node = node;
-type = virXMLPropString(node, "type");
-if (!type) {
+typestr = virXMLPropString(node, "type");
+if (!typestr) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("missing audio 'type' attribute"));
goto error;
}
-if ((def->type = virDomainAudioTypeTypeFromString(type)) < 0) {
+if ((type = virDomainAudioTypeTypeFromString(typestr)) < 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unknown audio type '%s'"), type);
+ _("unknown audio type '%s'"), typestr);
goto error;
}
+def->type = type;
tmp = virXMLPropString(node, "id");
if (!tmp) {
@@ -13163,7 +13165,7 @@ virDomainAudioDefParseXML(virDomainXMLOption *xmlopt
G_GNUC_UNUSED,
if (outputNode && virDomainAudioCommonParse(&def->output, outputNode,
ctxt) < 0)
goto error;
-switch ((virDomainAudioType) def->type) {
+switch (def->type) {
case VIR_DOMAIN_AUDIO_TYPE_NONE:
break;
@@ -25465,7 +25467,7 @@ virDomainAudioDefFormat(virBuffer *buf,
virBufferAsprintf(buf, "id, type);
-switch ((virDomainAudioType)def->type) {
+switch (def->type) {
case VIR_DOMAIN_AUDIO_TYPE_NONE:
break;
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index cf8481f1f6..462c61a807 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1543,7 +1543,7 @@ struct _virDomainAudioIOSDL {
};
struct _virDomainAudioDef {
-int type;
+virDomainAudioType type;
unsigned int id;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index d6c5308ef0..dcc060bde9 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7670,7 +7670,7 @@ qemuBuildAudioCommandLineArg(virCommand *cmd,
qemuBuildAudioCommonArg(&buf, "in", &def->input);
qemuBuildAudioCommonArg(&buf, "out", &def->output);
-switch ((virDomainAudioType)def->type) {
+switch (def->type) {
case VIR_DOMAIN_AUDIO_TYPE_NONE:
break;
@@ -7859,7 +7859,7 @@ qemuBuildAudioCommandLineEnv(virCommand *cmd,
qemuBuildAudioCommonEnv(cmd, "QEMU_AUDIO_ADC_", &audio->input);
qemuBuildAudioCommonEnv(cmd, "QEMU_AUDIO_DAC_", &audio->output);
-switch ((virDomainAudioType)audio->type) {
+switch (audio->type) {
case VIR_DOMAIN_AUDIO_TYPE_NONE:
break;
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index 141203f979..e6ddb43113 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -4223,7 +4223,7 @@ qemuValidateDomainDeviceDefAudio(virDomainAudioDef *audio,
}
}
-switch ((virDomainAudioType)audio->type) {
+switch (audio->type) {
case VIR_DOMAIN_AUDIO_TYPE_NONE:
break;
--
2.26.3
[libvirt PATCH 06/10] virDomainIOMMUDefParseXML: Use virXMLProp*
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `aw_bits`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute.
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 69 +++---
1 file changed, 17 insertions(+), 52 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 1350c46039..e35c38caa3 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14805,71 +14805,36 @@ virDomainIOMMUDefParseXML(xmlNodePtr node,
{
VIR_XPATH_NODE_AUTORESTORE(ctxt)
xmlNodePtr driver;
-int val;
-g_autofree char *tmp = NULL;
g_autofree virDomainIOMMUDef *iommu = NULL;
ctxt->node = node;
iommu = g_new0(virDomainIOMMUDef, 1);
-if (!(tmp = virXMLPropString(node, "model"))) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
- _("missing model for IOMMU device"));
+if (virXMLPropEnum(node, "model", virDomainIOMMUModelTypeFromString,
+ VIR_XML_PROP_REQUIRED, &iommu->model) < 0)
return NULL;
-}
-
-if ((val = virDomainIOMMUModelTypeFromString(tmp)) < 0) {
-virReportError(VIR_ERR_XML_ERROR, _("unknown IOMMU model: %s"), tmp);
-return NULL;
-}
-
-iommu->model = val;
if ((driver = virXPathNode("./driver", ctxt))) {
-VIR_FREE(tmp);
-if ((tmp = virXMLPropString(driver, "intremap"))) {
-if ((val = virTristateSwitchTypeFromString(tmp)) < 0) {
-virReportError(VIR_ERR_XML_ERROR, _("unknown intremap value:
%s"), tmp);
-return NULL;
-}
-iommu->intremap = val;
-}
+if (virXMLPropTristateSwitch(driver, "intremap", VIR_XML_PROP_NONE,
+ &iommu->intremap) < 0)
+return NULL;
-VIR_FREE(tmp);
-if ((tmp = virXMLPropString(driver, "caching_mode"))) {
-if ((val = virTristateSwitchTypeFromString(tmp)) < 0) {
-virReportError(VIR_ERR_XML_ERROR, _("unknown caching_mode
value: %s"), tmp);
-return NULL;
-}
-iommu->caching_mode = val;
-}
-VIR_FREE(tmp);
-if ((tmp = virXMLPropString(driver, "iotlb"))) {
-if ((val = virTristateSwitchTypeFromString(tmp)) < 0) {
-virReportError(VIR_ERR_XML_ERROR, _("unknown iotlb value:
%s"), tmp);
-return NULL;
-}
-iommu->iotlb = val;
-}
+if (virXMLPropTristateSwitch(driver, "caching_mode", VIR_XML_PROP_NONE,
+ &iommu->caching_mode) < 0)
+return NULL;
-VIR_FREE(tmp);
-if ((tmp = virXMLPropString(driver, "eim"))) {
-if ((val = virTristateSwitchTypeFromString(tmp)) < 0) {
-virReportError(VIR_ERR_XML_ERROR, _("unknown eim value: %s"),
tmp);
-return NULL;
-}
-iommu->eim = val;
-}
+if (virXMLPropTristateSwitch(driver, "iotlb", VIR_XML_PROP_NONE,
+ &iommu->iotlb) < 0)
+return NULL;
-VIR_FREE(tmp);
-if ((tmp = virXMLPropString(driver, "aw_bits"))) {
-if (virStrToLong_ui(tmp, NULL, 10, &iommu->aw_bits) < 0) {
-virReportError(VIR_ERR_XML_ERROR, _("unknown aw_bits value:
%s"), tmp);
-return NULL;
-}
-}
+if (virXMLPropTristateSwitch(driver, "eim", VIR_XML_PROP_NONE,
+ &iommu->eim) < 0)
+return NULL;
+if (virXMLPropUInt(driver, "aw_bits", 10, VIR_XML_PROP_NONE,
+ &iommu->aw_bits) < 0)
+return NULL;
}
return g_steal_pointer(&iommu);
--
2.26.3
[libvirt PATCH 01/10] virDomainAudioPulseAudioParse: Use virXMLProp*
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `latency`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute.
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 10 ++
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b3ed3a9c5a..942d6f269a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13092,18 +13092,12 @@ static int
virDomainAudioPulseAudioParse(virDomainAudioIOPulseAudio *def,
xmlNodePtr node)
{
-g_autofree char *latency = virXMLPropString(node, "latency");
-
def->name = virXMLPropString(node, "name");
def->streamName = virXMLPropString(node, "streamName");
-if (latency &&
-virStrToLong_ui(latency, NULL, 10,
-&def->latency) < 0) {
-virReportError(VIR_ERR_XML_ERROR,
- _("cannot parse 'latency' value '%s'"), latency);
+if (virXMLPropUInt(node, "latency", 10, VIR_XML_PROP_NONE,
+ &def->latency) < 0)
return -1;
-}
return 0;
}
--
2.26.3
[libvirt PATCH 00/10] Refactor more XML parsing boilerplate code, part XII
For background, see https://listman.redhat.com/archives/libvir-list/2021-April/msg00668.html Tim Wiederhake (10): virDomainAudioPulseAudioParse: Use virXMLProp* virDomainAudioDef: Change type of "type" to virDomainAudioType virDomainAudioDef: Change type of "sdl.driver" to virDomainAudioSDLDriver virDomainAudioDefParseXML: Use virXMLProp* virDomainAudioDefParseXML: Don't ignore return value of virDomainAudio*Parse() virDomainIOMMUDefParseXML: Use virXMLProp* virStorageAdapterParseXML: Use g_autofree virStorageAdapterFCHost: Change type of "type" to virStorageAdapterType virStorageAdapterParseXML: Use virXMLProp* virDomainDeviceSpaprVioAddressParseXML: Use virXMLProp* src/bhyve/bhyve_command.c | 2 +- src/conf/device_conf.c | 14 +-- src/conf/domain_conf.c | 202 +++- src/conf/domain_conf.h | 4 +- src/conf/storage_adapter_conf.c | 38 ++ src/conf/storage_adapter_conf.h | 2 +- src/qemu/qemu_command.c | 4 +- src/qemu/qemu_validate.c| 2 +- 8 files changed, 97 insertions(+), 171 deletions(-) -- 2.26.3
[libvirt PATCH 03/10] virDomainAudioDef: Change type of "sdl.driver" to virDomainAudioSDLDriver
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 17 +
src/conf/domain_conf.h | 2 +-
2 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 758f699c2c..9e6719265f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13236,15 +13236,16 @@ virDomainAudioDefParseXML(virDomainXMLOption *xmlopt
G_GNUC_UNUSED,
break;
case VIR_DOMAIN_AUDIO_TYPE_SDL: {
-g_autofree char *driver = virXMLPropString(node, "driver");
-if (driver &&
-(def->backend.sdl.driver =
- virDomainAudioSDLDriverTypeFromString(driver)) <= 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unknown SDL driver '%s'"), driver);
-goto error;
+g_autofree char *driverstr = virXMLPropString(node, "driver");
+int driver;
+if (driverstr) {
+if ((driver = virDomainAudioSDLDriverTypeFromString(driverstr)) <=
0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown SDL driver '%s'"), driverstr);
+goto error;
+}
+def->backend.sdl.driver = driver;
}
-
if (inputNode)
virDomainAudioSDLParse(&def->backend.sdl.input, inputNode);
if (outputNode)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 462c61a807..fab856a5c7 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1578,7 +1578,7 @@ struct _virDomainAudioDef {
struct {
virDomainAudioIOSDL input;
virDomainAudioIOSDL output;
-int driver; /* virDomainAudioSDLDriver */
+virDomainAudioSDLDriver driver;
} sdl;
struct {
char *path;
--
2.26.3
[libvirt PATCH 04/10] virDomainAudioDefParseXML: Use virXMLProp*
This strictens the parser to disallow negative values (interpreted as
`UINT_MAX + value + 1`) for attribute `id`. Allowing negative
numbers to be interpreted this way makes no sense for this attribute.
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 74 --
1 file changed, 21 insertions(+), 53 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9e6719265f..2142e45fd5 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13122,40 +13122,18 @@ virDomainAudioDefParseXML(virDomainXMLOption *xmlopt
G_GNUC_UNUSED,
{
virDomainAudioDef *def;
VIR_XPATH_NODE_AUTORESTORE(ctxt)
-g_autofree char *tmp = NULL;
-g_autofree char *typestr = NULL;
-int type;
xmlNodePtr inputNode, outputNode;
def = g_new0(virDomainAudioDef, 1);
ctxt->node = node;
-typestr = virXMLPropString(node, "type");
-if (!typestr) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
- _("missing audio 'type' attribute"));
-goto error;
-}
-
-if ((type = virDomainAudioTypeTypeFromString(typestr)) < 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unknown audio type '%s'"), typestr);
+if (virXMLPropEnum(node, "type", virDomainAudioTypeTypeFromString,
+ VIR_XML_PROP_REQUIRED, &def->type) < 0)
goto error;
-}
-def->type = type;
-tmp = virXMLPropString(node, "id");
-if (!tmp) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("missing audio 'id' attribute"));
-goto error;
-}
-if (virStrToLong_ui(tmp, NULL, 10, &def->id) < 0 ||
-def->id == 0) {
-virReportError(VIR_ERR_XML_ERROR,
- _("invalid audio 'id' value '%s'"), tmp);
+if (virXMLPropUInt(node, "id", 10, VIR_XML_PROP_REQUIRED |
VIR_XML_PROP_NONZERO,
+ &def->id) < 0)
goto error;
-}
inputNode = virXPathNode("./input", ctxt);
outputNode = virXPathNode("./output", ctxt);
@@ -13191,29 +13169,25 @@ virDomainAudioDefParseXML(virDomainXMLOption *xmlopt
G_GNUC_UNUSED,
break;
case VIR_DOMAIN_AUDIO_TYPE_OSS: {
-g_autofree char *tryMMap = virXMLPropString(node, "tryMMap");
-g_autofree char *exclusive = virXMLPropString(node, "exclusive");
-g_autofree char *dspPolicy = virXMLPropString(node, "dspPolicy");
+int dspPolicySet;
-if (tryMMap && ((def->backend.oss.tryMMap =
- virTristateBoolTypeFromString(tryMMap)) <= 0)) {
-virReportError(VIR_ERR_XML_ERROR,
- _("unknown 'tryMMap' value '%s'"), tryMMap);
+if (virXMLPropTristateBool(node, "tryMMap", VIR_XML_PROP_NONE,
+ &def->backend.oss.tryMMap) < 0)
goto error;
-}
-if (exclusive && ((def->backend.oss.exclusive =
- virTristateBoolTypeFromString(exclusive)) <= 0)) {
-virReportError(VIR_ERR_XML_ERROR,
- _("unknown 'exclusive' value '%s'"), exclusive);
+if (virXMLPropTristateBool(node, "exclusive", VIR_XML_PROP_NONE,
+ &def->backend.oss.exclusive) < 0)
+goto error;
+
+if ((dspPolicySet = virXMLPropInt(node, "dspPolicy", 10,
VIR_XML_PROP_NONE,
+ &def->backend.oss.dspPolicy, 0)) < 0)
goto error;
-}
-if (dspPolicy) {
-if (virStrToLong_i(dspPolicy, NULL, 10,
- &def->backend.oss.dspPolicy) < 0) {
+if (dspPolicySet != 0) {
+if (def->backend.oss.dspPolicy < 0) {
virReportError(VIR_ERR_XML_ERROR,
- _("cannot parse 'dspPolicy' value '%s'"),
dspPolicy);
+ _("cannot parse 'dspPolicy' value '%i'"),
+ def->backend.oss.dspPolicy);
goto error;
}
def->backend.oss.dspPolicySet = true;
@@ -13236,16 +13210,10 @@ virDomainAudioDefParseXML(virDomainXMLOption *xmlopt
G_GNUC_UNUSED,
break;
case VIR_DOMAIN_AUDIO_TYPE_SDL: {
-g_autofree char *driverstr = virXMLPropString(node, "driver");
-int driver;
-if (driverstr) {
-if ((driver = virDomainAudioSDLDriver
[libvirt PATCH 05/10] virDomainAudioDefParseXML: Don't ignore return value of virDomainAudio*Parse()
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 50 +-
1 file changed, 30 insertions(+), 20 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2142e45fd5..1350c46039 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13155,17 +13155,21 @@ virDomainAudioDefParseXML(virDomainXMLOption *xmlopt
G_GNUC_UNUSED,
break;
case VIR_DOMAIN_AUDIO_TYPE_COREAUDIO:
-if (inputNode)
-virDomainAudioCoreAudioParse(&def->backend.coreaudio.input,
inputNode);
-if (outputNode)
-virDomainAudioCoreAudioParse(&def->backend.coreaudio.output,
outputNode);
+if (inputNode &&
+virDomainAudioCoreAudioParse(&def->backend.coreaudio.input,
inputNode) < 0)
+goto error;
+if (outputNode &&
+virDomainAudioCoreAudioParse(&def->backend.coreaudio.output,
outputNode) < 0)
+goto error;
break;
case VIR_DOMAIN_AUDIO_TYPE_JACK:
-if (inputNode)
-virDomainAudioJackParse(&def->backend.jack.input, inputNode);
-if (outputNode)
-virDomainAudioJackParse(&def->backend.jack.output, outputNode);
+if (inputNode &&
+virDomainAudioJackParse(&def->backend.jack.input, inputNode) < 0)
+goto error;
+if (outputNode &&
+virDomainAudioJackParse(&def->backend.jack.output, outputNode) < 0)
+goto error;
break;
case VIR_DOMAIN_AUDIO_TYPE_OSS: {
@@ -13193,20 +13197,24 @@ virDomainAudioDefParseXML(virDomainXMLOption *xmlopt
G_GNUC_UNUSED,
def->backend.oss.dspPolicySet = true;
}
-if (inputNode)
-virDomainAudioOSSParse(&def->backend.oss.input, inputNode);
-if (outputNode)
-virDomainAudioOSSParse(&def->backend.oss.output, outputNode);
+if (inputNode &&
+virDomainAudioOSSParse(&def->backend.oss.input, inputNode) < 0)
+goto error;
+if (outputNode &&
+virDomainAudioOSSParse(&def->backend.oss.output, outputNode) < 0)
+goto error;
break;
}
case VIR_DOMAIN_AUDIO_TYPE_PULSEAUDIO:
def->backend.pulseaudio.serverName = virXMLPropString(node,
"serverName");
-if (inputNode)
-virDomainAudioPulseAudioParse(&def->backend.pulseaudio.input,
inputNode);
-if (outputNode)
-virDomainAudioPulseAudioParse(&def->backend.pulseaudio.output,
outputNode);
+if (inputNode &&
+virDomainAudioPulseAudioParse(&def->backend.pulseaudio.input,
inputNode) < 0)
+goto error;
+if (outputNode &&
+virDomainAudioPulseAudioParse(&def->backend.pulseaudio.output,
outputNode) < 0)
+goto error;
break;
case VIR_DOMAIN_AUDIO_TYPE_SDL: {
@@ -13214,10 +13222,12 @@ virDomainAudioDefParseXML(virDomainXMLOption *xmlopt
G_GNUC_UNUSED,
VIR_XML_PROP_NONZERO, &def->backend.sdl.driver) < 0)
goto error;
-if (inputNode)
-virDomainAudioSDLParse(&def->backend.sdl.input, inputNode);
-if (outputNode)
-virDomainAudioSDLParse(&def->backend.sdl.output, outputNode);
+if (inputNode &&
+virDomainAudioSDLParse(&def->backend.sdl.input, inputNode) < 0)
+goto error;
+if (outputNode &&
+virDomainAudioSDLParse(&def->backend.sdl.output, outputNode) < 0)
+goto error;
break;
}
--
2.26.3
[libvirt PATCH 1/2] cpu_map: sync_qemu_i386.py: Use regex to look for begin mark
Signed-off-by: Tim Wiederhake
---
src/cpu_map/sync_qemu_i386.py | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/cpu_map/sync_qemu_i386.py b/src/cpu_map/sync_qemu_i386.py
index 1a98fa70d7..40c7d6e969 100755
--- a/src/cpu_map/sync_qemu_i386.py
+++ b/src/cpu_map/sync_qemu_i386.py
@@ -196,7 +196,7 @@ def read_builtin_x86_defs(filename):
"""Extract content between begin_mark and end_mark from file `filename` as
string, while expanding shorthand macros like "I486_FEATURES"."""
-begin_mark = "static X86CPUDefinition builtin_x86_defs[] = {\n"
+begin_mark = re.compile("^static X86CPUDefinition builtin_x86_defs\\[\\] =
{$")
end_mark = "};\n"
shorthand = re.compile("^#define ([A-Z0-9_]+_FEATURES) (.*)$")
lines = list()
@@ -205,10 +205,11 @@ def read_builtin_x86_defs(filename):
with open(filename, "rt") as f:
while True:
line = readline_cont(f)
-if line == begin_mark:
-break
if not line:
raise RuntimeError("begin mark not found")
+match = begin_mark.match(line)
+if match:
+break;
match = shorthand.match(line)
if match:
# TCG definitions are irrelevant for cpu models
--
2.26.3
[libvirt PATCH 2/2] cpu_map: sync_qemu_i386.py: Allow begin mark to contain `const`
This was introduced in qemu commit e11fd68996fb27c040552320f01a7d30a15a7cc1.
Signed-off-by: Tim Wiederhake
---
src/cpu_map/sync_qemu_i386.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/cpu_map/sync_qemu_i386.py b/src/cpu_map/sync_qemu_i386.py
index 40c7d6e969..fbf1edf5d0 100755
--- a/src/cpu_map/sync_qemu_i386.py
+++ b/src/cpu_map/sync_qemu_i386.py
@@ -196,7 +196,7 @@ def read_builtin_x86_defs(filename):
"""Extract content between begin_mark and end_mark from file `filename` as
string, while expanding shorthand macros like "I486_FEATURES"."""
-begin_mark = re.compile("^static X86CPUDefinition builtin_x86_defs\\[\\] =
{$")
+begin_mark = re.compile("^static( const)? X86CPUDefinition
builtin_x86_defs\\[\\] = {$")
end_mark = "};\n"
shorthand = re.compile("^#define ([A-Z0-9_]+_FEATURES) (.*)$")
lines = list()
--
2.26.3
[libvirt PATCH 0/2] Update sync_qemu_i386.py tool for changes in QEMU
QEMU commit e11fd68996fb27c040552320f01a7d30a15a7cc1 changed a line that was used by our tool as marker. Commit 1 prepares for the change, commit 2 actually makes the required adjustment. Cheers, Tim Tim Wiederhake (2): cpu_map: sync_qemu_i386.py: Use regex to look for begin mark cpu_map: sync_qemu_i386.py: Allow begin mark to contain `const` src/cpu_map/sync_qemu_i386.py | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) -- 2.26.3
Re: [libvirt PATCH v2 0/7] Enable sanitizers
Ping On Tue, 2021-05-18 at 10:41 +0200, Tim Wiederhake wrote: > Ping. > > On Thu, 2021-05-06 at 17:08 +0200, Tim Wiederhake wrote: > > This series enables and adds AddressSanitizer and > > UndefinedBehaviorSanitizer > > builds to the CI. > > > > See: > > https://clang.llvm.org/docs/AddressSanitizer.html and > > https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html > > > > These sanitizers already found some issues in libvirt, e.g. > > 4eb7c621985dad4de911ec394ac628bd1a5b29ab, > > 1294de209cee6643511265c7e2d4283c047cf652, > > 8b8c91f487592c6c067847ca59dde405ca17573f, or > > 1c34211c22de28127a509edbf2cf2f44cb0d891e. > > > > There exist two more relevant sanitizers, ThreadSanitizer and > > MemorySanitizer. > > Unfortunately, those two require an instrumented build of all > > dependencies, > > including libc, to work correctly. > > > > Note that clang and gcc have different implementations of these > > sanitizers, > > hence the introduction of two new jobs to the CI. The latter one > > issues a > > warning about the use of LD_PRELOAD in `virTestMain`, which in this > > particular case can be safely ignored by setting `ASAN_OPTIONS` to > > verify_asan_link_order=0` for the gcc build. > > > > Changes since V1: > > > > Incorporated changes suggested by Pavel, except for #6 (now #7): > > The > > statement > > in > > https://listman.redhat.com/archives/libvir-list/2021-May/msg00070.html > > on > > the sanitizers working with Fedora 33 is wrong, I was fooled by > > caching. The > > bug described there is present in Fedora 33, 34, and Rawhide. > > > > Cheers, > > Tim > > > > Tim Wiederhake (7): > > meson: Allow larger stack frames when instrumenting > > meson: Allow undefined symbols when sanitizers are enabled > > tests: virfilemock: realpath: Allow non-null second parameter > > openvz: Add missing symbols to libvirt_openvz.syms > > tests: openvzutilstest: Remove duplicate linking with > > libvirt_openvz.a > > virt-aa-helper: Remove duplicate linking with src/datatypes.o > > ci: Enable address and undefined behavior sanitizers > > > > .gitlab-ci.yml | 35 +++ > > build-aux/syntax-check.mk | 2 +- > > meson.build | 14 ++ > > src/libvirt_openvz.syms | 2 ++ > > src/security/meson.build | 1 - > > tests/meson.build | 2 +- > > tests/virfilemock.c | 20 > > 7 files changed, 61 insertions(+), 15 deletions(-) > > > > -- > > 2.26.3 > > > >
[libvirt PATCH] cpu_map: sync_qemu_i386.py: Remove superfluous semicolon
The semicolon in question makes the pipeline fail over a style checker
complaint.
Introduced-in: 360b8eb2d2cb1b6a8c9a78fa2c5be31dd7c74487
Signed-off-by: Tim Wiederhake
---
src/cpu_map/sync_qemu_i386.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/cpu_map/sync_qemu_i386.py b/src/cpu_map/sync_qemu_i386.py
index fbf1edf5d0..92bb58f75b 100755
--- a/src/cpu_map/sync_qemu_i386.py
+++ b/src/cpu_map/sync_qemu_i386.py
@@ -209,7 +209,7 @@ def read_builtin_x86_defs(filename):
raise RuntimeError("begin mark not found")
match = begin_mark.match(line)
if match:
-break;
+break
match = shorthand.match(line)
if match:
# TCG definitions are irrelevant for cpu models
--
2.31.1
[libvirt PATCH] cpu_map: Add cpu feature avx-vnni
"avx-vvni" was introduced to qemu in commit c1826ea6a052084f2e6a0bae9dd5932a727df039, adding it Cooperlake. This feature is currently not used by any libvirt CPU models, but its addition silences a warning from sync_qemu_i386.py: ``` warning: Unknown feature 'CPUID_7_1_EAX_AVX_VNNI' warning: Feature unknown to libvirt: CPUID_7_1_EAX_AVX_VNNI ``` Signed-off-by: Tim Wiederhake --- src/cpu_map/sync_qemu_i386.py | 1 + src/cpu_map/x86_features.xml | 3 +++ 2 files changed, 4 insertions(+) diff --git a/src/cpu_map/sync_qemu_i386.py b/src/cpu_map/sync_qemu_i386.py index 92bb58f75b..4dd9f3b84d 100755 --- a/src/cpu_map/sync_qemu_i386.py +++ b/src/cpu_map/sync_qemu_i386.py @@ -73,6 +73,7 @@ def translate_feature(name): "CPUID_7_0_EDX_SPEC_CTRL_SSBD": "ssbd", "CPUID_7_0_EDX_STIBP": "stibp", "CPUID_7_1_EAX_AVX512_BF16": "avx512-bf16", +"CPUID_7_1_EAX_AVX_VNNI": "avx-vnni", "CPUID_8000_0008_EBX_AMD_SSBD": "amd-ssbd", "CPUID_8000_0008_EBX_CLZERO": "clzero", "CPUID_8000_0008_EBX_IBPB": "ibpb", diff --git a/src/cpu_map/x86_features.xml b/src/cpu_map/x86_features.xml index e98b84f3ef..4cf3ff0804 100644 --- a/src/cpu_map/x86_features.xml +++ b/src/cpu_map/x86_features.xml @@ -363,6 +363,9 @@ + + + -- 2.31.1
Re: [PATCH 1/3] Don't call qsort() over NULL
On Mon, 2021-06-14 at 13:06 +0200, Michal Privoznik wrote: > In a few places it may happen that the array we want to sort is > still NULL (e.g. because there were no leases found, no paths for > secdriver to lock or no cache banks). However, passing NULL to > qsort() is undefined and even though glibc plays nicely we > shouldn't rely on undefined behaviour. > > Signed-off-by: Michal Privoznik Reviewed-by: Tim Wiederhake
Re: [PATCH 3/3] src: Use 1U for bit shifting
On Mon, 2021-06-14 at 13:06 +0200, Michal Privoznik wrote: > In a few places we take 1 and shift it left repeatedly. So much > that it won't longer fit into signed integer. The problem is that > this is undefined behaviour. Switching to 1U makes us stay within > boundaries. > > Signed-off-by: Michal Privoznik Reviewed-by: Tim Wiederhake
Re: [PATCH 2/3] tests: Don't pass INT_MAX to virFileReadAll()
On Mon, 2021-06-14 at 13:06 +0200, Michal Privoznik wrote:
> In a few occasions in tests we pass INT_MAX to
> virFileReadLimFD(). This is not safe because virFileReadAll()
> will call virFileReadLimFD() under the hood which takes the limit
> and adds 1 to it.
Calling virFileReadAll with "INT_MAX - 1" looks funny. Is it possible
to check for "maxlen >= INT_MAX" in virFileReadLimFD instead?
> And since we use signed integer for all of this
> an overflow will occur.
>
> Signed-off-by: Michal Privoznik
> ---
> tests/networkxml2firewalltest.c | 2 +-
> tests/testutils.c | 4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tests/networkxml2firewalltest.c
> b/tests/networkxml2firewalltest.c
> index 91336a0c55..facbc20a0c 100644
> --- a/tests/networkxml2firewalltest.c
> +++ b/tests/networkxml2firewalltest.c
> @@ -176,7 +176,7 @@ mymain(void)
>
> basefile =
> g_strdup_printf("%s/networkxml2firewalldata/base.args", abs_srcdir);
>
> - if (virFileReadAll(basefile, INT_MAX, &baseargs) < 0)
> + if (virFileReadAll(basefile, INT_MAX - 1, &baseargs) < 0)
> return EXIT_FAILURE;
>
> DO_TEST("nat-default");
> diff --git a/tests/testutils.c b/tests/testutils.c
> index eb3bd48b6a..4a63c6cc37 100644
> --- a/tests/testutils.c
> +++ b/tests/testutils.c
> @@ -313,7 +313,7 @@ virTestLoadFileJSON(const char *p, ...)
> if (!(path = virTestLoadFileGetPath(p, ap)))
> goto cleanup;
>
> - if (virFileReadAll(path, INT_MAX, &jsonstr) < 0)
> + if (virFileReadAll(path, INT_MAX - 1, &jsonstr) < 0)
> goto cleanup;
>
> if (!(ret = virJSONValueFromString(jsonstr)))
> @@ -562,7 +562,7 @@ virTestCompareToFileFull(const char *actual,
> if (virTestLoadFile(filename, &filecontent) < 0 &&
> !virTestGetRegenerate())
> return -1;
> } else {
> - if (virFileReadAll(filename, INT_MAX, &filecontent) < 0 &&
> !virTestGetRegenerate())
> + if (virFileReadAll(filename, INT_MAX - 1, &filecontent) < 0
> && !virTestGetRegenerate())
> return -1;
> }
>
Re: [PATCH] gitlab-ci: Don't build docs in 'sanitizer' jobs
On Tue, 2021-06-15 at 17:45 +0200, Peter Krempa wrote: > Docs are not sanitized, thus there's no point in building them. > > Signed-off-by: Peter Krempa Reviewed-by: Tim Wiederhake > --- > .gitlab-ci.yml | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml > index b5930a0a46..3fa616261e 100644 > --- a/.gitlab-ci.yml > +++ b/.gitlab-ci.yml > @@ -86,7 +86,7 @@ stages: > before_script: > - *script_variables > script: > - - meson build --werror -Db_lundef=false - > Db_sanitize="$SANITIZER" > + - meson build --werror -Ddocs=disabled -Db_lundef=false - > Db_sanitize="$SANITIZER" > - ninja -C build; > - ninja -C build test; >
[libvirt PATCH 04/16] virDomainFeaturesHyperVDefParse: Remove tautological "if"
Fix some line wrapping in the process.
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 163 +++--
1 file changed, 77 insertions(+), 86 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b778dfe463..3ba41869ec 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17292,115 +17292,106 @@ virDomainFeaturesHyperVDefParse(virDomainDef *def,
{
def->features[VIR_DOMAIN_FEATURE_HYPERV] = VIR_TRISTATE_SWITCH_ON;
-if (def->features[VIR_DOMAIN_FEATURE_HYPERV] == VIR_TRISTATE_SWITCH_ON) {
+node = xmlFirstElementChild(node);
+while (node != NULL) {
int feature;
virTristateSwitch value;
+xmlNodePtr child;
-node = xmlFirstElementChild(node);
-while (node != NULL) {
-xmlNodePtr child;
+feature = virDomainHypervTypeFromString((const char *)node->name);
+if (feature < 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported HyperV Enlightenment feature: %s"),
+ node->name);
+return -1;
+}
-feature = virDomainHypervTypeFromString((const char *)node->name);
-if (feature < 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unsupported HyperV Enlightenment feature:
%s"),
- node->name);
-return -1;
-}
+if (virXMLPropTristateSwitch(node, "state", VIR_XML_PROP_REQUIRED,
+ &value) < 0)
+return -1;
-if (virXMLPropTristateSwitch(node, "state",
- VIR_XML_PROP_REQUIRED, &value) < 0)
-return -1;
+def->hyperv_features[feature] = value;
-def->hyperv_features[feature] = value;
+switch ((virDomainHyperv) feature) {
+case VIR_DOMAIN_HYPERV_RELAXED:
+case VIR_DOMAIN_HYPERV_VAPIC:
+case VIR_DOMAIN_HYPERV_VPINDEX:
+case VIR_DOMAIN_HYPERV_RUNTIME:
+case VIR_DOMAIN_HYPERV_SYNIC:
+case VIR_DOMAIN_HYPERV_RESET:
+case VIR_DOMAIN_HYPERV_FREQUENCIES:
+case VIR_DOMAIN_HYPERV_REENLIGHTENMENT:
+case VIR_DOMAIN_HYPERV_TLBFLUSH:
+case VIR_DOMAIN_HYPERV_IPI:
+case VIR_DOMAIN_HYPERV_EVMCS:
+break;
-switch ((virDomainHyperv) feature) {
-case VIR_DOMAIN_HYPERV_RELAXED:
-case VIR_DOMAIN_HYPERV_VAPIC:
-case VIR_DOMAIN_HYPERV_VPINDEX:
-case VIR_DOMAIN_HYPERV_RUNTIME:
-case VIR_DOMAIN_HYPERV_SYNIC:
-case VIR_DOMAIN_HYPERV_RESET:
-case VIR_DOMAIN_HYPERV_FREQUENCIES:
-case VIR_DOMAIN_HYPERV_REENLIGHTENMENT:
-case VIR_DOMAIN_HYPERV_TLBFLUSH:
-case VIR_DOMAIN_HYPERV_IPI:
-case VIR_DOMAIN_HYPERV_EVMCS:
+case VIR_DOMAIN_HYPERV_STIMER:
+if (value != VIR_TRISTATE_SWITCH_ON)
break;
-case VIR_DOMAIN_HYPERV_STIMER:
-if (value != VIR_TRISTATE_SWITCH_ON)
-break;
+child = xmlFirstElementChild(node);
+while (child) {
+if (STRNEQ((const char *)child->name, "direct")) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported Hyper-V stimer feature: %s"),
+ child->name);
+return -1;
+}
-child = xmlFirstElementChild(node);
-while (child) {
-if (STRNEQ((const char *)child->name, "direct")) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unsupported Hyper-V stimer feature:
%s"),
- child->name);
-return -1;
-}
+if (virXMLPropTristateSwitch(child, "state",
VIR_XML_PROP_REQUIRED,
+ &def->hyperv_stimer_direct) < 0)
+return -1;
-if (virXMLPropTristateSwitch(child, "state",
- VIR_XML_PROP_REQUIRED,
- &def->hyperv_stimer_direct) <
0)
-return -1;
+child = xmlNextElementSibling(child);
+}
+break;
-child = xmlNextElementSibling(child);
-}
+case VIR_DOMAIN_HYPERV_SPINLOCKS:
+if (value != VIR_TRISTATE_SWITCH_ON)
break;
-
[libvirt PATCH 08/16] virDomainFeaturesKVMDefParse: Remove tautological "if"
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 30 ++
1 file changed, 14 insertions(+), 16 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 62565601ab..24529f3093 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17404,28 +17404,26 @@ virDomainFeaturesKVMDefParse(virDomainDef *def,
{
def->features[VIR_DOMAIN_FEATURE_KVM] = VIR_TRISTATE_SWITCH_ON;
-if (def->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON) {
+node = xmlFirstElementChild(node);
+while (node) {
int feature;
virTristateSwitch value;
-node = xmlFirstElementChild(node);
-while (node) {
-feature = virDomainKVMTypeFromString((const char *)node->name);
-if (feature < 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unsupported KVM feature: %s"),
- node->name);
-return -1;
-}
+feature = virDomainKVMTypeFromString((const char *)node->name);
+if (feature < 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported KVM feature: %s"),
+ node->name);
+return -1;
+}
-if (virXMLPropTristateSwitch(node, "state", VIR_XML_PROP_REQUIRED,
- &value) < 0)
-return -1;
+if (virXMLPropTristateSwitch(node, "state", VIR_XML_PROP_REQUIRED,
+ &value) < 0)
+return -1;
-def->kvm_features[feature] = value;
+def->kvm_features[feature] = value;
-node = xmlNextElementSibling(node);
-}
+node = xmlNextElementSibling(node);
}
return 0;
--
2.31.1
[libvirt PATCH 00/16] Refactor virDomainFeaturesDefParse
Some refactoring in preparation for adding support for qemu's "hv-passthrough" and the yet-to-be-merged "hv-defaults". Tim Wiederhake (16): virDomainFeaturesDefParse: Factor out HyperV parsing into separate function virDomainFeaturesHyperVDefParse: Inline hyperv/stimer parsing virDomainFeaturesHyperVDefParse: Remove ctxt virDomainFeaturesHyperVDefParse: Remove tautological "if" virDomainFeaturesDefParse: Factor out KVM parsing into separate function virDomainFeaturesKVMDefParse: Remove ctxt virDomainFeaturesKVMDefParse: Remove tautological "switch" virDomainFeaturesKVMDefParse: Remove tautological "if" virDomainFeaturesDefParse: Factor out XEN parsing into separate function virDomainFeaturesXENDefParse: Remove ctxt virDomainFeaturesXENDefParse: Remove tautological "if" virDomainFeaturesDefParse: Inline SMM parsing virDomainFeaturesDefParse: Inline MSRS parsing virDomainFeaturesDefParse: Factor out capabilities parsing into separate function virDomainFeaturesCapabilitiesDefParse: Remove ctxt virDomainFeaturesDefParse: Simplify APIC parsing src/conf/domain_conf.c | 557 ++--- 1 file changed, 296 insertions(+), 261 deletions(-) -- 2.31.1
[libvirt PATCH 09/16] virDomainFeaturesDefParse: Factor out XEN parsing into separate function
Only moving code, cleanup to follow.
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 108 -
1 file changed, 63 insertions(+), 45 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 24529f3093..e687f18afe 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17430,6 +17430,64 @@ virDomainFeaturesKVMDefParse(virDomainDef *def,
}
+static int
+virDomainFeaturesXENDefParse(virDomainDef *def,
+ xmlXPathContext *ctxt)
+{
+g_autofree xmlNodePtr *nodes = NULL;
+size_t i;
+int n;
+
+def->features[VIR_DOMAIN_FEATURE_XEN] = VIR_TRISTATE_SWITCH_ON;
+
+if (def->features[VIR_DOMAIN_FEATURE_XEN] == VIR_TRISTATE_SWITCH_ON) {
+int feature;
+virTristateSwitch value;
+
+if ((n = virXPathNodeSet("./features/xen/*", ctxt, &nodes)) < 0)
+return -1;
+
+for (i = 0; i < n; i++) {
+feature = virDomainXenTypeFromString((const char *)nodes[i]->name);
+if (feature < 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported Xen feature: %s"),
+ nodes[i]->name);
+return -1;
+}
+
+if (virXMLPropTristateSwitch(nodes[i], "state",
+ VIR_XML_PROP_REQUIRED, &value) < 0)
+return -1;
+
+def->xen_features[feature] = value;
+
+switch ((virDomainXen) feature) {
+case VIR_DOMAIN_XEN_E820_HOST:
+break;
+
+case VIR_DOMAIN_XEN_PASSTHROUGH:
+if (value != VIR_TRISTATE_SWITCH_ON)
+break;
+
+if (virXMLPropEnum(nodes[i], "mode",
+ virDomainXenPassthroughModeTypeFromString,
+ VIR_XML_PROP_NONZERO,
+ &def->xen_passthrough_mode) < 0)
+return -1;
+break;
+
+case VIR_DOMAIN_XEN_LAST:
+break;
+}
+}
+VIR_FREE(nodes);
+}
+
+return 0;
+}
+
+
static int
virDomainFeaturesDefParse(virDomainDef *def,
xmlXPathContextPtr ctxt)
@@ -17468,7 +17526,6 @@ virDomainFeaturesDefParse(virDomainDef *def,
case VIR_DOMAIN_FEATURE_VIRIDIAN:
case VIR_DOMAIN_FEATURE_PRIVNET:
case VIR_DOMAIN_FEATURE_MSRS:
-case VIR_DOMAIN_FEATURE_XEN:
def->features[val] = VIR_TRISTATE_SWITCH_ON;
break;
@@ -17482,6 +17539,11 @@ virDomainFeaturesDefParse(virDomainDef *def,
return -1;
break;
+case VIR_DOMAIN_FEATURE_XEN:
+if (virDomainFeaturesXENDefParse(def, ctxt) < 0)
+return -1;
+break;
+
case VIR_DOMAIN_FEATURE_CAPABILITIES: {
virDomainCapabilitiesPolicy policy;
@@ -17615,50 +17677,6 @@ virDomainFeaturesDefParse(virDomainDef *def,
}
VIR_FREE(nodes);
-if (def->features[VIR_DOMAIN_FEATURE_XEN] == VIR_TRISTATE_SWITCH_ON) {
-int feature;
-virTristateSwitch value;
-
-if ((n = virXPathNodeSet("./features/xen/*", ctxt, &nodes)) < 0)
-return -1;
-
-for (i = 0; i < n; i++) {
-feature = virDomainXenTypeFromString((const char *)nodes[i]->name);
-if (feature < 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unsupported Xen feature: %s"),
- nodes[i]->name);
-return -1;
-}
-
-if (virXMLPropTristateSwitch(nodes[i], "state",
- VIR_XML_PROP_REQUIRED, &value) < 0)
-return -1;
-
-def->xen_features[feature] = value;
-
-switch ((virDomainXen) feature) {
-case VIR_DOMAIN_XEN_E820_HOST:
-break;
-
-case VIR_DOMAIN_XEN_PASSTHROUGH:
-if (value != VIR_TRISTATE_SWITCH_ON)
-break;
-
-if (virXMLPropEnum(nodes[i], "mode",
- virDomainXenPassthroughModeTypeFromString,
- VIR_XML_PROP_NONZERO,
- &def->xen_passthrough_mode) < 0)
-return -1;
-break;
-
-case VIR_DOMAIN_XEN_LAST:
-break;
-}
-}
-VIR_FREE(nodes);
-}
-
if (def->features[VIR_DOMAIN_FEATURE_SMM] == VIR_TRISTATE_SWITCH_ON) {
int rv = virParseScaledValue("string(./features/smm/tseg)",
"string(./features/smm/tseg/@unit)",
--
2.31.1
[libvirt PATCH 01/16] virDomainFeaturesDefParse: Factor out HyperV parsing into separate function
Only moving code, cleanup to follow.
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 236 ++---
1 file changed, 127 insertions(+), 109 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f65509d8ec..8cf57db7ba 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17286,6 +17286,128 @@ virDomainResourceDefParse(xmlNodePtr node,
}
+static int
+virDomainFeaturesHyperVDefParse(virDomainDef *def,
+xmlXPathContext *ctxt)
+{
+g_autofree xmlNodePtr *nodes = NULL;
+size_t i;
+int n;
+
+def->features[VIR_DOMAIN_FEATURE_HYPERV] = VIR_TRISTATE_SWITCH_ON;
+
+if (def->features[VIR_DOMAIN_FEATURE_HYPERV] == VIR_TRISTATE_SWITCH_ON) {
+int feature;
+virTristateSwitch value;
+if ((n = virXPathNodeSet("./features/hyperv/*", ctxt, &nodes)) < 0)
+return -1;
+
+for (i = 0; i < n; i++) {
+feature = virDomainHypervTypeFromString((const char
*)nodes[i]->name);
+if (feature < 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported HyperV Enlightenment feature:
%s"),
+ nodes[i]->name);
+return -1;
+}
+
+if (virXMLPropTristateSwitch(nodes[i], "state",
+ VIR_XML_PROP_REQUIRED, &value) < 0)
+return -1;
+
+def->hyperv_features[feature] = value;
+
+switch ((virDomainHyperv) feature) {
+case VIR_DOMAIN_HYPERV_RELAXED:
+case VIR_DOMAIN_HYPERV_VAPIC:
+case VIR_DOMAIN_HYPERV_VPINDEX:
+case VIR_DOMAIN_HYPERV_RUNTIME:
+case VIR_DOMAIN_HYPERV_SYNIC:
+case VIR_DOMAIN_HYPERV_STIMER:
+case VIR_DOMAIN_HYPERV_RESET:
+case VIR_DOMAIN_HYPERV_FREQUENCIES:
+case VIR_DOMAIN_HYPERV_REENLIGHTENMENT:
+case VIR_DOMAIN_HYPERV_TLBFLUSH:
+case VIR_DOMAIN_HYPERV_IPI:
+case VIR_DOMAIN_HYPERV_EVMCS:
+break;
+
+case VIR_DOMAIN_HYPERV_SPINLOCKS:
+if (value != VIR_TRISTATE_SWITCH_ON)
+break;
+
+if (virXMLPropUInt(nodes[i], "retries", 0,
+ VIR_XML_PROP_REQUIRED,
+ &def->hyperv_spinlocks) < 0)
+return -1;
+
+if (def->hyperv_spinlocks < 0xFFF) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("HyperV spinlock retry count must be "
+ "at least 4095"));
+return -1;
+}
+break;
+
+case VIR_DOMAIN_HYPERV_VENDOR_ID:
+if (value != VIR_TRISTATE_SWITCH_ON)
+break;
+
+if (!(def->hyperv_vendor_id = virXMLPropString(nodes[i],
+ "value"))) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("missing 'value' attribute for "
+ "HyperV feature 'vendor_id'"));
+return -1;
+}
+
+if (strlen(def->hyperv_vendor_id) >
VIR_DOMAIN_HYPERV_VENDOR_ID_MAX) {
+virReportError(VIR_ERR_XML_ERROR,
+ _("HyperV vendor_id value must not be more "
+ "than %d characters."),
+ VIR_DOMAIN_HYPERV_VENDOR_ID_MAX);
+return -1;
+}
+
+/* ensure that the string can be passed to qemu */
+if (strchr(def->hyperv_vendor_id, ',')) {
+virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("HyperV vendor_id value is invalid"));
+return -1;
+}
+break;
+
+case VIR_DOMAIN_HYPERV_LAST:
+break;
+}
+}
+VIR_FREE(nodes);
+}
+
+if (def->hyperv_features[VIR_DOMAIN_HYPERV_STIMER] ==
VIR_TRISTATE_SWITCH_ON) {
+if ((n = virXPathNodeSet("./features/hyperv/stimer/*", ctxt, &nodes))
< 0)
+return -1;
+
+for (i = 0; i < n; i++) {
+if (STRNEQ((const char *)nodes[i]->name, "direct")) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported Hyper-V stimer feature: %s"),
+
[libvirt PATCH 12/16] virDomainFeaturesDefParse: Inline SMM parsing
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 41 ++---
1 file changed, 26 insertions(+), 15 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 02c06d5ab9..b411c1fb8c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17554,8 +17554,7 @@ virDomainFeaturesDefParse(virDomainDef *def,
case VIR_DOMAIN_FEATURE_HAP:
case VIR_DOMAIN_FEATURE_PMU:
case VIR_DOMAIN_FEATURE_PVSPINLOCK:
-case VIR_DOMAIN_FEATURE_VMPORT:
-case VIR_DOMAIN_FEATURE_SMM: {
+case VIR_DOMAIN_FEATURE_VMPORT: {
virTristateSwitch state;
if (virXMLPropTristateSwitch(nodes[i], "state",
@@ -17569,6 +17568,31 @@ virDomainFeaturesDefParse(virDomainDef *def,
break;
}
+case VIR_DOMAIN_FEATURE_SMM: {
+virTristateSwitch state;
+
+if (virXMLPropTristateSwitch(nodes[i], "state",
+ VIR_XML_PROP_NONE, &state) < 0)
+return -1;
+
+if ((state == VIR_TRISTATE_SWITCH_ABSENT) ||
+(state == VIR_TRISTATE_SWITCH_ON)) {
+int rv = virParseScaledValue("string(./features/smm/tseg)",
+
"string(./features/smm/tseg/@unit)",
+ ctxt,
+ &def->tseg_size,
+ 1024 * 1024, /* Defaults to
mebibytes */
+ ULLONG_MAX,
+ false);
+if (rv < 0)
+return -1;
+
+def->features[val] = VIR_TRISTATE_SWITCH_ON;
+def->tseg_specified = rv != 0;
+}
+break;
+}
+
case VIR_DOMAIN_FEATURE_GIC:
if (virXMLPropEnum(nodes[i], "version",
virGICVersionTypeFromString,
VIR_XML_PROP_NONZERO, &def->gic_version) < 0)
@@ -17670,19 +17694,6 @@ virDomainFeaturesDefParse(virDomainDef *def,
}
VIR_FREE(nodes);
-if (def->features[VIR_DOMAIN_FEATURE_SMM] == VIR_TRISTATE_SWITCH_ON) {
-int rv = virParseScaledValue("string(./features/smm/tseg)",
- "string(./features/smm/tseg/@unit)",
- ctxt,
- &def->tseg_size,
- 1024 * 1024, /* Defaults to mebibytes */
- ULLONG_MAX,
- false);
-if (rv < 0)
-return -1;
-def->tseg_specified = rv;
-}
-
if (def->features[VIR_DOMAIN_FEATURE_MSRS] == VIR_TRISTATE_SWITCH_ON) {
virDomainMsrsUnknown unknown;
xmlNodePtr node = NULL;
--
2.31.1
[libvirt PATCH 06/16] virDomainFeaturesKVMDefParse: Remove ctxt
Iterating over all child elements of a node does not require xpath.
By doing away with xpath for this code, the code can be simplified.
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 22 +-
1 file changed, 9 insertions(+), 13 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 384740c364..2ad4cbc5a3 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17400,26 +17400,21 @@ virDomainFeaturesHyperVDefParse(virDomainDef *def,
static int
virDomainFeaturesKVMDefParse(virDomainDef *def,
- xmlXPathContext *ctxt)
+ xmlNodePtr node)
{
-g_autofree xmlNodePtr *nodes = NULL;
-size_t i;
-int n;
-
def->features[VIR_DOMAIN_FEATURE_KVM] = VIR_TRISTATE_SWITCH_ON;
if (def->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON) {
int feature;
virTristateSwitch value;
-if ((n = virXPathNodeSet("./features/kvm/*", ctxt, &nodes)) < 0)
-return -1;
-for (i = 0; i < n; i++) {
-feature = virDomainKVMTypeFromString((const char *)nodes[i]->name);
+node = xmlFirstElementChild(node);
+while (node) {
+feature = virDomainKVMTypeFromString((const char *)node->name);
if (feature < 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("unsupported KVM feature: %s"),
- nodes[i]->name);
+ node->name);
return -1;
}
@@ -17427,7 +17422,7 @@ virDomainFeaturesKVMDefParse(virDomainDef *def,
case VIR_DOMAIN_KVM_HIDDEN:
case VIR_DOMAIN_KVM_DEDICATED:
case VIR_DOMAIN_KVM_POLLCONTROL:
-if (virXMLPropTristateSwitch(nodes[i], "state",
+if (virXMLPropTristateSwitch(node, "state",
VIR_XML_PROP_REQUIRED,
&value) < 0)
return -1;
@@ -17438,8 +17433,9 @@ virDomainFeaturesKVMDefParse(virDomainDef *def,
case VIR_DOMAIN_KVM_LAST:
break;
}
+
+node = xmlNextElementSibling(node);
}
-VIR_FREE(nodes);
}
return 0;
@@ -17494,7 +17490,7 @@ virDomainFeaturesDefParse(virDomainDef *def,
break;
case VIR_DOMAIN_FEATURE_KVM:
-if (virDomainFeaturesKVMDefParse(def, ctxt) < 0)
+if (virDomainFeaturesKVMDefParse(def, nodes[i]) < 0)
return -1;
break;
--
2.31.1
[libvirt PATCH 05/16] virDomainFeaturesDefParse: Factor out KVM parsing into separate function
Only moving code, cleanup to follow.
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 88 +-
1 file changed, 53 insertions(+), 35 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3ba41869ec..384740c364 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17398,6 +17398,54 @@ virDomainFeaturesHyperVDefParse(virDomainDef *def,
}
+static int
+virDomainFeaturesKVMDefParse(virDomainDef *def,
+ xmlXPathContext *ctxt)
+{
+g_autofree xmlNodePtr *nodes = NULL;
+size_t i;
+int n;
+
+def->features[VIR_DOMAIN_FEATURE_KVM] = VIR_TRISTATE_SWITCH_ON;
+
+if (def->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON) {
+int feature;
+virTristateSwitch value;
+if ((n = virXPathNodeSet("./features/kvm/*", ctxt, &nodes)) < 0)
+return -1;
+
+for (i = 0; i < n; i++) {
+feature = virDomainKVMTypeFromString((const char *)nodes[i]->name);
+if (feature < 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported KVM feature: %s"),
+ nodes[i]->name);
+return -1;
+}
+
+switch ((virDomainKVM) feature) {
+case VIR_DOMAIN_KVM_HIDDEN:
+case VIR_DOMAIN_KVM_DEDICATED:
+case VIR_DOMAIN_KVM_POLLCONTROL:
+if (virXMLPropTristateSwitch(nodes[i], "state",
+ VIR_XML_PROP_REQUIRED,
+ &value) < 0)
+return -1;
+
+def->kvm_features[feature] = value;
+break;
+
+case VIR_DOMAIN_KVM_LAST:
+break;
+}
+}
+VIR_FREE(nodes);
+}
+
+return 0;
+}
+
+
static int
virDomainFeaturesDefParse(virDomainDef *def,
xmlXPathContextPtr ctxt)
@@ -17435,7 +17483,6 @@ virDomainFeaturesDefParse(virDomainDef *def,
case VIR_DOMAIN_FEATURE_PAE:
case VIR_DOMAIN_FEATURE_VIRIDIAN:
case VIR_DOMAIN_FEATURE_PRIVNET:
-case VIR_DOMAIN_FEATURE_KVM:
case VIR_DOMAIN_FEATURE_MSRS:
case VIR_DOMAIN_FEATURE_XEN:
def->features[val] = VIR_TRISTATE_SWITCH_ON;
@@ -17446,6 +17493,11 @@ virDomainFeaturesDefParse(virDomainDef *def,
return -1;
break;
+case VIR_DOMAIN_FEATURE_KVM:
+if (virDomainFeaturesKVMDefParse(def, ctxt) < 0)
+return -1;
+break;
+
case VIR_DOMAIN_FEATURE_CAPABILITIES: {
virDomainCapabilitiesPolicy policy;
@@ -17579,40 +17631,6 @@ virDomainFeaturesDefParse(virDomainDef *def,
}
VIR_FREE(nodes);
-if (def->features[VIR_DOMAIN_FEATURE_KVM] == VIR_TRISTATE_SWITCH_ON) {
-int feature;
-virTristateSwitch value;
-if ((n = virXPathNodeSet("./features/kvm/*", ctxt, &nodes)) < 0)
-return -1;
-
-for (i = 0; i < n; i++) {
-feature = virDomainKVMTypeFromString((const char *)nodes[i]->name);
-if (feature < 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unsupported KVM feature: %s"),
- nodes[i]->name);
-return -1;
-}
-
-switch ((virDomainKVM) feature) {
-case VIR_DOMAIN_KVM_HIDDEN:
-case VIR_DOMAIN_KVM_DEDICATED:
-case VIR_DOMAIN_KVM_POLLCONTROL:
-if (virXMLPropTristateSwitch(nodes[i], "state",
- VIR_XML_PROP_REQUIRED,
- &value) < 0)
-return -1;
-
-def->kvm_features[feature] = value;
-break;
-
-case VIR_DOMAIN_KVM_LAST:
-break;
-}
-}
-VIR_FREE(nodes);
-}
-
if (def->features[VIR_DOMAIN_FEATURE_XEN] == VIR_TRISTATE_SWITCH_ON) {
int feature;
virTristateSwitch value;
--
2.31.1
[libvirt PATCH 15/16] virDomainFeaturesCapabilitiesDefParse: Remove ctxt
Iterating over all child elements of a node does not require xpath.
By doing away with xpath for this code, the code can be simplified.
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 22 --
1 file changed, 8 insertions(+), 14 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 84c684b004..50717c4f44 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17483,12 +17483,8 @@ virDomainFeaturesXENDefParse(virDomainDef *def,
static int
virDomainFeaturesCapabilitiesDefParse(virDomainDef *def,
- xmlNodePtr node,
- xmlXPathContext *ctxt)
+ xmlNodePtr node)
{
-g_autofree xmlNodePtr *nodes = NULL;
-size_t i;
-int n;
virDomainCapabilitiesPolicy policy;
if (virXMLPropEnumDefault(node, "policy",
@@ -17499,27 +17495,25 @@ virDomainFeaturesCapabilitiesDefParse(virDomainDef
*def,
def->features[VIR_DOMAIN_FEATURE_CAPABILITIES] = policy;
-if ((n = virXPathNodeSet("./features/capabilities/*", ctxt, &nodes)) < 0)
-return -1;
-
-for (i = 0; i < n; i++) {
+node = xmlFirstElementChild(node);
+while (node) {
virTristateSwitch state;
-int val = virDomainProcessCapsFeatureTypeFromString((const char
*)nodes[i]->name);
+int val = virDomainProcessCapsFeatureTypeFromString((const char
*)node->name);
if (val < 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unexpected capability feature '%s'"),
nodes[i]->name);
+ _("unexpected capability feature '%s'"),
node->name);
return -1;
}
-if (virXMLPropTristateSwitch(nodes[i], "state", VIR_XML_PROP_NONE,
- &state) < 0)
+if (virXMLPropTristateSwitch(node, "state", VIR_XML_PROP_NONE, &state)
< 0)
return -1;
if (state == VIR_TRISTATE_SWITCH_ABSENT)
state = VIR_TRISTATE_SWITCH_ON;
def->caps_features[val] = state;
+node = xmlNextElementSibling(node);
}
return 0;
@@ -17594,7 +17588,7 @@ virDomainFeaturesDefParse(virDomainDef *def,
break;
case VIR_DOMAIN_FEATURE_CAPABILITIES: {
-if (virDomainFeaturesCapabilitiesDefParse(def, nodes[i], ctxt) < 0)
+if (virDomainFeaturesCapabilitiesDefParse(def, nodes[i]) < 0)
return -1;
break;
}
--
2.31.1
[libvirt PATCH 07/16] virDomainFeaturesKVMDefParse: Remove tautological "switch"
`feature` is always one of the values listed in the switch,
ensured by `virDomainKVMTypeFromString` above.
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 18 --
1 file changed, 4 insertions(+), 14 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2ad4cbc5a3..62565601ab 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17418,21 +17418,11 @@ virDomainFeaturesKVMDefParse(virDomainDef *def,
return -1;
}
-switch ((virDomainKVM) feature) {
-case VIR_DOMAIN_KVM_HIDDEN:
-case VIR_DOMAIN_KVM_DEDICATED:
-case VIR_DOMAIN_KVM_POLLCONTROL:
-if (virXMLPropTristateSwitch(node, "state",
- VIR_XML_PROP_REQUIRED,
- &value) < 0)
-return -1;
-
-def->kvm_features[feature] = value;
-break;
+if (virXMLPropTristateSwitch(node, "state", VIR_XML_PROP_REQUIRED,
+ &value) < 0)
+return -1;
-case VIR_DOMAIN_KVM_LAST:
-break;
-}
+def->kvm_features[feature] = value;
node = xmlNextElementSibling(node);
}
--
2.31.1
[libvirt PATCH 14/16] virDomainFeaturesDefParse: Factor out capabilities parsing into separate function
Cleanup to follow. This removes the last re-use of `nodes` in this function,
eliminating two VIR_FREEs.
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 78 +-
1 file changed, 46 insertions(+), 32 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 915303adcd..84c684b004 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17481,6 +17481,51 @@ virDomainFeaturesXENDefParse(virDomainDef *def,
}
+static int
+virDomainFeaturesCapabilitiesDefParse(virDomainDef *def,
+ xmlNodePtr node,
+ xmlXPathContext *ctxt)
+{
+g_autofree xmlNodePtr *nodes = NULL;
+size_t i;
+int n;
+virDomainCapabilitiesPolicy policy;
+
+if (virXMLPropEnumDefault(node, "policy",
+ virDomainCapabilitiesPolicyTypeFromString,
+ VIR_XML_PROP_NONE, &policy,
+ VIR_DOMAIN_CAPABILITIES_POLICY_DEFAULT) < 0)
+return -1;
+
+def->features[VIR_DOMAIN_FEATURE_CAPABILITIES] = policy;
+
+if ((n = virXPathNodeSet("./features/capabilities/*", ctxt, &nodes)) < 0)
+return -1;
+
+for (i = 0; i < n; i++) {
+virTristateSwitch state;
+int val = virDomainProcessCapsFeatureTypeFromString((const char
*)nodes[i]->name);
+if (val < 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unexpected capability feature '%s'"),
nodes[i]->name);
+return -1;
+}
+
+
+if (virXMLPropTristateSwitch(nodes[i], "state", VIR_XML_PROP_NONE,
+ &state) < 0)
+return -1;
+
+if (state == VIR_TRISTATE_SWITCH_ABSENT)
+state = VIR_TRISTATE_SWITCH_ON;
+
+def->caps_features[val] = state;
+}
+
+return 0;
+}
+
+
static int
virDomainFeaturesDefParse(virDomainDef *def,
xmlXPathContextPtr ctxt)
@@ -17549,15 +17594,8 @@ virDomainFeaturesDefParse(virDomainDef *def,
break;
case VIR_DOMAIN_FEATURE_CAPABILITIES: {
-virDomainCapabilitiesPolicy policy;
-
-if (virXMLPropEnumDefault(nodes[i], "policy",
-
virDomainCapabilitiesPolicyTypeFromString,
- VIR_XML_PROP_NONE, &policy,
- VIR_DOMAIN_CAPABILITIES_POLICY_DEFAULT)
< 0)
+if (virDomainFeaturesCapabilitiesDefParse(def, nodes[i], ctxt) < 0)
return -1;
-
-def->features[val] = policy;
break;
}
@@ -17703,31 +17741,7 @@ virDomainFeaturesDefParse(virDomainDef *def,
break;
}
}
-VIR_FREE(nodes);
-if ((n = virXPathNodeSet("./features/capabilities/*", ctxt, &nodes)) < 0)
-return -1;
-
-for (i = 0; i < n; i++) {
-virTristateSwitch state;
-int val = virDomainProcessCapsFeatureTypeFromString((const char
*)nodes[i]->name);
-if (val < 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unexpected capability feature '%s'"),
nodes[i]->name);
-return -1;
-}
-
-
-if (virXMLPropTristateSwitch(nodes[i], "state", VIR_XML_PROP_NONE,
- &state) < 0)
-return -1;
-
-if (state == VIR_TRISTATE_SWITCH_ABSENT)
-state = VIR_TRISTATE_SWITCH_ON;
-
-def->caps_features[val] = state;
-}
-VIR_FREE(nodes);
return 0;
}
--
2.31.1
[libvirt PATCH 02/16] virDomainFeaturesHyperVDefParse: Inline hyperv/stimer parsing
Iterating over all child elements of a node does not require xpath.
By doing away with xpath for this code, the code can be inlined and
simplified. This also removes the re-use of `nodes`, elimininating
two VIR_FREEs.
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 46 ++
1 file changed, 24 insertions(+), 22 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8cf57db7ba..4ec5557eba 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17303,6 +17303,8 @@ virDomainFeaturesHyperVDefParse(virDomainDef *def,
return -1;
for (i = 0; i < n; i++) {
+xmlNodePtr child;
+
feature = virDomainHypervTypeFromString((const char
*)nodes[i]->name);
if (feature < 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -17323,7 +17325,6 @@ virDomainFeaturesHyperVDefParse(virDomainDef *def,
case VIR_DOMAIN_HYPERV_VPINDEX:
case VIR_DOMAIN_HYPERV_RUNTIME:
case VIR_DOMAIN_HYPERV_SYNIC:
-case VIR_DOMAIN_HYPERV_STIMER:
case VIR_DOMAIN_HYPERV_RESET:
case VIR_DOMAIN_HYPERV_FREQUENCIES:
case VIR_DOMAIN_HYPERV_REENLIGHTENMENT:
@@ -17332,6 +17333,28 @@ virDomainFeaturesHyperVDefParse(virDomainDef *def,
case VIR_DOMAIN_HYPERV_EVMCS:
break;
+case VIR_DOMAIN_HYPERV_STIMER:
+if (value != VIR_TRISTATE_SWITCH_ON)
+break;
+
+child = xmlFirstElementChild(nodes[i]);
+while (child) {
+if (STRNEQ((const char *)child->name, "direct")) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported Hyper-V stimer feature:
%s"),
+ child->name);
+return -1;
+}
+
+if (virXMLPropTristateSwitch(child, "state",
+ VIR_XML_PROP_REQUIRED,
+ &def->hyperv_stimer_direct) <
0)
+return -1;
+
+child = xmlNextElementSibling(child);
+}
+break;
+
case VIR_DOMAIN_HYPERV_SPINLOCKS:
if (value != VIR_TRISTATE_SWITCH_ON)
break;
@@ -17381,27 +17404,6 @@ virDomainFeaturesHyperVDefParse(virDomainDef *def,
break;
}
}
-VIR_FREE(nodes);
-}
-
-if (def->hyperv_features[VIR_DOMAIN_HYPERV_STIMER] ==
VIR_TRISTATE_SWITCH_ON) {
-if ((n = virXPathNodeSet("./features/hyperv/stimer/*", ctxt, &nodes))
< 0)
-return -1;
-
-for (i = 0; i < n; i++) {
-if (STRNEQ((const char *)nodes[i]->name, "direct")) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unsupported Hyper-V stimer feature: %s"),
- nodes[i]->name);
-return -1;
-}
-
-if (virXMLPropTristateSwitch(nodes[i], "state",
- VIR_XML_PROP_REQUIRED,
- &def->hyperv_stimer_direct) < 0)
-return -1;
-}
-VIR_FREE(nodes);
}
return 0;
--
2.31.1
[libvirt PATCH 03/16] virDomainFeaturesHyperVDefParse: Remove ctxt
Iterating over all child elements of a node does not require xpath.
By doing away with xpath for this code, the code can be simplified.
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 27 ---
1 file changed, 12 insertions(+), 15 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 4ec5557eba..b778dfe463 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17288,32 +17288,27 @@ virDomainResourceDefParse(xmlNodePtr node,
static int
virDomainFeaturesHyperVDefParse(virDomainDef *def,
-xmlXPathContext *ctxt)
+xmlNodePtr node)
{
-g_autofree xmlNodePtr *nodes = NULL;
-size_t i;
-int n;
-
def->features[VIR_DOMAIN_FEATURE_HYPERV] = VIR_TRISTATE_SWITCH_ON;
if (def->features[VIR_DOMAIN_FEATURE_HYPERV] == VIR_TRISTATE_SWITCH_ON) {
int feature;
virTristateSwitch value;
-if ((n = virXPathNodeSet("./features/hyperv/*", ctxt, &nodes)) < 0)
-return -1;
-for (i = 0; i < n; i++) {
+node = xmlFirstElementChild(node);
+while (node != NULL) {
xmlNodePtr child;
-feature = virDomainHypervTypeFromString((const char
*)nodes[i]->name);
+feature = virDomainHypervTypeFromString((const char *)node->name);
if (feature < 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("unsupported HyperV Enlightenment feature:
%s"),
- nodes[i]->name);
+ node->name);
return -1;
}
-if (virXMLPropTristateSwitch(nodes[i], "state",
+if (virXMLPropTristateSwitch(node, "state",
VIR_XML_PROP_REQUIRED, &value) < 0)
return -1;
@@ -17337,7 +17332,7 @@ virDomainFeaturesHyperVDefParse(virDomainDef *def,
if (value != VIR_TRISTATE_SWITCH_ON)
break;
-child = xmlFirstElementChild(nodes[i]);
+child = xmlFirstElementChild(node);
while (child) {
if (STRNEQ((const char *)child->name, "direct")) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -17359,7 +17354,7 @@ virDomainFeaturesHyperVDefParse(virDomainDef *def,
if (value != VIR_TRISTATE_SWITCH_ON)
break;
-if (virXMLPropUInt(nodes[i], "retries", 0,
+if (virXMLPropUInt(node, "retries", 0,
VIR_XML_PROP_REQUIRED,
&def->hyperv_spinlocks) < 0)
return -1;
@@ -17376,7 +17371,7 @@ virDomainFeaturesHyperVDefParse(virDomainDef *def,
if (value != VIR_TRISTATE_SWITCH_ON)
break;
-if (!(def->hyperv_vendor_id = virXMLPropString(nodes[i],
+if (!(def->hyperv_vendor_id = virXMLPropString(node,
"value"))) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("missing 'value' attribute for "
@@ -17403,6 +17398,8 @@ virDomainFeaturesHyperVDefParse(virDomainDef *def,
case VIR_DOMAIN_HYPERV_LAST:
break;
}
+
+node = xmlNextElementSibling(node);
}
}
@@ -17454,7 +17451,7 @@ virDomainFeaturesDefParse(virDomainDef *def,
break;
case VIR_DOMAIN_FEATURE_HYPERV:
-if (virDomainFeaturesHyperVDefParse(def, ctxt) < 0)
+if (virDomainFeaturesHyperVDefParse(def, nodes[i]) < 0)
return -1;
break;
--
2.31.1
[libvirt PATCH 10/16] virDomainFeaturesXENDefParse: Remove ctxt
Iterating over all child elements of a node does not require xpath.
By doing away with xpath for this code, the code can be simplified.
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 25 ++---
1 file changed, 10 insertions(+), 15 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e687f18afe..45c4b9cedf 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17432,31 +17432,25 @@ virDomainFeaturesKVMDefParse(virDomainDef *def,
static int
virDomainFeaturesXENDefParse(virDomainDef *def,
- xmlXPathContext *ctxt)
+ xmlNodePtr node)
{
-g_autofree xmlNodePtr *nodes = NULL;
-size_t i;
-int n;
-
def->features[VIR_DOMAIN_FEATURE_XEN] = VIR_TRISTATE_SWITCH_ON;
if (def->features[VIR_DOMAIN_FEATURE_XEN] == VIR_TRISTATE_SWITCH_ON) {
int feature;
virTristateSwitch value;
-if ((n = virXPathNodeSet("./features/xen/*", ctxt, &nodes)) < 0)
-return -1;
-
-for (i = 0; i < n; i++) {
-feature = virDomainXenTypeFromString((const char *)nodes[i]->name);
+node = xmlFirstElementChild(node);
+while (node) {
+feature = virDomainXenTypeFromString((const char *)node->name);
if (feature < 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("unsupported Xen feature: %s"),
- nodes[i]->name);
+ node->name);
return -1;
}
-if (virXMLPropTristateSwitch(nodes[i], "state",
+if (virXMLPropTristateSwitch(node, "state",
VIR_XML_PROP_REQUIRED, &value) < 0)
return -1;
@@ -17470,7 +17464,7 @@ virDomainFeaturesXENDefParse(virDomainDef *def,
if (value != VIR_TRISTATE_SWITCH_ON)
break;
-if (virXMLPropEnum(nodes[i], "mode",
+if (virXMLPropEnum(node, "mode",
virDomainXenPassthroughModeTypeFromString,
VIR_XML_PROP_NONZERO,
&def->xen_passthrough_mode) < 0)
@@ -17480,8 +17474,9 @@ virDomainFeaturesXENDefParse(virDomainDef *def,
case VIR_DOMAIN_XEN_LAST:
break;
}
+
+node = xmlNextElementSibling(node);
}
-VIR_FREE(nodes);
}
return 0;
@@ -17540,7 +17535,7 @@ virDomainFeaturesDefParse(virDomainDef *def,
break;
case VIR_DOMAIN_FEATURE_XEN:
-if (virDomainFeaturesXENDefParse(def, ctxt) < 0)
+if (virDomainFeaturesXENDefParse(def, nodes[i]) < 0)
return -1;
break;
--
2.31.1
[libvirt PATCH 11/16] virDomainFeaturesXENDefParse: Remove tautological "if"
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 58 --
1 file changed, 28 insertions(+), 30 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 45c4b9cedf..02c06d5ab9 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17436,47 +17436,45 @@ virDomainFeaturesXENDefParse(virDomainDef *def,
{
def->features[VIR_DOMAIN_FEATURE_XEN] = VIR_TRISTATE_SWITCH_ON;
-if (def->features[VIR_DOMAIN_FEATURE_XEN] == VIR_TRISTATE_SWITCH_ON) {
+node = xmlFirstElementChild(node);
+while (node) {
int feature;
virTristateSwitch value;
-node = xmlFirstElementChild(node);
-while (node) {
-feature = virDomainXenTypeFromString((const char *)node->name);
-if (feature < 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unsupported Xen feature: %s"),
- node->name);
-return -1;
-}
-
-if (virXMLPropTristateSwitch(node, "state",
- VIR_XML_PROP_REQUIRED, &value) < 0)
-return -1;
+feature = virDomainXenTypeFromString((const char *)node->name);
+if (feature < 0) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unsupported Xen feature: %s"),
+ node->name);
+return -1;
+}
-def->xen_features[feature] = value;
+if (virXMLPropTristateSwitch(node, "state",
+ VIR_XML_PROP_REQUIRED, &value) < 0)
+return -1;
-switch ((virDomainXen) feature) {
-case VIR_DOMAIN_XEN_E820_HOST:
-break;
+def->xen_features[feature] = value;
-case VIR_DOMAIN_XEN_PASSTHROUGH:
-if (value != VIR_TRISTATE_SWITCH_ON)
-break;
+switch ((virDomainXen) feature) {
+case VIR_DOMAIN_XEN_E820_HOST:
+break;
-if (virXMLPropEnum(node, "mode",
- virDomainXenPassthroughModeTypeFromString,
- VIR_XML_PROP_NONZERO,
- &def->xen_passthrough_mode) < 0)
-return -1;
+case VIR_DOMAIN_XEN_PASSTHROUGH:
+if (value != VIR_TRISTATE_SWITCH_ON)
break;
-case VIR_DOMAIN_XEN_LAST:
-break;
-}
+if (virXMLPropEnum(node, "mode",
+ virDomainXenPassthroughModeTypeFromString,
+ VIR_XML_PROP_NONZERO,
+ &def->xen_passthrough_mode) < 0)
+return -1;
+break;
-node = xmlNextElementSibling(node);
+case VIR_DOMAIN_XEN_LAST:
+break;
}
+
+node = xmlNextElementSibling(node);
}
return 0;
--
2.31.1
[libvirt PATCH 13/16] virDomainFeaturesDefParse: Inline MSRS parsing
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 26 --
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b411c1fb8c..915303adcd 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17518,10 +17518,21 @@ virDomainFeaturesDefParse(virDomainDef *def,
case VIR_DOMAIN_FEATURE_PAE:
case VIR_DOMAIN_FEATURE_VIRIDIAN:
case VIR_DOMAIN_FEATURE_PRIVNET:
-case VIR_DOMAIN_FEATURE_MSRS:
def->features[val] = VIR_TRISTATE_SWITCH_ON;
break;
+case VIR_DOMAIN_FEATURE_MSRS: {
+virDomainMsrsUnknown unknown;
+if (virXMLPropEnum(nodes[i], "unknown",
+ virDomainMsrsUnknownTypeFromString,
+ VIR_XML_PROP_REQUIRED, &unknown) < 0)
+return -1;
+
+def->features[val] = VIR_TRISTATE_SWITCH_ON;
+def->msrs_features[VIR_DOMAIN_MSRS_UNKNOWN] = unknown;
+break;
+}
+
case VIR_DOMAIN_FEATURE_HYPERV:
if (virDomainFeaturesHyperVDefParse(def, nodes[i]) < 0)
return -1;
@@ -17694,19 +17705,6 @@ virDomainFeaturesDefParse(virDomainDef *def,
}
VIR_FREE(nodes);
-if (def->features[VIR_DOMAIN_FEATURE_MSRS] == VIR_TRISTATE_SWITCH_ON) {
-virDomainMsrsUnknown unknown;
-xmlNodePtr node = NULL;
-if ((node = virXPathNode("./features/msrs", ctxt)) == NULL)
-return -1;
-
-if (virXMLPropEnum(node, "unknown", virDomainMsrsUnknownTypeFromString,
- VIR_XML_PROP_REQUIRED, &unknown) < 0)
-return -1;
-
-def->msrs_features[VIR_DOMAIN_MSRS_UNKNOWN] = unknown;
-}
-
if ((n = virXPathNodeSet("./features/capabilities/*", ctxt, &nodes)) < 0)
return -1;
--
2.31.1
[libvirt PATCH 16/16] virDomainFeaturesDefParse: Simplify APIC parsing
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 23 ++-
1 file changed, 10 insertions(+), 13 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 50717c4f44..d78f846a52 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -17532,7 +17532,6 @@ virDomainFeaturesDefParse(virDomainDef *def,
return -1;
for (i = 0; i < n; i++) {
-g_autofree char *tmp = NULL;
int val = virDomainFeatureTypeFromString((const char *)nodes[i]->name);
if (val < 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -17541,18 +17540,6 @@ virDomainFeaturesDefParse(virDomainDef *def,
}
switch ((virDomainFeature) val) {
-case VIR_DOMAIN_FEATURE_APIC:
-if ((tmp = virXPathString("string(./features/apic/@eoi)", ctxt))) {
-int eoi;
-if ((eoi = virTristateSwitchTypeFromString(tmp)) <= 0) {
-virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unknown value for attribute eoi: '%s'"),
- tmp);
-return -1;
-}
-def->apic_eoi = eoi;
-}
-G_GNUC_FALLTHROUGH;
case VIR_DOMAIN_FEATURE_ACPI:
case VIR_DOMAIN_FEATURE_PAE:
case VIR_DOMAIN_FEATURE_VIRIDIAN:
@@ -17560,6 +17547,16 @@ virDomainFeaturesDefParse(virDomainDef *def,
def->features[val] = VIR_TRISTATE_SWITCH_ON;
break;
+case VIR_DOMAIN_FEATURE_APIC: {
+virTristateSwitch eoi;
+if (virXMLPropTristateSwitch(nodes[i], "eoi", VIR_XML_PROP_NONE,
&eoi) < 0)
+return -1;
+
+def->features[val] = VIR_TRISTATE_SWITCH_ON;
+def->apic_eoi = eoi;
+break;
+}
+
case VIR_DOMAIN_FEATURE_MSRS: {
virDomainMsrsUnknown unknown;
if (virXMLPropEnum(nodes[i], "unknown",
--
2.31.1
[libvirt PATCH] docs: Fix some typos
Signed-off-by: Tim Wiederhake --- docs/kbase/live_full_disk_backup.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/kbase/live_full_disk_backup.rst b/docs/kbase/live_full_disk_backup.rst index 1605ec05d2..562a9e87b0 100644 --- a/docs/kbase/live_full_disk_backup.rst +++ b/docs/kbase/live_full_disk_backup.rst @@ -97,7 +97,7 @@ virDomainBackupBegin() API: Assuming a guest with a single disk image, create a temporary live QCOW2 overlay (commonly called as "external snapshot") to track the live guest writes. Then backup the original disk image while the guest (live QEMU) keeps writing to the temporary -overlay. Finally, perform the "active block-commit" opertion to +overlay. Finally, perform the "active block-commit" operation to live-merge the temporary overlay disk contents into the original image — i.e. the backing file — and "pivot" the live QEMU process to point to it. @@ -138,7 +138,7 @@ it. you have to explicitly clean the libvirt metadata using ``virsh snapshot-delete vm1 --metadata [name|--current]``. -#. Now, take a backup the orignal image, ``base.raw``, to a different +#. Now, take a backup the original image, ``base.raw``, to a different location using ``cp`` or ``rsync``:: $ cp /var/lib/libvirt/images/base.raw -- 2.31.1
[libvirt PATCH 4/6] virDomainSEVDefParseXML: Use automatic memory management
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index dda615a8ba..dd803e6df5 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14719,7 +14719,7 @@ virDomainSEVDefParseXML(xmlNodePtr sevNode,
xmlXPathContextPtr ctxt)
{
VIR_XPATH_NODE_AUTORESTORE(ctxt)
-virDomainSEVDef *def;
+g_autoptr(virDomainSEVDef) def = NULL;
unsigned long policy;
int rc = -1;
@@ -14765,10 +14765,9 @@ virDomainSEVDefParseXML(xmlNodePtr sevNode,
def->dh_cert = virXPathString("string(./dhCert)", ctxt);
def->session = virXPathString("string(./session)", ctxt);
-return def;
+return g_steal_pointer(&def);
error:
-virDomainSEVDefFree(def);
return NULL;
}
--
2.31.1
[libvirt PATCH 1/6] conf: Add AUTOPTR_CLEANUP_FUNC for virDomainSEVDef
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 2 +-
src/conf/domain_conf.h | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 04c10df0a9..c1438d85f4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -3490,7 +3490,7 @@ virDomainResctrlDefFree(virDomainResctrlDef *resctrl)
}
-static void
+void
virDomainSEVDefFree(virDomainSEVDef *def)
{
if (!def)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 4d9d499b16..a14a2090f8 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2661,6 +2661,8 @@ struct _virDomainSEVDef {
unsigned int reduced_phys_bits;
};
+void virDomainSEVDefFree(virDomainSEVDef *def);
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(virDomainSEVDef, virDomainSEVDefFree);
typedef enum {
VIR_DOMAIN_IOMMU_MODEL_INTEL,
--
2.31.1
[libvirt PATCH 0/6] More XML parsing boiler code refactoring
In the spirit of https://listman.redhat.com/archives/libvir-list/2021-May/msg00550.html. Tim Wiederhake (6): conf: Add AUTOPTR_CLEANUP_FUNC for virDomainSEVDef conf: virDomainSEVDef: Change type of "sectype" to virDomainLaunchSecurity virDomainSEVDefParseXML: Use virXMLPropEnum virDomainSEVDefParseXML: Use automatic memory management virDomainSEVDefParseXML: Remove superfluous `goto`s virDomainSEVDefParseXML: Remove superfluous variable initialization src/conf/domain_conf.c | 41 +++-- src/conf/domain_conf.h | 4 +++- 2 files changed, 14 insertions(+), 31 deletions(-) -- 2.31.1
[libvirt PATCH 2/6] conf: virDomainSEVDef: Change type of "sectype" to virDomainLaunchSecurity
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 6 --
src/conf/domain_conf.h | 2 +-
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c1438d85f4..68ab18f3ab 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14722,6 +14722,7 @@ virDomainSEVDefParseXML(xmlNodePtr sevNode,
virDomainSEVDef *def;
unsigned long policy;
g_autofree char *type = NULL;
+int sectype;
int rc = -1;
def = g_new0(virDomainSEVDef, 1);
@@ -14734,8 +14735,8 @@ virDomainSEVDefParseXML(xmlNodePtr sevNode,
goto error;
}
-def->sectype = virDomainLaunchSecurityTypeFromString(type);
-switch ((virDomainLaunchSecurity) def->sectype) {
+sectype = virDomainLaunchSecurityTypeFromString(type);
+switch ((virDomainLaunchSecurity) sectype) {
case VIR_DOMAIN_LAUNCH_SECURITY_SEV:
break;
case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
@@ -14746,6 +14747,7 @@ virDomainSEVDefParseXML(xmlNodePtr sevNode,
type);
goto error;
}
+def->sectype = sectype;
if (virXPathULongHex("string(./policy)", ctxt, &policy) < 0) {
virReportError(VIR_ERR_XML_ERROR, "%s",
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index a14a2090f8..c31531c93b 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -2651,7 +2651,7 @@ typedef enum {
struct _virDomainSEVDef {
-int sectype; /* enum virDomainLaunchSecurity */
+virDomainLaunchSecurity sectype;
char *dh_cert;
char *session;
unsigned int policy;
--
2.31.1
[libvirt PATCH 5/6] virDomainSEVDefParseXML: Remove superfluous `goto`s
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 11 ---
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index dd803e6df5..db8ec23d70 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14730,12 +14730,12 @@ virDomainSEVDefParseXML(xmlNodePtr sevNode,
if (virXMLPropEnum(sevNode, "type", virDomainLaunchSecurityTypeFromString,
VIR_XML_PROP_NONZERO | VIR_XML_PROP_REQUIRED,
&def->sectype) < 0)
-goto error;
+return NULL;
if (virXPathULongHex("string(./policy)", ctxt, &policy) < 0) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("failed to get launch security policy"));
-goto error;
+return NULL;
}
/* the following attributes are platform dependent and if missing, we can
@@ -14747,7 +14747,7 @@ virDomainSEVDefParseXML(xmlNodePtr sevNode,
} else if (rc == -2) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("Invalid format for launch security cbitpos"));
-goto error;
+return NULL;
}
rc = virXPathUInt("string(./reducedPhysBits)", ctxt,
@@ -14758,7 +14758,7 @@ virDomainSEVDefParseXML(xmlNodePtr sevNode,
virReportError(VIR_ERR_XML_ERROR, "%s",
_("Invalid format for launch security "
"reduced-phys-bits"));
-goto error;
+return NULL;
}
def->policy = policy;
@@ -14766,9 +14766,6 @@ virDomainSEVDefParseXML(xmlNodePtr sevNode,
def->session = virXPathString("string(./session)", ctxt);
return g_steal_pointer(&def);
-
- error:
-return NULL;
}
--
2.31.1
[libvirt PATCH 3/6] virDomainSEVDefParseXML: Use virXMLPropEnum
Signed-off-by: Tim Wiederhake
---
src/conf/domain_conf.c | 23 +++
1 file changed, 3 insertions(+), 20 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 68ab18f3ab..dda615a8ba 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14721,33 +14721,16 @@ virDomainSEVDefParseXML(xmlNodePtr sevNode,
VIR_XPATH_NODE_AUTORESTORE(ctxt)
virDomainSEVDef *def;
unsigned long policy;
-g_autofree char *type = NULL;
-int sectype;
int rc = -1;
def = g_new0(virDomainSEVDef, 1);
ctxt->node = sevNode;
-if (!(type = virXMLPropString(sevNode, "type"))) {
-virReportError(VIR_ERR_XML_ERROR, "%s",
- _("missing launch security type"));
-goto error;
-}
-
-sectype = virDomainLaunchSecurityTypeFromString(type);
-switch ((virDomainLaunchSecurity) sectype) {
-case VIR_DOMAIN_LAUNCH_SECURITY_SEV:
-break;
-case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
-case VIR_DOMAIN_LAUNCH_SECURITY_LAST:
-default:
-virReportError(VIR_ERR_XML_ERROR,
- _("unsupported launch security type '%s'"),
- type);
+if (virXMLPropEnum(sevNode, "type", virDomainLaunchSecurityTypeFromString,
+ VIR_XML_PROP_NONZERO | VIR_XML_PROP_REQUIRED,
+ &def->sectype) < 0)
goto error;
-}
-def->sectype = sectype;
if (virXPathULongHex("string(./policy)", ctxt, &policy) < 0) {
virReportError(VIR_ERR_XML_ERROR, "%s",
--
2.31.1
[libvirt PATCH 6/6] virDomainSEVDefParseXML: Remove superfluous variable initialization
Signed-off-by: Tim Wiederhake --- src/conf/domain_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index db8ec23d70..2d8ae7e860 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14721,7 +14721,7 @@ virDomainSEVDefParseXML(xmlNodePtr sevNode, VIR_XPATH_NODE_AUTORESTORE(ctxt) g_autoptr(virDomainSEVDef) def = NULL; unsigned long policy; -int rc = -1; +int rc; def = g_new0(virDomainSEVDef, 1); -- 2.31.1
Re: [libvirt][PATCH v4 1/4] conf: Introduce SGX related element into domain xml
On Thu, 2021-07-01 at 20:10 +0800, Haibin Huang wrote:
> From: Lin Yang
>
>
> 1024
>
Please also update "docs/schemas/domaincommon.rng".
> ---
> src/conf/domain_conf.c | 106 +-
> --
> src/conf/domain_conf.h | 10
> src/conf/virconftypes.h | 3 ++
> 3 files changed, 91 insertions(+), 28 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index ef67efa1da..4336dafd82 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -1336,6 +1336,7 @@ VIR_ENUM_IMPL(virDomainLaunchSecurity,
> VIR_DOMAIN_LAUNCH_SECURITY_LAST,
> "",
> "sev",
> + "sgx",
> );
>
> static virClassPtr virDomainObjClass;
> @@ -3409,6 +3410,16 @@ virDomainSEVDefFree(virDomainSEVDefPtr def)
> }
>
>
> +static void
> +virDomainSGXDefFree(virDomainSGXDefPtr def)
> +{
> + if (!def)
> + return;
> +
> + VIR_FREE(def);
> +}
> +
> +
> void virDomainDefFree(virDomainDefPtr def)
> {
> size_t i;
> @@ -3597,6 +3608,7 @@ void virDomainDefFree(virDomainDefPtr def)
> (def->ns.free)(def->namespaceData);
>
> virDomainSEVDefFree(def->sev);
> + virDomainSGXDefFree(def->sgx);
>
> xmlFreeNode(def->metadata);
>
> @@ -16700,39 +16712,17 @@ virDomainMemoryTargetDefParseXML(xmlNodePtr
> node,
> return 0;
> }
>
> -
> static virDomainSEVDefPtr
> -virDomainSEVDefParseXML(xmlNodePtr sevNode,
> - xmlXPathContextPtr ctxt)
> +virDomainSEVDefParseXML(xmlXPathContextPtr ctxt)
> {
> VIR_XPATH_NODE_AUTORESTORE(ctxt);
> virDomainSEVDefPtr def;
> unsigned long policy;
> - g_autofree char *type = NULL;
>
> if (VIR_ALLOC(def) < 0)
> return NULL;
>
> - ctxt->node = sevNode;
> -
> - if (!(type = virXMLPropString(sevNode, "type"))) {
> - virReportError(VIR_ERR_XML_ERROR, "%s",
> - _("missing launch security type"));
> - goto error;
> - }
> -
> - def->sectype = virDomainLaunchSecurityTypeFromString(type);
> - switch ((virDomainLaunchSecurity) def->sectype) {
> - case VIR_DOMAIN_LAUNCH_SECURITY_SEV:
> - break;
> - case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
> - case VIR_DOMAIN_LAUNCH_SECURITY_LAST:
> - default:
> - virReportError(VIR_ERR_XML_ERROR,
> - _("unsupported launch security type '%s'"),
> - type);
> - goto error;
> - }
> + def->sectype = VIR_DOMAIN_LAUNCH_SECURITY_SEV;
>
> if (virXPathUInt("string(./cbitpos)", ctxt, &def->cbitpos) < 0)
> {
> virReportError(VIR_ERR_XML_ERROR, "%s",
> @@ -16764,6 +16754,63 @@ virDomainSEVDefParseXML(xmlNodePtr sevNode,
> return NULL;
> }
>
> +static virDomainSGXDefPtr
> +virDomainSGXDefParseXML(xmlXPathContextPtr ctxt)
> +{
> + VIR_XPATH_NODE_AUTORESTORE(ctxt);
I do not believe that this is necessary.
> + virDomainSGXDefPtr def;
> +
> + if (VIR_ALLOC(def) < 0)
> + return NULL;
> +
> + def->sectype = VIR_DOMAIN_LAUNCH_SECURITY_SGX;
> +
> + if (virDomainParseMemory("./epc_size", "./epc_size/@unit", ctxt,
> + &def->epc_size, false, false) < 0)
> + goto error;
> +
> + return def;
> +
> + error:
> + virDomainSGXDefFree(def);
> + return NULL;
> +}
> +
> +static int
> +virDomainLaunchSecurityDefParseXML(xmlNodePtr launchSecurityNode,
> + xmlXPathContextPtr ctxt,
> + virDomainDefPtr def)
> +{
> + VIR_XPATH_NODE_AUTORESTORE(ctxt);
> + g_autofree char *type = NULL;
> +
> + ctxt->node = launchSecurityNode;
> +
> + if (!(type = virXMLPropString(launchSecurityNode, "type"))) {
> + virReportError(VIR_ERR_XML_ERROR, "%s",
> + _("missing launch security type"));
> + return -1;
> + }
> +
> + switch ((virDomainLaunchSecurity)
> virDomainLaunchSecurityTypeFromString(type)) {
> + case VIR_DOMAIN_LAUNCH_SECURITY_SEV:
> + def->sev = virDomainSEVDefParseXML(ctxt);
I believe this should return "-1" when "def->sev == NULL".
> + break;
> + case VIR_DOMAIN_LAUNCH_SECURITY_SGX:
> + def->sgx = virDomainSGXDefParseXML(ctxt);
Similar.
Regards,
Tim
> + break;
> + case VIR_DOMAIN_LAUNCH_SECURITY_NONE:
> + case VIR_DOMAIN_LAUNCH_SECURITY_LAST:
> + default:
> + virReportError(VIR_ERR_XML_ERROR,
> + _("unsupported launch security type '%s'"),
> + type);
> + return -1;
> + }
> +
> + return 0;
> +}
> +
> static virDomainMemoryDefPtr
> virDomainMemoryDefParseXML(virDomainXMLOptionPtr xmlopt,
> xmlNodePtr memdevNode,
> @@ -7,12 +22274,15 @@ virDomainDefParseXML(xmlDocPtr xml,
> ctxt->node = node;
> VIR_FREE(nodes);
>
> - /* Check for SEV feature */
> - if ((node = virXPathNode("./launchSecurity", ctxt)) !=
Re: [libvirt][PATCH v4 3/4] qemu: Add command-line to enable SGX
On Thu, 2021-07-01 at 20:10 +0800, Haibin Huang wrote:
> From: Lin Yang
>
> If SGX is defined in domain, add the argument to enable
> SGX in -cpu :
>
> -cpu ,+sgx,+sgx-debug,+sgx1,+sgx-encls-c,
> +sgx-enclv,+sgx-exinfo,+sgx-kss,+sgx-mode64,
> +sgx-provisionkey,+sgx-tokenkey,+sgx2,+sgxlc
> ---
> src/qemu/qemu_command.c | 6 ++
> 1 file changed, 6 insertions(+)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 2c3785886c..fb05acbc94 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -6405,6 +6405,12 @@ qemuBuildCpuModelArgStr(virQEMUDriverPtr
> driver,
>
> case VIR_CPU_MODE_CUSTOM:
> virBufferAdd(buf, cpu->model, -1);
> + if(def->sgx)
Space between "if" and "(".
Regards,
Tim
> + virBufferAdd(buf,
> + ",+sgx,+sgx-debug,+sgx1,+sgx-encls-c,+sgx-
> enclv,+sgx-exinfo,"
> + "+sgx-kss,+sgx-mode64,+sgx-
> provisionkey,+sgx-tokenkey,+sgx2,"
> + "+sgxlc",
> + -1);
> break;
>
> case VIR_CPU_MODE_LAST:
Re: [libvirt][PATCH v4 2/4] qemu: Add command-line to generate SGX EPC memory backend
On Thu, 2021-07-01 at 20:10 +0800, Haibin Huang wrote:
> From: Lin Yang
>
> According to the result parsing from xml, add the argument of
> SGX EPC memory backend into QEMU command line:
>
> -object memory-backend-epc,id=mem1,size=K,prealloc \
> -sgx-epc id=epc1,memdev=mem1
> ---
> src/qemu/qemu_command.c | 24
> 1 file changed, 24 insertions(+)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 01812cd39b..2c3785886c 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -9869,6 +9869,27 @@ qemuBuildVsockCommandLine(virCommandPtr cmd,
> }
>
>
> +static int
> +qemuBuildSGXCommandLine(virCommandPtr cmd, virDomainSGXDefPtr sgx)
> +{
> + g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
> +
> + if (!sgx)
> + return 0;
> +
> + VIR_DEBUG("sgx->epc_size=%lluKiB", sgx->epc_size);
> +
> + virBufferAsprintf(&buf, "memory-backend-
> epc,id=mem1,size=%lluK,prealloc", sgx->epc_size);
> + virCommandAddArg(cmd, "-object");
> + virCommandAddArgBuffer(cmd, &buf);
virCommandAddArgFormat?
> +
> + virCommandAddArg(cmd, "-sgx-epc");
> + virCommandAddArg(cmd, "id=epc1,memdev=mem1");
> +
> + return 0;
> +}
> +
> +
> /*
> * Constructs a argv suitable for launching qemu with config defined
> * for a given virtual machine.
> @@ -10154,6 +10175,9 @@ qemuBuildCommandLine(virQEMUDriverPtr driver,
> cfg->logTimestamp)
> virCommandAddArgList(cmd, "-msg", "timestamp=on", NULL);
>
> + if (qemuBuildSGXCommandLine(cmd, def->sgx) < 0)
> + return NULL;
> +
Personal opinion: I would not add this to the end of the function, but
place it next to the call to "qemuBuildSEVCommandLine(...)". Or replace
the call to qemuBuildSEVCommandLine() with a
"qemuBuildSecurityCommandLine()", which in turn calls
qemuBuild{SEV,SGX}CommandLine().
Regards,
Tim
> return g_steal_pointer(&cmd);
> }
>
Re: [libvirt][PATCH v4 4/4] Support to query SGX capability
On Thu, 2021-07-01 at 20:10 +0800, Haibin Huang wrote: > 1.Add SGX feature in domain capabilities > 2.Get sgx capabilities by query-sgx-capabilities > 3.Transfer the B to KB for epc_size > 4.Delete sgx1 and sgx2 > 5.add unit test for get capabilities > > Signed-off-by: Haibin Huang > --- > src/conf/domain_capabilities.c | 29 > src/conf/domain_capabilities.h | 13 ++ > src/libvirt_private.syms | 2 +- > src/qemu/qemu_capabilities.c | 146 ++ > src/qemu/qemu_capabilities.h | 6 + > src/qemu/qemu_command.c | 2 +- > src/qemu/qemu_monitor.c | 10 ++ > src/qemu/qemu_monitor.h | 3 + > src/qemu/qemu_monitor_json.c | 91 +++ > src/qemu/qemu_monitor_json.h | 3 + > tests/domaincapsdata/bhyve_basic.x86_64.xml | 1 + > tests/domaincapsdata/bhyve_fbuf.x86_64.xml | 1 + > tests/domaincapsdata/bhyve_uefi.x86_64.xml | 1 + > tests/domaincapsdata/empty.xml | 1 + > tests/domaincapsdata/libxl-xenfv.xml | 1 + > tests/domaincapsdata/libxl-xenpv.xml | 1 + > .../domaincapsdata/qemu_1.5.3-q35.x86_64.xml | 1 + > .../domaincapsdata/qemu_1.5.3-tcg.x86_64.xml | 1 + > tests/domaincapsdata/qemu_1.5.3.x86_64.xml | 1 + > .../domaincapsdata/qemu_1.6.0-q35.x86_64.xml | 1 + > .../domaincapsdata/qemu_1.6.0-tcg.x86_64.xml | 1 + > tests/domaincapsdata/qemu_1.6.0.x86_64.xml | 1 + > .../domaincapsdata/qemu_1.7.0-q35.x86_64.xml | 1 + > .../domaincapsdata/qemu_1.7.0-tcg.x86_64.xml | 1 + > tests/domaincapsdata/qemu_1.7.0.x86_64.xml | 1 + > .../domaincapsdata/qemu_2.1.1-q35.x86_64.xml | 1 + > .../domaincapsdata/qemu_2.1.1-tcg.x86_64.xml | 1 + > tests/domaincapsdata/qemu_2.1.1.x86_64.xml | 1 + > .../domaincapsdata/qemu_2.10.0-q35.x86_64.xml | 1 + > .../domaincapsdata/qemu_2.10.0-tcg.x86_64.xml | 1 + > .../qemu_2.10.0-virt.aarch64.xml | 1 + > tests/domaincapsdata/qemu_2.10.0.aarch64.xml | 1 + > tests/domaincapsdata/qemu_2.10.0.ppc64.xml | 1 + > tests/domaincapsdata/qemu_2.10.0.s390x.xml | 1 + > tests/domaincapsdata/qemu_2.10.0.x86_64.xml | 1 + > .../domaincapsdata/qemu_2.11.0-q35.x86_64.xml | 1 + > .../domaincapsdata/qemu_2.11.0-tcg.x86_64.xml | 1 + > tests/domaincapsdata/qemu_2.11.0.s390x.xml | 1 + > tests/domaincapsdata/qemu_2.11.0.x86_64.xml | 1 + > .../domaincapsdata/qemu_2.12.0-q35.x86_64.xml | 1 + > .../domaincapsdata/qemu_2.12.0-tcg.x86_64.xml | 1 + > .../qemu_2.12.0-virt.aarch64.xml | 1 + > tests/domaincapsdata/qemu_2.12.0.aarch64.xml | 1 + > tests/domaincapsdata/qemu_2.12.0.ppc64.xml | 1 + > tests/domaincapsdata/qemu_2.12.0.s390x.xml | 1 + > tests/domaincapsdata/qemu_2.12.0.x86_64.xml | 1 + > .../domaincapsdata/qemu_2.4.0-q35.x86_64.xml | 1 + > .../domaincapsdata/qemu_2.4.0-tcg.x86_64.xml | 1 + > tests/domaincapsdata/qemu_2.4.0.x86_64.xml | 1 + > .../domaincapsdata/qemu_2.5.0-q35.x86_64.xml | 1 + > .../domaincapsdata/qemu_2.5.0-tcg.x86_64.xml | 1 + > tests/domaincapsdata/qemu_2.5.0.x86_64.xml | 1 + > .../domaincapsdata/qemu_2.6.0-q35.x86_64.xml | 1 + > .../domaincapsdata/qemu_2.6.0-tcg.x86_64.xml | 1 + > .../qemu_2.6.0-virt.aarch64.xml | 1 + > tests/domaincapsdata/qemu_2.6.0.aarch64.xml | 1 + > tests/domaincapsdata/qemu_2.6.0.ppc64.xml | 1 + > tests/domaincapsdata/qemu_2.6.0.x86_64.xml | 1 + > .../domaincapsdata/qemu_2.7.0-q35.x86_64.xml | 1 + > .../domaincapsdata/qemu_2.7.0-tcg.x86_64.xml | 1 + > tests/domaincapsdata/qemu_2.7.0.s390x.xml | 1 + > tests/domaincapsdata/qemu_2.7.0.x86_64.xml | 1 + > .../domaincapsdata/qemu_2.8.0-q35.x86_64.xml | 1 + > .../domaincapsdata/qemu_2.8.0-tcg.x86_64.xml | 1 + > tests/domaincapsdata/qemu_2.8.0.s390x.xml | 1 + > tests/domaincapsdata/qemu_2.8.0.x86_64.xml | 1 + > .../domaincapsdata/qemu_2.9.0-q35.x86_64.xml | 1 + > .../domaincapsdata/qemu_2.9.0-tcg.x86_64.xml | 1 + > tests/domaincapsdata/qemu_2.9.0.ppc64.xml | 1 + > tests/domaincapsdata/qemu_2.9.0.s390x.xml | 1 + > tests/domaincapsdata/qemu_2.9.0.x86_64.xml | 1 + > .../domaincapsdata/qemu_3.0.0-q35.x86_64.xml | 1 + > .../domaincapsdata/qemu_3.0.0-tcg.x86_64.xml | 1 + > tests/domaincapsdata/qemu_3.0.0.ppc64.xml | 1 + > tests/domaincapsdata/qemu_3.0.0.s390x.xml | 1 + > tests/domaincapsdata/qemu_3.0.0.x86_64.xml | 1 + > .../domaincapsdata/qemu_3.1.0-q35.x86_64.xml | 1 + > .../domaincapsdata/qemu_3.1.0-tcg.x86_64.xml | 1 + > tests/domaincapsdata/qemu_3.1.0.ppc64.xml | 1 + > tests/domaincapsdata/qemu_3.1.0.x86_64.xml | 1 + > .../domaincapsdata/qemu_4.0.0-q35.x86_64.xml | 1 + > .../domaincapsdata/qemu_4.0.0-tcg.x86_64.xml | 1 +
[libvirt PATCH 03/10] qemuMonitorGetAllBlockStatsInfo: Assign hash table only on success
`virHashNew` cannot return NULL, the check is not needed.
Signed-off-by: Tim Wiederhake
---
src/qemu/qemu_monitor.c | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 19fcc5658b..86aabc98c3 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2155,23 +2155,23 @@ qemuMonitorGetAllBlockStatsInfo(qemuMonitor *mon,
bool backingChain)
{
int ret;
+GHashTable *stats = virHashNew(g_free);
+
VIR_DEBUG("ret_stats=%p, backing=%d", ret_stats, backingChain);
QEMU_CHECK_MONITOR(mon);
-if (!(*ret_stats = virHashNew(g_free)))
-goto error;
-
-ret = qemuMonitorJSONGetAllBlockStatsInfo(mon, *ret_stats, backingChain);
+*ret_stats = NULL;
+ret = qemuMonitorJSONGetAllBlockStatsInfo(mon, stats, backingChain);
if (ret < 0)
goto error;
+*ret_stats = stats;
return ret;
error:
-virHashFree(*ret_stats);
-*ret_stats = NULL;
+virHashFree(stats);
return -1;
}
--
2.31.1
[libvirt PATCH 00/10] virHashNew refactorings
"virHashNew" cannot return NULL, yet we check for NULL in various places. This series is the first of several that remove these checks. Where applicable, the functions are refactored to use automatic memory management by means of g_autoptr etc. as well. Tim Wiederhake (10): qemuMonitorGetAllBlockStatsInfo: Clean up line break qemuMonitorGetAllBlockStatsInfo: Remove superfluous variable initialization qemuMonitorGetAllBlockStatsInfo: Assign hash table only on success qemuMonitorGetAllBlockStatsInfo: Use automatic memory management qemuMonitorGetBlockInfo: Remove superfluous variable "ret" qemuMonitorGetBlockInfo: Use automatic memory management qemuMonitorGetBlockInfo: `virHashNew` cannot return NULL qemuMonitorGetChardevInfo: Remove superfluous variable "ret" qemuMonitorGetChardevInfo: Use automatic memory management qemuMonitorGetChardevInfo: `virHashNew` cannot return NULL src/qemu/qemu_monitor.c | 54 - 1 file changed, 16 insertions(+), 38 deletions(-) -- 2.31.1
[libvirt PATCH 02/10] qemuMonitorGetAllBlockStatsInfo: Remove superfluous variable initialization
Signed-off-by: Tim Wiederhake
---
src/qemu/qemu_monitor.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index df1e41f68f..19fcc5658b 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2154,7 +2154,7 @@ qemuMonitorGetAllBlockStatsInfo(qemuMonitor *mon,
GHashTable **ret_stats,
bool backingChain)
{
-int ret = -1;
+int ret;
VIR_DEBUG("ret_stats=%p, backing=%d", ret_stats, backingChain);
QEMU_CHECK_MONITOR(mon);
--
2.31.1
[libvirt PATCH 01/10] qemuMonitorGetAllBlockStatsInfo: Clean up line break
Signed-off-by: Tim Wiederhake --- src/qemu/qemu_monitor.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 8f35b4240f..df1e41f68f 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2162,8 +2162,7 @@ qemuMonitorGetAllBlockStatsInfo(qemuMonitor *mon, if (!(*ret_stats = virHashNew(g_free))) goto error; -ret = qemuMonitorJSONGetAllBlockStatsInfo(mon, *ret_stats, - backingChain); +ret = qemuMonitorJSONGetAllBlockStatsInfo(mon, *ret_stats, backingChain); if (ret < 0) goto error; -- 2.31.1
[libvirt PATCH 04/10] qemuMonitorGetAllBlockStatsInfo: Use automatic memory management
Signed-off-by: Tim Wiederhake
---
src/qemu/qemu_monitor.c | 10 +++---
1 file changed, 3 insertions(+), 7 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 86aabc98c3..f08b43bbfb 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2155,7 +2155,7 @@ qemuMonitorGetAllBlockStatsInfo(qemuMonitor *mon,
bool backingChain)
{
int ret;
-GHashTable *stats = virHashNew(g_free);
+g_autoptr(GHashTable) stats = virHashNew(g_free);
VIR_DEBUG("ret_stats=%p, backing=%d", ret_stats, backingChain);
@@ -2165,14 +2165,10 @@ qemuMonitorGetAllBlockStatsInfo(qemuMonitor *mon,
ret = qemuMonitorJSONGetAllBlockStatsInfo(mon, stats, backingChain);
if (ret < 0)
-goto error;
+return -1;
-*ret_stats = stats;
+*ret_stats = g_steal_pointer(&stats);
return ret;
-
- error:
-virHashFree(stats);
-return -1;
}
--
2.31.1
[libvirt PATCH 08/10] qemuMonitorGetChardevInfo: Remove superfluous variable "ret"
Signed-off-by: Tim Wiederhake
---
src/qemu/qemu_monitor.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index dba2fb1982..f03d6106f9 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2885,7 +2885,6 @@ int
qemuMonitorGetChardevInfo(qemuMonitor *mon,
GHashTable **retinfo)
{
-int ret;
GHashTable *info = NULL;
VIR_DEBUG("retinfo=%p", retinfo);
@@ -2895,9 +2894,7 @@ qemuMonitorGetChardevInfo(qemuMonitor *mon,
if (!(info = virHashNew(qemuMonitorChardevInfoFree)))
goto error;
-ret = qemuMonitorJSONGetChardevInfo(mon, info);
-
-if (ret < 0)
+if (qemuMonitorJSONGetChardevInfo(mon, info) < 0)
goto error;
*retinfo = info;
--
2.31.1
[libvirt PATCH 05/10] qemuMonitorGetBlockInfo: Remove superfluous variable "ret"
Signed-off-by: Tim Wiederhake
---
src/qemu/qemu_monitor.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index f08b43bbfb..de0f6ccf7d 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2103,7 +2103,6 @@ qemuDomainDiskInfoFree(void *value)
GHashTable *
qemuMonitorGetBlockInfo(qemuMonitor *mon)
{
-int ret;
GHashTable *table;
QEMU_CHECK_MONITOR_NULL(mon);
@@ -2111,9 +2110,7 @@ qemuMonitorGetBlockInfo(qemuMonitor *mon)
if (!(table = virHashNew(qemuDomainDiskInfoFree)))
return NULL;
-ret = qemuMonitorJSONGetBlockInfo(mon, table);
-
-if (ret < 0) {
+if (qemuMonitorJSONGetBlockInfo(mon, table) < 0) {
virHashFree(table);
return NULL;
}
--
2.31.1
[libvirt PATCH 07/10] qemuMonitorGetBlockInfo: `virHashNew` cannot return NULL
Signed-off-by: Tim Wiederhake
---
src/qemu/qemu_monitor.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 9f0b20db09..dba2fb1982 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2103,13 +2103,10 @@ qemuDomainDiskInfoFree(void *value)
GHashTable *
qemuMonitorGetBlockInfo(qemuMonitor *mon)
{
-g_autoptr(GHashTable) table = NULL;
+g_autoptr(GHashTable) table = virHashNew(qemuDomainDiskInfoFree);
QEMU_CHECK_MONITOR_NULL(mon);
-if (!(table = virHashNew(qemuDomainDiskInfoFree)))
-return NULL;
-
if (qemuMonitorJSONGetBlockInfo(mon, table) < 0) {
return NULL;
}
--
2.31.1
[libvirt PATCH 06/10] qemuMonitorGetBlockInfo: Use automatic memory management
Signed-off-by: Tim Wiederhake
---
src/qemu/qemu_monitor.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index de0f6ccf7d..9f0b20db09 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2103,7 +2103,7 @@ qemuDomainDiskInfoFree(void *value)
GHashTable *
qemuMonitorGetBlockInfo(qemuMonitor *mon)
{
-GHashTable *table;
+g_autoptr(GHashTable) table = NULL;
QEMU_CHECK_MONITOR_NULL(mon);
@@ -2111,11 +2111,10 @@ qemuMonitorGetBlockInfo(qemuMonitor *mon)
return NULL;
if (qemuMonitorJSONGetBlockInfo(mon, table) < 0) {
-virHashFree(table);
return NULL;
}
-return table;
+return g_steal_pointer(&table);
}
--
2.31.1
[libvirt PATCH 10/10] qemuMonitorGetChardevInfo: `virHashNew` cannot return NULL
Signed-off-by: Tim Wiederhake
---
src/qemu/qemu_monitor.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 2c43cc0788..a63056f0a0 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2885,16 +2885,13 @@ int
qemuMonitorGetChardevInfo(qemuMonitor *mon,
GHashTable **retinfo)
{
-g_autoptr(GHashTable) info = NULL;
+g_autoptr(GHashTable) info = virHashNew(qemuMonitorChardevInfoFree);
VIR_DEBUG("retinfo=%p", retinfo);
QEMU_CHECK_MONITOR(mon);
*retinfo = NULL;
-if (!(info = virHashNew(qemuMonitorChardevInfoFree)))
-return -1;
-
if (qemuMonitorJSONGetChardevInfo(mon, info) < 0)
return -1;
--
2.31.1
[libvirt PATCH 09/10] qemuMonitorGetChardevInfo: Use automatic memory management
Signed-off-by: Tim Wiederhake
---
src/qemu/qemu_monitor.c | 16 ++--
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index f03d6106f9..2c43cc0788 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2885,25 +2885,21 @@ int
qemuMonitorGetChardevInfo(qemuMonitor *mon,
GHashTable **retinfo)
{
-GHashTable *info = NULL;
+g_autoptr(GHashTable) info = NULL;
VIR_DEBUG("retinfo=%p", retinfo);
-QEMU_CHECK_MONITOR_GOTO(mon, error);
+QEMU_CHECK_MONITOR(mon);
+*retinfo = NULL;
if (!(info = virHashNew(qemuMonitorChardevInfoFree)))
-goto error;
+return -1;
if (qemuMonitorJSONGetChardevInfo(mon, info) < 0)
-goto error;
+return -1;
-*retinfo = info;
+*retinfo = g_steal_pointer(&info);
return 0;
-
- error:
-virHashFree(info);
-*retinfo = NULL;
-return -1;
}
--
2.31.1
[libvirt PATCH v2 00/11] virHashNew refactorings
"virHashNew" cannot return NULL, yet we check for NULL in various places. This series is the first of several that remove these checks. Where applicable, the functions are refactored to use automatic memory management by means of g_autoptr etc. as well. v1: https://listman.redhat.com/archives/libvir-list/2021-July/msg00074.html Changes since v1: * Fixed a memory leak that was introduced in patch #3 and fixed in patch #4 * Split up patches 3 and 4 into three patches Regards, Tim Tim Wiederhake (11): qemuMonitorGetAllBlockStatsInfo: Clean up line break qemuMonitorGetAllBlockStatsInfo: Remove superfluous variable initialization qemuMonitorGetAllBlockStatsInfo: Assign hash table only on success qemuMonitorGetAllBlockStatsInfo: Use automatic memory management qemuMonitorGetAllBlockStatsInfo: `virHashNew` cannot return NULL qemuMonitorGetBlockInfo: Remove superfluous variable "ret" qemuMonitorGetBlockInfo: Use automatic memory management qemuMonitorGetBlockInfo: `virHashNew` cannot return NULL qemuMonitorGetChardevInfo: Remove superfluous variable "ret" qemuMonitorGetChardevInfo: Use automatic memory management qemuMonitorGetChardevInfo: `virHashNew` cannot return NULL src/qemu/qemu_monitor.c | 53 - 1 file changed, 15 insertions(+), 38 deletions(-) -- 2.31.1
[libvirt PATCH v2 01/11] qemuMonitorGetAllBlockStatsInfo: Clean up line break
Signed-off-by: Tim Wiederhake --- src/qemu/qemu_monitor.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 8f35b4240f..df1e41f68f 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2162,8 +2162,7 @@ qemuMonitorGetAllBlockStatsInfo(qemuMonitor *mon, if (!(*ret_stats = virHashNew(g_free))) goto error; -ret = qemuMonitorJSONGetAllBlockStatsInfo(mon, *ret_stats, - backingChain); +ret = qemuMonitorJSONGetAllBlockStatsInfo(mon, *ret_stats, backingChain); if (ret < 0) goto error; -- 2.31.1
[libvirt PATCH v2 02/11] qemuMonitorGetAllBlockStatsInfo: Remove superfluous variable initialization
Signed-off-by: Tim Wiederhake
---
src/qemu/qemu_monitor.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index df1e41f68f..19fcc5658b 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2154,7 +2154,7 @@ qemuMonitorGetAllBlockStatsInfo(qemuMonitor *mon,
GHashTable **ret_stats,
bool backingChain)
{
-int ret = -1;
+int ret;
VIR_DEBUG("ret_stats=%p, backing=%d", ret_stats, backingChain);
QEMU_CHECK_MONITOR(mon);
--
2.31.1
[libvirt PATCH v2 03/11] qemuMonitorGetAllBlockStatsInfo: Assign hash table only on success
`virHashNew` cannot return NULL, the check is not needed.
Signed-off-by: Tim Wiederhake
---
src/qemu/qemu_monitor.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 19fcc5658b..d24531832b 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2155,22 +2155,24 @@ qemuMonitorGetAllBlockStatsInfo(qemuMonitor *mon,
bool backingChain)
{
int ret;
+GHashTable *stats = NULL;
VIR_DEBUG("ret_stats=%p, backing=%d", ret_stats, backingChain);
QEMU_CHECK_MONITOR(mon);
-if (!(*ret_stats = virHashNew(g_free)))
+if (!(stats = virHashNew(g_free)))
goto error;
-ret = qemuMonitorJSONGetAllBlockStatsInfo(mon, *ret_stats, backingChain);
+ret = qemuMonitorJSONGetAllBlockStatsInfo(mon, stats, backingChain);
if (ret < 0)
goto error;
+*ret_stats = stats;
return ret;
error:
-virHashFree(*ret_stats);
+virHashFree(stats);
*ret_stats = NULL;
return -1;
}
--
2.31.1
[libvirt PATCH v2 04/11] qemuMonitorGetAllBlockStatsInfo: Use automatic memory management
Signed-off-by: Tim Wiederhake
---
src/qemu/qemu_monitor.c | 14 +-
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index d24531832b..6ff7302360 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2155,26 +2155,22 @@ qemuMonitorGetAllBlockStatsInfo(qemuMonitor *mon,
bool backingChain)
{
int ret;
-GHashTable *stats = NULL;
+g_autoptr(GHashTable) stats = NULL;
+
VIR_DEBUG("ret_stats=%p, backing=%d", ret_stats, backingChain);
QEMU_CHECK_MONITOR(mon);
if (!(stats = virHashNew(g_free)))
-goto error;
+return -1;
ret = qemuMonitorJSONGetAllBlockStatsInfo(mon, stats, backingChain);
if (ret < 0)
-goto error;
+return -1;
-*ret_stats = stats;
+*ret_stats = g_steal_pointer(&stats);
return ret;
-
- error:
-virHashFree(stats);
-*ret_stats = NULL;
-return -1;
}
--
2.31.1
[libvirt PATCH v2 08/11] qemuMonitorGetBlockInfo: `virHashNew` cannot return NULL
Signed-off-by: Tim Wiederhake
---
src/qemu/qemu_monitor.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 2253c96cb5..f2c35ab173 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2103,13 +2103,10 @@ qemuDomainDiskInfoFree(void *value)
GHashTable *
qemuMonitorGetBlockInfo(qemuMonitor *mon)
{
-g_autoptr(GHashTable) table = NULL;
+g_autoptr(GHashTable) table = virHashNew(qemuDomainDiskInfoFree);
QEMU_CHECK_MONITOR_NULL(mon);
-if (!(table = virHashNew(qemuDomainDiskInfoFree)))
-return NULL;
-
if (qemuMonitorJSONGetBlockInfo(mon, table) < 0) {
return NULL;
}
--
2.31.1
[libvirt PATCH v2 06/11] qemuMonitorGetBlockInfo: Remove superfluous variable "ret"
Signed-off-by: Tim Wiederhake
---
src/qemu/qemu_monitor.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index dd0658f93c..933d4a0154 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2103,7 +2103,6 @@ qemuDomainDiskInfoFree(void *value)
GHashTable *
qemuMonitorGetBlockInfo(qemuMonitor *mon)
{
-int ret;
GHashTable *table;
QEMU_CHECK_MONITOR_NULL(mon);
@@ -2111,9 +2110,7 @@ qemuMonitorGetBlockInfo(qemuMonitor *mon)
if (!(table = virHashNew(qemuDomainDiskInfoFree)))
return NULL;
-ret = qemuMonitorJSONGetBlockInfo(mon, table);
-
-if (ret < 0) {
+if (qemuMonitorJSONGetBlockInfo(mon, table) < 0) {
virHashFree(table);
return NULL;
}
--
2.31.1
[libvirt PATCH v2 07/11] qemuMonitorGetBlockInfo: Use automatic memory management
Signed-off-by: Tim Wiederhake
---
src/qemu/qemu_monitor.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 933d4a0154..2253c96cb5 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2103,7 +2103,7 @@ qemuDomainDiskInfoFree(void *value)
GHashTable *
qemuMonitorGetBlockInfo(qemuMonitor *mon)
{
-GHashTable *table;
+g_autoptr(GHashTable) table = NULL;
QEMU_CHECK_MONITOR_NULL(mon);
@@ -2111,11 +2111,10 @@ qemuMonitorGetBlockInfo(qemuMonitor *mon)
return NULL;
if (qemuMonitorJSONGetBlockInfo(mon, table) < 0) {
-virHashFree(table);
return NULL;
}
-return table;
+return g_steal_pointer(&table);
}
--
2.31.1
[libvirt PATCH v2 09/11] qemuMonitorGetChardevInfo: Remove superfluous variable "ret"
Signed-off-by: Tim Wiederhake
---
src/qemu/qemu_monitor.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index f2c35ab173..cb59fc7b7b 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2884,7 +2884,6 @@ int
qemuMonitorGetChardevInfo(qemuMonitor *mon,
GHashTable **retinfo)
{
-int ret;
GHashTable *info = NULL;
VIR_DEBUG("retinfo=%p", retinfo);
@@ -2894,9 +2893,7 @@ qemuMonitorGetChardevInfo(qemuMonitor *mon,
if (!(info = virHashNew(qemuMonitorChardevInfoFree)))
goto error;
-ret = qemuMonitorJSONGetChardevInfo(mon, info);
-
-if (ret < 0)
+if (qemuMonitorJSONGetChardevInfo(mon, info) < 0)
goto error;
*retinfo = info;
--
2.31.1
[libvirt PATCH v2 05/11] qemuMonitorGetAllBlockStatsInfo: `virHashNew` cannot return NULL
Signed-off-by: Tim Wiederhake
---
src/qemu/qemu_monitor.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 6ff7302360..dd0658f93c 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2155,15 +2155,12 @@ qemuMonitorGetAllBlockStatsInfo(qemuMonitor *mon,
bool backingChain)
{
int ret;
-g_autoptr(GHashTable) stats = NULL;
+g_autoptr(GHashTable) stats = virHashNew(g_free);
VIR_DEBUG("ret_stats=%p, backing=%d", ret_stats, backingChain);
QEMU_CHECK_MONITOR(mon);
-if (!(stats = virHashNew(g_free)))
-return -1;
-
ret = qemuMonitorJSONGetAllBlockStatsInfo(mon, stats, backingChain);
if (ret < 0)
--
2.31.1
[libvirt PATCH v2 10/11] qemuMonitorGetChardevInfo: Use automatic memory management
Signed-off-by: Tim Wiederhake
---
src/qemu/qemu_monitor.c | 16 ++--
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index cb59fc7b7b..4489b809f4 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2884,25 +2884,21 @@ int
qemuMonitorGetChardevInfo(qemuMonitor *mon,
GHashTable **retinfo)
{
-GHashTable *info = NULL;
+g_autoptr(GHashTable) info = NULL;
VIR_DEBUG("retinfo=%p", retinfo);
-QEMU_CHECK_MONITOR_GOTO(mon, error);
+QEMU_CHECK_MONITOR(mon);
+*retinfo = NULL;
if (!(info = virHashNew(qemuMonitorChardevInfoFree)))
-goto error;
+return -1;
if (qemuMonitorJSONGetChardevInfo(mon, info) < 0)
-goto error;
+return -1;
-*retinfo = info;
+*retinfo = g_steal_pointer(&info);
return 0;
-
- error:
-virHashFree(info);
-*retinfo = NULL;
-return -1;
}
--
2.31.1
[libvirt PATCH v2 11/11] qemuMonitorGetChardevInfo: `virHashNew` cannot return NULL
Signed-off-by: Tim Wiederhake
---
src/qemu/qemu_monitor.c | 5 +
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 4489b809f4..ef24b2ca1e 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -2884,16 +2884,13 @@ int
qemuMonitorGetChardevInfo(qemuMonitor *mon,
GHashTable **retinfo)
{
-g_autoptr(GHashTable) info = NULL;
+g_autoptr(GHashTable) info = virHashNew(qemuMonitorChardevInfoFree);
VIR_DEBUG("retinfo=%p", retinfo);
QEMU_CHECK_MONITOR(mon);
*retinfo = NULL;
-if (!(info = virHashNew(qemuMonitorChardevInfoFree)))
-return -1;
-
if (qemuMonitorJSONGetChardevInfo(mon, info) < 0)
return -1;
--
2.31.1
