[PATCH 00/21] Refactoring conf to use XPath
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
Signed-off-by: Kristina Hanicova <khanicov@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
Signed-off-by: Kristina Hanicova <khanicov@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
Signed-off-by: Kristina Hanicova <khanicov@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
Signed-off-by: Kristina Hanicova <khanicov@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
Signed-off-by: Kristina Hanicova <khanicov@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
Signed-off-by: Kristina Hanicova <khanicov@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
Signed-off-by: Kristina Hanicova <khanicov@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
Signed-off-by: Kristina Hanicova <khanicov@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
Signed-off-by: Kristina Hanicova <khanicov@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
On Thu, Apr 15, 2021 at 16:26:24 +0200, Kristina Hanicova wrote:
Signed-off-by: Kristina Hanicova <khanicov@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.
Signed-off-by: Kristina Hanicova <khanicov@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
Signed-off-by: Kristina Hanicova <khanicov@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
Signed-off-by: Kristina Hanicova <khanicov@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
Signed-off-by: Kristina Hanicova <khanicov@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
Signed-off-by: Kristina Hanicova <khanicov@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
Signed-off-by: Kristina Hanicova <khanicov@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
Signed-off-by: Kristina Hanicova <khanicov@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
Signed-off-by: Kristina Hanicova <khanicov@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
Signed-off-by: Kristina Hanicova <khanicov@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
Signed-off-by: Kristina Hanicova <khanicov@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
Signed-off-by: Kristina Hanicova <khanicov@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
Signed-off-by: Kristina Hanicova <khanicov@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
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
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
participants (4)
-
Kristina Hanicova -
Michal Privoznik -
Peter Krempa -
Tim Wiederhake