[PATCH 00/43] XML parsing helper cleanup
This series cleans up the use of virXMLParse* macros and tries to unify and remove the uncommon ones. In next step since virXMLParse itself has features which can replace open-coded bits in conf parsers for all various object types we have, this series cleans them up too. Peter Krempa (43): util: xml: Expose all arguments of virXMLParseHelper in virXMLParse macro conf: nwfilderbindigobj: Register automatic cleanup for virNWFilterBindingObj virNWFilterBindingObjParse: Refactor XML parsing code vbox: snapshot_conf: Don't allocate XPath context explicitly virVBoxSnapshotConfGet(RW|RO)DisksPathsFromLibvirtXML: Refactor virNWFilterBindingDefParse: Properly use virXMLParse conf: nwfilterbinding: Provide only virNWFilterBindingDefParse tests: qemuxml2argv: Use virXMLParse properly conf: networkport: Remove virNetworkPortDefParseNode conf: networkportdef: Provide only virNetworkPortDefParse testParseXMLDocFromFile: Refactor control flow testParseXMLDocFromFile: Remove 'type' argument testParseXMLDocFromFile: Validate that the replaced node is identical with parsed root conf: storage: Remove virStoragePoolDefParseNode conf: storage: Provide only virStoragePoolDefParse conf: storage: Remove virStorageVolDefParseNode conf: storage: Provide only virStorageVolDefParse conf: snapshot: Remove virDomainSnapshotDefParseNode conf: secret: Refactor secretXMLParseNode conf: secret: Provide only virSecretDefParse prlsdkParseSnapshotTree: Simplify XML parsing code conf: checkpoint: Remove virDomainCheckpointDefParseNode conf: backup: Remove virDomainBackupDefParseNode conf: nwfilter: Remove virNWFilterDefParseNode conf: nwfilter: Provide only virNWFilterDefParse conf: node_device: Remove virNodeDeviceDefParseNode conf: nodedev: Provide only virNodeDeviceDefParse conf: interface: Remove virInterfaceDefParseNode conf: interface: Remove virInterfaceDefParseFile conf: network: Remove virNetworkDefParseNode conf: network: Provide only virNetworkDefParse conf: domain: Remove virDomainObjParseNode conf: domain: Simplify validation in virDomainDefParse virDomainDefParseNode: Pass only the XPath context as argument virsh: Use proper helper for parsing XML in virshDumpXML util: xml: Remove virXMLParseString qemu: capabilities: Convert virQEMUCapsLoadCache to virXMLParse util: xml: Remove virXMLParseFile util: xml: Remove virXMLParseCtxt conf: savecookie: Remove virSaveCookieParseNode security: aa-helper: Use virXMLParse instead of virXMLParseString test_driver: Make callers of testOpenParse ensure the root element name util: xml: Remove virXMLParseStringCtxtRoot src/conf/backup_conf.c | 42 ++------ src/conf/backup_conf.h | 10 +- src/conf/checkpoint_conf.c | 42 +++----- src/conf/domain_conf.c | 85 +++++----------- src/conf/domain_conf.h | 7 +- src/conf/interface_conf.c | 54 ++-------- src/conf/interface_conf.h | 7 +- src/conf/network_conf.c | 50 ++-------- src/conf/network_conf.h | 16 +-- src/conf/node_device_conf.c | 57 ++--------- src/conf/node_device_conf.h | 25 ++--- src/conf/nwfilter_conf.c | 48 ++------- src/conf/nwfilter_conf.h | 12 +-- src/conf/secret_conf.c | 44 ++------ src/conf/secret_conf.h | 7 +- src/conf/snapshot_conf.c | 57 ++++------- src/conf/snapshot_conf.h | 13 +-- src/conf/storage_conf.c | 108 +++----------------- src/conf/storage_conf.h | 31 ++---- src/conf/virnetworkobj.c | 6 +- src/conf/virnetworkportdef.c | 49 ++------- src/conf/virnetworkportdef.h | 12 +-- src/conf/virnwfilterbindingdef.c | 51 ++-------- src/conf/virnwfilterbindingdef.h | 11 +- src/conf/virnwfilterbindingobj.c | 66 +++--------- src/conf/virnwfilterbindingobj.h | 3 +- src/conf/virnwfilterbindingobjlist.c | 2 +- src/conf/virnwfilterobj.c | 2 +- src/conf/virsavecookie.c | 31 ++---- src/conf/virsecretobj.c | 2 +- src/conf/virstorageobj.c | 4 +- src/esx/esx_network_driver.c | 2 +- src/esx/esx_storage_backend_vmfs.c | 4 +- src/hypervisor/domain_driver.c | 7 +- src/libvirt_private.syms | 41 +++----- src/network/bridge_driver.c | 10 +- src/node_device/node_device_driver.c | 8 +- src/nwfilter/nwfilter_driver.c | 4 +- src/qemu/qemu_capabilities.c | 15 +-- src/qemu/qemu_domain.c | 9 +- src/qemu/qemu_migration_cookie.c | 10 +- src/qemu/qemu_process.c | 2 +- src/secret/secret_driver.c | 2 +- src/security/virt-aa-helper.c | 9 +- src/storage/storage_driver.c | 10 +- src/test/test_driver.c | 138 +++++++++++--------------- src/util/virxml.h | 50 ++-------- src/vbox/vbox_network.c | 2 +- src/vbox/vbox_snapshot_conf.c | 129 +++++++----------------- src/vbox/vbox_storage.c | 2 +- src/vz/vz_sdk.c | 18 +--- tests/networkxml2conftest.c | 2 +- tests/networkxml2firewalltest.c | 2 +- tests/networkxml2xmltest.c | 2 +- tests/networkxml2xmlupdatetest.c | 2 +- tests/nodedevmdevctltest.c | 8 +- tests/nodedevxml2xmltest.c | 3 +- tests/nwfilterxml2firewalltest.c | 2 +- tests/nwfilterxml2xmltest.c | 2 +- tests/qemuxml2argvtest.c | 19 +--- tests/secretxml2xmltest.c | 2 +- tests/storagepoolxml2argvtest.c | 2 +- tests/storagepoolxml2xmltest.c | 2 +- tests/storagevolxml2argvtest.c | 8 +- tests/storagevolxml2xmltest.c | 4 +- tests/virnetworkportxml2xmltest.c | 2 +- tests/virnwfilterbindingxml2xmltest.c | 2 +- tests/virschematest.c | 2 +- tools/virsh-util.c | 5 +- 69 files changed, 399 insertions(+), 1098 deletions(-) -- 2.37.3
The generic helper also has helper code to validate the root element and create an XPath context. Many places in the code duplicate code for doing these operations. Extend the helper to provide all arguments and fix all callers. In many cases this patch refactores the passing of the 'validate' field into a separate variable to avoid quirky looking arguments. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/backup_conf.c | 5 +++-- src/conf/checkpoint_conf.c | 2 +- src/conf/domain_conf.c | 6 ++++-- src/conf/interface_conf.c | 3 ++- src/conf/network_conf.c | 2 +- src/conf/node_device_conf.c | 3 ++- src/conf/nwfilter_conf.c | 5 +++-- src/conf/secret_conf.c | 5 +++-- src/conf/snapshot_conf.c | 5 +++-- src/conf/storage_conf.c | 6 ++++-- src/conf/virnetworkportdef.c | 4 ++-- src/conf/virnwfilterbindingdef.c | 4 ++-- src/conf/virnwfilterbindingobj.c | 3 ++- src/test/test_driver.c | 2 +- src/util/virxml.h | 8 ++++++-- src/vbox/vbox_snapshot_conf.c | 6 +++--- src/vz/vz_sdk.c | 3 ++- tests/qemuxml2argvtest.c | 3 ++- 18 files changed, 46 insertions(+), 29 deletions(-) diff --git a/src/conf/backup_conf.c b/src/conf/backup_conf.c index 0f2fba115d..7dfc8ee635 100644 --- a/src/conf/backup_conf.c +++ b/src/conf/backup_conf.c @@ -277,9 +277,10 @@ virDomainBackupDefParseString(const char *xmlStr, virDomainBackupDef *ret = NULL; g_autoptr(xmlDoc) xml = NULL; int keepBlanksDefault = xmlKeepBlanksDefault(0); + bool validate = !(flags & VIR_DOMAIN_BACKUP_PARSE_INTERNAL); - if ((xml = virXMLParse(NULL, xmlStr, _("(domain_backup)"), "domainbackup.rng", - !(flags & VIR_DOMAIN_BACKUP_PARSE_INTERNAL)))) { + if ((xml = virXMLParse(NULL, xmlStr, _("(domain_backup)"), + NULL, NULL, "domainbackup.rng", validate))) { xmlKeepBlanksDefault(keepBlanksDefault); ret = virDomainBackupDefParseNode(xml, xmlDocGetRootElement(xml), xmlopt, flags); diff --git a/src/conf/checkpoint_conf.c b/src/conf/checkpoint_conf.c index 338cf10d12..0d2d2050da 100644 --- a/src/conf/checkpoint_conf.c +++ b/src/conf/checkpoint_conf.c @@ -212,7 +212,7 @@ virDomainCheckpointDefParseString(const char *xmlStr, g_autoptr(xmlDoc) xml = NULL; int keepBlanksDefault = xmlKeepBlanksDefault(0); - if ((xml = virXMLParse(NULL, xmlStr, _("(domain_checkpoint)"), + if ((xml = virXMLParse(NULL, xmlStr, _("(domain_checkpoint)"), NULL, NULL, "domaincheckpoint.rng", true))) { xmlKeepBlanksDefault(keepBlanksDefault); ret = virDomainCheckpointDefParseNode(xml, xmlDocGetRootElement(xml), diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a609cc4f68..b3202c9f76 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -19083,8 +19083,10 @@ virDomainDefParse(const char *xmlStr, virDomainDef *def = NULL; int keepBlanksDefault = xmlKeepBlanksDefault(0); xmlNodePtr root; - if (!(xml = virXMLParse(filename, xmlStr, _("(domain_definition)"), "domain.rng", - flags & VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA))) + bool validate = flags & VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA; + + if (!(xml = virXMLParse(filename, xmlStr, _("(domain_definition)"), + NULL, NULL, "domain.rng", validate))) goto cleanup; root = xmlDocGetRootElement(xml); diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c index dc61b378b9..a3f6b6bed6 100644 --- a/src/conf/interface_conf.c +++ b/src/conf/interface_conf.c @@ -700,9 +700,10 @@ virInterfaceDefParse(const char *xmlStr, unsigned int flags) { g_autoptr(xmlDoc) xml = NULL; + bool validate = flags & VIR_INTERFACE_DEFINE_VALIDATE; xml = virXMLParse(filename, xmlStr, _("(interface_definition)"), - "interface.rng", flags & VIR_INTERFACE_DEFINE_VALIDATE); + NULL, NULL, "interface.rng", validate); if (!xml) return NULL; diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index b8450b75a9..b1d77a80c3 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -2036,7 +2036,7 @@ virNetworkDefParse(const char *xmlStr, int keepBlanksDefault = xmlKeepBlanksDefault(0); if ((xml = virXMLParse(filename, xmlStr, _("(network_definition)"), - "network.rng", validate))) + NULL, NULL, "network.rng", validate))) def = virNetworkDefParseNode(xml, xmlDocGetRootElement(xml), xmlopt); xmlKeepBlanksDefault(keepBlanksDefault); diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index d5bfc098b2..1db9a3240a 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -2508,7 +2508,8 @@ virNodeDeviceDefParse(const char *str, g_autoptr(xmlDoc) xml = NULL; g_autoptr(virNodeDeviceDef) def = NULL; - if (!(xml = virXMLParse(filename, str, _("(node_device_definition)"), NULL, false)) || + if (!(xml = virXMLParse(filename, str, _("(node_device_definition)"), + NULL, NULL, NULL, false)) || !(def = virNodeDeviceDefParseNode(xml, xmlDocGetRootElement(xml), create, virt_type))) return NULL; diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index b5fd266457..44ea056823 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -2713,9 +2713,10 @@ virNWFilterDefParse(const char *xmlStr, { virNWFilterDef *def = NULL; g_autoptr(xmlDoc) xml = NULL; + bool validate = flags & VIR_NWFILTER_DEFINE_VALIDATE; - if ((xml = virXMLParse(filename, xmlStr, _("(nwfilter_definition)"), "nwfilter.rng", - flags & VIR_NWFILTER_DEFINE_VALIDATE))) { + if ((xml = virXMLParse(filename, xmlStr, _("(nwfilter_definition)"), + NULL, NULL, "nwfilter.rng", validate))) { def = virNWFilterDefParseNode(xml, xmlDocGetRootElement(xml)); } diff --git a/src/conf/secret_conf.c b/src/conf/secret_conf.c index 011fdaa12b..02c2e38964 100644 --- a/src/conf/secret_conf.c +++ b/src/conf/secret_conf.c @@ -193,9 +193,10 @@ virSecretDefParse(const char *xmlStr, { g_autoptr(xmlDoc) xml = NULL; virSecretDef *ret = NULL; + bool validate = flags & VIR_SECRET_DEFINE_VALIDATE; - if ((xml = virXMLParse(filename, xmlStr, _("(definition_of_secret)"), "secret.rng", - flags & VIR_SECRET_DEFINE_VALIDATE))) { + if ((xml = virXMLParse(filename, xmlStr, _("(definition_of_secret)"), + NULL, NULL, "secret.rng", validate))) { ret = secretXMLParseNode(xml, xmlDocGetRootElement(xml)); } diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index ae635edd08..a5974053f4 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -421,9 +421,10 @@ virDomainSnapshotDefParseString(const char *xmlStr, virDomainSnapshotDef *ret = NULL; g_autoptr(xmlDoc) xml = NULL; int keepBlanksDefault = xmlKeepBlanksDefault(0); + bool validate = flags & VIR_DOMAIN_SNAPSHOT_PARSE_VALIDATE; - if ((xml = virXMLParse(NULL, xmlStr, _("(domain_snapshot)"), "domainsnapshot.rng", - flags & VIR_DOMAIN_SNAPSHOT_PARSE_VALIDATE))) { + if ((xml = virXMLParse(NULL, xmlStr, _("(domain_snapshot)"), + NULL, NULL, "domainsnapshot.rng", validate))) { xmlKeepBlanksDefault(keepBlanksDefault); ret = virDomainSnapshotDefParseNode(xml, xmlDocGetRootElement(xml), xmlopt, parseOpaque, diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 79f16aadf3..d7375a5160 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1001,9 +1001,10 @@ virStoragePoolDefParse(const char *xmlStr, { virStoragePoolDef *ret = NULL; g_autoptr(xmlDoc) xml = NULL; + bool validate = flags & VIR_STORAGE_POOL_DEFINE_VALIDATE; if ((xml = virXMLParse(filename, xmlStr, _("(storage_pool_definition)"), - "storagepool.rng", flags & VIR_STORAGE_POOL_DEFINE_VALIDATE))) { + NULL, NULL, "storagepool.rng", validate))) { ret = virStoragePoolDefParseNode(xml, xmlDocGetRootElement(xml)); } @@ -1470,7 +1471,8 @@ virStorageVolDefParse(virStoragePoolDef *pool, virStorageVolDef *ret = NULL; g_autoptr(xmlDoc) xml = NULL; - if ((xml = virXMLParse(filename, xmlStr, _("(storage_volume_definition)"), NULL, false))) { + if ((xml = virXMLParse(filename, xmlStr, _("(storage_volume_definition)"), + NULL, NULL, NULL, false))) { ret = virStorageVolDefParseNode(pool, xml, xmlDocGetRootElement(xml), flags); } diff --git a/src/conf/virnetworkportdef.c b/src/conf/virnetworkportdef.c index 39fa895fae..40cadc4ae8 100644 --- a/src/conf/virnetworkportdef.c +++ b/src/conf/virnetworkportdef.c @@ -279,10 +279,10 @@ virNetworkPortDefParse(const char *xmlStr, { virNetworkPortDef *def = NULL; g_autoptr(xmlDoc) xml = NULL; + bool validate = flags & VIR_NETWORK_PORT_CREATE_VALIDATE; if ((xml = virXMLParse(filename, xmlStr, _("(networkport_definition)"), - "networkport.rng", - flags & VIR_NETWORK_PORT_CREATE_VALIDATE))) { + NULL, NULL, "networkport.rng", validate))) { def = virNetworkPortDefParseNode(xml, xmlDocGetRootElement(xml)); } diff --git a/src/conf/virnwfilterbindingdef.c b/src/conf/virnwfilterbindingdef.c index cc50ba944a..e58bab3f08 100644 --- a/src/conf/virnwfilterbindingdef.c +++ b/src/conf/virnwfilterbindingdef.c @@ -182,10 +182,10 @@ virNWFilterBindingDefParse(const char *xmlStr, { virNWFilterBindingDef *def = NULL; g_autoptr(xmlDoc) xml = NULL; + bool validate = flags & VIR_NWFILTER_BINDING_CREATE_VALIDATE; if ((xml = virXMLParse(filename, xmlStr, _("(nwfilterbinding_definition)"), - "nwfilterbinding.rng", - flags & VIR_NWFILTER_BINDING_CREATE_VALIDATE))) { + NULL, NULL, "nwfilterbinding.rng", validate))) { def = virNWFilterBindingDefParseNode(xml, xmlDocGetRootElement(xml)); } diff --git a/src/conf/virnwfilterbindingobj.c b/src/conf/virnwfilterbindingobj.c index 47455c7e35..cc6009d1f2 100644 --- a/src/conf/virnwfilterbindingobj.c +++ b/src/conf/virnwfilterbindingobj.c @@ -257,7 +257,8 @@ virNWFilterBindingObjParse(const char *xmlStr, virNWFilterBindingObj *obj = NULL; g_autoptr(xmlDoc) xml = NULL; - if ((xml = virXMLParse(filename, xmlStr, _("(nwfilterbinding_status)"), NULL, false))) { + if ((xml = virXMLParse(filename, xmlStr, _("(nwfilterbinding_status)"), + NULL, NULL, NULL, false))) { obj = virNWFilterBindingObjParseNode(xml, xmlDocGetRootElement(xml)); } diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 686ff051a8..1c10124564 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -851,7 +851,7 @@ testParseXMLDocFromFile(xmlNodePtr node, const char *file, const char *type) if ((relFile = virXMLPropString(node, "file"))) { absFile = testBuildFilename(file, relFile); - if (!(doc = virXMLParse(absFile, NULL, type, NULL, false))) + if (!(doc = virXMLParse(absFile, NULL, type, NULL, NULL, NULL, false))) return NULL; ret = xmlCopyNode(xmlDocGetRootElement(doc), 1); diff --git a/src/util/virxml.h b/src/util/virxml.h index af58d44835..a80e795f6b 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -204,13 +204,17 @@ virXMLPickShellSafeComment(const char *str1, * @filename: file to parse, or NULL for string parsing * @xmlStr: if @filename is NULL, a string to parse * @url: if @filename is NULL, an optional filename to attribute the parse to + * @rootelement: if non-NULL, validate that the root element name equals to this parameter + * @ctxt: if non-NULL, filled with a new XPath context including populating the root node + * @schemafile: name of the appropriate schema file for the parsed XML for validation (may be NULL) + * @validate: if true and @schemafile is non-NULL, validate the XML against @schemafile * * Parse xml from either a file or a string. * * Return the parsed document object, or NULL on failure. */ -#define virXMLParse(filename, xmlStr, url, schemafile, validate) \ - virXMLParseHelper(VIR_FROM_THIS, filename, xmlStr, url, NULL, NULL, schemafile, validate) +#define virXMLParse(filename, xmlStr, url, rootelement, ctxt, schemafile, validate) \ + virXMLParseHelper(VIR_FROM_THIS, filename, xmlStr, url, rootelement, ctxt, schemafile, validate) /** * virXMLParseString: diff --git a/src/vbox/vbox_snapshot_conf.c b/src/vbox/vbox_snapshot_conf.c index 90afac179e..6dce9cdf0f 100644 --- a/src/vbox/vbox_snapshot_conf.c +++ b/src/vbox/vbox_snapshot_conf.c @@ -584,7 +584,7 @@ virVBoxSnapshotConfLoadVboxFile(const char *filePath, machineDescription = g_new0(virVBoxSnapshotConfMachine, 1); - xml = virXMLParse(filePath, NULL, NULL, NULL, false); + xml = virXMLParse(filePath, NULL, NULL, NULL, NULL, NULL, false); if (xml == NULL) { virReportError(VIR_ERR_XML_ERROR, "%s", _("Unable to parse the xml")); @@ -1214,7 +1214,7 @@ virVBoxSnapshotConfGetRWDisksPathsFromLibvirtXML(const char *filePath, _("filePath is null")); goto cleanup; } - xml = virXMLParse(filePath, NULL, NULL, NULL, false); + xml = virXMLParse(filePath, NULL, NULL, NULL, NULL, NULL, false); if (xml == NULL) { virReportError(VIR_ERR_XML_ERROR, "%s", _("Unable to parse the xml")); @@ -1271,7 +1271,7 @@ virVBoxSnapshotConfGetRODisksPathsFromLibvirtXML(const char *filePath, _("filePath is null")); goto cleanup; } - xml = virXMLParse(filePath, NULL, NULL, NULL, false); + xml = virXMLParse(filePath, NULL, NULL, NULL, NULL, NULL, false); if (xml == NULL) { virReportError(VIR_ERR_XML_ERROR, "%s", _("Unable to parse the xml")); diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index ecf610d7db..8fb7a9948d 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -4581,7 +4581,8 @@ prlsdkParseSnapshotTree(const char *treexml) if (*treexml == '\0') return snapshots; - if (!(xml = virXMLParse(NULL, treexml, _("(snapshot_tree)"), NULL, false))) + if (!(xml = virXMLParse(NULL, treexml, _("(snapshot_tree)"), + NULL, NULL, NULL, false))) goto cleanup; root = xmlDocGetRootElement(xml); diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index c8b69bbd7a..89bed6a46e 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -672,7 +672,8 @@ testCompareXMLToArgv(const void *data) if (testCheckExclusiveFlags(info->flags) < 0) goto cleanup; - if (!(xml = virXMLParse(info->infile, NULL, "(domain_definition)", NULL, false))) + if (!(xml = virXMLParse(info->infile, NULL, "(domain_definition)", + NULL, NULL, NULL, false))) goto cleanup; root = xmlDocGetRootElement(xml); -- 2.37.3
On a Tuesday in 2022, Peter Krempa wrote:
The generic helper also has helper code to validate the root element and create an XPath context. Many places in the code duplicate code for doing these operations.
Extend the helper to provide all arguments and fix all callers.
In many cases this patch refactores the passing of the 'validate'
*refactors
field into a separate variable to avoid quirky looking arguments.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/backup_conf.c | 5 +++-- src/conf/checkpoint_conf.c | 2 +- src/conf/domain_conf.c | 6 ++++-- src/conf/interface_conf.c | 3 ++- src/conf/network_conf.c | 2 +- src/conf/node_device_conf.c | 3 ++- src/conf/nwfilter_conf.c | 5 +++-- src/conf/secret_conf.c | 5 +++-- src/conf/snapshot_conf.c | 5 +++-- src/conf/storage_conf.c | 6 ++++-- src/conf/virnetworkportdef.c | 4 ++-- src/conf/virnwfilterbindingdef.c | 4 ++-- src/conf/virnwfilterbindingobj.c | 3 ++- src/test/test_driver.c | 2 +- src/util/virxml.h | 8 ++++++-- src/vbox/vbox_snapshot_conf.c | 6 +++--- src/vz/vz_sdk.c | 3 ++- tests/qemuxml2argvtest.c | 3 ++- 18 files changed, 46 insertions(+), 29 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/virnwfilterbindingobj.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/conf/virnwfilterbindingobj.h b/src/conf/virnwfilterbindingobj.h index 93f693936e..17f855bda1 100644 --- a/src/conf/virnwfilterbindingobj.h +++ b/src/conf/virnwfilterbindingobj.h @@ -26,6 +26,7 @@ #include "virobject.h" typedef struct _virNWFilterBindingObj virNWFilterBindingObj; +G_DEFINE_AUTOPTR_CLEANUP_FUNC(virNWFilterBindingObj, virObjectUnref); virNWFilterBindingObj * virNWFilterBindingObjNew(void); -- 2.37.3
Remove the redundant root node checking and XPath context creation by using virXMLParse properly. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/virnwfilterbindingobj.c | 65 +++++----------------------- src/conf/virnwfilterbindingobj.h | 2 +- src/conf/virnwfilterbindingobjlist.c | 2 +- src/libvirt_private.syms | 2 +- 4 files changed, 14 insertions(+), 57 deletions(-) diff --git a/src/conf/virnwfilterbindingobj.c b/src/conf/virnwfilterbindingobj.c index cc6009d1f2..5ff53f7380 100644 --- a/src/conf/virnwfilterbindingobj.c +++ b/src/conf/virnwfilterbindingobj.c @@ -202,74 +202,31 @@ virNWFilterBindingObjDelete(const virNWFilterBindingObj *obj, } -static virNWFilterBindingObj * -virNWFilterBindingObjParseXML(xmlDocPtr doc, - xmlXPathContextPtr ctxt) +virNWFilterBindingObj * +virNWFilterBindingObjParse(const char *filename) { - virNWFilterBindingObj *ret; + g_autoptr(virNWFilterBindingObj) ret = NULL; + g_autoptr(xmlDoc) xml = NULL; + g_autoptr(xmlXPathContext) ctxt = NULL; xmlNodePtr node; + if (!(xml = virXMLParse(filename, NULL, _("(nwfilterbinding_status)"), + "filterbindingstatus", &ctxt, NULL, false))) + return NULL; + if (!(ret = virNWFilterBindingObjNew())) return NULL; if (!(node = virXPathNode("./filterbinding", ctxt))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("filter binding status missing content")); - goto cleanup; - } - - if (!(ret->def = virNWFilterBindingDefParseNode(doc, node))) - goto cleanup; - - return ret; - - cleanup: - virObjectUnref(ret); - return NULL; -} - - -static virNWFilterBindingObj * -virNWFilterBindingObjParseNode(xmlDocPtr doc, - xmlNodePtr root) -{ - g_autoptr(xmlXPathContext) ctxt = NULL; - - if (STRNEQ((const char *)root->name, "filterbindingstatus")) { - virReportError(VIR_ERR_XML_ERROR, - _("unknown root element '%s' for filter binding"), - root->name); return NULL; } - if (!(ctxt = virXMLXPathContextNew(doc))) + if (!(ret->def = virNWFilterBindingDefParseNode(xml, node))) return NULL; - ctxt->node = root; - return virNWFilterBindingObjParseXML(doc, ctxt); -} - - -static virNWFilterBindingObj * -virNWFilterBindingObjParse(const char *xmlStr, - const char *filename) -{ - virNWFilterBindingObj *obj = NULL; - g_autoptr(xmlDoc) xml = NULL; - - if ((xml = virXMLParse(filename, xmlStr, _("(nwfilterbinding_status)"), - NULL, NULL, NULL, false))) { - obj = virNWFilterBindingObjParseNode(xml, xmlDocGetRootElement(xml)); - } - - return obj; -} - - -virNWFilterBindingObj * -virNWFilterBindingObjParseFile(const char *filename) -{ - return virNWFilterBindingObjParse(NULL, filename); + return g_steal_pointer(&ret); } diff --git a/src/conf/virnwfilterbindingobj.h b/src/conf/virnwfilterbindingobj.h index 17f855bda1..33aa575436 100644 --- a/src/conf/virnwfilterbindingobj.h +++ b/src/conf/virnwfilterbindingobj.h @@ -64,7 +64,7 @@ virNWFilterBindingObjDelete(const virNWFilterBindingObj *obj, const char *statusDir); virNWFilterBindingObj * -virNWFilterBindingObjParseFile(const char *filename); +virNWFilterBindingObjParse(const char *filename); char * virNWFilterBindingObjFormat(const virNWFilterBindingObj *obj); diff --git a/src/conf/virnwfilterbindingobjlist.c b/src/conf/virnwfilterbindingobjlist.c index 9a46fa06b9..a349f5c195 100644 --- a/src/conf/virnwfilterbindingobjlist.c +++ b/src/conf/virnwfilterbindingobjlist.c @@ -266,7 +266,7 @@ virNWFilterBindingObjListLoadStatus(virNWFilterBindingObjList *bindings, if ((statusFile = virNWFilterBindingObjConfigFile(statusDir, name)) == NULL) goto error; - if (!(obj = virNWFilterBindingObjParseFile(statusFile))) + if (!(obj = virNWFilterBindingObjParse(statusFile))) goto error; def = virNWFilterBindingObjGetDef(obj); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 5be40dbefe..11862bb1a9 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1377,7 +1377,7 @@ virNWFilterBindingObjFormat; virNWFilterBindingObjGetDef; virNWFilterBindingObjGetRemoving; virNWFilterBindingObjNew; -virNWFilterBindingObjParseFile; +virNWFilterBindingObjParse; virNWFilterBindingObjSave; virNWFilterBindingObjSetDef; virNWFilterBindingObjSetRemoving; -- 2.37.3
Use the one provided via virXMLParse. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/vbox/vbox_snapshot_conf.c | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/src/vbox/vbox_snapshot_conf.c b/src/vbox/vbox_snapshot_conf.c index 6dce9cdf0f..e175f1964e 100644 --- a/src/vbox/vbox_snapshot_conf.c +++ b/src/vbox/vbox_snapshot_conf.c @@ -584,15 +584,12 @@ virVBoxSnapshotConfLoadVboxFile(const char *filePath, machineDescription = g_new0(virVBoxSnapshotConfMachine, 1); - xml = virXMLParse(filePath, NULL, NULL, NULL, NULL, NULL, false); + xml = virXMLParse(filePath, NULL, NULL, NULL, &xPathContext, NULL, false); if (xml == NULL) { virReportError(VIR_ERR_XML_ERROR, "%s", _("Unable to parse the xml")); goto cleanup; } - if (!(xPathContext = virXMLXPathContextNew(xml))) - goto cleanup; - if (xmlXPathRegisterNs(xPathContext, BAD_CAST "vbox", BAD_CAST VBOX_SETTINGS_NS) < 0) { @@ -603,8 +600,6 @@ virVBoxSnapshotConfLoadVboxFile(const char *filePath, } /* Retrieve MachineNode */ - cur = xmlDocGetRootElement(xml); - xPathContext->node = cur; machineNode = virXPathNode("./vbox:Machine", xPathContext); if (machineNode == NULL) { virReportError(VIR_ERR_XML_ERROR, "%s", @@ -1214,17 +1209,13 @@ virVBoxSnapshotConfGetRWDisksPathsFromLibvirtXML(const char *filePath, _("filePath is null")); goto cleanup; } - xml = virXMLParse(filePath, NULL, NULL, NULL, NULL, NULL, false); + xml = virXMLParse(filePath, NULL, NULL, NULL, &xPathContext, NULL, false); if (xml == NULL) { virReportError(VIR_ERR_XML_ERROR, "%s", _("Unable to parse the xml")); goto cleanup; } - if (!(xPathContext = virXMLXPathContextNew(xml))) - goto cleanup; - - xPathContext->node = xmlDocGetRootElement(xml); if ((nodeSize = virXPathNodeSet("/domainsnapshot/disks/disk", xPathContext, &nodes)) < 0) goto cleanup; @@ -1271,17 +1262,13 @@ virVBoxSnapshotConfGetRODisksPathsFromLibvirtXML(const char *filePath, _("filePath is null")); goto cleanup; } - xml = virXMLParse(filePath, NULL, NULL, NULL, NULL, NULL, false); + xml = virXMLParse(filePath, NULL, NULL, NULL, &xPathContext, NULL, false); if (xml == NULL) { virReportError(VIR_ERR_XML_ERROR, "%s", _("Unable to parse the xml")); goto cleanup; } - if (!(xPathContext = virXMLXPathContextNew(xml))) - goto cleanup; - - xPathContext->node = xmlDocGetRootElement(xml); if ((nodeSize = virXPathNodeSet("/domainsnapshot/domain/devices/disk", xPathContext, &nodes)) < 0) -- 2.37.3
virVBoxSnapshotConfGetRWDisksPathsFromLibvirtXML and virVBoxSnapshotConfGetRODisksPathsFromLibvirtXML were doing the same thing, except for one XPath query. Factor out the common code into a helper and bring it up to modern standard. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/vbox/vbox_snapshot_conf.c | 116 +++++++++++----------------------- 1 file changed, 36 insertions(+), 80 deletions(-) diff --git a/src/vbox/vbox_snapshot_conf.c b/src/vbox/vbox_snapshot_conf.c index e175f1964e..2f3f48d688 100644 --- a/src/vbox/vbox_snapshot_conf.c +++ b/src/vbox/vbox_snapshot_conf.c @@ -1187,60 +1187,55 @@ virVBoxSnapshotConfIsCurrentSnapshot(virVBoxSnapshotConfMachine *machine, return STREQ(snapshot->uuid, machine->currentSnapshot); } -/* - *getRWDisksPathsFromLibvirtXML: Parse a libvirt XML snapshot file, allocates and - *fills a list of read-write disk paths. - *return array length on success, -1 on failure. - */ -int -virVBoxSnapshotConfGetRWDisksPathsFromLibvirtXML(const char *filePath, - char ***rwDisksPath) +static int +virVBoxSnapshotConfGetDisksPathsFromLibvirtXML(const char *filePath, + char ***disksPath, + const char *xpath) { - int result = -1; size_t i = 0; - g_auto(GStrv) ret = NULL; g_autoptr(xmlDoc) xml = NULL; g_autoptr(xmlXPathContext) xPathContext = NULL; - xmlNodePtr *nodes = NULL; + g_autofree xmlNodePtr *nodes = NULL; int nodeSize = 0; - *rwDisksPath = NULL; + + *disksPath = NULL; + if (filePath == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("filePath is null")); - goto cleanup; - } - xml = virXMLParse(filePath, NULL, NULL, NULL, &xPathContext, NULL, false); - if (xml == NULL) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("Unable to parse the xml")); - goto cleanup; + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("filePath is null")); + return -1; } - if ((nodeSize = virXPathNodeSet("/domainsnapshot/disks/disk", - xPathContext, &nodes)) < 0) - goto cleanup; + if (!(xml = virXMLParse(filePath, NULL, NULL, NULL, &xPathContext, NULL, false))) + return -1; + + if ((nodeSize = virXPathNodeSet(xpath, xPathContext, &nodes)) < 0) + return -1; - ret = g_new0(char *, nodeSize); + *disksPath = g_new0(char *, nodeSize); for (i = 0; i < nodeSize; i++) { - xmlNodePtr node = nodes[i]; - xmlNodePtr sourceNode; - - xPathContext->node = node; - sourceNode = virXPathNode("./source", xPathContext); - if (sourceNode) - ret[i] = virXMLPropString(sourceNode, "file"); + xPathContext->node = nodes[i]; + (*disksPath)[i] = virXPathString("string(./source/@file)", xPathContext); } - *rwDisksPath = g_steal_pointer(&ret); - result = 0; - cleanup: - if (result < 0) - nodeSize = -1; - VIR_FREE(nodes); return nodeSize; } + +/* + *getRWDisksPathsFromLibvirtXML: Parse a libvirt XML snapshot file, allocates and + *fills a list of read-write disk paths. + *return array length on success, -1 on failure. + */ +int +virVBoxSnapshotConfGetRWDisksPathsFromLibvirtXML(const char *filePath, + char ***rwDisksPath) +{ + return virVBoxSnapshotConfGetDisksPathsFromLibvirtXML(filePath, rwDisksPath, + "/domainsnapshot/disks/disk"); +} + + /* *getRODisksPathsFromLibvirtXML: *Parse a libvirt XML snapshot file, allocates and fills *a list of read-only disk paths (the parents of the read-write disks). @@ -1250,50 +1245,11 @@ int virVBoxSnapshotConfGetRODisksPathsFromLibvirtXML(const char *filePath, char ***roDisksPath) { - int result = -1; - size_t i = 0; - g_auto(GStrv) ret = NULL; - g_autoptr(xmlDoc) xml = NULL; - g_autoptr(xmlXPathContext) xPathContext = NULL; - xmlNodePtr *nodes = NULL; - int nodeSize = 0; - if (filePath == NULL) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("filePath is null")); - goto cleanup; - } - xml = virXMLParse(filePath, NULL, NULL, NULL, &xPathContext, NULL, false); - if (xml == NULL) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("Unable to parse the xml")); - goto cleanup; - } - - if ((nodeSize = virXPathNodeSet("/domainsnapshot/domain/devices/disk", - xPathContext, - &nodes)) < 0) - goto cleanup; - ret = g_new0(char *, nodeSize); - - for (i = 0; i < nodeSize; i++) { - xmlNodePtr node = nodes[i]; - xmlNodePtr sourceNode; - - xPathContext->node = node; - sourceNode = virXPathNode("./source", xPathContext); - if (sourceNode) - ret[i] = virXMLPropString(sourceNode, "file"); - } - *roDisksPath = g_steal_pointer(&ret); - result = 0; - - cleanup: - if (result < 0) - nodeSize = -1; - VIR_FREE(nodes); - return nodeSize; + return virVBoxSnapshotConfGetDisksPathsFromLibvirtXML(filePath, roDisksPath, + "/domainsnapshot/domain/devices/disk"); } + /* *hardDiskUuidByLocation: Return the uuid of the hard disk whose location is 'location' *return a valid uuid, or NULL on failure -- 2.37.3
Fetch the XPath context and validate the node by using virXMLParse's features. This allows to completely remove virNWFilterBindingDefParseNode as all callers now properly validate the root element name and have a XPath context handy. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/virnwfilterbindingdef.c | 34 ++++++-------------------------- src/conf/virnwfilterbindingdef.h | 3 +-- src/conf/virnwfilterbindingobj.c | 4 +++- src/libvirt_private.syms | 2 +- 4 files changed, 11 insertions(+), 32 deletions(-) diff --git a/src/conf/virnwfilterbindingdef.c b/src/conf/virnwfilterbindingdef.c index e58bab3f08..524010c4c4 100644 --- a/src/conf/virnwfilterbindingdef.c +++ b/src/conf/virnwfilterbindingdef.c @@ -71,7 +71,7 @@ virNWFilterBindingDefCopy(virNWFilterBindingDef *src) } -static virNWFilterBindingDef * +virNWFilterBindingDef * virNWFilterBindingDefParseXML(xmlXPathContextPtr ctxt) { virNWFilterBindingDef *ret; @@ -154,42 +154,20 @@ virNWFilterBindingDefParseXML(xmlXPathContextPtr ctxt) } -virNWFilterBindingDef * -virNWFilterBindingDefParseNode(xmlDocPtr xml, - xmlNodePtr root) -{ - g_autoptr(xmlXPathContext) ctxt = NULL; - - if (STRNEQ((const char *)root->name, "filterbinding")) { - virReportError(VIR_ERR_XML_ERROR, - "%s", - _("unknown root element for nwfilter binding")); - return NULL; - } - - if (!(ctxt = virXMLXPathContextNew(xml))) - return NULL; - - ctxt->node = root; - return virNWFilterBindingDefParseXML(ctxt); -} - - static virNWFilterBindingDef * virNWFilterBindingDefParse(const char *xmlStr, const char *filename, unsigned int flags) { - virNWFilterBindingDef *def = NULL; g_autoptr(xmlDoc) xml = NULL; + g_autoptr(xmlXPathContext) ctxt = NULL; bool validate = flags & VIR_NWFILTER_BINDING_CREATE_VALIDATE; - if ((xml = virXMLParse(filename, xmlStr, _("(nwfilterbinding_definition)"), - NULL, NULL, "nwfilterbinding.rng", validate))) { - def = virNWFilterBindingDefParseNode(xml, xmlDocGetRootElement(xml)); - } + if (!(xml = virXMLParse(filename, xmlStr, _("(nwfilterbinding_definition)"), + "filterbinding", &ctxt, "nwfilterbinding.rng", validate))) + return NULL; - return def; + return virNWFilterBindingDefParseXML(ctxt); } diff --git a/src/conf/virnwfilterbindingdef.h b/src/conf/virnwfilterbindingdef.h index 4bf0f252f8..a5497d5a1c 100644 --- a/src/conf/virnwfilterbindingdef.h +++ b/src/conf/virnwfilterbindingdef.h @@ -47,8 +47,7 @@ virNWFilterBindingDef * virNWFilterBindingDefCopy(virNWFilterBindingDef *src); virNWFilterBindingDef * -virNWFilterBindingDefParseNode(xmlDocPtr xml, - xmlNodePtr root); +virNWFilterBindingDefParseXML(xmlXPathContextPtr ctxt); virNWFilterBindingDef * virNWFilterBindingDefParseString(const char *xml, diff --git a/src/conf/virnwfilterbindingobj.c b/src/conf/virnwfilterbindingobj.c index 5ff53f7380..6e67c5c7b6 100644 --- a/src/conf/virnwfilterbindingobj.c +++ b/src/conf/virnwfilterbindingobj.c @@ -223,7 +223,9 @@ virNWFilterBindingObjParse(const char *filename) return NULL; } - if (!(ret->def = virNWFilterBindingDefParseNode(xml, node))) + ctxt->node = node; + + if (!(ret->def = virNWFilterBindingDefParseXML(ctxt))) return NULL; return g_steal_pointer(&ret); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 11862bb1a9..6e85a8c6cb 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1365,8 +1365,8 @@ virNWFilterBindingDefFormat; virNWFilterBindingDefFormatBuf; virNWFilterBindingDefFree; virNWFilterBindingDefParseFile; -virNWFilterBindingDefParseNode; virNWFilterBindingDefParseString; +virNWFilterBindingDefParseXML; # conf/virnwfilterbindingobj.h -- 2.37.3
Remove the virNWFilterBindingDefParseString/File thin wrappers. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/virnwfilterbindingdef.c | 17 +---------------- src/conf/virnwfilterbindingdef.h | 8 +++----- src/libvirt_private.syms | 3 +-- src/nwfilter/nwfilter_driver.c | 2 +- tests/virnwfilterbindingxml2xmltest.c | 2 +- 5 files changed, 7 insertions(+), 25 deletions(-) diff --git a/src/conf/virnwfilterbindingdef.c b/src/conf/virnwfilterbindingdef.c index 524010c4c4..592731fc1b 100644 --- a/src/conf/virnwfilterbindingdef.c +++ b/src/conf/virnwfilterbindingdef.c @@ -154,7 +154,7 @@ virNWFilterBindingDefParseXML(xmlXPathContextPtr ctxt) } -static virNWFilterBindingDef * +virNWFilterBindingDef * virNWFilterBindingDefParse(const char *xmlStr, const char *filename, unsigned int flags) @@ -171,21 +171,6 @@ virNWFilterBindingDefParse(const char *xmlStr, } -virNWFilterBindingDef * -virNWFilterBindingDefParseString(const char *xmlStr, - unsigned int flags) -{ - return virNWFilterBindingDefParse(xmlStr, NULL, flags); -} - - -virNWFilterBindingDef * -virNWFilterBindingDefParseFile(const char *filename) -{ - return virNWFilterBindingDefParse(NULL, filename, 0); -} - - char * virNWFilterBindingDefFormat(const virNWFilterBindingDef *def) { diff --git a/src/conf/virnwfilterbindingdef.h b/src/conf/virnwfilterbindingdef.h index a5497d5a1c..272ad686a0 100644 --- a/src/conf/virnwfilterbindingdef.h +++ b/src/conf/virnwfilterbindingdef.h @@ -50,11 +50,9 @@ virNWFilterBindingDef * virNWFilterBindingDefParseXML(xmlXPathContextPtr ctxt); virNWFilterBindingDef * -virNWFilterBindingDefParseString(const char *xml, - unsigned int flags); - -virNWFilterBindingDef * -virNWFilterBindingDefParseFile(const char *filename); +virNWFilterBindingDefParse(const char *xmlStr, + const char *filename, + unsigned int flags); char * virNWFilterBindingDefFormat(const virNWFilterBindingDef *def); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6e85a8c6cb..b8165c07d9 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1364,8 +1364,7 @@ virNWFilterBindingDefCopy; virNWFilterBindingDefFormat; virNWFilterBindingDefFormatBuf; virNWFilterBindingDefFree; -virNWFilterBindingDefParseFile; -virNWFilterBindingDefParseString; +virNWFilterBindingDefParse; virNWFilterBindingDefParseXML; diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index 9cb306909c..f9fc09bbd3 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -739,7 +739,7 @@ nwfilterBindingCreateXML(virConnectPtr conn, return NULL; } - def = virNWFilterBindingDefParseString(xml, flags); + def = virNWFilterBindingDefParse(xml, NULL, flags); if (!def) return NULL; diff --git a/tests/virnwfilterbindingxml2xmltest.c b/tests/virnwfilterbindingxml2xmltest.c index 0c93b58a0c..bde21968fe 100644 --- a/tests/virnwfilterbindingxml2xmltest.c +++ b/tests/virnwfilterbindingxml2xmltest.c @@ -41,7 +41,7 @@ testCompareXMLToXMLFiles(const char *xml) virResetLastError(); - if (!(dev = virNWFilterBindingDefParseFile(xml))) + if (!(dev = virNWFilterBindingDefParse(NULL, xml, 0))) goto fail; if (!(actual = virNWFilterBindingDefFormat(dev))) -- 2.37.3
Don't validate the root node and don't allocate a private XPath context when virXMLParse can do that internally. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemuxml2argvtest.c | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 89bed6a46e..8785c96ce1 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -626,7 +626,6 @@ testCompareXMLToArgv(const void *data) qemuDomainObjPrivate *priv = NULL; g_autoptr(xmlDoc) xml = NULL; g_autoptr(xmlXPathContext) ctxt = NULL; - xmlNodePtr root; g_autofree char *archstr = NULL; virArch arch = VIR_ARCH_NONE; g_autoptr(virIdentity) sysident = virIdentityGetSystem(); @@ -673,21 +672,9 @@ testCompareXMLToArgv(const void *data) goto cleanup; if (!(xml = virXMLParse(info->infile, NULL, "(domain_definition)", - NULL, NULL, NULL, false))) + "domain", &ctxt, NULL, false))) goto cleanup; - root = xmlDocGetRootElement(xml); - if (!virXMLNodeNameEqual(root, "domain")) { - VIR_TEST_VERBOSE("unexpected root element <%s>, expecting <domain>", - root->name); - goto cleanup; - } - - if (!(ctxt = virXMLXPathContextNew(xml))) - goto cleanup; - - ctxt->node = root; - if ((archstr = virXPathString("string(./os/type[1]/@arch)", ctxt))) arch = virArchFromString(archstr); @@ -722,7 +709,7 @@ testCompareXMLToArgv(const void *data) parseFlags |= VIR_DOMAIN_DEF_PARSE_INACTIVE; - if (!(vm->def = virDomainDefParseNode(xml, root, driver.xmlopt, NULL, + if (!(vm->def = virDomainDefParseNode(xml, ctxt->node, driver.xmlopt, NULL, parseFlags))) { err = virGetLastError(); if (!err) { -- 2.37.3
The function is exported but used only intenally, additionally everything it did for the only caller can be replaced by properly using virXMLParse. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/virnetworkportdef.c | 32 +++++--------------------------- src/conf/virnetworkportdef.h | 4 ---- src/libvirt_private.syms | 1 - 3 files changed, 5 insertions(+), 32 deletions(-) diff --git a/src/conf/virnetworkportdef.c b/src/conf/virnetworkportdef.c index 40cadc4ae8..651e4c3329 100644 --- a/src/conf/virnetworkportdef.c +++ b/src/conf/virnetworkportdef.c @@ -251,42 +251,20 @@ virNetworkPortDefParseXML(xmlXPathContextPtr ctxt) } -virNetworkPortDef * -virNetworkPortDefParseNode(xmlDocPtr xml, - xmlNodePtr root) -{ - g_autoptr(xmlXPathContext) ctxt = NULL; - - if (STRNEQ((const char *)root->name, "networkport")) { - virReportError(VIR_ERR_XML_ERROR, - "%s", - _("unknown root element for network port")); - return NULL; - } - - if (!(ctxt = virXMLXPathContextNew(xml))) - return NULL; - - ctxt->node = root; - return virNetworkPortDefParseXML(ctxt); -} - - static virNetworkPortDef * virNetworkPortDefParse(const char *xmlStr, const char *filename, unsigned int flags) { - virNetworkPortDef *def = NULL; g_autoptr(xmlDoc) xml = NULL; + g_autoptr(xmlXPathContext) ctxt = NULL; bool validate = flags & VIR_NETWORK_PORT_CREATE_VALIDATE; - if ((xml = virXMLParse(filename, xmlStr, _("(networkport_definition)"), - NULL, NULL, "networkport.rng", validate))) { - def = virNetworkPortDefParseNode(xml, xmlDocGetRootElement(xml)); - } + if (!(xml = virXMLParse(filename, xmlStr, _("(networkport_definition)"), + "networkport", &ctxt, "networkport.rng", validate))) + return NULL; - return def; + return virNetworkPortDefParseXML(ctxt); } diff --git a/src/conf/virnetworkportdef.h b/src/conf/virnetworkportdef.h index c661534046..908a7c8795 100644 --- a/src/conf/virnetworkportdef.h +++ b/src/conf/virnetworkportdef.h @@ -80,10 +80,6 @@ void virNetworkPortDefFree(virNetworkPortDef *port); G_DEFINE_AUTOPTR_CLEANUP_FUNC(virNetworkPortDef, virNetworkPortDefFree); -virNetworkPortDef * -virNetworkPortDefParseNode(xmlDocPtr xml, - xmlNodePtr root); - virNetworkPortDef * virNetworkPortDefParseString(const char *xml, unsigned int flags); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index b8165c07d9..903c4196df 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1327,7 +1327,6 @@ virNetworkPortDefFormat; virNetworkPortDefFormatBuf; virNetworkPortDefFree; virNetworkPortDefParseFile; -virNetworkPortDefParseNode; virNetworkPortDefParseString; virNetworkPortDefSaveStatus; -- 2.37.3
Replace the two helpers virNetworkPortDefParseString/File with the common helper. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 2 +- src/conf/virnetworkobj.c | 2 +- src/conf/virnetworkportdef.c | 17 +---------------- src/conf/virnetworkportdef.h | 8 +++----- src/libvirt_private.syms | 3 +-- src/network/bridge_driver.c | 2 +- tests/virnetworkportxml2xmltest.c | 2 +- 7 files changed, 9 insertions(+), 27 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b3202c9f76..adbd97e632 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -29537,7 +29537,7 @@ virDomainNetCreatePort(virConnectPtr conn, VIR_FREE(portxml); if (!(portxml = virNetworkPortGetXMLDesc(port, 0)) || - !(portdef = virNetworkPortDefParseString(portxml, 0)) || + !(portdef = virNetworkPortDefParse(portxml, NULL, 0)) || virDomainNetDefActualFromNetworkPort(iface, portdef) < 0) { virErrorPreserveLast(&save_err); virNetworkPortDelete(port, 0); diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c index b4c61fff85..7621fa3380 100644 --- a/src/conf/virnetworkobj.c +++ b/src/conf/virnetworkobj.c @@ -1833,7 +1833,7 @@ virNetworkObjLoadAllPorts(virNetworkObj *net, file = g_strdup_printf("%s/%s.xml", dir, de->d_name); - portdef = virNetworkPortDefParseFile(file); + portdef = virNetworkPortDefParse(NULL, file, 0); if (!portdef) { VIR_WARN("Cannot parse port %s", file); continue; diff --git a/src/conf/virnetworkportdef.c b/src/conf/virnetworkportdef.c index 651e4c3329..035e3fe758 100644 --- a/src/conf/virnetworkportdef.c +++ b/src/conf/virnetworkportdef.c @@ -251,7 +251,7 @@ virNetworkPortDefParseXML(xmlXPathContextPtr ctxt) } -static virNetworkPortDef * +virNetworkPortDef * virNetworkPortDefParse(const char *xmlStr, const char *filename, unsigned int flags) @@ -268,21 +268,6 @@ virNetworkPortDefParse(const char *xmlStr, } -virNetworkPortDef * -virNetworkPortDefParseString(const char *xmlStr, - unsigned int flags) -{ - return virNetworkPortDefParse(xmlStr, NULL, flags); -} - - -virNetworkPortDef * -virNetworkPortDefParseFile(const char *filename) -{ - return virNetworkPortDefParse(NULL, filename, 0); -} - - char * virNetworkPortDefFormat(const virNetworkPortDef *def) { diff --git a/src/conf/virnetworkportdef.h b/src/conf/virnetworkportdef.h index 908a7c8795..48e73dbefd 100644 --- a/src/conf/virnetworkportdef.h +++ b/src/conf/virnetworkportdef.h @@ -81,11 +81,9 @@ virNetworkPortDefFree(virNetworkPortDef *port); G_DEFINE_AUTOPTR_CLEANUP_FUNC(virNetworkPortDef, virNetworkPortDefFree); virNetworkPortDef * -virNetworkPortDefParseString(const char *xml, - unsigned int flags); - -virNetworkPortDef * -virNetworkPortDefParseFile(const char *filename); +virNetworkPortDefParse(const char *xmlStr, + const char *filename, + unsigned int flags); char * virNetworkPortDefFormat(const virNetworkPortDef *def); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 903c4196df..3ef3210184 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1326,8 +1326,7 @@ virNetworkObjUpdateAssignDef; virNetworkPortDefFormat; virNetworkPortDefFormatBuf; virNetworkPortDefFree; -virNetworkPortDefParseFile; -virNetworkPortDefParseString; +virNetworkPortDefParse; virNetworkPortDefSaveStatus; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 7c6430b4e3..e4f5e93779 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -4844,7 +4844,7 @@ networkPortCreateXML(virNetworkPtr net, def = virNetworkObjGetDef(obj); - if (!(portdef = virNetworkPortDefParseString(xmldesc, flags))) + if (!(portdef = virNetworkPortDefParse(xmldesc, NULL, flags))) goto cleanup; if (virNetworkPortCreateXMLEnsureACL(net->conn, def, portdef) < 0) diff --git a/tests/virnetworkportxml2xmltest.c b/tests/virnetworkportxml2xmltest.c index 237624a752..21e3c6f697 100644 --- a/tests/virnetworkportxml2xmltest.c +++ b/tests/virnetworkportxml2xmltest.c @@ -38,7 +38,7 @@ testCompareXMLToXMLFiles(const char *expected) g_autofree char *actual = NULL; g_autoptr(virNetworkPortDef) dev = NULL; - if (!(dev = virNetworkPortDefParseFile(expected))) + if (!(dev = virNetworkPortDefParse(NULL, expected, 0))) return -1; if (!(actual = virNetworkPortDefFormat(dev))) -- 2.37.3
Move few variables definitions closer to usage, add comments explaining what's happening and simplify the control flow. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/test/test_driver.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 1c10124564..5740f54a58 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -841,32 +841,34 @@ testDomainObjCheckTaint(virDomainObj *obj) } static xmlNodePtr -testParseXMLDocFromFile(xmlNodePtr node, const char *file, const char *type) +testParseXMLDocFromFile(xmlNodePtr node, + const char *file, + const char *type) { - xmlNodePtr ret = NULL; g_autoptr(xmlDoc) doc = NULL; - g_autofree char *absFile = NULL; g_autofree char *relFile = NULL; if ((relFile = virXMLPropString(node, "file"))) { - absFile = testBuildFilename(file, relFile); + g_autofree char *absFile = testBuildFilename(file, relFile); + xmlNodePtr newnode = NULL; if (!(doc = virXMLParse(absFile, NULL, type, NULL, NULL, NULL, false))) return NULL; - ret = xmlCopyNode(xmlDocGetRootElement(doc), 1); - if (!ret) { + if (!(newnode = xmlCopyNode(xmlDocGetRootElement(doc), 1))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Failed to copy XML node")); return NULL; } - xmlReplaceNode(node, ret); + + /* place 'newnode' in place of 'node' in the original XML document object */ + xmlReplaceNode(node, newnode); + /* free the original node */ xmlFreeNode(node); - } else { - ret = node; + return newnode; } - return ret; + return node; } static int -- 2.37.3
virXMLParse ignores the 'url' argument which is what 'type' was passed to it as when a filename is used as source for the XML. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/test/test_driver.c | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 5740f54a58..26c6c95c28 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -842,8 +842,7 @@ testDomainObjCheckTaint(virDomainObj *obj) static xmlNodePtr testParseXMLDocFromFile(xmlNodePtr node, - const char *file, - const char *type) + const char *file) { g_autoptr(xmlDoc) doc = NULL; g_autofree char *relFile = NULL; @@ -852,7 +851,7 @@ testParseXMLDocFromFile(xmlNodePtr node, g_autofree char *absFile = testBuildFilename(file, relFile); xmlNodePtr newnode = NULL; - if (!(doc = virXMLParse(absFile, NULL, type, NULL, NULL, NULL, false))) + if (!(doc = virXMLParse(absFile, NULL, NULL, NULL, NULL, NULL, false))) return NULL; if (!(newnode = xmlCopyNode(xmlDocGetRootElement(doc), 1))) { @@ -969,8 +968,7 @@ testParseDomainSnapshots(testDriver *privconn, for (i = 0; i < nsdata->num_snap_nodes; i++) { virDomainMomentObj *snap; g_autoptr(virDomainSnapshotDef) def = NULL; - xmlNodePtr node = testParseXMLDocFromFile(nodes[i], file, - "domainsnapshot"); + xmlNodePtr node = testParseXMLDocFromFile(nodes[i], file); if (!node) return -1; @@ -1024,7 +1022,7 @@ testParseDomains(testDriver *privconn, for (i = 0; i < num; i++) { g_autoptr(virDomainDef) def = NULL; testDomainNamespaceDef *nsdata; - xmlNodePtr node = testParseXMLDocFromFile(nodes[i], file, "domain"); + xmlNodePtr node = testParseXMLDocFromFile(nodes[i], file); if (!node) goto error; @@ -1088,7 +1086,7 @@ testParseNetworks(testDriver *privconn, for (i = 0; i < num; i++) { g_autoptr(virNetworkDef) def = NULL; - xmlNodePtr node = testParseXMLDocFromFile(nodes[i], file, "network"); + xmlNodePtr node = testParseXMLDocFromFile(nodes[i], file); if (!node) return -1; @@ -1124,8 +1122,7 @@ testParseInterfaces(testDriver *privconn, for (i = 0; i < num; i++) { g_autoptr(virInterfaceDef) def = NULL; - xmlNodePtr node = testParseXMLDocFromFile(nodes[i], file, - "interface"); + xmlNodePtr node = testParseXMLDocFromFile(nodes[i], file); if (!node) return -1; @@ -1165,8 +1162,7 @@ testOpenVolumesForPool(const char *file, return -1; for (i = 0; i < num; i++) { - xmlNodePtr node = testParseXMLDocFromFile(nodes[i], file, - "volume"); + xmlNodePtr node = testParseXMLDocFromFile(nodes[i], file); if (!node) return -1; @@ -1209,8 +1205,7 @@ testParseStorage(testDriver *privconn, for (i = 0; i < num; i++) { virStoragePoolDef *def; - xmlNodePtr node = testParseXMLDocFromFile(nodes[i], file, - "pool"); + xmlNodePtr node = testParseXMLDocFromFile(nodes[i], file); if (!node) return -1; @@ -1258,8 +1253,8 @@ testParseNodedevs(testDriver *privconn, for (i = 0; i < num; i++) { virNodeDeviceDef *def; - xmlNodePtr node = testParseXMLDocFromFile(nodes[i], file, - "nodedev"); + xmlNodePtr node = testParseXMLDocFromFile(nodes[i], file); + if (!node) return -1; -- 2.37.3
When replacing a definition node by contents of a file the root node in the file must match the replaced node. Enforce that by passing the original node name as the 'rootnode' argument of virXMLParse. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/test/test_driver.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 26c6c95c28..5eae22f591 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -851,7 +851,8 @@ testParseXMLDocFromFile(xmlNodePtr node, g_autofree char *absFile = testBuildFilename(file, relFile); xmlNodePtr newnode = NULL; - if (!(doc = virXMLParse(absFile, NULL, NULL, NULL, NULL, NULL, false))) + if (!(doc = virXMLParse(absFile, NULL, NULL, (const char *) node->name, + NULL, NULL, false))) return NULL; if (!(newnode = xmlCopyNode(xmlDocGetRootElement(doc), 1))) { -- 2.37.3
Replace it by proper use of virXMLParse to validate the root node and allocate the context. The use in the test driver can be directly replaced by virStoragePoolDefParseXML as both are validated. The change to the storage driver isn't trivial though as it requires careful xpath context juggling to parse the nested volumes properly. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/storage_conf.c | 34 ++++++---------------------------- src/conf/storage_conf.h | 4 ---- src/libvirt_private.syms | 1 - src/test/test_driver.c | 19 +++++++------------ 4 files changed, 13 insertions(+), 45 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index d7375a5160..c11f40488d 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -972,43 +972,21 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt) } -virStoragePoolDef * -virStoragePoolDefParseNode(xmlDocPtr xml, - xmlNodePtr root) -{ - g_autoptr(xmlXPathContext) ctxt = NULL; - - if (!virXMLNodeNameEqual(root, "pool")) { - virReportError(VIR_ERR_XML_ERROR, - _("unexpected root element <%s>, " - "expecting <pool>"), - root->name); - return NULL; - } - - if (!(ctxt = virXMLXPathContextNew(xml))) - return NULL; - - ctxt->node = root; - return virStoragePoolDefParseXML(ctxt); -} - - static virStoragePoolDef * virStoragePoolDefParse(const char *xmlStr, const char *filename, unsigned int flags) { - virStoragePoolDef *ret = NULL; g_autoptr(xmlDoc) xml = NULL; + g_autoptr(xmlXPathContext) ctxt = NULL; bool validate = flags & VIR_STORAGE_POOL_DEFINE_VALIDATE; - if ((xml = virXMLParse(filename, xmlStr, _("(storage_pool_definition)"), - NULL, NULL, "storagepool.rng", validate))) { - ret = virStoragePoolDefParseNode(xml, xmlDocGetRootElement(xml)); - } - return ret; + if (!(xml = virXMLParse(filename, xmlStr, _("(storage_pool_definition)"), + "pool", &ctxt, "storagepool.rng", validate))) + return NULL; + + return virStoragePoolDefParseXML(ctxt); } diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index a1bf243935..5eee5b6881 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -278,10 +278,6 @@ virStoragePoolDefParseString(const char *xml, virStoragePoolDef * virStoragePoolDefParseFile(const char *filename); -virStoragePoolDef * -virStoragePoolDefParseNode(xmlDocPtr xml, - xmlNodePtr root); - char * virStoragePoolDefFormat(virStoragePoolDef *def); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3ef3210184..ddc2394fb9 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1054,7 +1054,6 @@ virStoragePartedFsTypeToString; virStoragePoolDefFormat; virStoragePoolDefFree; virStoragePoolDefParseFile; -virStoragePoolDefParseNode; virStoragePoolDefParseSourceString; virStoragePoolDefParseString; virStoragePoolFormatDiskTypeFromString; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 5eae22f591..27cf3ced43 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1145,20 +1145,15 @@ testParseInterfaces(testDriver *privconn, static int testOpenVolumesForPool(const char *file, xmlXPathContextPtr ctxt, - virStoragePoolObj *obj, - int objidx) + virStoragePoolObj *obj) { virStoragePoolDef *def = virStoragePoolObjGetDef(obj); size_t i; int num; - g_autofree char *vol_xpath = NULL; g_autofree xmlNodePtr *nodes = NULL; g_autoptr(virStorageVolDef) volDef = NULL; - /* Find storage volumes */ - vol_xpath = g_strdup_printf("/node/pool[%d]/volume", objidx); - - num = virXPathNodeSet(vol_xpath, ctxt, &nodes); + num = virXPathNodeSet("/pool/volume", ctxt, &nodes); if (num < 0) return -1; @@ -1195,6 +1190,7 @@ testParseStorage(testDriver *privconn, const char *file, xmlXPathContextPtr ctxt) { + VIR_XPATH_NODE_AUTORESTORE(ctxt) int num; size_t i; virStoragePoolObj *obj; @@ -1206,12 +1202,11 @@ testParseStorage(testDriver *privconn, for (i = 0; i < num; i++) { virStoragePoolDef *def; - xmlNodePtr node = testParseXMLDocFromFile(nodes[i], file); - if (!node) + + if (!(ctxt->node = testParseXMLDocFromFile(nodes[i], file))) return -1; - def = virStoragePoolDefParseNode(ctxt->doc, node); - if (!def) + if (!(def = virStoragePoolDefParseXML(ctxt))) return -1; if (!(obj = virStoragePoolObjListAdd(privconn->pools, &def, 0))) { @@ -1226,7 +1221,7 @@ testParseStorage(testDriver *privconn, virStoragePoolObjSetActive(obj, true); /* Find storage volumes */ - if (testOpenVolumesForPool(file, ctxt, obj, i+1) < 0) { + if (testOpenVolumesForPool(file, ctxt, obj) < 0) { virStoragePoolObjEndAPI(&obj); return -1; } -- 2.37.3
Replace the virStoragePoolDefParseString/File thin wrappers by virStoragePoolDefParse. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 2 +- src/conf/storage_conf.c | 17 +---------------- src/conf/storage_conf.h | 8 +++----- src/conf/virstorageobj.c | 2 +- src/libvirt_private.syms | 3 +-- src/storage/storage_driver.c | 4 ++-- src/test/test_driver.c | 4 ++-- tests/storagepoolxml2argvtest.c | 2 +- tests/storagepoolxml2xmltest.c | 2 +- tests/storagevolxml2argvtest.c | 4 ++-- tests/storagevolxml2xmltest.c | 2 +- 11 files changed, 16 insertions(+), 34 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index adbd97e632..2a3ea9641f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -29914,7 +29914,7 @@ virDomainStorageSourceTranslateSourcePool(virStorageSource *src, if (!(poolxml = virStoragePoolGetXMLDesc(pool, 0))) return -1; - if (!(pooldef = virStoragePoolDefParseString(poolxml, 0))) + if (!(pooldef = virStoragePoolDefParse(poolxml, NULL, 0))) return -1; src->srcpool->pooltype = pooldef->type; diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index c11f40488d..0c095c73e0 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -972,7 +972,7 @@ virStoragePoolDefParseXML(xmlXPathContextPtr ctxt) } -static virStoragePoolDef * +virStoragePoolDef * virStoragePoolDefParse(const char *xmlStr, const char *filename, unsigned int flags) @@ -990,21 +990,6 @@ virStoragePoolDefParse(const char *xmlStr, } -virStoragePoolDef * -virStoragePoolDefParseString(const char *xmlStr, - unsigned int flags) -{ - return virStoragePoolDefParse(xmlStr, NULL, flags); -} - - -virStoragePoolDef * -virStoragePoolDefParseFile(const char *filename) -{ - return virStoragePoolDefParse(NULL, filename, 0); -} - - static int virStoragePoolSourceFormat(virBuffer *buf, virStoragePoolOptions *options, diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 5eee5b6881..ead54ca7c6 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -272,11 +272,9 @@ virStoragePoolDef * virStoragePoolDefParseXML(xmlXPathContextPtr ctxt); virStoragePoolDef * -virStoragePoolDefParseString(const char *xml, - unsigned int flags); - -virStoragePoolDef * -virStoragePoolDefParseFile(const char *filename); +virStoragePoolDefParse(const char *xmlStr, + const char *filename, + unsigned int flags); char * virStoragePoolDefFormat(virStoragePoolDef *def); diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c index e1465c8376..77128a4846 100644 --- a/src/conf/virstorageobj.c +++ b/src/conf/virstorageobj.c @@ -1579,7 +1579,7 @@ virStoragePoolObjLoad(virStoragePoolObjList *pools, VIR_DEBUG("loading storage pool config XML '%s'", path); - if (!(def = virStoragePoolDefParseFile(path))) + if (!(def = virStoragePoolDefParse(NULL, path, 0))) return NULL; if (!virStringMatchesNameSuffix(file, def->name, ".xml")) { diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ddc2394fb9..8fec8d9027 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1053,9 +1053,8 @@ virStoragePoolCapsNew; virStoragePartedFsTypeToString; virStoragePoolDefFormat; virStoragePoolDefFree; -virStoragePoolDefParseFile; +virStoragePoolDefParse; virStoragePoolDefParseSourceString; -virStoragePoolDefParseString; virStoragePoolFormatDiskTypeFromString; virStoragePoolFormatDiskTypeToString; virStoragePoolFormatFileSystemNetTypeToString; diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index fccf0fcf52..aefe638083 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -721,7 +721,7 @@ storagePoolCreateXML(virConnectPtr conn, VIR_EXCLUSIVE_FLAGS_RET(VIR_STORAGE_POOL_BUILD_OVERWRITE, VIR_STORAGE_POOL_BUILD_NO_OVERWRITE, NULL); - if (!(newDef = virStoragePoolDefParseString(xml, 0))) + if (!(newDef = virStoragePoolDefParse(xml, NULL, 0))) goto cleanup; if (virStoragePoolCreateXMLEnsureACL(conn, newDef) < 0) @@ -801,7 +801,7 @@ storagePoolDefineXML(virConnectPtr conn, virCheckFlags(VIR_STORAGE_POOL_DEFINE_VALIDATE, NULL); - if (!(newDef = virStoragePoolDefParseString(xml, flags))) + if (!(newDef = virStoragePoolDefParse(xml, NULL, flags))) goto cleanup; if (virXMLCheckIllegalChars("name", newDef->name, "\n") < 0) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 27cf3ced43..4245b32c15 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -6631,7 +6631,7 @@ testStoragePoolCreateXML(virConnectPtr conn, virCheckFlags(0, NULL); virObjectLock(privconn); - if (!(newDef = virStoragePoolDefParseString(xml, 0))) + if (!(newDef = virStoragePoolDefParse(xml, NULL, 0))) goto cleanup; if (!(obj = virStoragePoolObjListAdd(privconn->pools, &newDef, @@ -6693,7 +6693,7 @@ testStoragePoolDefineXML(virConnectPtr conn, virCheckFlags(VIR_STORAGE_POOL_DEFINE_VALIDATE, NULL); virObjectLock(privconn); - if (!(newDef = virStoragePoolDefParseString(xml, flags))) + if (!(newDef = virStoragePoolDefParse(xml, NULL, flags))) goto cleanup; newDef->capacity = defaultPoolCap; diff --git a/tests/storagepoolxml2argvtest.c b/tests/storagepoolxml2argvtest.c index 04ff45f98d..e8e40d695e 100644 --- a/tests/storagepoolxml2argvtest.c +++ b/tests/storagepoolxml2argvtest.c @@ -27,7 +27,7 @@ testCompareXMLToArgvFiles(bool shouldFail, g_autofree char *src = NULL; g_autoptr(virCommand) cmd = NULL; - if (!(def = virStoragePoolDefParseFile(poolxml))) + if (!(def = virStoragePoolDefParse(NULL, poolxml, 0))) goto cleanup; defTypeStr = virStoragePoolTypeToString(def->type); diff --git a/tests/storagepoolxml2xmltest.c b/tests/storagepoolxml2xmltest.c index 76d6f54b33..6a48594197 100644 --- a/tests/storagepoolxml2xmltest.c +++ b/tests/storagepoolxml2xmltest.c @@ -19,7 +19,7 @@ testCompareXMLToXMLFiles(const char *inxml, const char *outxml) g_autofree char *actual = NULL; g_autoptr(virStoragePoolDef) dev = NULL; - if (!(dev = virStoragePoolDefParseFile(inxml))) + if (!(dev = virStoragePoolDefParse(NULL, inxml, 0))) return -1; if (!(actual = virStoragePoolDefFormat(dev))) diff --git a/tests/storagevolxml2argvtest.c b/tests/storagevolxml2argvtest.c index 6f1fc7b38c..22b4cda5c5 100644 --- a/tests/storagevolxml2argvtest.c +++ b/tests/storagevolxml2argvtest.c @@ -48,7 +48,7 @@ testCompareXMLToArgvFiles(bool shouldFail, g_autoptr(virStorageVolDef) inputvol = NULL; g_autoptr(virStoragePoolDef) inputpool = NULL; - if (!(def = virStoragePoolDefParseFile(poolxml))) + if (!(def = virStoragePoolDefParse(NULL, poolxml, 0))) goto cleanup; if (!(obj = virStoragePoolObjNew())) { @@ -58,7 +58,7 @@ testCompareXMLToArgvFiles(bool shouldFail, virStoragePoolObjSetDef(obj, def); if (inputpoolxml) { - if (!(inputpool = virStoragePoolDefParseFile(inputpoolxml))) + if (!(inputpool = virStoragePoolDefParse(NULL, inputpoolxml, 0))) goto cleanup; } diff --git a/tests/storagevolxml2xmltest.c b/tests/storagevolxml2xmltest.c index 161cb7c6bc..7b229ac52a 100644 --- a/tests/storagevolxml2xmltest.c +++ b/tests/storagevolxml2xmltest.c @@ -19,7 +19,7 @@ testCompareXMLToXMLFiles(const char *poolxml, const char *inxml, g_autoptr(virStoragePoolDef) pool = NULL; g_autoptr(virStorageVolDef) dev = NULL; - if (!(pool = virStoragePoolDefParseFile(poolxml))) + if (!(pool = virStoragePoolDefParse(NULL, poolxml, 0))) return -1; if (!(dev = virStorageVolDefParseFile(pool, inxml, flags))) -- 2.37.3
Proper use of virXMLParse replaces everything the function provides. Callers can use virStorageVolDefParseXML instead. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/storage_conf.c | 37 ++++++------------------------------- src/conf/storage_conf.h | 7 +++---- src/libvirt_private.syms | 2 +- src/test/test_driver.c | 6 +++--- 4 files changed, 13 insertions(+), 39 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 0c095c73e0..703149241a 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1234,7 +1234,7 @@ virStorageCheckCompat(const char *compat) } -static virStorageVolDef * +virStorageVolDef * virStorageVolDefParseXML(virStoragePoolDef *pool, xmlXPathContextPtr ctxt, unsigned int flags) @@ -1401,45 +1401,20 @@ virStorageVolDefParseXML(virStoragePoolDef *pool, } -virStorageVolDef * -virStorageVolDefParseNode(virStoragePoolDef *pool, - xmlDocPtr xml, - xmlNodePtr root, - unsigned int flags) -{ - g_autoptr(xmlXPathContext) ctxt = NULL; - - if (!virXMLNodeNameEqual(root, "volume")) { - virReportError(VIR_ERR_XML_ERROR, - _("unexpected root element <%s>, " - "expecting <volume>"), - root->name); - return NULL; - } - - if (!(ctxt = virXMLXPathContextNew(xml))) - return NULL; - - ctxt->node = root; - return virStorageVolDefParseXML(pool, ctxt, flags); -} - - static virStorageVolDef * virStorageVolDefParse(virStoragePoolDef *pool, const char *xmlStr, const char *filename, unsigned int flags) { - virStorageVolDef *ret = NULL; g_autoptr(xmlDoc) xml = NULL; + g_autoptr(xmlXPathContext) ctxt = NULL; - if ((xml = virXMLParse(filename, xmlStr, _("(storage_volume_definition)"), - NULL, NULL, NULL, false))) { - ret = virStorageVolDefParseNode(pool, xml, xmlDocGetRootElement(xml), flags); - } + if (!(xml = virXMLParse(filename, xmlStr, _("(storage_volume_definition)"), + "volume", &ctxt, NULL, false))) + return NULL; - return ret; + return virStorageVolDefParseXML(pool, ctxt, flags); } diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index ead54ca7c6..14167af761 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -297,10 +297,9 @@ virStorageVolDefParseFile(virStoragePoolDef *pool, unsigned int flags); virStorageVolDef * -virStorageVolDefParseNode(virStoragePoolDef *pool, - xmlDocPtr xml, - xmlNodePtr root, - unsigned int flags); +virStorageVolDefParseXML(virStoragePoolDef *pool, + xmlXPathContextPtr ctxt, + unsigned int flags); char * virStorageVolDefFormat(virStoragePoolDef *pool, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 8fec8d9027..3191c9d5f7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1079,8 +1079,8 @@ virStorageVolDefFindByPath; virStorageVolDefFormat; virStorageVolDefFree; virStorageVolDefParseFile; -virStorageVolDefParseNode; virStorageVolDefParseString; +virStorageVolDefParseXML; virStorageVolDefRefreshAllocationTypeFromString; virStorageVolDefRefreshAllocationTypeToString; virStorageVolTypeFromString; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 4245b32c15..943c0834cd 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1147,6 +1147,7 @@ testOpenVolumesForPool(const char *file, xmlXPathContextPtr ctxt, virStoragePoolObj *obj) { + VIR_XPATH_NODE_AUTORESTORE(ctxt) virStoragePoolDef *def = virStoragePoolObjGetDef(obj); size_t i; int num; @@ -1158,11 +1159,10 @@ testOpenVolumesForPool(const char *file, return -1; for (i = 0; i < num; i++) { - xmlNodePtr node = testParseXMLDocFromFile(nodes[i], file); - if (!node) + if (!(ctxt->node = testParseXMLDocFromFile(nodes[i], file))) return -1; - if (!(volDef = virStorageVolDefParseNode(def, ctxt->doc, node, 0))) + if (!(volDef = virStorageVolDefParseXML(def, ctxt, 0))) return -1; if (!volDef->target.path) { -- 2.37.3
Remove the virStorageVolDefParseFile/String shim functions. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/storage_conf.c | 20 +------------------- src/conf/storage_conf.h | 12 ++++-------- src/esx/esx_storage_backend_vmfs.c | 4 ++-- src/libvirt_private.syms | 3 +-- src/storage/storage_driver.c | 6 ++---- src/test/test_driver.c | 4 ++-- src/vbox/vbox_storage.c | 2 +- tests/storagevolxml2argvtest.c | 4 ++-- tests/storagevolxml2xmltest.c | 2 +- 9 files changed, 16 insertions(+), 41 deletions(-) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 703149241a..4f2f9e7fb1 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1401,7 +1401,7 @@ virStorageVolDefParseXML(virStoragePoolDef *pool, } -static virStorageVolDef * +virStorageVolDef * virStorageVolDefParse(virStoragePoolDef *pool, const char *xmlStr, const char *filename, @@ -1418,24 +1418,6 @@ virStorageVolDefParse(virStoragePoolDef *pool, } -virStorageVolDef * -virStorageVolDefParseString(virStoragePoolDef *pool, - const char *xmlStr, - unsigned int flags) -{ - return virStorageVolDefParse(pool, xmlStr, NULL, flags); -} - - -virStorageVolDef * -virStorageVolDefParseFile(virStoragePoolDef *pool, - const char *filename, - unsigned int flags) -{ - return virStorageVolDefParse(pool, NULL, filename, flags); -} - - static void virStorageVolTimestampFormat(virBuffer *buf, const char *name, struct timespec *ts) diff --git a/src/conf/storage_conf.h b/src/conf/storage_conf.h index 14167af761..bbfdbc2f2f 100644 --- a/src/conf/storage_conf.h +++ b/src/conf/storage_conf.h @@ -287,14 +287,10 @@ typedef enum { } virStorageVolDefParseFlags; virStorageVolDef * -virStorageVolDefParseString(virStoragePoolDef *pool, - const char *xml, - unsigned int flags); - -virStorageVolDef * -virStorageVolDefParseFile(virStoragePoolDef *pool, - const char *filename, - unsigned int flags); +virStorageVolDefParse(virStoragePoolDef *pool, + const char *xmlStr, + const char *filename, + unsigned int flags); virStorageVolDef * virStorageVolDefParseXML(virStoragePoolDef *pool, diff --git a/src/esx/esx_storage_backend_vmfs.c b/src/esx/esx_storage_backend_vmfs.c index c718597d72..4efcc2fc75 100644 --- a/src/esx/esx_storage_backend_vmfs.c +++ b/src/esx/esx_storage_backend_vmfs.c @@ -861,7 +861,7 @@ esxStorageVolCreateXML(virStoragePoolPtr pool, } /* Parse config */ - def = virStorageVolDefParseString(&poolDef, xmldesc, 0); + def = virStorageVolDefParse(&poolDef, xmldesc, NULL, 0); if (!def) goto cleanup; @@ -1063,7 +1063,7 @@ esxStorageVolCreateXMLFrom(virStoragePoolPtr pool, sourceVolume->name); /* Parse config */ - def = virStorageVolDefParseString(&poolDef, xmldesc, 0); + def = virStorageVolDefParse(&poolDef, xmldesc, NULL, 0); if (!def) goto cleanup; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3191c9d5f7..c1ecd1ab04 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1078,8 +1078,7 @@ virStorageVolDefFindByName; virStorageVolDefFindByPath; virStorageVolDefFormat; virStorageVolDefFree; -virStorageVolDefParseFile; -virStorageVolDefParseString; +virStorageVolDefParse; virStorageVolDefParseXML; virStorageVolDefRefreshAllocationTypeFromString; virStorageVolDefRefreshAllocationTypeToString; diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index aefe638083..c25d9ca1ad 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1893,8 +1893,7 @@ storageVolCreateXML(virStoragePoolPtr pool, if ((backend = virStorageBackendForType(def->type)) == NULL) goto cleanup; - voldef = virStorageVolDefParseString(def, xmldesc, - VIR_VOL_XML_PARSE_OPT_CAPACITY); + voldef = virStorageVolDefParse(def, xmldesc, NULL, VIR_VOL_XML_PARSE_OPT_CAPACITY); if (voldef == NULL) goto cleanup; @@ -2067,8 +2066,7 @@ storageVolCreateXMLFrom(virStoragePoolPtr pool, goto cleanup; } - voldef = virStorageVolDefParseString(def, xmldesc, - VIR_VOL_XML_PARSE_NO_CAPACITY); + voldef = virStorageVolDefParse(def, xmldesc, NULL, VIR_VOL_XML_PARSE_NO_CAPACITY); if (voldef == NULL) goto cleanup; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 943c0834cd..5a4e240d35 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -7198,7 +7198,7 @@ testStorageVolCreateXML(virStoragePoolPtr pool, return NULL; def = virStoragePoolObjGetDef(obj); - privvol = virStorageVolDefParseString(def, xmldesc, 0); + privvol = virStorageVolDefParse(def, xmldesc, NULL, 0); if (privvol == NULL) goto cleanup; @@ -7257,7 +7257,7 @@ testStorageVolCreateXMLFrom(virStoragePoolPtr pool, return NULL; def = virStoragePoolObjGetDef(obj); - privvol = virStorageVolDefParseString(def, xmldesc, 0); + privvol = virStorageVolDefParse(def, xmldesc, NULL, 0); if (privvol == NULL) goto cleanup; diff --git a/src/vbox/vbox_storage.c b/src/vbox/vbox_storage.c index be609033e1..7d1cee562f 100644 --- a/src/vbox/vbox_storage.c +++ b/src/vbox/vbox_storage.c @@ -423,7 +423,7 @@ vboxStorageVolCreateXML(virStoragePoolPtr pool, memset(&poolDef, 0, sizeof(poolDef)); poolDef.type = VIR_STORAGE_POOL_DIR; - if ((def = virStorageVolDefParseString(&poolDef, xml, 0)) == NULL) + if ((def = virStorageVolDefParse(&poolDef, xml, NULL, 0)) == NULL) goto cleanup; if (!def->name || diff --git a/tests/storagevolxml2argvtest.c b/tests/storagevolxml2argvtest.c index 22b4cda5c5..0b7848d9ea 100644 --- a/tests/storagevolxml2argvtest.c +++ b/tests/storagevolxml2argvtest.c @@ -65,11 +65,11 @@ testCompareXMLToArgvFiles(bool shouldFail, if (inputvolxml) parse_flags |= VIR_VOL_XML_PARSE_NO_CAPACITY; - if (!(vol = virStorageVolDefParseFile(def, volxml, parse_flags))) + if (!(vol = virStorageVolDefParse(def, NULL, volxml, parse_flags))) goto cleanup; if (inputvolxml && - !(inputvol = virStorageVolDefParseFile(inputpool, inputvolxml, 0))) + !(inputvol = virStorageVolDefParse(inputpool, NULL, inputvolxml, 0))) goto cleanup; testSetVolumeType(vol, def); diff --git a/tests/storagevolxml2xmltest.c b/tests/storagevolxml2xmltest.c index 7b229ac52a..3ea54e6a7a 100644 --- a/tests/storagevolxml2xmltest.c +++ b/tests/storagevolxml2xmltest.c @@ -22,7 +22,7 @@ testCompareXMLToXMLFiles(const char *poolxml, const char *inxml, if (!(pool = virStoragePoolDefParse(NULL, poolxml, 0))) return -1; - if (!(dev = virStorageVolDefParseFile(pool, inxml, flags))) + if (!(dev = virStorageVolDefParse(pool, NULL, inxml, flags))) return -1; if (!(actual = virStorageVolDefFormat(pool, dev))) -- 2.37.3
Check the root XML node name and fetch XPath context by properly configuring virXMLParse. Callers can use virDomainSnapshotDefParse instead. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/snapshot_conf.c | 40 +++++++++------------------------------- src/conf/snapshot_conf.h | 13 +++++++------ src/test/test_driver.c | 18 ++++++++---------- 3 files changed, 24 insertions(+), 47 deletions(-) diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index a5974053f4..afdc11876d 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -197,7 +197,7 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node, * If flags does not include * VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL, then current is ignored. */ -static virDomainSnapshotDef * +virDomainSnapshotDef * virDomainSnapshotDefParse(xmlXPathContextPtr ctxt, virDomainXMLOption *xmlopt, void *parseOpaque, @@ -389,27 +389,6 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt, return g_steal_pointer(&def); } -virDomainSnapshotDef * -virDomainSnapshotDefParseNode(xmlDocPtr xml, - xmlNodePtr root, - virDomainXMLOption *xmlopt, - void *parseOpaque, - bool *current, - unsigned int flags) -{ - g_autoptr(xmlXPathContext) ctxt = NULL; - - if (!virXMLNodeNameEqual(root, "domainsnapshot")) { - virReportError(VIR_ERR_XML_ERROR, "%s", _("domainsnapshot")); - return NULL; - } - - if (!(ctxt = virXMLXPathContextNew(xml))) - return NULL; - - ctxt->node = root; - return virDomainSnapshotDefParse(ctxt, xmlopt, parseOpaque, current, flags); -} virDomainSnapshotDef * virDomainSnapshotDefParseString(const char *xmlStr, @@ -418,21 +397,20 @@ virDomainSnapshotDefParseString(const char *xmlStr, bool *current, unsigned int flags) { - virDomainSnapshotDef *ret = NULL; + g_autoptr(xmlXPathContext) ctxt = NULL; g_autoptr(xmlDoc) xml = NULL; int keepBlanksDefault = xmlKeepBlanksDefault(0); bool validate = flags & VIR_DOMAIN_SNAPSHOT_PARSE_VALIDATE; - if ((xml = virXMLParse(NULL, xmlStr, _("(domain_snapshot)"), - NULL, NULL, "domainsnapshot.rng", validate))) { - xmlKeepBlanksDefault(keepBlanksDefault); - ret = virDomainSnapshotDefParseNode(xml, xmlDocGetRootElement(xml), - xmlopt, parseOpaque, - current, flags); - } + xml = virXMLParse(NULL, xmlStr, _("(domain_snapshot)"), + "domainsnapshot", &ctxt, "domainsnapshot.rng", validate); + xmlKeepBlanksDefault(keepBlanksDefault); - return ret; + if (!xml) + return NULL; + + return virDomainSnapshotDefParse(ctxt, xmlopt, parseOpaque, current, flags); } diff --git a/src/conf/snapshot_conf.h b/src/conf/snapshot_conf.h index 1f787f1a94..fec4a5a912 100644 --- a/src/conf/snapshot_conf.h +++ b/src/conf/snapshot_conf.h @@ -102,12 +102,13 @@ virDomainSnapshotDef *virDomainSnapshotDefParseString(const char *xmlStr, void *parseOpaque, bool *current, unsigned int flags); -virDomainSnapshotDef *virDomainSnapshotDefParseNode(xmlDocPtr xml, - xmlNodePtr root, - virDomainXMLOption *xmlopt, - void *parseOpaque, - bool *current, - unsigned int flags); +virDomainSnapshotDef * +virDomainSnapshotDefParse(xmlXPathContextPtr ctxt, + virDomainXMLOption *xmlopt, + void *parseOpaque, + bool *current, + unsigned int flags); + virDomainSnapshotDef *virDomainSnapshotDefNew(void); char *virDomainSnapshotDefFormat(const char *uuidstr, virDomainSnapshotDef *def, diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 5a4e240d35..50c8a7e2be 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -961,25 +961,23 @@ testParseDomainSnapshots(testDriver *privconn, const char *file, xmlXPathContextPtr ctxt) { + VIR_XPATH_NODE_AUTORESTORE(ctxt) size_t i; testDomainNamespaceDef *nsdata = domobj->def->namespaceData; xmlNodePtr *nodes = nsdata->snap_nodes; - bool cur; for (i = 0; i < nsdata->num_snap_nodes; i++) { virDomainMomentObj *snap; g_autoptr(virDomainSnapshotDef) def = NULL; - xmlNodePtr node = testParseXMLDocFromFile(nodes[i], file); - if (!node) + unsigned int parseFlags = VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL | + VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE; + bool cur; + + if (!(ctxt->node = testParseXMLDocFromFile(nodes[i], file))) return -1; - def = virDomainSnapshotDefParseNode(ctxt->doc, node, - privconn->xmlopt, - NULL, - &cur, - VIR_DOMAIN_SNAPSHOT_PARSE_INTERNAL | - VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE); - if (!def) + if (!(def = virDomainSnapshotDefParse(ctxt, privconn->xmlopt, NULL, + &cur, parseFlags))) return -1; if (!(snap = virDomainSnapshotAssignDef(domobj->snapshots, &def))) -- 2.37.3
Rename it to virSecretParseXML and move the root node validation and context fetching into the caller (by properly calling virXMLParse). Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/secret_conf.c | 29 ++++++++--------------------- 1 file changed, 8 insertions(+), 21 deletions(-) diff --git a/src/conf/secret_conf.c b/src/conf/secret_conf.c index 02c2e38964..d41d8157cd 100644 --- a/src/conf/secret_conf.c +++ b/src/conf/secret_conf.c @@ -123,28 +123,15 @@ virSecretDefParseUsage(xmlXPathContextPtr ctxt, return 0; } + static virSecretDef * -secretXMLParseNode(xmlDocPtr xml, xmlNodePtr root) +virSecretParseXML(xmlXPathContext *ctxt) { - g_autoptr(xmlXPathContext) ctxt = NULL; g_autoptr(virSecretDef) def = NULL; g_autofree char *ephemeralstr = NULL; g_autofree char *privatestr = NULL; g_autofree char *uuidstr = NULL; - if (!virXMLNodeNameEqual(root, "secret")) { - virReportError(VIR_ERR_XML_ERROR, - _("unexpected root element <%s>, " - "expecting <secret>"), - root->name); - return NULL; - } - - if (!(ctxt = virXMLXPathContextNew(xml))) - return NULL; - - ctxt->node = root; - def = g_new0(virSecretDef, 1); if ((ephemeralstr = virXPathString("string(./@ephemeral)", ctxt))) { @@ -186,21 +173,21 @@ secretXMLParseNode(xmlDocPtr xml, xmlNodePtr root) return g_steal_pointer(&def); } + static virSecretDef * virSecretDefParse(const char *xmlStr, const char *filename, unsigned int flags) { g_autoptr(xmlDoc) xml = NULL; - virSecretDef *ret = NULL; + g_autoptr(xmlXPathContext) ctxt = NULL; bool validate = flags & VIR_SECRET_DEFINE_VALIDATE; - if ((xml = virXMLParse(filename, xmlStr, _("(definition_of_secret)"), - NULL, NULL, "secret.rng", validate))) { - ret = secretXMLParseNode(xml, xmlDocGetRootElement(xml)); - } + if (!(xml = virXMLParse(filename, xmlStr, _("(definition_of_secret)"), + "secret", &ctxt, "secret.rng", validate))) + return NULL; - return ret; + return virSecretParseXML(ctxt); } virSecretDef * -- 2.37.3
Replace the virSecretDefParseFile/String shims by calls to virSecretDefParse. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/secret_conf.c | 14 +------------- src/conf/secret_conf.h | 7 +++++-- src/conf/virsecretobj.c | 2 +- src/libvirt_private.syms | 3 +-- src/secret/secret_driver.c | 2 +- tests/secretxml2xmltest.c | 2 +- 6 files changed, 10 insertions(+), 20 deletions(-) diff --git a/src/conf/secret_conf.c b/src/conf/secret_conf.c index d41d8157cd..629a4a57f7 100644 --- a/src/conf/secret_conf.c +++ b/src/conf/secret_conf.c @@ -174,7 +174,7 @@ virSecretParseXML(xmlXPathContext *ctxt) } -static virSecretDef * +virSecretDef * virSecretDefParse(const char *xmlStr, const char *filename, unsigned int flags) @@ -190,18 +190,6 @@ virSecretDefParse(const char *xmlStr, return virSecretParseXML(ctxt); } -virSecretDef * -virSecretDefParseString(const char *xmlStr, - unsigned int flags) -{ - return virSecretDefParse(xmlStr, NULL, flags); -} - -virSecretDef * -virSecretDefParseFile(const char *filename) -{ - return virSecretDefParse(NULL, filename, 0); -} static int virSecretDefFormatUsage(virBuffer *buf, diff --git a/src/conf/secret_conf.h b/src/conf/secret_conf.h index 36d50407fd..dee98899ac 100644 --- a/src/conf/secret_conf.h +++ b/src/conf/secret_conf.h @@ -35,8 +35,11 @@ struct _virSecretDef { void virSecretDefFree(virSecretDef *def); G_DEFINE_AUTOPTR_CLEANUP_FUNC(virSecretDef, virSecretDefFree); -virSecretDef *virSecretDefParseString(const char *xml, unsigned int flags); -virSecretDef *virSecretDefParseFile(const char *filename); +virSecretDef * +virSecretDefParse(const char *xmlStr, + const char *filename, + unsigned int flags); + char *virSecretDefFormat(const virSecretDef *def); #define VIR_CONNECT_LIST_SECRETS_FILTERS_EPHEMERAL \ diff --git a/src/conf/virsecretobj.c b/src/conf/virsecretobj.c index 4032cd1e9a..4929f74155 100644 --- a/src/conf/virsecretobj.c +++ b/src/conf/virsecretobj.c @@ -870,7 +870,7 @@ virSecretLoad(virSecretObjList *secrets, virSecretDef *def = NULL; virSecretObj *obj = NULL; - if (!(def = virSecretDefParseFile(path))) + if (!(def = virSecretDefParse(NULL, path, 0))) goto cleanup; if (virSecretLoadValidateUUID(def, file) < 0) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index c1ecd1ab04..6334cbb448 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1010,8 +1010,7 @@ virObjectEventStateQueue; # conf/secret_conf.h virSecretDefFormat; virSecretDefFree; -virSecretDefParseFile; -virSecretDefParseString; +virSecretDefParse; virSecretUsageTypeFromString; virSecretUsageTypeToString; diff --git a/src/secret/secret_driver.c b/src/secret/secret_driver.c index 6328589fa4..bd981a8ace 100644 --- a/src/secret/secret_driver.c +++ b/src/secret/secret_driver.c @@ -207,7 +207,7 @@ secretDefineXML(virConnectPtr conn, virCheckFlags(VIR_SECRET_DEFINE_VALIDATE, NULL); - if (!(def = virSecretDefParseString(xml, flags))) + if (!(def = virSecretDefParse(xml, NULL, flags))) return NULL; if (virSecretDefineXMLEnsureACL(conn, def) < 0) diff --git a/tests/secretxml2xmltest.c b/tests/secretxml2xmltest.c index 828e44a32b..eb4d3e143c 100644 --- a/tests/secretxml2xmltest.c +++ b/tests/secretxml2xmltest.c @@ -13,7 +13,7 @@ testCompareXMLToXMLFiles(const char *inxml, const char *outxml) g_autofree char *actual = NULL; g_autoptr(virSecretDef) secret = NULL; - if (!(secret = virSecretDefParseFile(inxml))) + if (!(secret = virSecretDefParse(NULL, inxml, 0))) return -1; if (!(actual = virSecretDefFormat(secret))) -- 2.37.3
Use features of virXMLParse to validate root node and fetch XPath context. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/vz/vz_sdk.c | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index 8fb7a9948d..8cd3348f5c 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -4566,7 +4566,6 @@ prlsdkParseSnapshotTree(const char *treexml) virDomainSnapshotObjList *ret = NULL; g_autoptr(xmlDoc) xml = NULL; g_autoptr(xmlXPathContext) ctxt = NULL; - xmlNodePtr root; xmlNodePtr *nodes = NULL; virDomainSnapshotDef *def = NULL; virDomainMomentObj *snapshot; @@ -4582,21 +4581,9 @@ prlsdkParseSnapshotTree(const char *treexml) return snapshots; if (!(xml = virXMLParse(NULL, treexml, _("(snapshot_tree)"), - NULL, NULL, NULL, false))) + "ParallelsSavedStates", &ctxt, NULL, false))) goto cleanup; - root = xmlDocGetRootElement(xml); - if (!virXMLNodeNameEqual(root, "ParallelsSavedStates")) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unexpected root element: '%s'"), root->name); - goto cleanup; - } - - if (!(ctxt = virXMLXPathContextNew(xml))) - goto cleanup; - - ctxt->node = root; - if ((n = virXPathNodeSet("//SavedStateItem", ctxt, &nodes)) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("cannot extract snapshot nodes")); @@ -4604,7 +4591,7 @@ prlsdkParseSnapshotTree(const char *treexml) } for (i = 0; i < n; i++) { - if (nodes[i]->parent == root) + if (nodes[i]->parent == xmlDocGetRootElement(xml)) continue; def = g_new0(virDomainSnapshotDef, 1); -- 2.37.3
Replace all it does by properly using virXMLParse. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/checkpoint_conf.c | 36 ++++++++---------------------------- 1 file changed, 8 insertions(+), 28 deletions(-) diff --git a/src/conf/checkpoint_conf.c b/src/conf/checkpoint_conf.c index 0d2d2050da..ea20bfdd14 100644 --- a/src/conf/checkpoint_conf.c +++ b/src/conf/checkpoint_conf.c @@ -181,26 +181,6 @@ virDomainCheckpointDefParse(xmlXPathContextPtr ctxt, return ret; } -static virDomainCheckpointDef * -virDomainCheckpointDefParseNode(xmlDocPtr xml, - xmlNodePtr root, - virDomainXMLOption *xmlopt, - void *parseOpaque, - unsigned int flags) -{ - g_autoptr(xmlXPathContext) ctxt = NULL; - - if (!virXMLNodeNameEqual(root, "domaincheckpoint")) { - virReportError(VIR_ERR_XML_ERROR, "%s", _("domaincheckpoint")); - return NULL; - } - - if (!(ctxt = virXMLXPathContextNew(xml))) - return NULL; - - ctxt->node = root; - return virDomainCheckpointDefParse(ctxt, xmlopt, parseOpaque, flags); -} virDomainCheckpointDef * virDomainCheckpointDefParseString(const char *xmlStr, @@ -208,19 +188,19 @@ virDomainCheckpointDefParseString(const char *xmlStr, void *parseOpaque, unsigned int flags) { - virDomainCheckpointDef *ret = NULL; g_autoptr(xmlDoc) xml = NULL; + g_autoptr(xmlXPathContext) ctxt = NULL; int keepBlanksDefault = xmlKeepBlanksDefault(0); - if ((xml = virXMLParse(NULL, xmlStr, _("(domain_checkpoint)"), NULL, NULL, - "domaincheckpoint.rng", true))) { - xmlKeepBlanksDefault(keepBlanksDefault); - ret = virDomainCheckpointDefParseNode(xml, xmlDocGetRootElement(xml), - xmlopt, parseOpaque, flags); - } + xml = virXMLParse(NULL, xmlStr, _("(domain_checkpoint)"), + "domaincheckpoint", &ctxt, "domaincheckpoint.rng", true); + xmlKeepBlanksDefault(keepBlanksDefault); - return ret; + if (!xml) + return NULL; + + return virDomainCheckpointDefParse(ctxt, xmlopt, parseOpaque, flags); } -- 2.37.3
Rename virDomainBackupDefParse to virDomainBackupDefParseXML and use it in place of virDomainBackupDefParseNode. This is possible as virXMLParse can be used to replace XPath context allocation and root node checking. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/backup_conf.c | 41 ++++++++++------------------------------ src/conf/backup_conf.h | 10 +++++----- src/libvirt_private.syms | 2 +- src/qemu/qemu_domain.c | 9 ++++++--- 4 files changed, 22 insertions(+), 40 deletions(-) diff --git a/src/conf/backup_conf.c b/src/conf/backup_conf.c index 7dfc8ee635..ad5633388d 100644 --- a/src/conf/backup_conf.c +++ b/src/conf/backup_conf.c @@ -190,10 +190,10 @@ virDomainBackupDefParsePrivate(virDomainBackupDef *def, } -static virDomainBackupDef * -virDomainBackupDefParse(xmlXPathContextPtr ctxt, - virDomainXMLOption *xmlopt, - unsigned int flags) +virDomainBackupDef * +virDomainBackupDefParseXML(xmlXPathContextPtr ctxt, + virDomainXMLOption *xmlopt, + unsigned int flags) { g_autoptr(virDomainBackupDef) def = NULL; g_autofree xmlNodePtr *nodes = NULL; @@ -274,41 +274,20 @@ virDomainBackupDefParseString(const char *xmlStr, virDomainXMLOption *xmlopt, unsigned int flags) { - virDomainBackupDef *ret = NULL; g_autoptr(xmlDoc) xml = NULL; + g_autoptr(xmlXPathContext) ctxt = NULL; int keepBlanksDefault = xmlKeepBlanksDefault(0); bool validate = !(flags & VIR_DOMAIN_BACKUP_PARSE_INTERNAL); - if ((xml = virXMLParse(NULL, xmlStr, _("(domain_backup)"), - NULL, NULL, "domainbackup.rng", validate))) { - xmlKeepBlanksDefault(keepBlanksDefault); - ret = virDomainBackupDefParseNode(xml, xmlDocGetRootElement(xml), - xmlopt, flags); - } - xmlKeepBlanksDefault(keepBlanksDefault); - - return ret; -} - + xml = virXMLParse(NULL, xmlStr, _("(domain_backup)"), + "domainbackup", &ctxt, "domainbackup.rng", validate); -virDomainBackupDef * -virDomainBackupDefParseNode(xmlDocPtr xml, - xmlNodePtr root, - virDomainXMLOption *xmlopt, - unsigned int flags) -{ - g_autoptr(xmlXPathContext) ctxt = NULL; - - if (!virXMLNodeNameEqual(root, "domainbackup")) { - virReportError(VIR_ERR_XML_ERROR, "%s", _("domainbackup")); - return NULL; - } + xmlKeepBlanksDefault(keepBlanksDefault); - if (!(ctxt = virXMLXPathContextNew(xml))) + if (!xml) return NULL; - ctxt->node = root; - return virDomainBackupDefParse(ctxt, xmlopt, flags); + return virDomainBackupDefParseXML(ctxt, xmlopt, flags); } diff --git a/src/conf/backup_conf.h b/src/conf/backup_conf.h index dc66b75892..9c3532a546 100644 --- a/src/conf/backup_conf.h +++ b/src/conf/backup_conf.h @@ -105,16 +105,16 @@ typedef enum { VIR_DOMAIN_BACKUP_PARSE_INTERNAL = 1 << 0, } virDomainBackupParseFlags; +virDomainBackupDef * +virDomainBackupDefParseXML(xmlXPathContextPtr ctxt, + virDomainXMLOption *xmlopt, + unsigned int flags); + virDomainBackupDef * virDomainBackupDefParseString(const char *xmlStr, virDomainXMLOption *xmlopt, unsigned int flags); -virDomainBackupDef * -virDomainBackupDefParseNode(xmlDocPtr xml, - xmlNodePtr root, - virDomainXMLOption *xmlopt, - unsigned int flags); void virDomainBackupDefFree(virDomainBackupDef *def); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6334cbb448..0adcf20f0c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -46,8 +46,8 @@ virAccessPermStorageVolTypeToString; virDomainBackupAlignDisks; virDomainBackupDefFormat; virDomainBackupDefFree; -virDomainBackupDefParseNode; virDomainBackupDefParseString; +virDomainBackupDefParseXML; # conf/capabilities.h diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ee024d17cd..ee35ef586f 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2860,6 +2860,7 @@ static int qemuDomainObjPrivateXMLParseBackups(qemuDomainObjPrivate *priv, xmlXPathContextPtr ctxt) { + VIR_XPATH_NODE_AUTORESTORE(ctxt) g_autofree xmlNodePtr *nodes = NULL; ssize_t nnodes = 0; @@ -2875,9 +2876,11 @@ qemuDomainObjPrivateXMLParseBackups(qemuDomainObjPrivate *priv, if (nnodes == 0) return 0; - if (!(priv->backup = virDomainBackupDefParseNode(ctxt->doc, nodes[0], - priv->driver->xmlopt, - VIR_DOMAIN_BACKUP_PARSE_INTERNAL))) + ctxt->node = nodes[0]; + + if (!(priv->backup = virDomainBackupDefParseXML(ctxt, + priv->driver->xmlopt, + VIR_DOMAIN_BACKUP_PARSE_INTERNAL))) return -1; return 0; -- 2.37.3
Use virXMLParse to fetch the XML context and validate the top level XML element name so that virNWFilterDefParseNode is no longer needed. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/nwfilter_conf.c | 32 +++++--------------------------- src/conf/nwfilter_conf.h | 4 ---- 2 files changed, 5 insertions(+), 31 deletions(-) diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index 44ea056823..00728782d1 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -2685,42 +2685,20 @@ virNWFilterDefParseXML(xmlXPathContextPtr ctxt) } -virNWFilterDef * -virNWFilterDefParseNode(xmlDocPtr xml, - xmlNodePtr root) -{ - g_autoptr(xmlXPathContext) ctxt = NULL; - - if (STRNEQ((const char *)root->name, "filter")) { - virReportError(VIR_ERR_XML_ERROR, - "%s", - _("unknown root element for nw filter")); - return NULL; - } - - if (!(ctxt = virXMLXPathContextNew(xml))) - return NULL; - - ctxt->node = root; - return virNWFilterDefParseXML(ctxt); -} - - static virNWFilterDef * virNWFilterDefParse(const char *xmlStr, const char *filename, unsigned int flags) { - virNWFilterDef *def = NULL; g_autoptr(xmlDoc) xml = NULL; + g_autoptr(xmlXPathContext) ctxt = NULL; bool validate = flags & VIR_NWFILTER_DEFINE_VALIDATE; - if ((xml = virXMLParse(filename, xmlStr, _("(nwfilter_definition)"), - NULL, NULL, "nwfilter.rng", validate))) { - def = virNWFilterDefParseNode(xml, xmlDocGetRootElement(xml)); - } + if (!(xml = virXMLParse(filename, xmlStr, _("(nwfilter_definition)"), + "filter", &ctxt, "nwfilter.rng", validate))) + return NULL; - return def; + return virNWFilterDefParseXML(ctxt); } diff --git a/src/conf/nwfilter_conf.h b/src/conf/nwfilter_conf.h index b67364714a..b8a970f00b 100644 --- a/src/conf/nwfilter_conf.h +++ b/src/conf/nwfilter_conf.h @@ -533,10 +533,6 @@ int virNWFilterDeleteDef(const char *configDir, virNWFilterDef *def); -virNWFilterDef * -virNWFilterDefParseNode(xmlDocPtr xml, - xmlNodePtr root); - char * virNWFilterDefFormat(const virNWFilterDef *def); -- 2.37.3
Replace virNWFilterDefParseString/File with the common function. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/nwfilter_conf.c | 17 +---------------- src/conf/nwfilter_conf.h | 8 +++----- src/conf/virnwfilterobj.c | 2 +- src/libvirt_private.syms | 3 +-- src/nwfilter/nwfilter_driver.c | 2 +- tests/nwfilterxml2firewalltest.c | 2 +- tests/nwfilterxml2xmltest.c | 2 +- 7 files changed, 9 insertions(+), 27 deletions(-) diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index 00728782d1..9a95ae6c12 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -2685,7 +2685,7 @@ virNWFilterDefParseXML(xmlXPathContextPtr ctxt) } -static virNWFilterDef * +virNWFilterDef * virNWFilterDefParse(const char *xmlStr, const char *filename, unsigned int flags) @@ -2702,21 +2702,6 @@ virNWFilterDefParse(const char *xmlStr, } -virNWFilterDef * -virNWFilterDefParseString(const char *xmlStr, - unsigned int flags) -{ - return virNWFilterDefParse(xmlStr, NULL, flags); -} - - -virNWFilterDef * -virNWFilterDefParseFile(const char *filename) -{ - return virNWFilterDefParse(NULL, filename, 0); -} - - int virNWFilterSaveConfig(const char *configDir, virNWFilterDef *def) diff --git a/src/conf/nwfilter_conf.h b/src/conf/nwfilter_conf.h index b8a970f00b..1a38e4198c 100644 --- a/src/conf/nwfilter_conf.h +++ b/src/conf/nwfilter_conf.h @@ -541,11 +541,9 @@ virNWFilterSaveConfig(const char *configDir, virNWFilterDef *def); virNWFilterDef * -virNWFilterDefParseString(const char *xml, - unsigned int flags); - -virNWFilterDef * -virNWFilterDefParseFile(const char *filename); +virNWFilterDefParse(const char *xmlStr, + const char *filename, + unsigned int flags); typedef int (*virNWFilterTriggerRebuildCallback)(void *opaque); diff --git a/src/conf/virnwfilterobj.c b/src/conf/virnwfilterobj.c index f9c1b049d5..e8dfe66b3c 100644 --- a/src/conf/virnwfilterobj.c +++ b/src/conf/virnwfilterobj.c @@ -578,7 +578,7 @@ virNWFilterObjListLoadConfig(virNWFilterObjList *nwfilters, if (!(configFile = virFileBuildPath(configDir, name, ".xml"))) goto error; - if (!(def = virNWFilterDefParseFile(configFile))) + if (!(def = virNWFilterDefParse(NULL, configFile, 0))) goto error; if (STRNEQ(name, def->name)) { diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0adcf20f0c..6836c99fff 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -952,8 +952,7 @@ virNWFilterConfLayerInit; virNWFilterConfLayerShutdown; virNWFilterDefFormat; virNWFilterDefFree; -virNWFilterDefParseFile; -virNWFilterDefParseString; +virNWFilterDefParse; virNWFilterDeleteDef; virNWFilterJumpTargetTypeToString; virNWFilterPrintStateMatchFlags; diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index f9fc09bbd3..8e45096eaa 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -545,7 +545,7 @@ nwfilterDefineXMLFlags(virConnectPtr conn, return NULL; } - if (!(def = virNWFilterDefParseString(xml, flags))) + if (!(def = virNWFilterDefParse(xml, NULL, flags))) goto cleanup; if (virNWFilterDefineXMLFlagsEnsureACL(conn, def) < 0) diff --git a/tests/nwfilterxml2firewalltest.c b/tests/nwfilterxml2firewalltest.c index 1514002b8f..bd112cef83 100644 --- a/tests/nwfilterxml2firewalltest.c +++ b/tests/nwfilterxml2firewalltest.c @@ -255,7 +255,7 @@ virNWFilterDefToInst(const char *xml, { size_t i; int ret = -1; - virNWFilterDef *def = virNWFilterDefParseFile(xml); + virNWFilterDef *def = virNWFilterDefParse(NULL, xml, 0); if (!def) return -1; diff --git a/tests/nwfilterxml2xmltest.c b/tests/nwfilterxml2xmltest.c index c2b6dc575b..ca037ad9a0 100644 --- a/tests/nwfilterxml2xmltest.c +++ b/tests/nwfilterxml2xmltest.c @@ -21,7 +21,7 @@ testCompareXMLToXMLFiles(const char *inxml, const char *outxml, virResetLastError(); - if (!(dev = virNWFilterDefParseFile(inxml))) { + if (!(dev = virNWFilterDefParse(NULL, inxml, 0))) { if (expect_error) { virResetLastError(); goto done; -- 2.37.3
Both callers be easily made to call virNodeDeviceDefParseXML directly. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/node_device_conf.c | 34 ++++++---------------------------- src/conf/node_device_conf.h | 7 +++---- src/libvirt_private.syms | 2 +- src/test/test_driver.c | 7 +++---- 4 files changed, 13 insertions(+), 37 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 1db9a3240a..7e50904828 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -2370,7 +2370,7 @@ virNodeDevCapsDefParseXML(xmlXPathContextPtr ctxt, } -static virNodeDeviceDef * +virNodeDeviceDef * virNodeDeviceDefParseXML(xmlXPathContextPtr ctxt, int create, const char *virt_type) @@ -2473,30 +2473,6 @@ virNodeDeviceDefParseXML(xmlXPathContextPtr ctxt, } -virNodeDeviceDef * -virNodeDeviceDefParseNode(xmlDocPtr xml, - xmlNodePtr root, - int create, - const char *virt_type) -{ - g_autoptr(xmlXPathContext) ctxt = NULL; - - if (!virXMLNodeNameEqual(root, "device")) { - virReportError(VIR_ERR_XML_ERROR, - _("unexpected root element <%s> " - "expecting <device>"), - root->name); - return NULL; - } - - if (!(ctxt = virXMLXPathContextNew(xml))) - return NULL; - - ctxt->node = root; - return virNodeDeviceDefParseXML(ctxt, create, virt_type); -} - - static virNodeDeviceDef * virNodeDeviceDefParse(const char *str, const char *filename, @@ -2506,12 +2482,14 @@ virNodeDeviceDefParse(const char *str, void *opaque) { g_autoptr(xmlDoc) xml = NULL; + g_autoptr(xmlXPathContext) ctxt = NULL; g_autoptr(virNodeDeviceDef) def = NULL; if (!(xml = virXMLParse(filename, str, _("(node_device_definition)"), - NULL, NULL, NULL, false)) || - !(def = virNodeDeviceDefParseNode(xml, xmlDocGetRootElement(xml), - create, virt_type))) + "device", &ctxt, NULL, false))) + return NULL; + + if (!(def = virNodeDeviceDefParseXML(ctxt, create, virt_type))) return NULL; if (parserCallbacks) { diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index 21622c62ac..feef22910d 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -391,10 +391,9 @@ virNodeDeviceDefParseFile(const char *filename, void *opaque); virNodeDeviceDef * -virNodeDeviceDefParseNode(xmlDocPtr xml, - xmlNodePtr root, - int create, - const char *virt_type); +virNodeDeviceDefParseXML(xmlXPathContextPtr ctxt, + int create, + const char *virt_type); int virNodeDeviceGetWWNs(virNodeDeviceDef *def, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 6836c99fff..ffab2b7c43 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -866,8 +866,8 @@ virNodeDeviceCapsListExport; virNodeDeviceDefFormat; virNodeDeviceDefFree; virNodeDeviceDefParseFile; -virNodeDeviceDefParseNode; virNodeDeviceDefParseString; +virNodeDeviceDefParseXML; virNodeDeviceGetAPMatrixDynamicCaps; virNodeDeviceGetCSSDynamicCaps; virNodeDeviceGetMdevParentDynamicCaps; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 50c8a7e2be..7c7ef1b924 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1236,6 +1236,7 @@ testParseNodedevs(testDriver *privconn, const char *file, xmlXPathContextPtr ctxt) { + VIR_XPATH_NODE_AUTORESTORE(ctxt) int num; size_t i; virNodeDeviceObj *obj; @@ -1247,13 +1248,11 @@ testParseNodedevs(testDriver *privconn, for (i = 0; i < num; i++) { virNodeDeviceDef *def; - xmlNodePtr node = testParseXMLDocFromFile(nodes[i], file); - if (!node) + if (!(ctxt->node = testParseXMLDocFromFile(nodes[i], file))) return -1; - def = virNodeDeviceDefParseNode(ctxt->doc, node, 0, NULL); - if (!def) + if (!(def = virNodeDeviceDefParseXML(ctxt, 0, NULL))) return -1; if (!(obj = virNodeDeviceObjListAssignDef(privconn->devs, def))) { -- 2.37.3
Replace the thin wrappers virNodeDeviceDefParseString/File by directly calling the main parser. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/node_device_conf.c | 24 +----------------------- src/conf/node_device_conf.h | 18 ++++++------------ src/hypervisor/domain_driver.c | 7 +++---- src/libvirt_private.syms | 3 +-- src/node_device/node_device_driver.c | 8 ++++---- src/test/test_driver.c | 6 ++---- tests/nodedevmdevctltest.c | 8 ++++---- tests/nodedevxml2xmltest.c | 3 +-- 8 files changed, 22 insertions(+), 55 deletions(-) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index 7e50904828..bdfbbab434 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -2473,7 +2473,7 @@ virNodeDeviceDefParseXML(xmlXPathContextPtr ctxt, } -static virNodeDeviceDef * +virNodeDeviceDef * virNodeDeviceDefParse(const char *str, const char *filename, int create, @@ -2513,28 +2513,6 @@ virNodeDeviceDefParse(const char *str, } -virNodeDeviceDef * -virNodeDeviceDefParseString(const char *str, - int create, - const char *virt_type, - virNodeDeviceDefParserCallbacks *parserCallbacks, - void *opaque) -{ - return virNodeDeviceDefParse(str, NULL, create, virt_type, parserCallbacks, opaque); -} - - -virNodeDeviceDef * -virNodeDeviceDefParseFile(const char *filename, - int create, - const char *virt_type, - virNodeDeviceDefParserCallbacks *parserCallbacks, - void *opaque) -{ - return virNodeDeviceDefParse(NULL, filename, create, virt_type, parserCallbacks, opaque); -} - - /* * Return fc_host dev's WWNN and WWPN */ diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index feef22910d..a556358632 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -377,18 +377,12 @@ typedef struct _virNodeDeviceDefParserCallbacks { } virNodeDeviceDefParserCallbacks; virNodeDeviceDef * -virNodeDeviceDefParseString(const char *str, - int create, - const char *virt_type, - virNodeDeviceDefParserCallbacks *callbacks, - void *opaque); - -virNodeDeviceDef * -virNodeDeviceDefParseFile(const char *filename, - int create, - const char *virt_type, - virNodeDeviceDefParserCallbacks *callbacks, - void *opaque); +virNodeDeviceDefParse(const char *str, + const char *filename, + int create, + const char *virt_type, + virNodeDeviceDefParserCallbacks *parserCallbacks, + void *opaque); virNodeDeviceDef * virNodeDeviceDefParseXML(xmlXPathContextPtr ctxt, diff --git a/src/hypervisor/domain_driver.c b/src/hypervisor/domain_driver.c index bb1da7ac6b..c154f00eea 100644 --- a/src/hypervisor/domain_driver.c +++ b/src/hypervisor/domain_driver.c @@ -395,8 +395,7 @@ virDomainDriverNodeDeviceReset(virNodeDevicePtr dev, if (!xml) return -1; - def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE, NULL, - NULL, NULL); + def = virNodeDeviceDefParse(xml, NULL, EXISTING_DEVICE, NULL, NULL, NULL); if (!def) return -1; @@ -441,7 +440,7 @@ virDomainDriverNodeDeviceReAttach(virNodeDevicePtr dev, if (!xml) return -1; - def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE, NULL, NULL, NULL); + def = virNodeDeviceDefParse(xml, NULL, EXISTING_DEVICE, NULL, NULL, NULL); if (!def) return -1; @@ -489,7 +488,7 @@ virDomainDriverNodeDeviceDetachFlags(virNodeDevicePtr dev, if (!xml) return -1; - def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE, NULL, NULL, NULL); + def = virNodeDeviceDefParse(xml, NULL, EXISTING_DEVICE, NULL, NULL, NULL); if (!def) return -1; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ffab2b7c43..9cbbfef093 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -865,8 +865,7 @@ virNodeDevCapTypeToString; virNodeDeviceCapsListExport; virNodeDeviceDefFormat; virNodeDeviceDefFree; -virNodeDeviceDefParseFile; -virNodeDeviceDefParseString; +virNodeDeviceDefParse; virNodeDeviceDefParseXML; virNodeDeviceGetAPMatrixDynamicCaps; virNodeDeviceGetCSSDynamicCaps; diff --git a/src/node_device/node_device_driver.c b/src/node_device/node_device_driver.c index fa3cfcf24c..8e93b0dd6f 100644 --- a/src/node_device/node_device_driver.c +++ b/src/node_device/node_device_driver.c @@ -884,8 +884,8 @@ nodeDeviceCreateXML(virConnectPtr conn, virt_type = virConnectGetType(conn); - if (!(def = virNodeDeviceDefParseString(xmlDesc, CREATE_DEVICE, virt_type, - &driver->parserCallbacks, NULL))) + if (!(def = virNodeDeviceDefParse(xmlDesc, NULL, CREATE_DEVICE, virt_type, + &driver->parserCallbacks, NULL))) return NULL; if (virNodeDeviceCreateXMLEnsureACL(conn, def) < 0) @@ -1405,8 +1405,8 @@ nodeDeviceDefineXML(virConnect *conn, virt_type = virConnectGetType(conn); - if (!(def = virNodeDeviceDefParseString(xmlDesc, CREATE_DEVICE, virt_type, - &driver->parserCallbacks, NULL))) + if (!(def = virNodeDeviceDefParse(xmlDesc, NULL, CREATE_DEVICE, virt_type, + &driver->parserCallbacks, NULL))) return NULL; if (virNodeDeviceDefineXMLEnsureACL(conn, def) < 0) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 7c7ef1b924..72786da568 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -7635,8 +7635,7 @@ testNodeDeviceMockCreateVport(testDriver *driver, if (!xml) goto cleanup; - if (!(def = virNodeDeviceDefParseString(xml, EXISTING_DEVICE, NULL, NULL, - NULL))) + if (!(def = virNodeDeviceDefParse(xml, NULL, EXISTING_DEVICE, NULL, NULL, NULL))) goto cleanup; VIR_FREE(def->name); @@ -7698,8 +7697,7 @@ testNodeDeviceCreateXML(virConnectPtr conn, virCheckFlags(0, NULL); - if (!(def = virNodeDeviceDefParseString(xmlDesc, CREATE_DEVICE, NULL, - NULL, NULL))) + if (!(def = virNodeDeviceDefParse(xmlDesc, NULL, CREATE_DEVICE, NULL, NULL, NULL))) goto cleanup; /* We run this simply for validation - it essentially validates that diff --git a/tests/nodedevmdevctltest.c b/tests/nodedevmdevctltest.c index dee4639922..02e85d4779 100644 --- a/tests/nodedevmdevctltest.c +++ b/tests/nodedevmdevctltest.c @@ -70,8 +70,8 @@ testMdevctlCmd(virMdevctlCommand cmd_type, return -1; } - if (!(def = virNodeDeviceDefParseFile(mdevxml, create, VIRT_TYPE, - &parser_callbacks, NULL))) + if (!(def = virNodeDeviceDefParse(NULL, mdevxml, create, VIRT_TYPE, + &parser_callbacks, NULL))) return -1; /* this function will set a stdin buffer containing the json configuration @@ -142,8 +142,8 @@ testMdevctlAutostart(const void *data G_GNUC_UNUSED) abs_srcdir); g_autoptr(virCommandDryRunToken) dryRunToken = virCommandDryRunTokenNew(); - if (!(def = virNodeDeviceDefParseFile(mdevxml, CREATE_DEVICE, VIRT_TYPE, - &parser_callbacks, NULL))) + if (!(def = virNodeDeviceDefParse(NULL, mdevxml, CREATE_DEVICE, VIRT_TYPE, + &parser_callbacks, NULL))) return -1; virCommandSetDryRun(dryRunToken, &buf, true, true, NULL, NULL); diff --git a/tests/nodedevxml2xmltest.c b/tests/nodedevxml2xmltest.c index 68a4041d8c..d1c0652e7d 100644 --- a/tests/nodedevxml2xmltest.c +++ b/tests/nodedevxml2xmltest.c @@ -23,8 +23,7 @@ testCompareXMLToXMLFiles(const char *xml, const char *outfile) if (virTestLoadFile(xml, &xmlData) < 0) goto fail; - if (!(dev = virNodeDeviceDefParseString(xmlData, EXISTING_DEVICE, NULL, - NULL, NULL))) + if (!(dev = virNodeDeviceDefParse(xmlData, NULL, EXISTING_DEVICE, NULL, NULL, NULL))) goto fail; /* Calculate some things that are not read in */ -- 2.37.3
Both callers be easily made to call virInterfaceDefParseXML directly. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/interface_conf.c | 35 +++++------------------------------ src/conf/interface_conf.h | 4 ++-- src/libvirt_private.syms | 2 +- src/test/test_driver.c | 8 ++++---- 4 files changed, 12 insertions(+), 37 deletions(-) diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c index a3f6b6bed6..628199c6f3 100644 --- a/src/conf/interface_conf.c +++ b/src/conf/interface_conf.c @@ -33,9 +33,6 @@ VIR_ENUM_IMPL(virInterface, "ethernet", "bridge", "bond", "vlan", ); -static virInterfaceDef * -virInterfaceDefParseXML(xmlXPathContextPtr ctxt, int parentIfType); - static int virInterfaceDefDevFormat(virBuffer *buf, const virInterfaceDef *def, virInterfaceType parentIfType); @@ -565,7 +562,7 @@ virInterfaceDefParseVlan(virInterfaceDef *def, } -static virInterfaceDef * +virInterfaceDef * virInterfaceDefParseXML(xmlXPathContextPtr ctxt, int parentIfType) { @@ -672,42 +669,20 @@ virInterfaceDefParseXML(xmlXPathContextPtr ctxt, } -virInterfaceDef * -virInterfaceDefParseNode(xmlDocPtr xml, - xmlNodePtr root) -{ - g_autoptr(xmlXPathContext) ctxt = NULL; - - if (!virXMLNodeNameEqual(root, "interface")) { - virReportError(VIR_ERR_XML_ERROR, - _("unexpected root element <%s>, " - "expecting <interface>"), - root->name); - return NULL; - } - - if (!(ctxt = virXMLXPathContextNew(xml))) - return NULL; - - ctxt->node = root; - return virInterfaceDefParseXML(ctxt, VIR_INTERFACE_TYPE_LAST); -} - - static virInterfaceDef * virInterfaceDefParse(const char *xmlStr, const char *filename, unsigned int flags) { g_autoptr(xmlDoc) xml = NULL; + g_autoptr(xmlXPathContext) ctxt = NULL; bool validate = flags & VIR_INTERFACE_DEFINE_VALIDATE; - xml = virXMLParse(filename, xmlStr, _("(interface_definition)"), - NULL, NULL, "interface.rng", validate); - if (!xml) + if (!(xml = virXMLParse(filename, xmlStr, _("(interface_definition)"), + "interface", &ctxt, "interface.rng", validate))) return NULL; - return virInterfaceDefParseNode(xml, xmlDocGetRootElement(xml)); + return virInterfaceDefParseXML(ctxt, VIR_INTERFACE_TYPE_LAST); } diff --git a/src/conf/interface_conf.h b/src/conf/interface_conf.h index fa6bce9a00..b8927d7b4f 100644 --- a/src/conf/interface_conf.h +++ b/src/conf/interface_conf.h @@ -162,8 +162,8 @@ virInterfaceDef * virInterfaceDefParseFile(const char *filename); virInterfaceDef * -virInterfaceDefParseNode(xmlDocPtr xml, - xmlNodePtr root); +virInterfaceDefParseXML(xmlXPathContextPtr ctxt, + int parentIfType); char * virInterfaceDefFormat(const virInterfaceDef *def); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 9cbbfef093..1251bba896 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -793,8 +793,8 @@ virDomainDiskDefValidateStartupPolicy; virInterfaceDefFormat; virInterfaceDefFree; virInterfaceDefParseFile; -virInterfaceDefParseNode; virInterfaceDefParseString; +virInterfaceDefParseXML; # conf/netdev_bandwidth_conf.h diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 72786da568..5f98c05c34 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1110,6 +1110,7 @@ testParseInterfaces(testDriver *privconn, const char *file, xmlXPathContextPtr ctxt) { + VIR_XPATH_NODE_AUTORESTORE(ctxt) int num; size_t i; virInterfaceObj *obj; @@ -1121,12 +1122,11 @@ testParseInterfaces(testDriver *privconn, for (i = 0; i < num; i++) { g_autoptr(virInterfaceDef) def = NULL; - xmlNodePtr node = testParseXMLDocFromFile(nodes[i], file); - if (!node) + + if (!(ctxt->node = testParseXMLDocFromFile(nodes[i], file))) return -1; - def = virInterfaceDefParseNode(ctxt->doc, node); - if (!def) + if (!(def = virInterfaceDefParseXML(ctxt, VIR_INTERFACE_TYPE_LAST))) return -1; if (!(obj = virInterfaceObjListAssignDef(privconn->ifaces, &def))) -- 2.37.3
The function was not used. Remove it and merge virInterfaceDefParse into virInterfaceDefParseString. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/interface_conf.c | 24 ++++-------------------- src/conf/interface_conf.h | 3 --- src/libvirt_private.syms | 1 - 3 files changed, 4 insertions(+), 24 deletions(-) diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c index 628199c6f3..b31fdce101 100644 --- a/src/conf/interface_conf.c +++ b/src/conf/interface_conf.c @@ -669,16 +669,15 @@ virInterfaceDefParseXML(xmlXPathContextPtr ctxt, } -static virInterfaceDef * -virInterfaceDefParse(const char *xmlStr, - const char *filename, - unsigned int flags) +virInterfaceDef * +virInterfaceDefParseString(const char *xmlStr, + unsigned int flags) { g_autoptr(xmlDoc) xml = NULL; g_autoptr(xmlXPathContext) ctxt = NULL; bool validate = flags & VIR_INTERFACE_DEFINE_VALIDATE; - if (!(xml = virXMLParse(filename, xmlStr, _("(interface_definition)"), + if (!(xml = virXMLParse(NULL, xmlStr, _("(interface_definition)"), "interface", &ctxt, "interface.rng", validate))) return NULL; @@ -686,21 +685,6 @@ virInterfaceDefParse(const char *xmlStr, } -virInterfaceDef * -virInterfaceDefParseString(const char *xmlStr, - unsigned int flags) -{ - return virInterfaceDefParse(xmlStr, NULL, flags); -} - - -virInterfaceDef * -virInterfaceDefParseFile(const char *filename) -{ - return virInterfaceDefParse(NULL, filename, 0); -} - - static int virInterfaceBridgeDefFormat(virBuffer *buf, const virInterfaceDef *def) diff --git a/src/conf/interface_conf.h b/src/conf/interface_conf.h index b8927d7b4f..1272216300 100644 --- a/src/conf/interface_conf.h +++ b/src/conf/interface_conf.h @@ -158,9 +158,6 @@ virInterfaceDef * virInterfaceDefParseString(const char *xmlStr, unsigned int flags); -virInterfaceDef * -virInterfaceDefParseFile(const char *filename); - virInterfaceDef * virInterfaceDefParseXML(xmlXPathContextPtr ctxt, int parentIfType); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 1251bba896..a24e0db5d7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -792,7 +792,6 @@ virDomainDiskDefValidateStartupPolicy; # conf/interface_conf.h virInterfaceDefFormat; virInterfaceDefFree; -virInterfaceDefParseFile; virInterfaceDefParseString; virInterfaceDefParseXML; -- 2.37.3
Both callers can be easily converted to call virNetworkDefParseXML directly. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/network_conf.c | 37 ++++++++----------------------------- src/conf/network_conf.h | 5 ----- src/libvirt_private.syms | 1 - src/test/test_driver.c | 8 ++++---- 4 files changed, 12 insertions(+), 39 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index b1d77a80c3..3574c0214b 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -2032,15 +2032,17 @@ virNetworkDefParse(const char *xmlStr, bool validate) { g_autoptr(xmlDoc) xml = NULL; - virNetworkDef *def = NULL; + g_autoptr(xmlXPathContext) ctxt = NULL; int keepBlanksDefault = xmlKeepBlanksDefault(0); - if ((xml = virXMLParse(filename, xmlStr, _("(network_definition)"), - NULL, NULL, "network.rng", validate))) - def = virNetworkDefParseNode(xml, xmlDocGetRootElement(xml), xmlopt); - + xml = virXMLParse(filename, xmlStr, _("(network_definition)"), + "network", &ctxt, "network.rng", validate); xmlKeepBlanksDefault(keepBlanksDefault); - return def; + + if (!xml) + return NULL; + + return virNetworkDefParseXML(ctxt, xmlopt); } @@ -2061,29 +2063,6 @@ virNetworkDefParseFile(const char *filename, } -virNetworkDef * -virNetworkDefParseNode(xmlDocPtr xml, - xmlNodePtr root, - virNetworkXMLOption *xmlopt) -{ - g_autoptr(xmlXPathContext) ctxt = NULL; - - if (!virXMLNodeNameEqual(root, "network")) { - virReportError(VIR_ERR_XML_ERROR, - _("unexpected root element <%s>, " - "expecting <network>"), - root->name); - return NULL; - } - - if (!(ctxt = virXMLXPathContextNew(xml))) - return NULL; - - ctxt->node = root; - return virNetworkDefParseXML(ctxt, xmlopt); -} - - static int virNetworkDNSDefFormat(virBuffer *buf, const virNetworkDNSDef *def) diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 48b07ea13c..49d3ce6b30 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -333,11 +333,6 @@ virNetworkDef * virNetworkDefParseFile(const char *filename, virNetworkXMLOption *xmlopt); -virNetworkDef * -virNetworkDefParseNode(xmlDocPtr xml, - xmlNodePtr root, - virNetworkXMLOption *xmlopt); - char * virNetworkDefFormat(const virNetworkDef *def, virNetworkXMLOption *xmlopt, diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a24e0db5d7..3ce2cc1a7e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -829,7 +829,6 @@ virNetworkDefFree; virNetworkDefGetIPByIndex; virNetworkDefGetRouteByIndex; virNetworkDefParseFile; -virNetworkDefParseNode; virNetworkDefParseString; virNetworkDefParseXML; virNetworkDefUpdateSection; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 5f98c05c34..4c4eabc9fc 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1074,6 +1074,7 @@ testParseNetworks(testDriver *privconn, const char *file, xmlXPathContextPtr ctxt) { + VIR_XPATH_NODE_AUTORESTORE(ctxt) int num; size_t i; virNetworkObj *obj; @@ -1085,12 +1086,11 @@ testParseNetworks(testDriver *privconn, for (i = 0; i < num; i++) { g_autoptr(virNetworkDef) def = NULL; - xmlNodePtr node = testParseXMLDocFromFile(nodes[i], file); - if (!node) + + if (!(ctxt->node = testParseXMLDocFromFile(nodes[i], file))) return -1; - def = virNetworkDefParseNode(ctxt->doc, node, NULL); - if (!def) + if (!(def = virNetworkDefParseXML(ctxt, NULL))) return -1; if (!(obj = virNetworkObjAssignDef(privconn->networks, def, 0))) -- 2.37.3
Replace virNetworkDefParseString/File by direct calls to virNetworkDefParse. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 2 +- src/conf/network_conf.c | 21 ++------------------- src/conf/network_conf.h | 11 ++++------- src/conf/virnetworkobj.c | 2 +- src/esx/esx_network_driver.c | 2 +- src/libvirt_private.syms | 3 +-- src/network/bridge_driver.c | 8 ++++---- src/qemu/qemu_process.c | 2 +- src/test/test_driver.c | 8 ++++---- src/vbox/vbox_network.c | 2 +- tests/networkxml2conftest.c | 2 +- tests/networkxml2firewalltest.c | 2 +- tests/networkxml2xmltest.c | 2 +- tests/networkxml2xmlupdatetest.c | 2 +- 14 files changed, 24 insertions(+), 45 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2a3ea9641f..5bb5cfa500 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -29728,7 +29728,7 @@ virDomainNetResolveActualType(virDomainNetDef *iface) if (!(xml = virNetworkGetXMLDesc(net, 0))) return -1; - if (!(def = virNetworkDefParseString(xml, NULL, false))) + if (!(def = virNetworkDefParse(xml, NULL, NULL, false))) return -1; switch ((virNetworkForwardType) def->forward.type) { diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 3574c0214b..be43894050 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -315,7 +315,7 @@ virNetworkDefCopy(virNetworkDef *def, if (!(xml = virNetworkDefFormat(def, xmlopt, flags))) return NULL; - return virNetworkDefParseString(xml, xmlopt, false); + return virNetworkDefParse(xml, NULL, xmlopt, false); } @@ -2025,7 +2025,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt, } -static virNetworkDef * +virNetworkDef * virNetworkDefParse(const char *xmlStr, const char *filename, virNetworkXMLOption *xmlopt, @@ -2046,23 +2046,6 @@ virNetworkDefParse(const char *xmlStr, } -virNetworkDef * -virNetworkDefParseString(const char *xmlStr, - virNetworkXMLOption *xmlopt, - bool validate) -{ - return virNetworkDefParse(xmlStr, NULL, xmlopt, validate); -} - - -virNetworkDef * -virNetworkDefParseFile(const char *filename, - virNetworkXMLOption *xmlopt) -{ - return virNetworkDefParse(NULL, filename, xmlopt, false); -} - - static int virNetworkDNSDefFormat(virBuffer *buf, const virNetworkDNSDef *def) diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 49d3ce6b30..2b2e9d15f0 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -325,13 +325,10 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt, virNetworkXMLOption *xmlopt); virNetworkDef * -virNetworkDefParseString(const char *xmlStr, - virNetworkXMLOption *xmlopt, - bool validate); - -virNetworkDef * -virNetworkDefParseFile(const char *filename, - virNetworkXMLOption *xmlopt); +virNetworkDefParse(const char *xmlStr, + const char *filename, + virNetworkXMLOption *xmlopt, + bool validate); char * virNetworkDefFormat(const virNetworkDef *def, diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c index 7621fa3380..d23eb2f401 100644 --- a/src/conf/virnetworkobj.c +++ b/src/conf/virnetworkobj.c @@ -967,7 +967,7 @@ virNetworkLoadConfig(virNetworkObjList *nets, if ((autostart = virFileLinkPointsTo(autostartLink, configFile)) < 0) goto error; - if (!(def = virNetworkDefParseFile(configFile, xmlopt))) + if (!(def = virNetworkDefParse(NULL, configFile, xmlopt, false))) goto error; if (STRNEQ(name, def->name)) { diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c index bf9630ce9d..1261b3f1fa 100644 --- a/src/esx/esx_network_driver.c +++ b/src/esx/esx_network_driver.c @@ -296,7 +296,7 @@ esxNetworkDefineXMLFlags(virConnectPtr conn, const char *xml, return NULL; /* Parse network XML */ - def = virNetworkDefParseString(xml, NULL, !!(flags & VIR_NETWORK_DEFINE_VALIDATE)); + def = virNetworkDefParse(xml, NULL, NULL, !!(flags & VIR_NETWORK_DEFINE_VALIDATE)); if (!def) return NULL; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3ce2cc1a7e..5714e0c114 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -828,8 +828,7 @@ virNetworkDefForwardIf; virNetworkDefFree; virNetworkDefGetIPByIndex; virNetworkDefGetRouteByIndex; -virNetworkDefParseFile; -virNetworkDefParseString; +virNetworkDefParse; virNetworkDefParseXML; virNetworkDefUpdateSection; virNetworkDHCPLeaseTimeUnitTypeFromString; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index e4f5e93779..a5973e26cd 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -3017,8 +3017,8 @@ networkCreateXMLFlags(virConnectPtr conn, virCheckFlags(VIR_NETWORK_CREATE_VALIDATE, NULL); - if (!(newDef = virNetworkDefParseString(xml, network_driver->xmlopt, - !!(flags & VIR_NETWORK_CREATE_VALIDATE)))) + if (!(newDef = virNetworkDefParse(xml, NULL, network_driver->xmlopt, + !!(flags & VIR_NETWORK_CREATE_VALIDATE)))) goto cleanup; if (virNetworkCreateXMLFlagsEnsureACL(conn, newDef) < 0) @@ -3082,8 +3082,8 @@ networkDefineXMLFlags(virConnectPtr conn, virCheckFlags(VIR_NETWORK_DEFINE_VALIDATE, NULL); - if (!(def = virNetworkDefParseString(xml, network_driver->xmlopt, - !!(flags & VIR_NETWORK_DEFINE_VALIDATE)))) + if (!(def = virNetworkDefParse(xml, NULL, network_driver->xmlopt, + !!(flags & VIR_NETWORK_DEFINE_VALIDATE)))) goto cleanup; defAlias = def; /* so we can still ref the object after nullifying def */ diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 998f4aa63c..d23857353a 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -4852,7 +4852,7 @@ qemuProcessGetNetworkAddress(const char *netname, if (!xml) return -1; - netdef = virNetworkDefParseString(xml, NULL, false); + netdef = virNetworkDefParse(xml, NULL, NULL, false); if (!netdef) return -1; diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 4c4eabc9fc..9f0517e89e 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -5559,8 +5559,8 @@ testNetworkCreateXMLFlags(virConnectPtr conn, const char *xml, virCheckFlags(VIR_NETWORK_CREATE_VALIDATE, NULL); - if (!(newDef = virNetworkDefParseString(xml, NULL, - !!(flags & VIR_NETWORK_CREATE_VALIDATE)))) + if (!(newDef = virNetworkDefParse(xml, NULL, NULL, + !!(flags & VIR_NETWORK_CREATE_VALIDATE)))) goto cleanup; if (!(obj = virNetworkObjAssignDef(privconn->networks, newDef, @@ -5605,8 +5605,8 @@ testNetworkDefineXMLFlags(virConnectPtr conn, virCheckFlags(VIR_NETWORK_DEFINE_VALIDATE, NULL); - if (!(newDef = virNetworkDefParseString(xml, NULL, - !!(flags & VIR_NETWORK_DEFINE_VALIDATE)))) + if (!(newDef = virNetworkDefParse(xml, NULL, NULL, + !!(flags & VIR_NETWORK_DEFINE_VALIDATE)))) goto cleanup; if (!(obj = virNetworkObjAssignDef(privconn->networks, newDef, 0))) diff --git a/src/vbox/vbox_network.c b/src/vbox/vbox_network.c index 885fd48321..c3dea66102 100644 --- a/src/vbox/vbox_network.c +++ b/src/vbox/vbox_network.c @@ -402,7 +402,7 @@ vboxNetworkDefineCreateXML(virConnectPtr conn, const char *xml, bool start, VBOX_IID_INITIALIZE(&vboxnetiid); - if (!(def = virNetworkDefParseString(xml, NULL, validate)) || + if (!(def = virNetworkDefParse(xml, NULL, NULL, validate)) || (def->forward.type != VIR_NETWORK_FORWARD_NONE) || (def->nips == 0 || !def->ips)) goto cleanup; diff --git a/tests/networkxml2conftest.c b/tests/networkxml2conftest.c index bbeb849a39..726f073ddc 100644 --- a/tests/networkxml2conftest.c +++ b/tests/networkxml2conftest.c @@ -31,7 +31,7 @@ testCompareXMLToConfFiles(const char *inxml, const char *outconf, if (!(xmlopt = networkDnsmasqCreateXMLConf())) goto fail; - if (!(def = virNetworkDefParseFile(inxml, xmlopt))) + if (!(def = virNetworkDefParse(NULL, inxml, xmlopt, false))) goto fail; if (!(obj = virNetworkObjNew())) diff --git a/tests/networkxml2firewalltest.c b/tests/networkxml2firewalltest.c index ca793fd4ea..cb66a26294 100644 --- a/tests/networkxml2firewalltest.c +++ b/tests/networkxml2firewalltest.c @@ -95,7 +95,7 @@ static int testCompareXMLToArgvFiles(const char *xml, virCommandSetDryRun(dryRunToken, &buf, true, true, testCommandDryRun, NULL); - if (!(def = virNetworkDefParseFile(xml, NULL))) + if (!(def = virNetworkDefParse(NULL, xml, NULL, false))) return -1; if (networkAddFirewallRules(def) < 0) diff --git a/tests/networkxml2xmltest.c b/tests/networkxml2xmltest.c index 521f058acc..b0814c7529 100644 --- a/tests/networkxml2xmltest.c +++ b/tests/networkxml2xmltest.c @@ -33,7 +33,7 @@ testCompareXMLToXMLFiles(const char *inxml, const char *outxml, if (!(xmlopt = networkDnsmasqCreateXMLConf())) goto cleanup; - if (!(dev = virNetworkDefParseFile(inxml, xmlopt))) { + if (!(dev = virNetworkDefParse(NULL, inxml, xmlopt, false))) { result = TEST_COMPARE_NET_XML2XML_RESULT_FAIL_PARSE; goto cleanup; } diff --git a/tests/networkxml2xmlupdatetest.c b/tests/networkxml2xmlupdatetest.c index 58b6f70c96..afe2b1f574 100644 --- a/tests/networkxml2xmlupdatetest.c +++ b/tests/networkxml2xmlupdatetest.c @@ -25,7 +25,7 @@ testCompareXMLToXMLFiles(const char *netxml, const char *updatexml, if (virTestLoadFile(updatexml, &updateXmlData) < 0) return -1; - if (!(def = virNetworkDefParseFile(netxml, NULL))) + if (!(def = virNetworkDefParse(NULL, netxml, NULL, false))) goto fail; if (virNetworkDefUpdateSection(def, command, section, parentIndex, -- 2.37.3
virDomainObjParseFile is the only caller of virDomainObjParseNode. The code can be merged into it, simplified by using virXMLParse and the function removed. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 36 ++++++------------------------------ src/conf/domain_conf.h | 4 ---- src/libvirt_private.syms | 1 - 3 files changed, 6 insertions(+), 35 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5bb5cfa500..03f4b2fe33 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -19155,48 +19155,24 @@ virDomainDefParseNode(xmlDocPtr xml, virDomainObj * -virDomainObjParseNode(xmlDocPtr xml, - xmlNodePtr root, +virDomainObjParseFile(const char *filename, virDomainXMLOption *xmlopt, unsigned int flags) { + g_autoptr(xmlDoc) xml = NULL; g_autoptr(xmlXPathContext) ctxt = NULL; + int keepBlanksDefault = xmlKeepBlanksDefault(0); - if (!virXMLNodeNameEqual(root, "domstatus")) { - virReportError(VIR_ERR_XML_ERROR, - _("unexpected root element <%s>, " - "expecting <domstatus>"), - root->name); - return NULL; - } + xml = virXMLParse(filename, NULL, NULL, "domstatus", &ctxt, NULL, false); + xmlKeepBlanksDefault(keepBlanksDefault); - if (!(ctxt = virXMLXPathContextNew(xml))) + if (!xml) return NULL; - ctxt->node = root; return virDomainObjParseXML(ctxt, xmlopt, flags); } -virDomainObj * -virDomainObjParseFile(const char *filename, - virDomainXMLOption *xmlopt, - unsigned int flags) -{ - g_autoptr(xmlDoc) xml = NULL; - virDomainObj *obj = NULL; - int keepBlanksDefault = xmlKeepBlanksDefault(0); - - if ((xml = virXMLParseFile(filename))) { - obj = virDomainObjParseNode(xml, xmlDocGetRootElement(xml), - xmlopt, flags); - } - - xmlKeepBlanksDefault(keepBlanksDefault); - return obj; -} - - static bool virDomainTimerDefCheckABIStability(virDomainTimerDef *src, virDomainTimerDef *dst) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 352b88eae5..6eab1056c1 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3617,10 +3617,6 @@ virDomainDef *virDomainDefParseNode(xmlDocPtr doc, virDomainXMLOption *xmlopt, void *parseOpaque, unsigned int flags); -virDomainObj *virDomainObjParseNode(xmlDocPtr xml, - xmlNodePtr root, - virDomainXMLOption *xmlopt, - unsigned int flags); virDomainObj *virDomainObjParseFile(const char *filename, virDomainXMLOption *xmlopt, unsigned int flags); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 5714e0c114..00cb07709d 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -582,7 +582,6 @@ virDomainObjIsFailedPostcopy; virDomainObjIsPostcopy; virDomainObjNew; virDomainObjParseFile; -virDomainObjParseNode; virDomainObjRemoveTransientDef; virDomainObjSave; virDomainObjSetDefTransient; -- 2.37.3
Use virXMLParse's features to validate the top level element and fetch the XPath context. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 03f4b2fe33..bd882039e7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -19080,29 +19080,19 @@ virDomainDefParse(const char *xmlStr, unsigned int flags) { g_autoptr(xmlDoc) xml = NULL; - virDomainDef *def = NULL; + g_autoptr(xmlXPathContext) ctxt = NULL; int keepBlanksDefault = xmlKeepBlanksDefault(0); - xmlNodePtr root; bool validate = flags & VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA; - if (!(xml = virXMLParse(filename, xmlStr, _("(domain_definition)"), - NULL, NULL, "domain.rng", validate))) - goto cleanup; + xml = virXMLParse(filename, xmlStr, _("(domain_definition)"), + "domain", &ctxt, "domain.rng", validate); - root = xmlDocGetRootElement(xml); - if (!virXMLNodeNameEqual(root, "domain")) { - virReportError(VIR_ERR_XML_ERROR, - _("unexpected root element <%s>, " - "expecting <domain>"), - root->name); - goto cleanup; - } + xmlKeepBlanksDefault(keepBlanksDefault); - def = virDomainDefParseNode(xml, root, xmlopt, parseOpaque, flags); + if (!xml) + return NULL; - cleanup: - xmlKeepBlanksDefault(keepBlanksDefault); - return def; + return virDomainDefParseNode(xml, ctxt->node, xmlopt, parseOpaque, flags); } virDomainDef * -- 2.37.3
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/checkpoint_conf.c | 6 ++++-- src/conf/domain_conf.c | 11 ++--------- src/conf/domain_conf.h | 3 +-- src/conf/snapshot_conf.c | 16 ++++++++++------ src/qemu/qemu_migration_cookie.c | 10 ++++++---- src/test/test_driver.c | 8 ++++---- tests/qemuxml2argvtest.c | 3 +-- 7 files changed, 28 insertions(+), 29 deletions(-) diff --git a/src/conf/checkpoint_conf.c b/src/conf/checkpoint_conf.c index ea20bfdd14..6656089457 100644 --- a/src/conf/checkpoint_conf.c +++ b/src/conf/checkpoint_conf.c @@ -153,11 +153,13 @@ virDomainCheckpointDefParse(xmlXPathContextPtr ctxt, def->parent.parent_name = virXPathString("string(./parent/name)", ctxt); if ((domainNode = virXPathNode("./domain", ctxt))) { + VIR_XPATH_NODE_AUTORESTORE(ctxt) unsigned int domainParseFlags = VIR_DOMAIN_DEF_PARSE_INACTIVE | VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE; - def->parent.dom = virDomainDefParseNode(ctxt->node->doc, domainNode, - xmlopt, parseOpaque, + ctxt->node = domainNode; + + def->parent.dom = virDomainDefParseNode(ctxt, xmlopt, parseOpaque, domainParseFlags); if (!def->parent.dom) return NULL; diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index bd882039e7..bf165d0a64 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -19092,7 +19092,7 @@ virDomainDefParse(const char *xmlStr, if (!xml) return NULL; - return virDomainDefParseNode(xml, ctxt->node, xmlopt, parseOpaque, flags); + return virDomainDefParseNode(ctxt, xmlopt, parseOpaque, flags); } virDomainDef * @@ -19115,20 +19115,13 @@ virDomainDefParseFile(const char *filename, virDomainDef * -virDomainDefParseNode(xmlDocPtr xml, - xmlNodePtr root, +virDomainDefParseNode(xmlXPathContext *ctxt, virDomainXMLOption *xmlopt, void *parseOpaque, unsigned int flags) { - g_autoptr(xmlXPathContext) ctxt = NULL; g_autoptr(virDomainDef) def = NULL; - if (!(ctxt = virXMLXPathContextNew(xml))) - return NULL; - - ctxt->node = root; - if (!(def = virDomainDefParseXML(ctxt, xmlopt, flags))) return NULL; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 6eab1056c1..8f8a54bc41 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3612,8 +3612,7 @@ virDomainDef *virDomainDefParseFile(const char *filename, virDomainXMLOption *xmlopt, void *parseOpaque, unsigned int flags); -virDomainDef *virDomainDefParseNode(xmlDocPtr doc, - xmlNodePtr root, +virDomainDef *virDomainDefParseNode(xmlXPathContext *ctxt, virDomainXMLOption *xmlopt, void *parseOpaque, unsigned int flags); diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index afdc11876d..4b5b908d66 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -266,15 +266,15 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt, * clients will have to decide between best effort * initialization or outright failure. */ if ((domtype = virXPathString("string(./domain/@type)", ctxt))) { - xmlNodePtr domainNode = virXPathNode("./domain", ctxt); + VIR_XPATH_NODE_AUTORESTORE(ctxt) - if (!domainNode) { + if (!(ctxt->node = virXPathNode("./domain", ctxt))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing domain in snapshot")); return NULL; } - def->parent.dom = virDomainDefParseNode(ctxt->node->doc, domainNode, - xmlopt, parseOpaque, + + def->parent.dom = virDomainDefParseNode(ctxt, xmlopt, parseOpaque, domainflags); if (!def->parent.dom) return NULL; @@ -286,8 +286,12 @@ virDomainSnapshotDefParse(xmlXPathContextPtr ctxt, * VM. In case of absent, leave parent.inactiveDom NULL and use * parent.dom for config and live XML. */ if ((inactiveDomNode = virXPathNode("./inactiveDomain", ctxt))) { - def->parent.inactiveDom = virDomainDefParseNode(ctxt->node->doc, inactiveDomNode, - xmlopt, NULL, domainflags); + VIR_XPATH_NODE_AUTORESTORE(ctxt) + + ctxt->node = inactiveDomNode; + + def->parent.inactiveDom = virDomainDefParseNode(ctxt, xmlopt, NULL, + domainflags); if (!def->parent.inactiveDom) return NULL; } diff --git a/src/qemu/qemu_migration_cookie.c b/src/qemu/qemu_migration_cookie.c index a4e018e204..95e803b3e1 100644 --- a/src/qemu/qemu_migration_cookie.c +++ b/src/qemu/qemu_migration_cookie.c @@ -1261,7 +1261,6 @@ static int qemuMigrationCookieXMLParse(qemuMigrationCookie *mig, virQEMUDriver *driver, virQEMUCaps *qemuCaps, - xmlDocPtr doc, xmlXPathContextPtr ctxt, unsigned int flags) { @@ -1356,6 +1355,7 @@ qemuMigrationCookieXMLParse(qemuMigrationCookie *mig, if ((flags & QEMU_MIGRATION_COOKIE_PERSISTENT) && virXPathBoolean("count(./domain) > 0", ctxt)) { + VIR_XPATH_NODE_AUTORESTORE(ctxt) g_autofree xmlNodePtr *nodes = NULL; if ((virXPathNodeSet("./domain", ctxt, &nodes)) != 1) { @@ -1363,8 +1363,10 @@ qemuMigrationCookieXMLParse(qemuMigrationCookie *mig, _("Too many domain elements in migration cookie")); return -1; } - mig->persistent = virDomainDefParseNode(doc, nodes[0], - driver->xmlopt, qemuCaps, + + ctxt->node = nodes[0]; + + mig->persistent = virDomainDefParseNode(ctxt, driver->xmlopt, qemuCaps, VIR_DOMAIN_DEF_PARSE_INACTIVE | VIR_DOMAIN_DEF_PARSE_ABI_UPDATE_MIGRATION | VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE); @@ -1420,7 +1422,7 @@ qemuMigrationCookieXMLParseStr(qemuMigrationCookie *mig, if (!(doc = virXMLParseStringCtxt(xml, _("(qemu_migration_cookie)"), &ctxt))) return -1; - return qemuMigrationCookieXMLParse(mig, driver, qemuCaps, doc, ctxt, flags); + return qemuMigrationCookieXMLParse(mig, driver, qemuCaps, ctxt, flags); } diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 9f0517e89e..373e5f7846 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1019,14 +1019,14 @@ testParseDomains(testDriver *privconn, return -1; for (i = 0; i < num; i++) { + VIR_XPATH_NODE_AUTORESTORE(ctxt) g_autoptr(virDomainDef) def = NULL; testDomainNamespaceDef *nsdata; - xmlNodePtr node = testParseXMLDocFromFile(nodes[i], file); - if (!node) + + if (!(ctxt->node = testParseXMLDocFromFile(nodes[i], file))) goto error; - def = virDomainDefParseNode(ctxt->doc, node, - privconn->xmlopt, NULL, + def = virDomainDefParseNode(ctxt, privconn->xmlopt, NULL, VIR_DOMAIN_DEF_PARSE_INACTIVE); if (!def) goto error; diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 8785c96ce1..cad4c1abd5 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -709,8 +709,7 @@ testCompareXMLToArgv(const void *data) parseFlags |= VIR_DOMAIN_DEF_PARSE_INACTIVE; - if (!(vm->def = virDomainDefParseNode(xml, ctxt->node, driver.xmlopt, NULL, - parseFlags))) { + if (!(vm->def = virDomainDefParseNode(ctxt, driver.xmlopt, NULL, parseFlags))) { err = virGetLastError(); if (!err) { VIR_TEST_DEBUG("no error was reported for expected parse error"); -- 2.37.3
Use virXMLParseStringCtxt instead of virXMLParseString since the code requires a XPath context anyways. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tools/virsh-util.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tools/virsh-util.c b/tools/virsh-util.c index 8a20f627a1..3c4a084441 100644 --- a/tools/virsh-util.c +++ b/tools/virsh-util.c @@ -457,14 +457,11 @@ virshDumpXML(vshControl *ctl, } oldblanks = xmlKeepBlanksDefault(0); - doc = virXMLParseString(xml, url); + doc = virXMLParseStringCtxt(xml, url, &ctxt); xmlKeepBlanksDefault(oldblanks); if (!doc) return false; - if (!(ctxt = virXMLXPathContextNew(doc))) - return false; - if ((nnodes = virXPathNodeSet(xpath, ctxt, &nodes)) < 0) { return false; } -- 2.37.3
Most callers use virXMLParseStringCtxt. Convert the last use case and remove the helper. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 2 +- src/util/virxml.h | 12 ------------ 2 files changed, 1 insertion(+), 13 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index bf165d0a64..70501ca768 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -28851,7 +28851,7 @@ virDomainDefSetMetadata(virDomainDef *def, if (metadata) { /* parse and modify the xml from the user */ - if (!(doc = virXMLParseString(metadata, _("(metadata_xml)")))) + if (!(doc = virXMLParseStringCtxt(metadata, _("(metadata_xml)"), NULL))) return -1; if (virXMLInjectNamespace(doc->children, uri, key) < 0) diff --git a/src/util/virxml.h b/src/util/virxml.h index a80e795f6b..72d45f8018 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -216,18 +216,6 @@ virXMLPickShellSafeComment(const char *str1, #define virXMLParse(filename, xmlStr, url, rootelement, ctxt, schemafile, validate) \ virXMLParseHelper(VIR_FROM_THIS, filename, xmlStr, url, rootelement, ctxt, schemafile, validate) -/** - * virXMLParseString: - * @xmlStr: a string to parse - * @url: an optional filename to attribute the parse to - * - * Parse xml from a string. - * - * Return the parsed document object, or NULL on failure. - */ -#define virXMLParseString(xmlStr, url) \ - virXMLParseHelper(VIR_FROM_THIS, NULL, xmlStr, url, NULL, NULL, NULL, false) - /** * virXMLParseFile: * @filename: file to parse -- 2.37.3
Use virXMLParse so that the code doesn't have to explicitly allocate an XPath context and validate the root element. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_capabilities.c | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index e84310c79a..146c3daa39 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4359,22 +4359,9 @@ virQEMUCapsLoadCache(virArch hostArch, long long int l; unsigned long lu; - if (!(doc = virXMLParseFile(filename))) + if (!(doc = virXMLParse(filename, NULL, NULL, "qemuCaps", &ctxt, NULL, false))) return -1; - if (!(ctxt = virXMLXPathContextNew(doc))) - return -1; - - ctxt->node = xmlDocGetRootElement(doc); - - if (STRNEQ((const char *)ctxt->node->name, "qemuCaps")) { - virReportError(VIR_ERR_XML_ERROR, - _("unexpected root element <%s>, " - "expecting <qemuCaps>"), - ctxt->node->name); - return -1; - } - if (virXPathLongLong("string(./selfctime)", ctxt, &l) < 0) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing selfctime in QEMU capabilities XML")); -- 2.37.3
Most callers prefer using the XPath context. Convert the last user to use virXMLParseFileCtxt and remove the helper macro. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virxml.h | 11 ----------- tests/virschematest.c | 2 +- 2 files changed, 1 insertion(+), 12 deletions(-) diff --git a/src/util/virxml.h b/src/util/virxml.h index 72d45f8018..d773716a8c 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -216,17 +216,6 @@ virXMLPickShellSafeComment(const char *str1, #define virXMLParse(filename, xmlStr, url, rootelement, ctxt, schemafile, validate) \ virXMLParseHelper(VIR_FROM_THIS, filename, xmlStr, url, rootelement, ctxt, schemafile, validate) -/** - * virXMLParseFile: - * @filename: file to parse - * - * Parse xml from a file. - * - * Return the parsed document object, or NULL on failure. - */ -#define virXMLParseFile(filename) \ - virXMLParseHelper(VIR_FROM_THIS, filename, NULL, NULL, NULL, NULL, NULL, false) - /** * virXMLParseCtxt: * @filename: file to parse, or NULL for string parsing diff --git a/tests/virschematest.c b/tests/virschematest.c index dd13d82c62..cae502a338 100644 --- a/tests/virschematest.c +++ b/tests/virschematest.c @@ -51,7 +51,7 @@ testSchemaValidateXML(const void *args) bool shouldFail = virStringHasSuffix(data->xml_path, "-invalid.xml"); g_autoptr(xmlDoc) xml = NULL; - if (!(xml = virXMLParseFile(data->xml_path))) + if (!(xml = virXMLParseFileCtxt(data->xml_path, NULL))) return -1; if ((virXMLValidatorValidate(data->validator, xml) < 0) != shouldFail) -- 2.37.3
Convert the two outstanding uses to virXMLParseFileCtxt as they always pass a filename and remove the helper macro. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/virnetworkobj.c | 2 +- src/conf/virstorageobj.c | 2 +- src/util/virxml.h | 15 --------------- 3 files changed, 2 insertions(+), 17 deletions(-) diff --git a/src/conf/virnetworkobj.c b/src/conf/virnetworkobj.c index d23eb2f401..635d2ec0b0 100644 --- a/src/conf/virnetworkobj.c +++ b/src/conf/virnetworkobj.c @@ -858,7 +858,7 @@ virNetworkLoadState(virNetworkObjList *nets, if ((configFile = virNetworkConfigFile(stateDir, name)) == NULL) return NULL; - if (!(xml = virXMLParseCtxt(configFile, NULL, _("(network status)"), &ctxt))) + if (!(xml = virXMLParseFileCtxt(configFile, &ctxt))) return NULL; if (!(node = virXPathNode("//network", ctxt))) { diff --git a/src/conf/virstorageobj.c b/src/conf/virstorageobj.c index 77128a4846..98d9e0b97e 100644 --- a/src/conf/virstorageobj.c +++ b/src/conf/virstorageobj.c @@ -1622,7 +1622,7 @@ virStoragePoolObjLoadState(virStoragePoolObjList *pools, VIR_DEBUG("loading storage pool state XML '%s'", stateFile); - if (!(xml = virXMLParseCtxt(stateFile, NULL, _("(pool state)"), &ctxt))) + if (!(xml = virXMLParseFileCtxt(stateFile, &ctxt))) return NULL; if (!(node = virXPathNode("//pool", ctxt))) { diff --git a/src/util/virxml.h b/src/util/virxml.h index d773716a8c..9ec4920807 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -216,21 +216,6 @@ virXMLPickShellSafeComment(const char *str1, #define virXMLParse(filename, xmlStr, url, rootelement, ctxt, schemafile, validate) \ virXMLParseHelper(VIR_FROM_THIS, filename, xmlStr, url, rootelement, ctxt, schemafile, validate) -/** - * virXMLParseCtxt: - * @filename: file to parse, or NULL for string parsing - * @xmlStr: if @filename is NULL, a string to parse - * @url: if @filename is NULL, an optional filename to attribute the parse to - * @pctxt: if non-NULL, populate with a new context object on success, - * with (*pctxt)->node pre-set to the root node - * - * Parse xml from either a file or a string. - * - * Return the parsed document object, or NULL on failure. - */ -#define virXMLParseCtxt(filename, xmlStr, url, pctxt) \ - virXMLParseHelper(VIR_FROM_THIS, filename, xmlStr, url, NULL, pctxt, NULL, false) - /** * virXMLParseStringCtxt: * @xmlStr: a string to parse -- 2.37.3
The function provided just checking of the root XML node name which can be easily moved into the caller wich doesn't do that already and checking of the pointers which is trivial. Remove the helper. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/virsavecookie.c | 31 +++++++------------------------ 1 file changed, 7 insertions(+), 24 deletions(-) diff --git a/src/conf/virsavecookie.c b/src/conf/virsavecookie.c index 5fc9ca06e1..3a6361e745 100644 --- a/src/conf/virsavecookie.c +++ b/src/conf/virsavecookie.c @@ -32,26 +32,6 @@ VIR_LOG_INIT("conf.savecookie"); -static int -virSaveCookieParseNode(xmlXPathContextPtr ctxt, - virObject **obj, - virSaveCookieCallbacks *saveCookie) -{ - *obj = NULL; - - if (!virXMLNodeNameEqual(ctxt->node, "cookie")) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("XML does not contain expected 'cookie' element")); - return -1; - } - - if (!saveCookie || !saveCookie->parse) - return 0; - - return saveCookie->parse(ctxt, obj); -} - - int virSaveCookieParse(xmlXPathContextPtr ctxt, virObject **obj, @@ -64,7 +44,10 @@ virSaveCookieParse(xmlXPathContextPtr ctxt, if (!(ctxt->node = virXPathNode("./cookie", ctxt))) return 0; - return virSaveCookieParseNode(ctxt, obj, saveCookie); + if (!saveCookie || !saveCookie->parse) + return 0; + + return saveCookie->parse(ctxt, obj); } @@ -78,13 +61,13 @@ virSaveCookieParseString(const char *xml, *obj = NULL; - if (!xml) + if (!xml || !saveCookie || !saveCookie->parse) return 0; - if (!(doc = virXMLParseStringCtxt(xml, _("(save cookie)"), &ctxt))) + if (!(doc = virXMLParse(NULL, xml, _("(save cookie)"), "cookie", &ctxt, NULL, false))) return -1; - return virSaveCookieParseNode(ctxt, obj, saveCookie); + return saveCookie->parse(ctxt, obj); } -- 2.37.3
Use the helper with more features to validate the root XML element name instead of open-coding it. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/security/virt-aa-helper.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index f338488da3..33d600c00b 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -571,13 +571,8 @@ caps_mockup(vahControl * ctl, const char *xmlStr) g_autoptr(xmlXPathContext) ctxt = NULL; char *arch; - if (!(xml = virXMLParseStringCtxt(xmlStr, _("(domain_definition)"), - &ctxt))) { - return -1; - } - - if (!virXMLNodeNameEqual(ctxt->node, "domain")) { - vah_error(NULL, 0, _("unexpected root element, expecting <domain>")); + if (!(xml = virXMLParse(NULL, xmlStr, _("(domain_definition)"), + "domain", &ctxt, NULL, false))) { return -1; } -- 2.37.3
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/test/test_driver.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 373e5f7846..8675f8ad07 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1306,12 +1306,6 @@ testOpenParse(testDriver *privconn, const char *file, xmlXPathContextPtr ctxt) { - if (!virXMLNodeNameEqual(ctxt->node, "node")) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("Root element is not 'node'")); - return -1; - } - if (testParseNodeInfo(&privconn->nodeInfo, ctxt) < 0) return -1; if (testParseDomains(privconn, file, ctxt) < 0) @@ -1348,7 +1342,7 @@ testOpenFromFile(virConnectPtr conn, const char *file) if (!(privconn->caps = testBuildCapabilities(conn))) goto error; - if (!(doc = virXMLParseFileCtxt(file, &ctxt))) + if (!(doc = virXMLParse(file, NULL, NULL, "node", &ctxt, NULL, false))) goto error; privconn->numCells = 0; @@ -1411,8 +1405,8 @@ testOpenDefault(virConnectPtr conn) if (!(privconn->caps = testBuildCapabilities(conn))) goto error; - if (!(doc = virXMLParseStringCtxt(defaultConnXML, - _("(test driver)"), &ctxt))) + if (!(doc = virXMLParse(NULL, defaultConnXML, _("(test driver)"), + "node", &ctxt, NULL, false))) goto error; if (testOpenParse(privconn, NULL, ctxt) < 0) -- 2.37.3
Remove the seldom used helper in favor of full virXMLParse. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/conf/domain_conf.c | 6 ++++-- src/conf/storage_conf.c | 6 ++---- src/util/virxml.h | 4 ---- 3 files changed, 6 insertions(+), 10 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 70501ca768..7dba65cfeb 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13655,7 +13655,8 @@ virDomainDiskDefParse(const char *xmlStr, g_autoptr(xmlDoc) xml = NULL; g_autoptr(xmlXPathContext) ctxt = NULL; - if (!(xml = virXMLParseStringCtxtRoot(xmlStr, _("(disk_definition)"), "disk", &ctxt))) + if (!(xml = virXMLParse(NULL, xmlStr, _("(disk_definition)"), + "disk", &ctxt, NULL, false))) return NULL; return virDomainDiskDefParseXML(xmlopt, ctxt->node, ctxt, flags); @@ -13672,7 +13673,8 @@ virDomainDiskDefParseSource(const char *xmlStr, g_autoptr(virStorageSource) src = NULL; xmlNodePtr driverNode; - if (!(xml = virXMLParseStringCtxtRoot(xmlStr, _("(disk_definition)"), "disk", &ctxt))) + if (!(xml = virXMLParse(NULL, xmlStr, _("(disk_definition)"), + "disk", &ctxt, NULL, false))) return NULL; if (!(src = virDomainDiskDefParseSourceXML(xmlopt, ctxt->node, ctxt, flags))) diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index 4f2f9e7fb1..0f4fe1451e 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -667,10 +667,8 @@ virStoragePoolDefParseSourceString(const char *srcSpec, g_autoptr(xmlXPathContext) xpath_ctxt = NULL; g_autoptr(virStoragePoolSource) def = NULL; - if (!(doc = virXMLParseStringCtxtRoot(srcSpec, - _("(storage_source_specification)"), - "source", - &xpath_ctxt))) + if (!(doc = virXMLParse(NULL, srcSpec, _("(storage_source_specification)"), + "source", &xpath_ctxt, NULL, false))) return NULL; def = g_new0(virStoragePoolSource, 1); diff --git a/src/util/virxml.h b/src/util/virxml.h index 9ec4920807..2b442d60fe 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -230,10 +230,6 @@ virXMLPickShellSafeComment(const char *str1, #define virXMLParseStringCtxt(xmlStr, url, pctxt) \ virXMLParseHelper(VIR_FROM_THIS, NULL, xmlStr, url, NULL, pctxt, NULL, false) -/* virXMLParseStringCtxtRoot is same as above, except it also validates root node name */ -#define virXMLParseStringCtxtRoot(xmlStr, url, rootnode, pctxt) \ - virXMLParseHelper(VIR_FROM_THIS, NULL, xmlStr, url, rootnode, pctxt, NULL, false) - /** * virXMLParseFileCtxt: * @filename: file to parse -- 2.37.3
On a Tuesday in 2022, Peter Krempa wrote:
This series cleans up the use of virXMLParse* macros and tries to unify and remove the uncommon ones.
In next step since virXMLParse itself has features which can replace open-coded bits in conf parsers for all various object types we have, this series cleans them up too.
Peter Krempa (43): util: xml: Expose all arguments of virXMLParseHelper in virXMLParse macro conf: nwfilderbindigobj: Register automatic cleanup for virNWFilterBindingObj [...] security: aa-helper: Use virXMLParse instead of virXMLParseString test_driver: Make callers of testOpenParse ensure the root element name util: xml: Remove virXMLParseStringCtxtRoot
src/conf/backup_conf.c | 42 ++------ src/conf/backup_conf.h | 10 +- [...] tests/virschematest.c | 2 +- tools/virsh-util.c | 5 +- 69 files changed, 399 insertions(+), 1098 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Peter Krempa