9:26 a.m.
This series reworks the outdated way of parsing XML to parsing by XPath,
which is more obvious and saves a few lines of code.
Kristina Hanicova (21):
conf: Propagate xmlXPathContextPtr into
virDomainHostdevSubsysUSBDefParseXML()
Refactoring virDomainHostdevSubsysUSBDefParseXML() to use XPath
conf: Propagate xmlXPathContextPtr into virDomainBlkioDeviceParseXML()
Refactoring virDomainBlkioDeviceParseXML() to use XPath
conf: Propagate xmlXPathContextPtr into
virDomainHostdevSubsysPCIDefParseXML()
Refactoring virDomainHostdevSubsysPCIDefParseXML() to use XPath
conf: Propagate xmlXPathContextPtr into virDomainLeaseDefParseXML()
Refactoring virDomainLeaseDefParseXML() to use XPath
Refactoring virDomainDiskDefParseXML() to use XPath
Refactoring virDomainControllerDefParseXML() to use XPath
Refactoring virDomainFSDefParseXML() to use XPath
Refactoring virDomainNetDefParseXML() to use XPath
conf: Propagate xmlXPathContextPtr into
virDomainChrDefParseTargetXML()
Refactoring virDomainChrDefParseTargetXML() to use XPath
Refactoring virDomainChrSourceDefParseXML() to use XPath
Refactoring virDomainChrDefParseXML() to use XPath
Refactoring virDomainSmartcardDefParseXML() to use XPath
Refactoring virDomainGraphicsDefParseXMLSpice() to use XPath
conf: Propagate xmlXPathContextPtr into
virDomainVideoDriverDefParseXML()
Refactoring virDomainVideoDriverDefParseXML() to use XPath
Refactoring virDomainVideoDefParseXML() to use XPath
src/conf/domain_conf.c | 1786 +++++++++++++++++-----------------------
1 file changed, 777 insertions(+), 1009 deletions(-)
--
2.30.2
9:26 a.m.
New subject: [PATCH 01/21] conf: Propagate xmlXPathContextPtr into virDomainHostdevSubsysUSBDefParseXML()
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/conf/domain_conf.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 029f2d8d9c..d42167ae4c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6735,6 +6735,7 @@ virDomainDeviceInfoParseXML(virDomainXMLOption *xmlopt,
static int
virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
+ xmlXPathContextPtr ctxt G_GNUC_UNUSED,
virDomainHostdevDef *def)
{
bool got_product, got_vendor;
@@ -7479,7 +7480,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
break;
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
- if (virDomainHostdevSubsysUSBDefParseXML(sourcenode, def) < 0)
+ if (virDomainHostdevSubsysUSBDefParseXML(sourcenode, ctxt, def) < 0)
return -1;
break;
--
2.30.2
9:26 a.m.
New subject: [PATCH 02/21] Refactoring virDomainHostdevSubsysUSBDefParseXML() to use XPath
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/conf/domain_conf.c | 130 +++++++++++++++++++----------------------
1 file changed, 60 insertions(+), 70 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d42167ae4c..aca30d37d4 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6735,14 +6735,16 @@ virDomainDeviceInfoParseXML(virDomainXMLOption *xmlopt,
static int
virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
- xmlXPathContextPtr ctxt G_GNUC_UNUSED,
+ xmlXPathContextPtr ctxt,
virDomainHostdevDef *def)
{
bool got_product, got_vendor;
- xmlNodePtr cur;
virDomainHostdevSubsysUSB *usbsrc = &def->source.subsys.u.usb;
g_autofree char *startupPolicy = NULL;
g_autofree char *autoAddress = NULL;
+ VIR_XPATH_NODE_AUTORESTORE(ctxt)
+
+ ctxt->node = node;
if ((startupPolicy = virXMLPropString(node, "startupPolicy"))) {
def->startupPolicy =
@@ -6763,79 +6765,67 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
got_product = false;
got_vendor = false;
- cur = node->children;
- while (cur != NULL) {
- if (cur->type == XML_ELEMENT_NODE) {
- if (virXMLNodeNameEqual(cur, "vendor")) {
- g_autofree char *vendor = virXMLPropString(cur, "id");
-
- if (vendor) {
- got_vendor = true;
- if (virStrToLong_ui(vendor, NULL, 0, &usbsrc->vendor) < 0)
{
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("cannot parse vendor id %s"),
vendor);
- return -1;
- }
- } else {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("usb vendor needs id"));
- return -1;
- }
- } else if (virXMLNodeNameEqual(cur, "product")) {
- g_autofree char *product = virXMLPropString(cur, "id");
-
- if (product) {
- got_product = true;
- if (virStrToLong_ui(product, NULL, 0,
- &usbsrc->product) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("cannot parse product %s"),
- product);
- return -1;
- }
- } else {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("usb product needs id"));
- return -1;
- }
- } else if (virXMLNodeNameEqual(cur, "address")) {
- g_autofree char *bus = NULL;
- g_autofree char *device = NULL;
-
- bus = virXMLPropString(cur, "bus");
- if (bus) {
- if (virStrToLong_ui(bus, NULL, 0, &usbsrc->bus) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("cannot parse bus %s"), bus);
- return -1;
- }
- } else {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("usb address needs bus
id"));
- return -1;
- }
+ if (virXPathNode("./vendor", ctxt)) {
+ g_autofree char *vendor = NULL;
- device = virXMLPropString(cur, "device");
- if (device) {
- if (virStrToLong_ui(device, NULL, 0, &usbsrc->device) < 0)
{
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("cannot parse device %s"),
- device);
- return -1;
- }
- } else {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("usb address needs device id"));
- return -1;
- }
- } else {
+ if ((vendor = virXPathString("string(./vendor/@id)", ctxt))) {
+ got_vendor = true;
+ if (virStrToLong_ui(vendor, NULL, 0, &usbsrc->vendor) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unknown usb source type '%s'"),
- cur->name);
+ _("cannot parse vendor id %s"), vendor);
return -1;
}
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("usb vendor needs id"));
+ return -1;
+ }
+ }
+
+ if (virXPathNode("./product", ctxt)) {
+ g_autofree char *product = NULL;
+
+ if ((product = virXPathString("string(./product/@id)", ctxt))) {
+ got_product = true;
+ if (virStrToLong_ui(product, NULL, 0, &usbsrc->product) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("cannot parse product %s"), product);
+ return -1;
+ }
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("usb product needs id"));
+ return -1;
+ }
+ }
+
+ if (virXPathNode("./address", ctxt)) {
+ g_autofree char *bus = NULL;
+ g_autofree char *device = NULL;
+
+ if ((bus = virXPathString("string(./address/@bus)", ctxt))) {
+ if (virStrToLong_ui(bus, NULL, 0, &usbsrc->bus) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("cannot parse bus %s"), bus);
+ return -1;
+ }
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ "%s", _("usb address needs bus id"));
+ return -1;
+ }
+
+ if ((device = virXPathString("string(./address/@device)", ctxt))) {
+ if (virStrToLong_ui(device, NULL, 0, &usbsrc->device) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("cannot parse device %s"), device);
+ return -1;
+ }
+ } else {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("usb address needs device id"));
+ return -1;
}
- cur = cur->next;
}
if (got_vendor && usbsrc->vendor == 0) {
--
2.30.2
9:26 a.m.
New subject: [PATCH 03/21] conf: Propagate xmlXPathContextPtr into virDomainBlkioDeviceParseXML()
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/conf/domain_conf.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index aca30d37d4..33cc204507 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1698,6 +1698,7 @@ virBlkioDeviceArrayClear(virBlkioDevice *devices,
*/
static int
virDomainBlkioDeviceParseXML(xmlNodePtr root,
+ xmlXPathContextPtr ctxt G_GNUC_UNUSED,
virBlkioDevice *dev)
{
xmlNodePtr node;
@@ -20602,7 +20603,7 @@ virDomainDefTunablesParse(virDomainDef *def,
def->blkio.devices = g_new0(virBlkioDevice, n);
for (i = 0; i < n; i++) {
- if (virDomainBlkioDeviceParseXML(nodes[i],
+ if (virDomainBlkioDeviceParseXML(nodes[i], ctxt,
&def->blkio.devices[i]) < 0)
return -1;
def->blkio.ndevices++;
--
2.30.2
9:26 a.m.
New subject: [PATCH 04/21] Refactoring virDomainBlkioDeviceParseXML() to use XPath
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/conf/domain_conf.c | 116 +++++++++++++++++------------------------
1 file changed, 48 insertions(+), 68 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 33cc204507..65d5bb2c2a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1698,85 +1698,65 @@ virBlkioDeviceArrayClear(virBlkioDevice *devices,
*/
static int
virDomainBlkioDeviceParseXML(xmlNodePtr root,
- xmlXPathContextPtr ctxt G_GNUC_UNUSED,
+ xmlXPathContextPtr ctxt,
virBlkioDevice *dev)
{
- xmlNodePtr node;
g_autofree char *path = NULL;
+ g_autofree char *weight = NULL;
+ g_autofree char *read_bytes_sec = NULL;
+ g_autofree char *write_bytes_sec = NULL;
+ g_autofree char *read_iops_sec = NULL;
+ g_autofree char *write_iops_sec = NULL;
+ VIR_XPATH_NODE_AUTORESTORE(ctxt)
- for (node = root->children; node != NULL; node = node->next) {
- g_autofree char *c = NULL;
-
- if (node->type != XML_ELEMENT_NODE)
- continue;
-
- if (!(c = virXMLNodeContentString(node)))
- return -1;
-
- if (virXMLNodeNameEqual(node, "path")) {
- /* To avoid the need for explicit cleanup on failure,
- * don't set dev->path until we're assured of
- * success. Until then, store it in an autofree pointer.
- */
- if (!path)
- path = g_steal_pointer(&c);
- continue;
- }
+ ctxt->node = root;
- if (virXMLNodeNameEqual(node, "weight")) {
- if (virStrToLong_ui(c, NULL, 10, &dev->weight) < 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("could not parse weight %s"),
- c);
- return -1;
- }
- continue;
- }
+ /* To avoid the need for explicit cleanup on failure,
+ * don't set dev->path until we're assured of
+ * success. Until then, store it in an autofree pointer.
+ */
+ if (!(path = virXPathString("string(./path)", ctxt))) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("missing per-device path"));
+ return -1;
+ }
- if (virXMLNodeNameEqual(node, "read_bytes_sec")) {
- if (virStrToLong_ull(c, NULL, 10, &dev->rbps) < 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("could not parse read bytes sec %s"),
- c);
- return -1;
- }
- continue;
- }
+ if ((weight = virXPathString("string(./weight)", ctxt)) &&
+ (virStrToLong_ui(weight, NULL, 10, &dev->weight) < 0)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("could not parse weight %s"), weight);
+ return -1;
+ }
- if (virXMLNodeNameEqual(node, "write_bytes_sec")) {
- if (virStrToLong_ull(c, NULL, 10, &dev->wbps) < 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("could not parse write bytes sec %s"),
- c);
- return -1;
- }
- continue;
- }
+ if ((read_bytes_sec = virXPathString("string(./read_bytes_sec)", ctxt))
&&
+ (virStrToLong_ull(read_bytes_sec, NULL, 10, &dev->rbps) < 0)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("could not parse read bytes sec %s"),
+ read_bytes_sec);
+ return -1;
+ }
- if (virXMLNodeNameEqual(node, "read_iops_sec")) {
- if (virStrToLong_ui(c, NULL, 10, &dev->riops) < 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("could not parse read iops sec %s"),
- c);
- return -1;
- }
- continue;
- }
+ if ((write_bytes_sec = virXPathString("string(./write_bytes_sec)", ctxt))
&&
+ (virStrToLong_ull(write_bytes_sec, NULL, 10, &dev->wbps) < 0)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("could not parse write bytes sec %s"),
+ write_bytes_sec);
+ return -1;
+ }
- if (virXMLNodeNameEqual(node, "write_iops_sec")) {
- if (virStrToLong_ui(c, NULL, 10, &dev->wiops) < 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("could not parse write iops sec %s"),
- c);
- return -1;
- }
- continue;
- }
+ if ((read_iops_sec = virXPathString("string(./read_iops_sec)", ctxt))
&&
+ (virStrToLong_ui(read_iops_sec, NULL, 10, &dev->riops) < 0)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("could not parse read iops sec %s"),
+ read_iops_sec);
+ return -1;
}
- if (!path) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("missing per-device path"));
+ if ((write_iops_sec = virXPathString("string(./write_iops_sec)", ctxt))
&&
+ (virStrToLong_ui(write_iops_sec, NULL, 10, &dev->wiops) < 0)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("could not parse write iops sec %s"),
+ write_iops_sec);
return -1;
}
--
2.30.2
9:26 a.m.
New subject: [PATCH 05/21] conf: Propagate xmlXPathContextPtr into virDomainHostdevSubsysPCIDefParseXML()
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/conf/domain_conf.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 65d5bb2c2a..eee9c2e3e3 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6867,6 +6867,7 @@ virDomainHostdevSubsysPCIOrigStatesDefParseXML(xmlNodePtr node,
static int
virDomainHostdevSubsysPCIDefParseXML(xmlNodePtr node,
+ xmlXPathContextPtr ctxt G_GNUC_UNUSED,
virDomainHostdevDef *def,
unsigned int flags)
{
@@ -7434,7 +7435,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
switch (def->source.subsys.type) {
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
- if (virDomainHostdevSubsysPCIDefParseXML(sourcenode, def, flags) < 0)
+ if (virDomainHostdevSubsysPCIDefParseXML(sourcenode, ctxt, def, flags) < 0)
return -1;
backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT;
--
2.30.2
9:26 a.m.
New subject: [PATCH 06/21] Refactoring virDomainHostdevSubsysPCIDefParseXML() to use XPath
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/conf/domain_conf.c | 37 +++++++++++++------------------------
1 file changed, 13 insertions(+), 24 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index eee9c2e3e3..e808eade90 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6867,12 +6867,16 @@ virDomainHostdevSubsysPCIOrigStatesDefParseXML(xmlNodePtr node,
static int
virDomainHostdevSubsysPCIDefParseXML(xmlNodePtr node,
- xmlXPathContextPtr ctxt G_GNUC_UNUSED,
+ xmlXPathContextPtr ctxt,
virDomainHostdevDef *def,
unsigned int flags)
{
g_autofree char *filtering = NULL;
- xmlNodePtr cur;
+ xmlNodePtr address = NULL;
+ xmlNodePtr origstates = NULL;
+ VIR_XPATH_NODE_AUTORESTORE(ctxt)
+
+ ctxt->node = node;
if ((filtering = virXMLPropString(node, "writeFiltering"))) {
int val;
@@ -6885,29 +6889,14 @@ virDomainHostdevSubsysPCIDefParseXML(xmlNodePtr node,
def->writeFiltering = val;
}
- cur = node->children;
- while (cur != NULL) {
- if (cur->type == XML_ELEMENT_NODE) {
- if (virXMLNodeNameEqual(cur, "address")) {
- virPCIDeviceAddress *addr =
- &def->source.subsys.u.pci.addr;
+ if ((address = virXPathNode("./address", ctxt)) &&
+ virPCIDeviceAddressParseXML(address, &def->source.subsys.u.pci.addr) <
0)
+ return -1;
- if (virPCIDeviceAddressParseXML(cur, addr) < 0)
- return -1;
- } else if ((flags & VIR_DOMAIN_DEF_PARSE_PCI_ORIG_STATES) &&
- virXMLNodeNameEqual(cur, "origstates")) {
- virDomainHostdevOrigStates *states = &def->origstates;
- if (virDomainHostdevSubsysPCIOrigStatesDefParseXML(cur, states) < 0)
- return -1;
- } else {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unknown pci source type '%s'"),
- cur->name);
- return -1;
- }
- }
- cur = cur->next;
- }
+ if ((flags & VIR_DOMAIN_DEF_PARSE_PCI_ORIG_STATES) &&
+ (origstates = virXPathNode("./origstates", ctxt)) &&
+ virDomainHostdevSubsysPCIOrigStatesDefParseXML(origstates,
&def->origstates) < 0)
+ return -1;
return 0;
}
--
2.30.2
9:26 a.m.
New subject: [PATCH 07/21] conf: Propagate xmlXPathContextPtr into virDomainLeaseDefParseXML()
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/conf/domain_conf.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e808eade90..6a03727f2c 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8121,7 +8121,8 @@ virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDef
***seclabels_rtn,
/* Parse the XML definition for a lease
*/
static virDomainLeaseDef *
-virDomainLeaseDefParseXML(xmlNodePtr node)
+virDomainLeaseDefParseXML(xmlNodePtr node,
+ xmlXPathContextPtr ctxt G_GNUC_UNUSED)
{
virDomainLeaseDef *def;
xmlNodePtr cur;
@@ -16235,7 +16236,7 @@ virDomainDeviceDefParse(const char *xmlStr,
return NULL;
break;
case VIR_DOMAIN_DEVICE_LEASE:
- if (!(dev->data.lease = virDomainLeaseDefParseXML(node)))
+ if (!(dev->data.lease = virDomainLeaseDefParseXML(node, ctxt)))
return NULL;
break;
case VIR_DOMAIN_DEVICE_FS:
@@ -21148,7 +21149,7 @@ virDomainDefParseXML(xmlDocPtr xml,
if (n)
def->leases = g_new0(virDomainLeaseDef *, n);
for (i = 0; i < n; i++) {
- virDomainLeaseDef *lease = virDomainLeaseDefParseXML(nodes[i]);
+ virDomainLeaseDef *lease = virDomainLeaseDefParseXML(nodes[i], ctxt);
if (!lease)
goto error;
--
2.30.2
9:26 a.m.
New subject: [PATCH 08/21] Refactoring virDomainLeaseDefParseXML() to use XPath
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/conf/domain_conf.c | 34 ++++++++++------------------------
1 file changed, 10 insertions(+), 24 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6a03727f2c..808169883f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8122,48 +8122,34 @@ virSecurityDeviceLabelDefParseXML(virSecurityDeviceLabelDef
***seclabels_rtn,
*/
static virDomainLeaseDef *
virDomainLeaseDefParseXML(xmlNodePtr node,
- xmlXPathContextPtr ctxt G_GNUC_UNUSED)
+ xmlXPathContextPtr ctxt)
{
virDomainLeaseDef *def;
- xmlNodePtr cur;
g_autofree char *lockspace = NULL;
g_autofree char *key = NULL;
g_autofree char *path = NULL;
g_autofree char *offset = NULL;
+ VIR_XPATH_NODE_AUTORESTORE(ctxt)
+ ctxt->node = node;
def = g_new0(virDomainLeaseDef, 1);
- cur = node->children;
- while (cur != NULL) {
- if (cur->type == XML_ELEMENT_NODE) {
- if (!key && virXMLNodeNameEqual(cur, "key")) {
- if (!(key = virXMLNodeContentString(cur)))
- goto error;
- } else if (!lockspace &&
- virXMLNodeNameEqual(cur, "lockspace")) {
- if (!(lockspace = virXMLNodeContentString(cur)))
- goto error;
- } else if (!path &&
- virXMLNodeNameEqual(cur, "target")) {
- path = virXMLPropString(cur, "path");
- offset = virXMLPropString(cur, "offset");
- }
- }
- cur = cur->next;
- }
-
- if (!key) {
+ if (!(key = virXPathString("string(./key)", ctxt))) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("Missing 'key' element for lease"));
goto error;
}
- if (!path) {
+
+ if (!(lockspace = virXPathString("string(./lockspace)", ctxt)))
+ goto error;
+
+ if (!(path = virXPathString("string(./target/@path)", ctxt))) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("Missing 'target' element for lease"));
goto error;
}
- if (offset &&
+ if ((offset = virXPathString("string(./target/@offset)", ctxt)) &&
virStrToLong_ull(offset, NULL, 10, &def->offset) < 0) {
virReportError(VIR_ERR_XML_ERROR,
_("Malformed lease target offset %s"), offset);
--
2.30.2
9:26 a.m.
New subject: [PATCH 09/21] Refactoring virDomainDiskDefParseXML() to use XPath
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/conf/domain_conf.c | 241 ++++++++++++++++++++---------------------
1 file changed, 118 insertions(+), 123 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 808169883f..2af3ec6ec3 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -9259,7 +9259,12 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt,
unsigned int flags)
{
g_autoptr(virDomainDiskDef) def = NULL;
- xmlNodePtr cur;
+ xmlNodePtr source_node = NULL;
+ xmlNodePtr geometry_node = NULL;
+ xmlNodePtr driver_node = NULL;
+ xmlNodePtr mirror_node = NULL;
+ xmlNodePtr auth_node = NULL;
+ xmlNodePtr encryption_node = NULL;
VIR_XPATH_NODE_AUTORESTORE(ctxt)
bool source = false;
g_autoptr(virStorageEncryption) encryption = NULL;
@@ -9281,6 +9286,7 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt,
g_autofree char *product = NULL;
g_autofree char *domain_name = NULL;
g_autofree char *rotation_rate = NULL;
+ g_autofree char *index = NULL;
if (!(def = virDomainDiskDefNew(xmlopt)))
return NULL;
@@ -9320,139 +9326,128 @@ virDomainDiskDefParseXML(virDomainXMLOption *xmlopt,
rawio = virXMLPropString(node, "rawio");
sgio = virXMLPropString(node, "sgio");
- for (cur = node->children; cur != NULL; cur = cur->next) {
- if (cur->type != XML_ELEMENT_NODE)
- continue;
+ if ((source_node = virXPathNode("./source", ctxt))) {
+ if (virDomainStorageSourceParse(source_node, ctxt, def->src, flags, xmlopt)
< 0)
+ return NULL;
+ source = true;
+ startupPolicy = virXPathString("string(./source/@startupPolicy)",
ctxt);
- if (!source && virXMLNodeNameEqual(cur, "source")) {
- if (virDomainStorageSourceParse(cur, ctxt, def->src, flags, xmlopt) <
0)
- return NULL;
+ if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) &&
+ (index = virXPathString("string(./source/@index)", ctxt))
&&
+ virStrToLong_uip(index, NULL, 10, &def->src->id) < 0) {
+ virReportError(VIR_ERR_XML_ERROR, _("invalid disk index
'%s'"), index);
+ return NULL;
+ }
+ }
- source = true;
+ target = virXPathString("string(./target/@dev)", ctxt);
+ bus = virXPathString("string(./target/@bus)", ctxt);
+ tray = virXPathString("string(./target/@tray)", ctxt);
+ removable = virXPathString("string(./target/@removable)", ctxt);
+ rotation_rate = virXPathString("string(./target/@rotation_rate)", ctxt);
+ domain_name = virXPathString("string(./backenddomain/@name)", ctxt);
- startupPolicy = virXMLPropString(cur, "startupPolicy");
+ /* HACK: Work around for compat with Xen
+ * driver in previous libvirt releases */
+ if (target &&
+ STRPREFIX(target, "ioemu:"))
+ memmove(target, target+6, strlen(target)-5);
- if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) &&
- (tmp = virXMLPropString(cur, "index")) &&
- virStrToLong_uip(tmp, NULL, 10, &def->src->id) < 0) {
- virReportError(VIR_ERR_XML_ERROR, _("invalid disk index
'%s'"), tmp);
- return NULL;
- }
- VIR_FREE(tmp);
- } else if (!target &&
- virXMLNodeNameEqual(cur, "target")) {
- target = virXMLPropString(cur, "dev");
- bus = virXMLPropString(cur, "bus");
- tray = virXMLPropString(cur, "tray");
- removable = virXMLPropString(cur, "removable");
- rotation_rate = virXMLPropString(cur, "rotation_rate");
-
- /* HACK: Work around for compat with Xen
- * driver in previous libvirt releases */
- if (target &&
- STRPREFIX(target, "ioemu:"))
- memmove(target, target+6, strlen(target)-5);
- } else if (!domain_name &&
- virXMLNodeNameEqual(cur, "backenddomain")) {
- domain_name = virXMLPropString(cur, "name");
- } else if (virXMLNodeNameEqual(cur, "geometry")) {
- if (virDomainDiskDefGeometryParse(def, cur) < 0)
- return NULL;
- } else if (virXMLNodeNameEqual(cur, "blockio")) {
- logical_block_size =
- virXMLPropString(cur, "logical_block_size");
- if (logical_block_size &&
- virStrToLong_ui(logical_block_size, NULL, 0,
- &def->blockio.logical_block_size) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("invalid logical block size '%s'"),
- logical_block_size);
- return NULL;
- }
- physical_block_size =
- virXMLPropString(cur, "physical_block_size");
- if (physical_block_size &&
- virStrToLong_ui(physical_block_size, NULL, 0,
- &def->blockio.physical_block_size) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("invalid physical block size '%s'"),
- physical_block_size);
- return NULL;
- }
- } else if (!virDomainDiskGetDriver(def) &&
- virXMLNodeNameEqual(cur, "driver")) {
- if (virDomainVirtioOptionsParseXML(cur, &def->virtio) < 0)
- return NULL;
+ if ((geometry_node = virXPathNode("./geometry", ctxt)) &&
+ (virDomainDiskDefGeometryParse(def, geometry_node) < 0))
+ return NULL;
- if (virDomainDiskDefDriverParseXML(def, cur, ctxt) < 0)
- return NULL;
- } else if (!def->mirror &&
- virXMLNodeNameEqual(cur, "mirror") &&
- !(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE)) {
- if (virDomainDiskDefMirrorParse(def, cur, ctxt, flags, xmlopt) < 0)
- return NULL;
- } else if (!authdef &&
- virXMLNodeNameEqual(cur, "auth")) {
- if (!(authdef = virStorageAuthDefParse(cur, ctxt)))
- return NULL;
- def->diskElementAuth = true;
- } else if (virXMLNodeNameEqual(cur, "iotune")) {
- if (virDomainDiskDefIotuneParse(def, ctxt) < 0)
- return NULL;
- } else if (virXMLNodeNameEqual(cur, "readonly")) {
- def->src->readonly = true;
- } else if (virXMLNodeNameEqual(cur, "shareable")) {
- def->src->shared = true;
- } else if (virXMLNodeNameEqual(cur, "transient")) {
- def->transient = true;
- } else if (!encryption &&
- virXMLNodeNameEqual(cur, "encryption")) {
- if (!(encryption = virStorageEncryptionParseNode(cur, ctxt)))
- return NULL;
+ if ((logical_block_size =
+ virXPathString("string(./blockio/@logical_block_size)", ctxt))
&&
+ (virStrToLong_ui(logical_block_size, NULL, 0,
+ &def->blockio.logical_block_size) < 0)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("invalid logical block size '%s'"),
+ logical_block_size);
+ return NULL;
+ }
- def->diskElementEnc = true;
+ if ((physical_block_size =
+ virXPathString("string(./blockio/@physical_block_size)", ctxt))
&&
+ (virStrToLong_ui(physical_block_size, NULL, 0,
+ &def->blockio.physical_block_size) < 0)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("invalid physical block size '%s'"),
+ physical_block_size);
+ return NULL;
+ }
- } else if (!serial &&
- virXMLNodeNameEqual(cur, "serial")) {
- if (!(serial = virXMLNodeContentString(cur)))
- return NULL;
- } else if (!wwn &&
- virXMLNodeNameEqual(cur, "wwn")) {
- if (!(wwn = virXMLNodeContentString(cur)))
- return NULL;
+ if ((driver_node = virXPathNode("./driver", ctxt))) {
+ if (virDomainVirtioOptionsParseXML(driver_node, &def->virtio) < 0)
+ return NULL;
+ if (virDomainDiskDefDriverParseXML(def, driver_node, ctxt) < 0)
+ return NULL;
+ }
- if (!virValidateWWN(wwn))
- return NULL;
- } else if (!vendor &&
- virXMLNodeNameEqual(cur, "vendor")) {
- if (!(vendor = virXMLNodeContentString(cur)))
- return NULL;
+ if ((mirror_node = virXPathNode("./mirror", ctxt)) &&
+ !(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE) &&
+ (virDomainDiskDefMirrorParse(def, mirror_node, ctxt, flags, xmlopt) < 0))
+ return NULL;
- if (!virStringIsPrintable(vendor)) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("disk vendor is not printable string"));
- return NULL;
- }
- } else if (!product &&
- virXMLNodeNameEqual(cur, "product")) {
- if (!(product = virXMLNodeContentString(cur)))
- return NULL;
+ if ((auth_node = virXPathNode("./auth", ctxt))) {
+ if (!(authdef = virStorageAuthDefParse(auth_node, ctxt)))
+ return NULL;
+ def->diskElementAuth = true;
+ }
- if (!virStringIsPrintable(product)) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("disk product is not printable string"));
- return NULL;
- }
- } else if (virXMLNodeNameEqual(cur, "boot")) {
- /* boot is parsed as part of virDomainDeviceInfoParseXML */
- } else if ((flags & VIR_DOMAIN_DEF_PARSE_STATUS) &&
- virXMLNodeNameEqual(cur, "diskSecretsPlacement")) {
- g_autofree char *secretAuth = virXMLPropString(cur, "auth");
- g_autofree char *secretEnc = virXMLPropString(cur, "enc");
+ if (virXPathNode("./iotune", ctxt) &&
+ (virDomainDiskDefIotuneParse(def, ctxt) < 0))
+ return NULL;
- def->diskElementAuth = !!secretAuth;
- def->diskElementEnc = !!secretEnc;
- }
+ if (virXPathNode("./readonly", ctxt))
+ def->src->readonly = true;
+
+ if (virXPathNode("./shareable", ctxt))
+ def->src->shared = true;
+
+ if (virXPathNode("./transient", ctxt))
+ def->transient = true;
+
+ if ((encryption_node = virXPathNode("./encryption", ctxt))) {
+ if (!(encryption = virStorageEncryptionParseNode(encryption_node, ctxt)))
+ return NULL;
+ def->diskElementEnc = true;
+ }
+
+ if (virXPathNode("./serial", ctxt) &&
+ !(serial = virXPathString("string(./serial)", ctxt)))
+ return NULL;
+
+ if (virXPathNode("./wwn", ctxt)) {
+ if (!(wwn = virXPathString("string(./wwn)", ctxt)))
+ return NULL;
+ if (!virValidateWWN(wwn))
+ return NULL;
+ }
+
+ if ((vendor = virXPathString("string(./vendor)", ctxt)) &&
+ !virStringIsPrintable(vendor)) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("disk vendor is not printable string"));
+ return NULL;
+ }
+
+ if ((product = virXPathString("string(./product)", ctxt)) &&
+ !virStringIsPrintable(product)) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("disk product is not printable string"));
+ return NULL;
+ }
+
+ if ((flags & VIR_DOMAIN_DEF_PARSE_STATUS) &&
+ virXPathNode("./diskSecretsPlacement", ctxt)) {
+ g_autofree char *secretAuth =
+ virXPathString("string(./diskSecretsPlacement/@auth)", ctxt);
+ g_autofree char *secretEnc =
+ virXPathString("string(./diskSecretsPlacement/@enc)", ctxt);
+
+ def->diskElementAuth = !!secretAuth;
+ def->diskElementEnc = !!secretEnc;
}
/* Reset def->src->type in case when 'source' was not present */
--
2.30.2
9:35 a.m.
New subject: [PATCH 09/21] Refactoring virDomainDiskDefParseXML() to use XPath
On Thu, Apr 15, 2021 at 16:26:24 +0200, Kristina Hanicova wrote:
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/conf/domain_conf.c | 241 ++++++++++++++++++++---------------------
1 file changed, 118 insertions(+), 123 deletions(-)
I'm currently working on a bit deeper refactor of the disk parsing
function which also gets rid of much of the validation code. I'll send
it shortly hopefully.
9:26 a.m.
New subject: [PATCH 10/21] Refactoring virDomainControllerDefParseXML() to use XPath
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/conf/domain_conf.c | 69 ++++++++++++++++++------------------------
1 file changed, 29 insertions(+), 40 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 2af3ec6ec3..69f25c30a8 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -9844,9 +9844,7 @@ virDomainControllerDefParseXML(virDomainXMLOption *xmlopt,
{
g_autoptr(virDomainControllerDef) def = NULL;
int type = 0;
- xmlNodePtr cur = NULL;
- bool processedModel = false;
- bool processedTarget = false;
+ xmlNodePtr driver = NULL;
int numaNode = -1;
int ports = -1;
VIR_XPATH_NODE_AUTORESTORE(ctxt)
@@ -9903,45 +9901,36 @@ virDomainControllerDefParseXML(virDomainXMLOption *xmlopt,
def->idx = idxVal;
}
- cur = node->children;
- while (cur != NULL) {
- if (cur->type == XML_ELEMENT_NODE) {
- if (virXMLNodeNameEqual(cur, "driver")) {
- queues = virXMLPropString(cur, "queues");
- cmd_per_lun = virXMLPropString(cur, "cmd_per_lun");
- max_sectors = virXMLPropString(cur, "max_sectors");
- ioeventfd = virXMLPropString(cur, "ioeventfd");
- iothread = virXMLPropString(cur, "iothread");
+ if ((driver = virXPathNode("./driver", ctxt)) &&
+ (virDomainVirtioOptionsParseXML(driver, &def->virtio) < 0))
+ return NULL;
- if (virDomainVirtioOptionsParseXML(cur, &def->virtio) < 0)
- return NULL;
- } else if (virXMLNodeNameEqual(cur, "model")) {
- if (processedModel) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("Multiple <model> elements in "
- "controller definition not allowed"));
- return NULL;
- }
- modelName = virXMLPropString(cur, "name");
- processedModel = true;
- } else if (virXMLNodeNameEqual(cur, "target")) {
- if (processedTarget) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("Multiple <target> elements in "
- "controller definition not allowed"));
- return NULL;
- }
- chassisNr = virXMLPropString(cur, "chassisNr");
- chassis = virXMLPropString(cur, "chassis");
- port = virXMLPropString(cur, "port");
- busNr = virXMLPropString(cur, "busNr");
- hotplug = virXMLPropString(cur, "hotplug");
- targetIndex = virXMLPropString(cur, "index");
- processedTarget = true;
- }
- }
- cur = cur->next;
+ queues = virXPathString("string(./driver/@queues)", ctxt);
+ cmd_per_lun = virXPathString("string(./driver/@cmd_per_lun)", ctxt);
+ max_sectors = virXPathString("string(./driver/@max_sectors)", ctxt);
+ ioeventfd = virXPathString("string(./driver/@ioeventfd)", ctxt);
+ iothread = virXPathString("string(./driver/@iothread)", ctxt);
+
+ if (virXPathBoolean("boolean(count(./model) > 1)", ctxt)) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("Multiple <model> elements in "
+ "controller definition not allowed"));
+ return NULL;
+ }
+ modelName = virXPathString("string(./model/@name)", ctxt);
+
+ if (virXPathBoolean("boolean(count(./target) > 1)", ctxt)) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("Multiple <target> elements in "
+ "controller definition not allowed"));
+ return NULL;
}
+ chassisNr = virXPathString("string(./target/@chassisNr)", ctxt);
+ chassis = virXPathString("string(./target/@chassis)", ctxt);
+ port = virXPathString("string(./target/@port)", ctxt);
+ busNr = virXPathString("string(./target/@busNr)", ctxt);
+ hotplug = virXPathString("string(./target/@hotplug)", ctxt);
+ targetIndex = virXPathString("string(./target/@targetIndex)", ctxt);
/* node is parsed differently from target attributes because
* someone thought it should be a subelement instead...
--
2.30.2
9:26 a.m.
New subject: [PATCH 11/21] Refactoring virDomainFSDefParseXML() to use XPath
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/conf/domain_conf.c | 108 ++++++++++++++++++-----------------------
1 file changed, 48 insertions(+), 60 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 69f25c30a8..22700c3064 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10178,7 +10178,8 @@ virDomainFSDefParseXML(virDomainXMLOption *xmlopt,
{
VIR_XPATH_NODE_AUTORESTORE(ctxt)
virDomainFSDef *def;
- xmlNodePtr cur;
+ xmlNodePtr driver_node = NULL;
+ xmlNodePtr source_node = NULL;
g_autofree char *type = NULL;
g_autofree char *fsdriver = NULL;
g_autofree char *source = NULL;
@@ -10271,57 +10272,62 @@ virDomainFSDefParseXML(virDomainXMLOption *xmlopt,
1, ULLONG_MAX, false) < 0)
goto error;
- cur = node->children;
- while (cur != NULL) {
- if (cur->type == XML_ELEMENT_NODE) {
- if (!source &&
- virXMLNodeNameEqual(cur, "source")) {
-
- if (def->type == VIR_DOMAIN_FS_TYPE_MOUNT ||
- def->type == VIR_DOMAIN_FS_TYPE_BIND) {
- source = virXMLPropString(cur, "dir");
- } else if (def->type == VIR_DOMAIN_FS_TYPE_FILE) {
- source = virXMLPropString(cur, "file");
- } else if (def->type == VIR_DOMAIN_FS_TYPE_BLOCK) {
- source = virXMLPropString(cur, "dev");
- } else if (def->type == VIR_DOMAIN_FS_TYPE_TEMPLATE) {
- source = virXMLPropString(cur, "name");
- } else if (def->type == VIR_DOMAIN_FS_TYPE_RAM) {
- usage = virXMLPropString(cur, "usage");
- units = virXMLPropString(cur, "units");
- } else if (def->type == VIR_DOMAIN_FS_TYPE_VOLUME) {
- def->src->type = VIR_STORAGE_TYPE_VOLUME;
- if (virDomainDiskSourcePoolDefParse(cur,
&def->src->srcpool) < 0)
- goto error;
- }
- } else if (!target &&
- virXMLNodeNameEqual(cur, "target")) {
- target = virXMLPropString(cur, "dir");
- } else if (virXMLNodeNameEqual(cur, "readonly")) {
- def->readonly = true;
- } else if (virXMLNodeNameEqual(cur, "driver")) {
- if (!fsdriver)
- fsdriver = virXMLPropString(cur, "type");
- if (!wrpolicy)
- wrpolicy = virXMLPropString(cur, "wrpolicy");
- if (!format)
- format = virXMLPropString(cur, "format");
- if (virDomainVirtioOptionsParseXML(cur, &def->virtio) < 0)
- goto error;
- }
+ if ((source_node = virXPathNode("./source", ctxt))) {
+ if (def->type == VIR_DOMAIN_FS_TYPE_MOUNT ||
+ def->type == VIR_DOMAIN_FS_TYPE_BIND) {
+ source = virXPathString("string(./source/@dir)", ctxt);
+ } else if (def->type == VIR_DOMAIN_FS_TYPE_FILE) {
+ source = virXPathString("string(./source/@file)", ctxt);
+ } else if (def->type == VIR_DOMAIN_FS_TYPE_BLOCK) {
+ source = virXPathString("string(./source/@dev)", ctxt);
+ } else if (def->type == VIR_DOMAIN_FS_TYPE_TEMPLATE) {
+ source = virXPathString("string(./source/@name)", ctxt);
+ } else if (def->type == VIR_DOMAIN_FS_TYPE_RAM) {
+ usage = virXPathString("string(./source/@usage)", ctxt);
+ units = virXPathString("string(./source/@units)", ctxt);
+ } else if (def->type == VIR_DOMAIN_FS_TYPE_VOLUME) {
+ def->src->type = VIR_STORAGE_TYPE_VOLUME;
+ if (virDomainDiskSourcePoolDefParse(source_node,
&def->src->srcpool) < 0)
+ goto error;
}
- cur = cur->next;
}
- if (fsdriver) {
- if ((def->fsdriver = virDomainFSDriverTypeFromString(fsdriver)) <= 0) {
+ target = virXPathString("string(./target/@dir)", ctxt);
+
+ if (virXPathNode("./readonly", ctxt))
+ def->readonly = true;
+
+ if ((driver_node = virXPathNode("./driver", ctxt))) {
+ if (virDomainVirtioOptionsParseXML(driver_node, &def->virtio) < 0)
+ goto error;
+
+ if ((fsdriver = virXPathString("string(./driver/@type)", ctxt))
&&
+ ((def->fsdriver = virDomainFSDriverTypeFromString(fsdriver)) <= 0)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("unknown fs driver type '%s'"),
fsdriver);
goto error;
}
+
+ if ((wrpolicy = virXPathString("string(./driver/@wrpolicy)", ctxt))) {
+ if ((def->wrpolicy = virDomainFSWrpolicyTypeFromString(wrpolicy)) <= 0)
{
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown filesystem write policy
'%s'"), wrpolicy);
+ goto error;
+ }
+ } else {
+ def->wrpolicy = VIR_DOMAIN_FS_WRPOLICY_DEFAULT;
+ }
+
+ if ((format = virXPathString("string(./driver/@format)", ctxt))
&&
+ ((def->format = virStorageFileFormatTypeFromString(format)) <= 0)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown driver format value '%s'"),
format);
+ goto error;
+ }
}
+
if (def->fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS) {
g_autofree char *queue_size = virXPathString("string(./driver/@queue)",
ctxt);
g_autofree char *binary = virXPathString("string(./binary/@path)",
ctxt);
@@ -10379,24 +10385,6 @@ virDomainFSDefParseXML(virDomainXMLOption *xmlopt,
}
}
- if (format) {
- if ((def->format = virStorageFileFormatTypeFromString(format)) <= 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unknown driver format value '%s'"),
format);
- goto error;
- }
- }
-
- if (wrpolicy) {
- if ((def->wrpolicy = virDomainFSWrpolicyTypeFromString(wrpolicy)) <= 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unknown filesystem write policy '%s'"),
wrpolicy);
- goto error;
- }
- } else {
- def->wrpolicy = VIR_DOMAIN_FS_WRPOLICY_DEFAULT;
- }
-
if (source == NULL && def->type != VIR_DOMAIN_FS_TYPE_RAM
&& def->type != VIR_DOMAIN_FS_TYPE_VOLUME) {
virReportError(VIR_ERR_NO_SOURCE,
--
2.30.2
9:26 a.m.
New subject: [PATCH 12/21] Refactoring virDomainNetDefParseXML() to use XPath
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/conf/domain_conf.c | 338 +++++++++++++++++++----------------------
1 file changed, 156 insertions(+), 182 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 22700c3064..5ea2061766 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10712,7 +10712,13 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt,
{
virDomainNetDef *def;
virDomainHostdevDef *hostdev;
- xmlNodePtr cur;
+ xmlNodePtr source_node = NULL;
+ xmlNodePtr virtualport_node = NULL;
+ xmlNodePtr driver_node = NULL;
+ xmlNodePtr filterref_node = NULL;
+ xmlNodePtr actual_node = NULL;
+ xmlNodePtr vlan_node = NULL;
+ xmlNodePtr bandwidth_node = NULL;
xmlNodePtr tmpNode;
GHashTable *filterparams = NULL;
virDomainActualNetDef *actual = NULL;
@@ -10758,6 +10764,8 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt,
g_autofree char *vhostuser_type = NULL;
g_autofree char *trustGuestRxFilters = NULL;
g_autofree char *vhost_path = NULL;
+ g_autofree char *tap = NULL;
+ g_autofree char *vhost = NULL;
const char *prefix = xmlopt ? xmlopt->config.netPrefix : NULL;
if (!(def = virDomainNetDefNew(xmlopt)))
@@ -10786,194 +10794,157 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt,
goto error;
}
- cur = node->children;
- while (cur != NULL) {
- if (cur->type == XML_ELEMENT_NODE) {
- if (virXMLNodeNameEqual(cur, "source")) {
- xmlNodePtr tmpnode = ctxt->node;
+ if ((source_node = virXPathNode("./source", ctxt))) {
+ xmlNodePtr tmpnode = ctxt->node;
+ ctxt->node = source_node;
+ if (virDomainNetIPInfoParseXML(_("interface host IP"), ctxt,
&def->hostIP) < 0)
+ goto error;
+ ctxt->node = tmpnode;
- ctxt->node = cur;
- if (virDomainNetIPInfoParseXML(_("interface host IP"),
- ctxt, &def->hostIP) < 0)
- goto error;
- ctxt->node = tmpnode;
- }
- if (!macaddr && virXMLNodeNameEqual(cur, "mac")) {
- macaddr = virXMLPropString(cur, "address");
- macaddr_type = virXMLPropString(cur, "type");
- macaddr_check = virXMLPropString(cur, "check");
- } else if (!network &&
- def->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
- virXMLNodeNameEqual(cur, "source")) {
- network = virXMLPropString(cur, "network");
- portgroup = virXMLPropString(cur, "portgroup");
- if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE))
- portid = virXMLPropString(cur, "portid");
- } else if (!internal &&
- def->type == VIR_DOMAIN_NET_TYPE_INTERNAL &&
- virXMLNodeNameEqual(cur, "source")) {
- internal = virXMLPropString(cur, "name");
- } else if (!bridge &&
- def->type == VIR_DOMAIN_NET_TYPE_BRIDGE &&
- virXMLNodeNameEqual(cur, "source")) {
- bridge = virXMLPropString(cur, "bridge");
- } else if (!dev && def->type == VIR_DOMAIN_NET_TYPE_DIRECT
&&
- virXMLNodeNameEqual(cur, "source")) {
- dev = virXMLPropString(cur, "dev");
- mode = virXMLPropString(cur, "mode");
- } else if (!dev && def->type == VIR_DOMAIN_NET_TYPE_ETHERNET
&&
- virXMLNodeNameEqual(cur, "source")) {
- /* This clause is only necessary because from 2010 to
- * 2016 it was possible (but never documented) to
- * configure the name of the guest-side interface of
- * an openvz domain with <source dev='blah'/>. That
- * was blatant misuse of <source>, so was likely
- * (hopefully) never used, but just in case there was
- * somebody using it, we need to generate an error. If
- * the openvz driver is ever deprecated, this clause
- * can be removed from here.
- */
- if ((dev = virXMLPropString(cur, "dev"))) {
- virReportError(VIR_ERR_XML_ERROR,
- _("Invalid attempt to set <interface
type='ethernet'> "
- "device name with <source
dev='%s'/>. "
- "Use <target dev='%s'/> (for
host-side) "
- "or <guest dev='%s'/> (for
guest-side) instead."),
- dev, dev, dev);
- goto error;
- }
- } else if (!vhostuser_path && !vhostuser_mode &&
!vhostuser_type
- && def->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER
- && virXMLNodeNameEqual(cur, "source")) {
- vhostuser_type = virXMLPropString(cur, "type");
- vhostuser_path = virXMLPropString(cur, "path");
- vhostuser_mode = virXMLPropString(cur, "mode");
- if (virDomainChrSourceReconnectDefParseXML(&reconnect, cur, ctxt)
< 0)
- goto error;
+ if (def->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
+ network = virXPathString("string(./source/@network)", ctxt);
+ portgroup = virXPathString("string(./source/@portgroup)", ctxt);
+ if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE))
+ portid = virXPathString("string(./source/@portid)", ctxt);
+ }
- } else if (!dev
- && def->type == VIR_DOMAIN_NET_TYPE_VDPA
- && virXMLNodeNameEqual(cur, "source")) {
- dev = virXMLPropString(cur, "dev");
- } else if (!def->virtPortProfile
- && virXMLNodeNameEqual(cur, "virtualport")) {
- if (def->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
- if (!(def->virtPortProfile
- = virNetDevVPortProfileParse(cur,
-
VIR_VPORT_XML_GENERATE_MISSING_DEFAULTS))) {
- goto error;
- }
- } else if (def->type == VIR_DOMAIN_NET_TYPE_BRIDGE ||
- def->type == VIR_DOMAIN_NET_TYPE_DIRECT ||
- def->type == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
- if (!(def->virtPortProfile
- = virNetDevVPortProfileParse(cur,
-
VIR_VPORT_XML_GENERATE_MISSING_DEFAULTS|
-
VIR_VPORT_XML_REQUIRE_ALL_ATTRIBUTES|
- VIR_VPORT_XML_REQUIRE_TYPE))) {
- goto error;
- }
- } else {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("<virtualport> element unsupported
for"
- " <interface type='%s'>"),
type);
- goto error;
- }
- } else if (!address &&
- (def->type == VIR_DOMAIN_NET_TYPE_SERVER ||
- def->type == VIR_DOMAIN_NET_TYPE_CLIENT ||
- def->type == VIR_DOMAIN_NET_TYPE_MCAST ||
- def->type == VIR_DOMAIN_NET_TYPE_UDP) &&
- virXMLNodeNameEqual(cur, "source")) {
- address = virXMLPropString(cur, "address");
- port = virXMLPropString(cur, "port");
- if (!localaddr && def->type == VIR_DOMAIN_NET_TYPE_UDP) {
- xmlNodePtr tmpnode = ctxt->node;
- ctxt->node = cur;
- if ((tmpNode = virXPathNode("./local", ctxt))) {
- localaddr = virXMLPropString(tmpNode, "address");
- localport = virXMLPropString(tmpNode, "port");
- }
- ctxt->node = tmpnode;
- }
- } else if (!ifname &&
- virXMLNodeNameEqual(cur, "target")) {
- ifname = virXMLPropString(cur, "dev");
- managed_tap = virXMLPropString(cur, "managed");
- } else if ((!ifname_guest || !ifname_guest_actual) &&
- virXMLNodeNameEqual(cur, "guest")) {
- ifname_guest = virXMLPropString(cur, "dev");
- ifname_guest_actual = virXMLPropString(cur, "actual");
- } else if (!linkstate &&
- virXMLNodeNameEqual(cur, "link")) {
- linkstate = virXMLPropString(cur, "state");
- } else if (!script &&
- virXMLNodeNameEqual(cur, "script")) {
- script = virXMLPropString(cur, "path");
- } else if (!downscript &&
- virXMLNodeNameEqual(cur, "downscript")) {
- downscript = virXMLPropString(cur, "path");
- } else if (!domain_name &&
- virXMLNodeNameEqual(cur, "backenddomain")) {
- domain_name = virXMLPropString(cur, "name");
- } else if (virXMLNodeNameEqual(cur, "model")) {
- model = virXMLPropString(cur, "type");
- } else if (virXMLNodeNameEqual(cur, "driver")) {
- backend = virXMLPropString(cur, "name");
- txmode = virXMLPropString(cur, "txmode");
- ioeventfd = virXMLPropString(cur, "ioeventfd");
- event_idx = virXMLPropString(cur, "event_idx");
- queues = virXMLPropString(cur, "queues");
- rx_queue_size = virXMLPropString(cur, "rx_queue_size");
- tx_queue_size = virXMLPropString(cur, "tx_queue_size");
+ if (def->type == VIR_DOMAIN_NET_TYPE_INTERNAL)
+ internal = virXPathString("string(./source/@name)", ctxt);
- if (virDomainVirtioOptionsParseXML(cur, &def->virtio) < 0)
- goto error;
- } else if (virXMLNodeNameEqual(cur, "filterref")) {
- if (filter) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("Invalid specification of multiple
<filterref>s "
- "in a single <interface>"));
- goto error;
- }
- filter = virXMLPropString(cur, "filter");
- virHashFree(filterparams);
- filterparams = virNWFilterParseParamAttributes(cur);
- } else if (virXMLNodeNameEqual(cur, "boot")) {
- /* boot is parsed as part of virDomainDeviceInfoParseXML */
- } else if (!actual &&
- (flags & VIR_DOMAIN_DEF_PARSE_ACTUAL_NET) &&
- def->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
- virXMLNodeNameEqual(cur, "actual")) {
- if (virDomainActualNetDefParseXML(cur, ctxt, def,
- &actual, flags, xmlopt) < 0) {
- goto error;
- }
- } else if (virXMLNodeNameEqual(cur, "bandwidth")) {
- if (virNetDevBandwidthParse(&def->bandwidth,
- NULL,
- cur,
- def->type == VIR_DOMAIN_NET_TYPE_NETWORK)
< 0)
- goto error;
- } else if (virXMLNodeNameEqual(cur, "vlan")) {
- if (virNetDevVlanParse(cur, ctxt, &def->vlan) < 0)
- goto error;
- } else if (virXMLNodeNameEqual(cur, "backend")) {
- char *tmp = NULL;
+ if (def->type == VIR_DOMAIN_NET_TYPE_BRIDGE)
+ bridge = virXPathString("string(./source/@bridge)", ctxt);
- if ((tmp = virXMLPropString(cur, "tap")))
- def->backend.tap = virFileSanitizePath(tmp);
- VIR_FREE(tmp);
+ if (def->type == VIR_DOMAIN_NET_TYPE_DIRECT) {
+ dev = virXPathString("string(./source/@dev)", ctxt);
+ mode = virXPathString("string(./source/@mode)", ctxt);
+ }
- if (!vhost_path && (tmp = virXMLPropString(cur,
"vhost")))
- vhost_path = virFileSanitizePath(tmp);
- VIR_FREE(tmp);
+ /* This clause is only necessary because from 2010 to 2016 it was
+ * possible (but never documented) to configure the name of the
+ * guest-side interface of an openvz domain with <source
dev='blah'/>.
+ * That was blatant misuse of <source>, so was likely (hopefully)
+ * never used, but just in case there was somebody using it, we
+ * need to generate an error. If the openvz driver is ever
+ * deprecated, this clause can be removed from here.
+ */
+ if (def->type == VIR_DOMAIN_NET_TYPE_ETHERNET &&
+ (dev = virXPathString("string(./source/@dev)", ctxt))) {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("Invalid attempt to set <interface
type='ethernet'> "
+ "device name with <source dev='%s'/>.
"
+ "Use <target dev='%s'/> (for host-side)
"
+ "or <guest dev='%s'/> (for guest-side)
instead."),
+ dev, dev, dev);
+ goto error;
+ }
+
+ if (def->type == VIR_DOMAIN_NET_TYPE_VHOSTUSER) {
+ vhostuser_type = virXPathString("string(./source/@type)", ctxt);
+ vhostuser_path = virXPathString("string(./source/@path)", ctxt);
+ vhostuser_mode = virXPathString("string(./source/@mode)", ctxt);
+ if (virDomainChrSourceReconnectDefParseXML(&reconnect, source_node, ctxt)
< 0)
+ goto error;
+ }
+
+ if (def->type == VIR_DOMAIN_NET_TYPE_VDPA)
+ dev = virXPathString("string(./source/@dev)", ctxt);
+
+ if (def->type == VIR_DOMAIN_NET_TYPE_SERVER ||
+ def->type == VIR_DOMAIN_NET_TYPE_CLIENT ||
+ def->type == VIR_DOMAIN_NET_TYPE_MCAST ||
+ def->type == VIR_DOMAIN_NET_TYPE_UDP) {
+
+ address = virXPathString("string(./source/@address)", ctxt);
+ port = virXPathString("string(./source/@port)", ctxt);
+ if (def->type == VIR_DOMAIN_NET_TYPE_UDP) {
+ xmlNodePtr tmp_node = ctxt->node;
+ ctxt->node = source_node;
+ if ((tmpNode = virXPathNode("./local", ctxt))) {
+ localaddr = virXMLPropString(tmpNode, "address");
+ localport = virXMLPropString(tmpNode, "port");
+ }
+ ctxt->node = tmp_node;
}
}
- cur = cur->next;
}
- if (macaddr) {
+ if ((virtualport_node = virXPathNode("./virtualport", ctxt))) {
+ if (def->type == VIR_DOMAIN_NET_TYPE_NETWORK) {
+ if (!(def->virtPortProfile
+ = virNetDevVPortProfileParse(virtualport_node,
+ VIR_VPORT_XML_GENERATE_MISSING_DEFAULTS)))
{
+ goto error;
+ }
+ } else if (def->type == VIR_DOMAIN_NET_TYPE_BRIDGE ||
+ def->type == VIR_DOMAIN_NET_TYPE_DIRECT ||
+ def->type == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
+ if (!(def->virtPortProfile
+ = virNetDevVPortProfileParse(virtualport_node,
VIR_VPORT_XML_GENERATE_MISSING_DEFAULTS|
+ VIR_VPORT_XML_REQUIRE_ALL_ATTRIBUTES|
+ VIR_VPORT_XML_REQUIRE_TYPE))) {
+ goto error;
+ }
+ } else {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("<virtualport> element unsupported for"
+ " <interface type='%s'>"), type);
+ goto error;
+ }
+ }
+
+ ifname = virXPathString("string(./target/@dev)", ctxt);
+ managed_tap = virXPathString("string(./target/@managed)", ctxt);
+
+ ifname_guest = virXPathString("string(./guest/@dev)", ctxt);
+ ifname_guest_actual = virXPathString("string(./guest/@actual)", ctxt);
+
+ linkstate = virXPathString("string(./link/@state)", ctxt);
+ script = virXPathString("string(./script/@path)", ctxt);
+ downscript = virXPathString("string(./downscript/@path)", ctxt);
+ domain_name = virXPathString("string(./backenddomain/@name)", ctxt);
+ model = virXPathString("string(./model/@type)", ctxt);
+
+ if ((driver_node = virXPathNode("./driver", ctxt)) &&
+ (virDomainVirtioOptionsParseXML(driver_node, &def->virtio) < 0))
+ goto error;
+
+ backend = virXPathString("string(./driver/@name)", ctxt);
+ txmode = virXPathString("string(./driver/@txmode)", ctxt);
+ ioeventfd = virXPathString("string(./driver/@ioeventfd)", ctxt);
+ event_idx = virXPathString("string(./driver/@event_idx)", ctxt);
+ queues = virXPathString("string(./driver/@queues)", ctxt);
+ rx_queue_size = virXPathString("string(./driver/@rx_queue_size)", ctxt);
+ tx_queue_size = virXPathString("string(./driver/@tx_queue_size)", ctxt);
+
+ if ((filterref_node = virXPathNode("./filterref", ctxt))) {
+ filter = virXPathString("string(./filterref/@filter)", ctxt);
+ virHashFree(filterparams);
+ filterparams = virNWFilterParseParamAttributes(filterref_node);
+ }
+
+ if ((flags & VIR_DOMAIN_DEF_PARSE_ACTUAL_NET) &&
+ def->type == VIR_DOMAIN_NET_TYPE_NETWORK &&
+ (actual_node = virXPathNode("./actual", ctxt)) &&
+ (virDomainActualNetDefParseXML(actual_node, ctxt, def,
+ &actual, flags, xmlopt) < 0))
+ goto error;
+
+ if ((bandwidth_node = virXPathNode("./bandwidth", ctxt)) &&
+ (virNetDevBandwidthParse(&def->bandwidth, NULL, bandwidth_node,
+ def->type == VIR_DOMAIN_NET_TYPE_NETWORK) < 0))
+ goto error;
+
+ if ((vlan_node = virXPathNode("./vlan", ctxt)) &&
+ (virNetDevVlanParse(vlan_node, ctxt, &def->vlan) < 0))
+ goto error;
+
+ if ((tap = virXPathString("string(./backend/@tap)", ctxt)))
+ def->backend.tap = virFileSanitizePath(tap);
+
+ if ((vhost = virXPathString("string(./backend/@vhost)", ctxt)))
+ vhost_path = virFileSanitizePath(vhost);
+
+ if ((macaddr = virXPathString("string(./mac/@address)", ctxt))) {
if (virMacAddrParse((const char *)macaddr, &def->mac) < 0) {
virReportError(VIR_ERR_XML_ERROR,
_("unable to parse mac address '%s'"),
@@ -10991,8 +10962,9 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt,
def->mac_generated = true;
}
- if (macaddr_type) {
+ if ((macaddr_type = virXPathString("string(./mac/@type)", ctxt))) {
int tmp;
+
if ((tmp = virDomainNetMacTypeTypeFromString(macaddr_type)) <= 0) {
virReportError(VIR_ERR_XML_ERROR,
_("invalid mac address type value: '%s'. Valid
"
@@ -11002,8 +10974,10 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt,
}
def->mac_type = tmp;
}
- if (macaddr_check) {
+
+ if ((macaddr_check = virXPathString("string(./mac/@check)", ctxt))) {
int tmpCheck;
+
if ((tmpCheck = virTristateBoolTypeFromString(macaddr_check)) < 0) {
virReportError(VIR_ERR_XML_ERROR,
_("invalid mac address check value: '%s'"),
--
2.30.2
9:26 a.m.
New subject: [PATCH 13/21] conf: Propagate xmlXPathContextPtr into virDomainChrDefParseTargetXML()
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/conf/domain_conf.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 5ea2061766..163c47b7e0 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11614,6 +11614,7 @@ virDomainChrTargetModelFromString(int devtype,
static int
virDomainChrDefParseTargetXML(virDomainChrDef *def,
xmlNodePtr cur,
+ xmlXPathContextPtr ctxt G_GNUC_UNUSED,
unsigned int flags)
{
xmlNodePtr child;
@@ -12181,7 +12182,7 @@ virDomainChrDefParseXML(virDomainXMLOption *xmlopt,
if (cur->type == XML_ELEMENT_NODE) {
if (virXMLNodeNameEqual(cur, "target")) {
seenTarget = true;
- if (virDomainChrDefParseTargetXML(def, cur, flags) < 0)
+ if (virDomainChrDefParseTargetXML(def, cur, ctxt, flags) < 0)
goto error;
}
}
--
2.30.2
9:26 a.m.
New subject: [PATCH 14/21] Refactoring virDomainChrDefParseTargetXML() to use XPath
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/conf/domain_conf.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 163c47b7e0..ca9d54a47a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11614,16 +11614,18 @@ virDomainChrTargetModelFromString(int devtype,
static int
virDomainChrDefParseTargetXML(virDomainChrDef *def,
xmlNodePtr cur,
- xmlXPathContextPtr ctxt G_GNUC_UNUSED,
+ xmlXPathContextPtr ctxt,
unsigned int flags)
{
- xmlNodePtr child;
unsigned int port;
g_autofree char *targetType = virXMLPropString(cur, "type");
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;
if ((def->targetType =
virDomainChrTargetTypeFromString(def->deviceType,
@@ -11634,14 +11636,7 @@ virDomainChrDefParseTargetXML(virDomainChrDef *def,
return -1;
}
- child = cur->children;
- while (child != NULL) {
- if (child->type == XML_ELEMENT_NODE &&
- virXMLNodeNameEqual(child, "model")) {
- targetModel = virXMLPropString(child, "name");
- }
- child = child->next;
- }
+ targetModel = virXPathString("string(./model/@name)", ctxt);
if ((def->targetModel =
virDomainChrTargetModelFromString(def->deviceType,
--
2.30.2
9:26 a.m.
New subject: [PATCH 15/21] Refactoring virDomainChrSourceDefParseXML() to use XPath
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/conf/domain_conf.c | 190 ++++++++++++++++++++---------------------
1 file changed, 94 insertions(+), 96 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ca9d54a47a..0748c93567 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11929,112 +11929,112 @@ virDomainChrSourceDefParseXML(virDomainChrSourceDef *def,
virDomainChrDef *chr_def,
xmlXPathContextPtr ctxt)
{
- bool logParsed = false;
- bool protocolParsed = false;
- int sourceParsed = 0;
+ xmlNodePtr log = NULL;
+ xmlNodePtr protocol = NULL;
+ g_autofree xmlNodePtr *source = NULL;
+ int n = 0;
+ VIR_XPATH_NODE_AUTORESTORE(ctxt)
- for (; cur; cur = cur->next) {
- if (cur->type != XML_ELEMENT_NODE)
- continue;
+ ctxt->node = cur;
- if (virXMLNodeNameEqual(cur, "source")) {
- /* Parse only the first source element since only one is used
- * for chardev devices, the only exception is UDP type, where
- * user can specify two source elements. */
- if (sourceParsed >= 1 && def->type != VIR_DOMAIN_CHR_TYPE_UDP)
{
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("only one source element is allowed for "
- "character device"));
- goto error;
- } else if (sourceParsed >= 2) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("only two source elements are allowed for "
- "character device"));
- goto error;
- }
- sourceParsed++;
+ if ((n = virXPathNodeSet("./source", ctxt, &source)) > 0) {
+ /* Parse only the first source element since only one is used
+ * for chardev devices, the only exception is UDP type, where
+ * user can specify two source elements. */
+ if (n > 1 && def->type != VIR_DOMAIN_CHR_TYPE_UDP) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("only one source element is allowed for "
+ "character device"));
+ goto error;
+ } else if (n > 2) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("only two source elements are allowed for "
+ "character device"));
+ goto error;
+ }
- switch ((virDomainChrType) def->type) {
- case VIR_DOMAIN_CHR_TYPE_FILE:
- if (virDomainChrSourceDefParseFile(def, cur) < 0)
- goto error;
- break;
+ switch ((virDomainChrType) def->type) {
+ case VIR_DOMAIN_CHR_TYPE_FILE:
+ if (virDomainChrSourceDefParseFile(def, source[0]) < 0)
+ goto error;
+ break;
- case VIR_DOMAIN_CHR_TYPE_PTY:
- /* PTY path is only parsed from live xml. */
- if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE))
- def->data.file.path = virXMLPropString(cur, "path");
- break;
+ case VIR_DOMAIN_CHR_TYPE_PTY:
+ /* PTY path is only parsed from live xml. */
+ if (!(flags & VIR_DOMAIN_DEF_PARSE_INACTIVE))
+ def->data.file.path =
virXPathString("string(./source/@path)", ctxt);
+ break;
- case VIR_DOMAIN_CHR_TYPE_DEV:
- case VIR_DOMAIN_CHR_TYPE_PIPE:
- def->data.file.path = virXMLPropString(cur, "path");
- break;
+ case VIR_DOMAIN_CHR_TYPE_DEV:
+ case VIR_DOMAIN_CHR_TYPE_PIPE:
+ def->data.file.path = virXPathString("string(./source/@path)",
ctxt);
+ break;
- case VIR_DOMAIN_CHR_TYPE_UNIX:
- if (virDomainChrSourceDefParseUnix(def, cur, ctxt) < 0)
- goto error;
- break;
+ case VIR_DOMAIN_CHR_TYPE_UNIX:
+ if (virDomainChrSourceDefParseUnix(def, source[0], ctxt) < 0)
+ goto error;
+ break;
- case VIR_DOMAIN_CHR_TYPE_UDP:
- if (virDomainChrSourceDefParseUDP(def, cur) < 0)
+ case VIR_DOMAIN_CHR_TYPE_UDP:
+ if ((virDomainChrSourceDefParseUDP(def, source[0]) < 0) ||
+ (source[1] && virDomainChrSourceDefParseUDP(def, source[1]) <
0))
goto error;
- break;
+ break;
- case VIR_DOMAIN_CHR_TYPE_TCP:
- if (virDomainChrSourceDefParseTCP(def, cur, ctxt, flags) < 0)
- goto error;
- break;
+ case VIR_DOMAIN_CHR_TYPE_TCP:
+ if (virDomainChrSourceDefParseTCP(def, source[0], ctxt, flags) < 0)
+ goto error;
+ break;
- case VIR_DOMAIN_CHR_TYPE_SPICEPORT:
- def->data.spiceport.channel = virXMLPropString(cur,
"channel");
- break;
+ case VIR_DOMAIN_CHR_TYPE_SPICEPORT:
+ def->data.spiceport.channel =
virXPathString("string(./source/@channel)", ctxt);
+ break;
- case VIR_DOMAIN_CHR_TYPE_NMDM:
- def->data.nmdm.master = virXMLPropString(cur, "master");
- def->data.nmdm.slave = virXMLPropString(cur, "slave");
- break;
+ case VIR_DOMAIN_CHR_TYPE_NMDM:
+ def->data.nmdm.master =
virXPathString("string(./source/@master)", ctxt);
+ def->data.nmdm.slave = virXPathString("string(./source/@slave)",
ctxt);
+ break;
- case VIR_DOMAIN_CHR_TYPE_LAST:
- case VIR_DOMAIN_CHR_TYPE_NULL:
- case VIR_DOMAIN_CHR_TYPE_VC:
- case VIR_DOMAIN_CHR_TYPE_STDIO:
- case VIR_DOMAIN_CHR_TYPE_SPICEVMC:
- break;
- }
+ case VIR_DOMAIN_CHR_TYPE_LAST:
+ case VIR_DOMAIN_CHR_TYPE_NULL:
+ case VIR_DOMAIN_CHR_TYPE_VC:
+ case VIR_DOMAIN_CHR_TYPE_STDIO:
+ case VIR_DOMAIN_CHR_TYPE_SPICEVMC:
+ break;
+ }
- /* Check for an optional seclabel override in <source/>. */
- if (chr_def) {
- VIR_XPATH_NODE_AUTORESTORE(ctxt)
- ctxt->node = cur;
- if (virSecurityDeviceLabelDefParseXML(&def->seclabels,
- &def->nseclabels,
- ctxt,
- flags) < 0) {
- goto error;
- }
- }
- } else if (virXMLNodeNameEqual(cur, "log")) {
- if (logParsed) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("only one protocol element is allowed for "
- "character device"));
- goto error;
- }
- logParsed = true;
- if (virDomainChrSourceDefParseLog(def, cur) < 0)
- goto error;
- } else if (virXMLNodeNameEqual(cur, "protocol")) {
- if (protocolParsed) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("only one log element is allowed for "
- "character device"));
+ /* Check for an optional seclabel override in <source/>. */
+ if (chr_def) {
+ xmlNodePtr tmp = ctxt->node;
+ ctxt->node = source[0];
+ if (virSecurityDeviceLabelDefParseXML(&def->seclabels,
&def->nseclabels,
+ ctxt, flags) < 0) {
goto error;
}
- protocolParsed = true;
- if (virDomainChrSourceDefParseProtocol(def, cur) < 0)
- goto error;
+ ctxt->node = tmp;
+ }
+ }
+
+ if ((log = virXPathNode("./log", ctxt))) {
+ if (virXPathBoolean("boolean(count(./log) > 1)", ctxt)) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("only one log element is allowed for "
+ "character device"));
+ goto error;
+ }
+ if (virDomainChrSourceDefParseLog(def, log) < 0)
+ goto error;
+ }
+
+ if ((protocol = virXPathNode("./protocol", ctxt))) {
+ if (virXPathBoolean("boolean(count(./protocol) > 1)", ctxt)) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("only one protocol element is allowed for "
+ "character device"));
+ goto error;
}
+ if (virDomainChrSourceDefParseProtocol(def, protocol) < 0)
+ goto error;
}
return 0;
@@ -12188,7 +12188,7 @@ virDomainChrDefParseXML(virDomainXMLOption *xmlopt,
((def->targetType = virDomainChrDefaultTargetType(def->deviceType)) <
0))
goto error;
- if (virDomainChrSourceDefParseXML(def->source, node->children, flags, def,
+ if (virDomainChrSourceDefParseXML(def->source, node, flags, def,
ctxt) < 0)
goto error;
@@ -12312,7 +12312,7 @@ virDomainSmartcardDefParseXML(virDomainXMLOption *xmlopt,
}
cur = node->children;
- if (virDomainChrSourceDefParseXML(def->data.passthru, cur, flags,
+ if (virDomainChrSourceDefParseXML(def->data.passthru, node, flags,
NULL, ctxt) < 0)
return NULL;
@@ -14375,7 +14375,7 @@ virDomainRNGDefParseXML(virDomainXMLOption *xmlopt,
}
if (virDomainChrSourceDefParseXML(def->source.chardev,
- backends[0]->children, flags,
+ backends[0], flags,
NULL, ctxt) < 0)
goto error;
break;
@@ -15393,7 +15393,6 @@ virDomainRedirdevDefParseXML(virDomainXMLOption *xmlopt,
xmlXPathContextPtr ctxt,
unsigned int flags)
{
- xmlNodePtr cur;
virDomainRedirdevDef *def;
g_autofree char *bus = NULL;
g_autofree char *type = NULL;
@@ -15427,10 +15426,9 @@ virDomainRedirdevDefParseXML(virDomainXMLOption *xmlopt,
goto error;
}
- cur = node->children;
/* boot gets parsed in virDomainDeviceInfoParseXML
* source gets parsed in virDomainChrSourceDefParseXML */
- if (virDomainChrSourceDefParseXML(def->source, cur, flags,
+ if (virDomainChrSourceDefParseXML(def->source, node, flags,
NULL, ctxt) < 0)
goto error;
--
2.30.2
9:26 a.m.
New subject: [PATCH 16/21] Refactoring virDomainChrDefParseXML() to use XPath
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/conf/domain_conf.c | 25 +++++++++----------------
1 file changed, 9 insertions(+), 16 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0748c93567..8d53b30917 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12145,11 +12145,13 @@ virDomainChrDefParseXML(virDomainXMLOption *xmlopt,
xmlNodePtr node,
unsigned int flags)
{
- xmlNodePtr cur;
+ xmlNodePtr target;
const char *nodeName;
virDomainChrDef *def;
- bool seenTarget = false;
g_autofree char *type = NULL;
+ VIR_XPATH_NODE_AUTORESTORE(ctxt)
+
+ ctxt->node = node;
if (!(def = virDomainChrDefNew(xmlopt)))
return NULL;
@@ -12172,21 +12174,12 @@ virDomainChrDefParseXML(virDomainXMLOption *xmlopt,
goto error;
}
- cur = node->children;
- while (cur != NULL) {
- if (cur->type == XML_ELEMENT_NODE) {
- if (virXMLNodeNameEqual(cur, "target")) {
- seenTarget = true;
- if (virDomainChrDefParseTargetXML(def, cur, ctxt, flags) < 0)
- goto error;
- }
- }
- cur = cur->next;
- }
-
- if (!seenTarget &&
- ((def->targetType = virDomainChrDefaultTargetType(def->deviceType)) <
0))
+ if ((target = virXPathNode("./target", ctxt))) {
+ if (virDomainChrDefParseTargetXML(def, target, ctxt, flags) < 0)
+ goto error;
+ } else if ((def->targetType = virDomainChrDefaultTargetType(def->deviceType))
< 0) {
goto error;
+ }
if (virDomainChrSourceDefParseXML(def->source, node, flags, def,
ctxt) < 0)
--
2.30.2
9:26 a.m.
New subject: [PATCH 17/21] Refactoring virDomainSmartcardDefParseXML() to use XPath
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/conf/domain_conf.c | 59 ++++++++++++++++++------------------------
1 file changed, 25 insertions(+), 34 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 8d53b30917..ece34403a7 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12221,12 +12221,14 @@ virDomainSmartcardDefParseXML(virDomainXMLOption *xmlopt,
xmlXPathContextPtr ctxt,
unsigned int flags)
{
- xmlNodePtr cur;
g_autoptr(virDomainSmartcardDef) def = NULL;
- size_t i;
g_autofree char *mode = NULL;
g_autofree char *type = NULL;
+ g_autofree xmlNodePtr *certificates = NULL;
+ int n = 0;
+ VIR_XPATH_NODE_AUTORESTORE(ctxt)
+ ctxt->node = node;
def = g_new0(virDomainSmartcardDef, 1);
mode = virXMLPropString(node, "mode");
@@ -12247,42 +12249,32 @@ virDomainSmartcardDefParseXML(virDomainXMLOption *xmlopt,
break;
case VIR_DOMAIN_SMARTCARD_TYPE_HOST_CERTIFICATES:
- i = 0;
- cur = node->children;
- while (cur) {
- if (cur->type == XML_ELEMENT_NODE &&
- virXMLNodeNameEqual(cur, "certificate")) {
- if (i == 3) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("host-certificates mode needs "
- "exactly three certificates"));
- return NULL;
- }
- if (!(def->data.cert.file[i] = virXMLNodeContentString(cur)))
- return NULL;
-
- i++;
- } else if (cur->type == XML_ELEMENT_NODE &&
- virXMLNodeNameEqual(cur, "database") &&
- !def->data.cert.database) {
- if (!(def->data.cert.database = virXMLNodeContentString(cur)))
- return NULL;
-
- if (*def->data.cert.database != '/') {
- virReportError(VIR_ERR_XML_ERROR,
- _("expecting absolute path: %s"),
- def->data.cert.database);
- return NULL;
- }
- }
- cur = cur->next;
- }
- if (i < 3) {
+ n = virXPathNodeSet("./certificate", ctxt, &certificates);
+ if (n != VIR_DOMAIN_SMARTCARD_NUM_CERTIFICATES) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("host-certificates mode needs "
"exactly three certificates"));
return NULL;
}
+
+ if (!(def->data.cert.file[0] = virXMLNodeContentString(certificates[0])) ||
+ !(def->data.cert.file[1] = virXMLNodeContentString(certificates[1])) ||
+ !(def->data.cert.file[2] = virXMLNodeContentString(certificates[2])))
+ return NULL;
+
+ if (virXPathNode("./database", ctxt) &&
+ !def->data.cert.database) {
+ if (!(def->data.cert.database =
+ virXPathString("string(./database/text())", ctxt)))
+ return NULL;
+
+ if (*def->data.cert.database != '/') {
+ virReportError(VIR_ERR_XML_ERROR,
+ _("expecting absolute path: %s"),
+ def->data.cert.database);
+ return NULL;
+ }
+ }
break;
case VIR_DOMAIN_SMARTCARD_TYPE_PASSTHROUGH:
@@ -12304,7 +12296,6 @@ virDomainSmartcardDefParseXML(virDomainXMLOption *xmlopt,
return NULL;
}
- cur = node->children;
if (virDomainChrSourceDefParseXML(def->data.passthru, node, flags,
NULL, ctxt) < 0)
return NULL;
--
2.30.2
9:26 a.m.
New subject: [PATCH 18/21] Refactoring virDomainGraphicsDefParseXMLSpice() to use XPath
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/conf/domain_conf.c | 303 ++++++++++++++++-------------------------
1 file changed, 114 insertions(+), 189 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ece34403a7..1115a3104d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -13342,12 +13342,29 @@ virDomainGraphicsDefParseXMLSpice(virDomainGraphicsDef *def,
xmlXPathContextPtr ctxt,
unsigned int flags)
{
- xmlNodePtr cur;
int defaultModeVal;
+ int nameval;
+ g_autofree xmlNodePtr *node_list = NULL;
+ int n = 0;
+ size_t i = 0;
+ int value = 0;
g_autofree char *port = virXMLPropString(node, "port");
g_autofree char *tlsPort = virXMLPropString(node, "tlsPort");
g_autofree char *autoport = virXMLPropString(node, "autoport");
g_autofree char *defaultMode = virXMLPropString(node, "defaultMode");
+ g_autofree char *compression = NULL;
+ g_autofree char *jpeg_compression = NULL;
+ g_autofree char *zlib_compression = NULL;
+ g_autofree char *playback_compression = NULL;
+ g_autofree char *streaming_mode = NULL;
+ g_autofree char *copypaste = NULL;
+ g_autofree char *filetransfer_enable = NULL;
+ g_autofree char *gl_enable = NULL;
+ g_autofree char *mouse_mode = NULL;
+ g_autofree char *rendernode = NULL;
+ VIR_XPATH_NODE_AUTORESTORE(ctxt)
+
+ ctxt->node = node;
if (virDomainGraphicsListensParseXML(def, node, ctxt, flags) < 0)
return -1;
@@ -13403,207 +13420,115 @@ virDomainGraphicsDefParseXMLSpice(virDomainGraphicsDef *def,
def->type) < 0)
return -1;
- cur = node->children;
- while (cur != NULL) {
- if (cur->type == XML_ELEMENT_NODE) {
- if (virXMLNodeNameEqual(cur, "channel")) {
- int nameval, modeval;
- g_autofree char *name = NULL;
- g_autofree char *mode = NULL;
-
- name = virXMLPropString(cur, "name");
- mode = virXMLPropString(cur, "mode");
-
- if (!name || !mode) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("spice channel missing name/mode"));
- return -1;
- }
-
- if ((nameval = virDomainGraphicsSpiceChannelNameTypeFromString(name))
< 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unknown spice channel name %s"),
- name);
- return -1;
- }
- if ((modeval = virDomainGraphicsSpiceChannelModeTypeFromString(mode))
< 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unknown spice channel mode %s"),
- mode);
- return -1;
- }
-
- def->data.spice.channels[nameval] = modeval;
- } else if (virXMLNodeNameEqual(cur, "image")) {
- int compressionVal;
- g_autofree char *compression = virXMLPropString(cur,
"compression");
-
- if (!compression) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("spice image missing compression"));
- return -1;
- }
-
- if ((compressionVal =
- virDomainGraphicsSpiceImageCompressionTypeFromString(compression))
<= 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unknown spice image compression %s"),
- compression);
- return -1;
- }
-
- def->data.spice.image = compressionVal;
- } else if (virXMLNodeNameEqual(cur, "jpeg")) {
- int compressionVal;
- g_autofree char *compression = virXMLPropString(cur,
"compression");
-
- if (!compression) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("spice jpeg missing compression"));
- return -1;
- }
-
- if ((compressionVal =
- virDomainGraphicsSpiceJpegCompressionTypeFromString(compression))
<= 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unknown spice jpeg compression %s"),
- compression);
- return -1;
- }
-
- def->data.spice.jpeg = compressionVal;
- } else if (virXMLNodeNameEqual(cur, "zlib")) {
- int compressionVal;
- g_autofree char *compression = virXMLPropString(cur,
"compression");
-
- if (!compression) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("spice zlib missing compression"));
- return -1;
- }
-
- if ((compressionVal =
- virDomainGraphicsSpiceZlibCompressionTypeFromString(compression))
<= 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unknown spice zlib compression %s"),
- compression);
- return -1;
- }
-
- def->data.spice.zlib = compressionVal;
- } else if (virXMLNodeNameEqual(cur, "playback")) {
- int compressionVal;
- g_autofree char *compression = virXMLPropString(cur,
"compression");
-
- if (!compression) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("spice playback missing compression"));
- return -1;
- }
-
- if ((compressionVal =
- virTristateSwitchTypeFromString(compression)) <= 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("unknown spice playback compression"));
- return -1;
- }
-
- def->data.spice.playback = compressionVal;
- } else if (virXMLNodeNameEqual(cur, "streaming")) {
- int modeVal;
- g_autofree char *mode = virXMLPropString(cur, "mode");
-
- if (!mode) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("spice streaming missing mode"));
- return -1;
- }
- if ((modeVal =
- virDomainGraphicsSpiceStreamingModeTypeFromString(mode)) <= 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("unknown spice streaming mode"));
- return -1;
- }
-
- def->data.spice.streaming = modeVal;
- } else if (virXMLNodeNameEqual(cur, "clipboard")) {
- int copypasteVal;
- g_autofree char *copypaste = virXMLPropString(cur,
"copypaste");
-
- if (!copypaste) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("spice clipboard missing copypaste"));
- return -1;
- }
+ if ((n = virXPathNodeSet("./channel", ctxt, &node_list)) < 0)
+ return -1;
- if ((copypasteVal =
- virTristateBoolTypeFromString(copypaste)) <= 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unknown copypaste value '%s'"),
copypaste);
- return -1;
- }
+ for (i = 0; i < n; i++) {
+ g_autofree char *name = virXMLPropString(node_list[i], "name");
+ g_autofree char *mode = virXMLPropString(node_list[i], "mode");
- def->data.spice.copypaste = copypasteVal;
- } else if (virXMLNodeNameEqual(cur, "filetransfer")) {
- int enableVal;
- g_autofree char *enable = virXMLPropString(cur, "enable");
+ if ((nameval = virDomainGraphicsSpiceChannelNameTypeFromString(name)) < 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown spice channel name %s"), NULLSTR(name));
+ return -1;
+ }
+ if ((value = virDomainGraphicsSpiceChannelModeTypeFromString(mode)) < 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown spice channel mode %s"), NULLSTR(mode));
+ return -1;
+ }
+ def->data.spice.channels[nameval] = value;
+ }
- if (!enable) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("spice filetransfer missing enable"));
- return -1;
- }
+ if ((compression = virXPathString("string(./image/@compression)", ctxt)))
{
+ if ((value =
+ virDomainGraphicsSpiceImageCompressionTypeFromString(compression)) <= 0)
{
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown spice image compression %s"),
compression);
+ return -1;
+ }
+ def->data.spice.image = value;
+ }
- if ((enableVal =
- virTristateBoolTypeFromString(enable)) <= 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unknown enable value '%s'"),
enable);
- return -1;
- }
+ if ((jpeg_compression = virXPathString("string(./jpeg/@compression)",
ctxt))) {
+ if ((value =
+ virDomainGraphicsSpiceJpegCompressionTypeFromString(jpeg_compression)) <=
0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown spice jpeg compression %s"),
+ jpeg_compression);
+ return -1;
+ }
+ def->data.spice.jpeg = value;
+ }
- def->data.spice.filetransfer = enableVal;
- } else if (virXMLNodeNameEqual(cur, "gl")) {
- int enableVal;
- g_autofree char *enable = virXMLPropString(cur, "enable");
- g_autofree char *rendernode = virXMLPropString(cur,
"rendernode");
+ if ((zlib_compression = virXPathString("string(./zlib/@compression)",
ctxt))) {
+ if ((value =
+ virDomainGraphicsSpiceZlibCompressionTypeFromString(zlib_compression)) <=
0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown spice zlib compression %s"),
+ zlib_compression);
+ return -1;
+ }
+ def->data.spice.zlib = value;
+ }
- if (!enable) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("spice gl element missing enable"));
- return -1;
- }
+ if ((playback_compression =
virXPathString("string(./playback/@compression)", ctxt))) {
+ if ((value = virTristateSwitchTypeFromString(playback_compression)) <= 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("unknown spice playback compression"));
+ return -1;
+ }
+ def->data.spice.playback = value;
+ }
- if ((enableVal =
- virTristateBoolTypeFromString(enable)) <= 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unknown enable value '%s'"),
enable);
- return -1;
- }
+ if ((streaming_mode = virXPathString("string(./streaming/@mode)", ctxt)))
{
+ if ((value =
+ virDomainGraphicsSpiceStreamingModeTypeFromString(streaming_mode)) <= 0)
{
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("unknown spice streaming mode"));
+ return -1;
+ }
+ def->data.spice.streaming = value;
+ }
- def->data.spice.gl = enableVal;
- def->data.spice.rendernode = g_steal_pointer(&rendernode);
+ if ((copypaste = virXPathString("string(./clipboard/@copypaste)", ctxt)))
{
+ if ((value = virTristateBoolTypeFromString(copypaste)) <= 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown copypaste value '%s'"),
copypaste);
+ return -1;
+ }
+ def->data.spice.copypaste = value;
+ }
- } else if (virXMLNodeNameEqual(cur, "mouse")) {
- int modeVal;
- g_autofree char *mode = virXMLPropString(cur, "mode");
+ if ((filetransfer_enable = virXPathString("string(./filetransfer/@enable)",
ctxt))) {
+ if ((value = virTristateBoolTypeFromString(filetransfer_enable)) <= 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown enable value '%s'"),
+ filetransfer_enable);
+ return -1;
+ }
+ def->data.spice.filetransfer = value;
+ }
- if (!mode) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("spice mouse missing mode"));
- return -1;
- }
+ if ((gl_enable = virXPathString("string(./gl/@enable)", ctxt))) {
+ rendernode = virXPathString("string(./gl/@rendernode)", ctxt);
- if ((modeVal = virDomainGraphicsSpiceMouseModeTypeFromString(mode)) <=
0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unknown mouse mode value '%s'"),
- mode);
- return -1;
- }
+ if ((value = virTristateBoolTypeFromString(gl_enable)) <= 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown enable value '%s'"), gl_enable);
+ return -1;
+ }
+ def->data.spice.gl = value;
+ def->data.spice.rendernode = g_steal_pointer(&rendernode);
+ }
- def->data.spice.mousemode = modeVal;
- }
+ if ((mouse_mode = virXPathString("string(./mouse/@mode)", ctxt))) {
+ if ((value = virDomainGraphicsSpiceMouseModeTypeFromString(mouse_mode)) <= 0)
{
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown mouse mode value '%s'"),
mouse_mode);
+ return -1;
}
- cur = cur->next;
+ def->data.spice.mousemode = value;
}
return 0;
--
2.30.2
9:26 a.m.
New subject: [PATCH 19/21] conf: Propagate xmlXPathContextPtr into virDomainVideoDriverDefParseXML()
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/conf/domain_conf.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 1115a3104d..b9816c46ed 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -15048,7 +15048,8 @@ virDomainVideoResolutionDefParseXML(xmlNodePtr node)
}
static virDomainVideoDriverDef *
-virDomainVideoDriverDefParseXML(xmlNodePtr node)
+virDomainVideoDriverDefParseXML(xmlNodePtr node,
+ xmlXPathContextPtr ctxt G_GNUC_UNUSED)
{
xmlNodePtr cur;
virDomainVideoDriverDef *def;
@@ -15213,7 +15214,7 @@ virDomainVideoDefParseXML(virDomainXMLOption *xmlopt,
if (virDomainDeviceInfoParseXML(xmlopt, node, ctxt, &def->info, flags) <
0)
return NULL;
- def->driver = virDomainVideoDriverDefParseXML(node);
+ def->driver = virDomainVideoDriverDefParseXML(node, ctxt);
return g_steal_pointer(&def);
}
--
2.30.2
9:26 a.m.
New subject: [PATCH 20/21] Refactoring virDomainVideoDriverDefParseXML() to use XPath
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/conf/domain_conf.c | 17 ++++-------------
1 file changed, 4 insertions(+), 13 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b9816c46ed..a5dce8a8af 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -15049,25 +15049,16 @@ virDomainVideoResolutionDefParseXML(xmlNodePtr node)
static virDomainVideoDriverDef *
virDomainVideoDriverDefParseXML(xmlNodePtr node,
- xmlXPathContextPtr ctxt G_GNUC_UNUSED)
+ xmlXPathContextPtr ctxt)
{
- xmlNodePtr cur;
virDomainVideoDriverDef *def;
int val;
g_autofree char *vgaconf = NULL;
+ VIR_XPATH_NODE_AUTORESTORE(ctxt)
- cur = node->children;
- while (cur != NULL) {
- if (cur->type == XML_ELEMENT_NODE) {
- if (!vgaconf &&
- virXMLNodeNameEqual(cur, "driver")) {
- vgaconf = virXMLPropString(cur, "vgaconf");
- }
- }
- cur = cur->next;
- }
+ ctxt->node = node;
- if (!vgaconf)
+ if (!(vgaconf = virXPathString("string(./driver/@vgaconf)", ctxt)))
return NULL;
def = g_new0(virDomainVideoDriverDef, 1);
--
2.30.2
9:26 a.m.
New subject: [PATCH 21/21] Refactoring virDomainVideoDefParseXML() to use XPath
Signed-off-by: Kristina Hanicova <khanicov(a)redhat.com>
---
src/conf/domain_conf.c | 88 ++++++++++++++++--------------------------
1 file changed, 33 insertions(+), 55 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a5dce8a8af..54348c8c23 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -15080,7 +15080,9 @@ virDomainVideoDefParseXML(virDomainXMLOption *xmlopt,
unsigned int flags)
{
g_autoptr(virDomainVideoDef) def = NULL;
- xmlNodePtr cur;
+ xmlNodePtr driver;
+ xmlNodePtr accel_node;
+ xmlNodePtr res_node;
VIR_XPATH_NODE_AUTORESTORE(ctxt)
g_autofree char *type = NULL;
g_autofree char *driver_name = NULL;
@@ -15096,62 +15098,28 @@ virDomainVideoDefParseXML(virDomainXMLOption *xmlopt,
ctxt->node = node;
- cur = node->children;
- while (cur != NULL) {
- if (cur->type == XML_ELEMENT_NODE) {
- if (!type && !vram && !ram && !heads &&
- virXMLNodeNameEqual(cur, "model")) {
- xmlNodePtr child;
- type = virXMLPropString(cur, "type");
- ram = virXMLPropString(cur, "ram");
- vram = virXMLPropString(cur, "vram");
- vram64 = virXMLPropString(cur, "vram64");
- vgamem = virXMLPropString(cur, "vgamem");
- heads = virXMLPropString(cur, "heads");
-
- if ((primary = virXMLPropString(cur, "primary")) != NULL) {
- ignore_value(virStringParseYesNo(primary, &def->primary));
- VIR_FREE(primary);
- }
+ if ((primary = virXPathString("string(./model/@primary)", ctxt)) != NULL)
+ ignore_value(virStringParseYesNo(primary, &def->primary));
- child = cur->children;
- while (child != NULL) {
- if (child->type == XML_ELEMENT_NODE) {
- if (def->accel == NULL &&
- virXMLNodeNameEqual(child, "acceleration")) {
- if ((def->accel = virDomainVideoAccelDefParseXML(child))
== NULL)
- return NULL;
- }
- if (def->res == NULL &&
- virXMLNodeNameEqual(child, "resolution")) {
- if ((def->res =
virDomainVideoResolutionDefParseXML(child)) == NULL)
- return NULL;
- }
- }
- child = child->next;
- }
- }
- if (virXMLNodeNameEqual(cur, "driver")) {
- if (virDomainVirtioOptionsParseXML(cur, &def->virtio) < 0)
- return NULL;
- driver_name = virXMLPropString(cur, "name");
- }
- }
- cur = cur->next;
- }
+ if (def->accel == NULL &&
+ (accel_node = virXPathNode("./model/acceleration", ctxt)))
+ if ((def->accel = virDomainVideoAccelDefParseXML(accel_node)) == NULL)
+ return NULL;
- if (type) {
- if ((def->type = virDomainVideoTypeFromString(type)) < 0) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
- _("unknown video model '%s'"), type);
+ if (def->res == NULL &&
+ (res_node = virXPathNode("./model/resolution", ctxt)))
+ if ((def->res = virDomainVideoResolutionDefParseXML(res_node)) == NULL)
return NULL;
- }
- } else {
- def->type = VIR_DOMAIN_VIDEO_TYPE_DEFAULT;
+
+ if ((driver = virXPathNode("./driver", ctxt))) {
+ if (virDomainVirtioOptionsParseXML(driver, &def->virtio) < 0)
+ return NULL;
+ driver_name = virXPathString("string(./driver/@name)", ctxt);
}
if (driver_name) {
int backend;
+
if ((backend = virDomainVideoBackendTypeFromString(driver_name)) <= 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("unknown video driver '%s'"),
driver_name);
@@ -15162,7 +15130,17 @@ virDomainVideoDefParseXML(virDomainXMLOption *xmlopt,
def->backend = VIR_DOMAIN_VIDEO_BACKEND_TYPE_DEFAULT;
}
- if (ram) {
+ if ((type = virXPathString("string(./model/@type)", ctxt))) {
+ if ((def->type = virDomainVideoTypeFromString(type)) < 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown video model '%s'"), type);
+ return NULL;
+ }
+ } else {
+ def->type = VIR_DOMAIN_VIDEO_TYPE_DEFAULT;
+ }
+
+ if ((ram = virXPathString("string(./model/@ram)", ctxt))) {
if (virStrToLong_uip(ram, NULL, 10, &def->ram) < 0) {
virReportError(VIR_ERR_XML_ERROR,
_("cannot parse video ram '%s'"), ram);
@@ -15170,7 +15148,7 @@ virDomainVideoDefParseXML(virDomainXMLOption *xmlopt,
}
}
- if (vram) {
+ if ((vram = virXPathString("string(./model/@vram)", ctxt))) {
if (virStrToLong_uip(vram, NULL, 10, &def->vram) < 0) {
virReportError(VIR_ERR_XML_ERROR,
_("cannot parse video vram '%s'"), vram);
@@ -15178,7 +15156,7 @@ virDomainVideoDefParseXML(virDomainXMLOption *xmlopt,
}
}
- if (vram64) {
+ if ((vram64 = virXPathString("string(./model/@vram64)", ctxt))) {
if (virStrToLong_uip(vram64, NULL, 10, &def->vram64) < 0) {
virReportError(VIR_ERR_XML_ERROR,
_("cannot parse video vram64 '%s'"),
vram64);
@@ -15186,7 +15164,7 @@ virDomainVideoDefParseXML(virDomainXMLOption *xmlopt,
}
}
- if (vgamem) {
+ if ((vgamem = virXPathString("string(./model/@vgamem)", ctxt))) {
if (virStrToLong_uip(vgamem, NULL, 10, &def->vgamem) < 0) {
virReportError(VIR_ERR_XML_ERROR,
_("cannot parse video vgamem '%s'"),
vgamem);
@@ -15194,7 +15172,7 @@ virDomainVideoDefParseXML(virDomainXMLOption *xmlopt,
}
}
- if (heads) {
+ if ((heads = virXPathString("string(./model/@heads)", ctxt))) {
if (virStrToLong_uip(heads, NULL, 10, &def->heads) < 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("cannot parse video heads '%s'"), heads);
--
2.30.2
11:27 a.m.
On Thu, 2021-04-15 at 16:26 +0200, Kristina Hanicova wrote:
This series reworks the outdated way of parsing XML to parsing by
XPath,
which is more obvious and saves a few lines of code.
Kristina Hanicova (21):
conf: Propagate xmlXPathContextPtr into
virDomainHostdevSubsysUSBDefParseXML()
Refactoring virDomainHostdevSubsysUSBDefParseXML() to use XPath
conf: Propagate xmlXPathContextPtr into
virDomainBlkioDeviceParseXML()
Refactoring virDomainBlkioDeviceParseXML() to use XPath
conf: Propagate xmlXPathContextPtr into
virDomainHostdevSubsysPCIDefParseXML()
Refactoring virDomainHostdevSubsysPCIDefParseXML() to use XPath
conf: Propagate xmlXPathContextPtr into virDomainLeaseDefParseXML()
Refactoring virDomainLeaseDefParseXML() to use XPath
Refactoring virDomainDiskDefParseXML() to use XPath
Refactoring virDomainControllerDefParseXML() to use XPath
Refactoring virDomainFSDefParseXML() to use XPath
Refactoring virDomainNetDefParseXML() to use XPath
conf: Propagate xmlXPathContextPtr into
virDomainChrDefParseTargetXML()
Refactoring virDomainChrDefParseTargetXML() to use XPath
Refactoring virDomainChrSourceDefParseXML() to use XPath
Refactoring virDomainChrDefParseXML() to use XPath
Refactoring virDomainSmartcardDefParseXML() to use XPath
Refactoring virDomainGraphicsDefParseXMLSpice() to use XPath
conf: Propagate xmlXPathContextPtr into
virDomainVideoDriverDefParseXML()
Refactoring virDomainVideoDriverDefParseXML() to use XPath
Refactoring virDomainVideoDefParseXML() to use XPath
src/conf/domain_conf.c | 1786 +++++++++++++++++---------------------
--
1 file changed, 777 insertions(+), 1009 deletions(-)
Great to see that other people interested in cleaning up this part of
libvirt a bit, too!
Your approach seems to be going in the opposite direction of what I did
in the series I posted back in March [1]. That series rather reduces
xpath usage, than increasing it.
If you want to compare our versions, check out [2] and [3].
Kind regards,
Tim
[1]
https://listman.redhat.com/archives/libvir-list/2021-March/msg00853.html
[2]
https://listman.redhat.com/archives/libvir-list/2021-April/msg00232.html
[3] https://gitlab.com/twiederh/libvirt/-/compare/master...refactor_i
9:10 a.m.
On 4/15/21 6:27 PM, Tim Wiederhake wrote:
On Thu, 2021-04-15 at 16:26 +0200, Kristina Hanicova wrote:
> This series reworks the outdated way of parsing XML to parsing by
> XPath,
> which is more obvious and saves a few lines of code.
>
> Kristina Hanicova (21):
> conf: Propagate xmlXPathContextPtr into
> virDomainHostdevSubsysUSBDefParseXML()
> Refactoring virDomainHostdevSubsysUSBDefParseXML() to use XPath
> conf: Propagate xmlXPathContextPtr into
> virDomainBlkioDeviceParseXML()
> Refactoring virDomainBlkioDeviceParseXML() to use XPath
> conf: Propagate xmlXPathContextPtr into
> virDomainHostdevSubsysPCIDefParseXML()
> Refactoring virDomainHostdevSubsysPCIDefParseXML() to use XPath
> conf: Propagate xmlXPathContextPtr into virDomainLeaseDefParseXML()
> Refactoring virDomainLeaseDefParseXML() to use XPath
> Refactoring virDomainDiskDefParseXML() to use XPath
> Refactoring virDomainControllerDefParseXML() to use XPath
> Refactoring virDomainFSDefParseXML() to use XPath
> Refactoring virDomainNetDefParseXML() to use XPath
> conf: Propagate xmlXPathContextPtr into
> virDomainChrDefParseTargetXML()
> Refactoring virDomainChrDefParseTargetXML() to use XPath
> Refactoring virDomainChrSourceDefParseXML() to use XPath
> Refactoring virDomainChrDefParseXML() to use XPath
> Refactoring virDomainSmartcardDefParseXML() to use XPath
> Refactoring virDomainGraphicsDefParseXMLSpice() to use XPath
> conf: Propagate xmlXPathContextPtr into
> virDomainVideoDriverDefParseXML()
> Refactoring virDomainVideoDriverDefParseXML() to use XPath
> Refactoring virDomainVideoDefParseXML() to use XPath
>
> src/conf/domain_conf.c | 1786 +++++++++++++++++---------------------
> --
> 1 file changed, 777 insertions(+), 1009 deletions(-)
>
Great to see that other people interested in cleaning up this part of
libvirt a bit, too!
Your approach seems to be going in the opposite direction of what I did
in the series I posted back in March [1]. That series rather reduces
xpath usage, than increasing it.
If you want to compare our versions, check out [2] and [3].
Kind regards,
Tim
[1]
https://listman.redhat.com/archives/libvir-list/2021-March/msg00853.html
[2]
https://listman.redhat.com/archives/libvir-list/2021-April/msg00232.html
[3] https://gitlab.com/twiederh/libvirt/-/compare/master...refactor_i
Let me review your patches, but your's and Kristina's doesn't look like
opposing to me. I think Kristina's focus was mainly on removing while()
style of traversing elements in favor of XPATH. Your's is to replace
XMLPropString() + str2int/str2enum combo with one function.
Michal
1311
days inactive
1317
days old
24 comments
4 participants
participants (4)
-
Kristina Hanicova -
Michal Privoznik -
Peter Krempa -
Tim Wiederhake