[PATCH 0/5] src: move validation to virXMLParseHelper()
This series avoids duplication of the same code used for validation of XML document against rng schema and makes it easier to add validation of other objects in the future as well. Kristina Hanicova (5): src: add 'schema' and 'validate' variable to virXMLParseHelper() util: add xml validation against schema in virXMLParseHelper() domain_conf: replace validation with variables passed to virXMLParse() domain_conf: Remove redundant variable conf: replace validation with variables passed to virXMLParse() src/conf/backup_conf.c | 13 ++----------- src/conf/checkpoint_conf.c | 12 ++---------- src/conf/domain_conf.c | 27 +++++++-------------------- src/conf/interface_conf.c | 2 +- src/conf/network_conf.c | 2 +- src/conf/node_device_conf.c | 2 +- src/conf/nwfilter_conf.c | 2 +- src/conf/secret_conf.c | 2 +- src/conf/snapshot_conf.c | 15 ++------------- src/conf/storage_conf.c | 4 ++-- src/conf/virnetworkportdef.c | 2 +- src/conf/virnwfilterbindingdef.c | 2 +- src/conf/virnwfilterbindingobj.c | 2 +- src/test/test_driver.c | 2 +- src/util/virxml.c | 16 +++++++++++++++- src/util/virxml.h | 20 +++++++++++--------- src/vbox/vbox_snapshot_conf.c | 6 +++--- src/vz/vz_sdk.c | 2 +- tests/qemuxml2argvtest.c | 2 +- 19 files changed, 55 insertions(+), 80 deletions(-) -- 2.31.1
Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- src/conf/backup_conf.c | 2 +- src/conf/checkpoint_conf.c | 2 +- src/conf/domain_conf.c | 2 +- src/conf/interface_conf.c | 2 +- src/conf/network_conf.c | 2 +- src/conf/node_device_conf.c | 2 +- src/conf/nwfilter_conf.c | 2 +- src/conf/secret_conf.c | 2 +- src/conf/snapshot_conf.c | 2 +- src/conf/storage_conf.c | 4 ++-- src/conf/virnetworkportdef.c | 2 +- src/conf/virnwfilterbindingdef.c | 2 +- src/conf/virnwfilterbindingobj.c | 2 +- src/test/test_driver.c | 2 +- src/util/virxml.c | 6 +++++- src/util/virxml.h | 20 +++++++++++--------- src/vbox/vbox_snapshot_conf.c | 6 +++--- src/vz/vz_sdk.c | 2 +- tests/qemuxml2argvtest.c | 2 +- 19 files changed, 36 insertions(+), 30 deletions(-) diff --git a/src/conf/backup_conf.c b/src/conf/backup_conf.c index ac92bd4f26..9307357d84 100644 --- a/src/conf/backup_conf.c +++ b/src/conf/backup_conf.c @@ -281,7 +281,7 @@ virDomainBackupDefParseString(const char *xmlStr, g_autoptr(xmlDoc) xml = NULL; int keepBlanksDefault = xmlKeepBlanksDefault(0); - if ((xml = virXMLParse(NULL, xmlStr, _("(domain_backup)")))) { + if ((xml = virXMLParse(NULL, xmlStr, _("(domain_backup)"), NULL, false))) { xmlKeepBlanksDefault(keepBlanksDefault); ret = virDomainBackupDefParseNode(xml, xmlDocGetRootElement(xml), xmlopt, flags); diff --git a/src/conf/checkpoint_conf.c b/src/conf/checkpoint_conf.c index 7b26da416b..dd0e6035fa 100644 --- a/src/conf/checkpoint_conf.c +++ b/src/conf/checkpoint_conf.c @@ -226,7 +226,7 @@ virDomainCheckpointDefParseString(const char *xmlStr, xmlDocPtr xml; int keepBlanksDefault = xmlKeepBlanksDefault(0); - if ((xml = virXMLParse(NULL, xmlStr, _("(domain_checkpoint)")))) { + if ((xml = virXMLParse(NULL, xmlStr, _("(domain_checkpoint)"), NULL, false))) { xmlKeepBlanksDefault(keepBlanksDefault); ret = virDomainCheckpointDefParseNode(xml, xmlDocGetRootElement(xml), xmlopt, parseOpaque, flags); diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 15452ef6fc..b996a0cc3c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -20400,7 +20400,7 @@ virDomainDefParse(const char *xmlStr, virDomainDef *def = NULL; int keepBlanksDefault = xmlKeepBlanksDefault(0); xmlNodePtr root; - if (!(xml = virXMLParse(filename, xmlStr, _("(domain_definition)")))) + if (!(xml = virXMLParse(filename, xmlStr, _("(domain_definition)"), NULL, false))) goto cleanup; root = xmlDocGetRootElement(xml); diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c index d84ec66def..c4b763d0de 100644 --- a/src/conf/interface_conf.c +++ b/src/conf/interface_conf.c @@ -824,7 +824,7 @@ virInterfaceDefParse(const char *xmlStr, xmlDocPtr xml; virInterfaceDef *def = NULL; - if ((xml = virXMLParse(filename, xmlStr, _("(interface_definition)")))) { + if ((xml = virXMLParse(filename, xmlStr, _("(interface_definition)"), NULL, false))) { def = virInterfaceDefParseNode(xml, xmlDocGetRootElement(xml)); xmlFreeDoc(xml); } diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index e34ac52a68..fbd939a1f1 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -2091,7 +2091,7 @@ virNetworkDefParse(const char *xmlStr, virNetworkDef *def = NULL; int keepBlanksDefault = xmlKeepBlanksDefault(0); - if ((xml = virXMLParse(filename, xmlStr, _("(network_definition)")))) + if ((xml = virXMLParse(filename, xmlStr, _("(network_definition)"), NULL, false))) 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 d6093f4aa9..bda1a11b37 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -2177,7 +2177,7 @@ virNodeDeviceDefParse(const char *str, xmlDocPtr xml; g_autoptr(virNodeDeviceDef) def = NULL; - if ((xml = virXMLParse(filename, str, _("(node_device_definition)")))) { + if ((xml = virXMLParse(filename, str, _("(node_device_definition)"), NULL, false))) { def = virNodeDeviceDefParseNode(xml, xmlDocGetRootElement(xml), create, virt_type); xmlFreeDoc(xml); diff --git a/src/conf/nwfilter_conf.c b/src/conf/nwfilter_conf.c index fc81fd97ea..b7adae7161 100644 --- a/src/conf/nwfilter_conf.c +++ b/src/conf/nwfilter_conf.c @@ -2744,7 +2744,7 @@ virNWFilterDefParse(const char *xmlStr, virNWFilterDef *def = NULL; xmlDocPtr xml; - if ((xml = virXMLParse(filename, xmlStr, _("(nwfilter_definition)")))) { + if ((xml = virXMLParse(filename, xmlStr, _("(nwfilter_definition)"), NULL, false))) { def = virNWFilterDefParseNode(xml, xmlDocGetRootElement(xml)); xmlFreeDoc(xml); } diff --git a/src/conf/secret_conf.c b/src/conf/secret_conf.c index 7bfdc58b83..5968b4365c 100644 --- a/src/conf/secret_conf.c +++ b/src/conf/secret_conf.c @@ -196,7 +196,7 @@ virSecretDefParse(const char *xmlStr, xmlDocPtr xml; virSecretDef *ret = NULL; - if ((xml = virXMLParse(filename, xmlStr, _("(definition_of_secret)")))) { + if ((xml = virXMLParse(filename, xmlStr, _("(definition_of_secret)"), NULL, false))) { ret = secretXMLParseNode(xml, xmlDocGetRootElement(xml)); xmlFreeDoc(xml); } diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index 0592640dd9..3282627044 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -462,7 +462,7 @@ virDomainSnapshotDefParseString(const char *xmlStr, xmlDocPtr xml; int keepBlanksDefault = xmlKeepBlanksDefault(0); - if ((xml = virXMLParse(NULL, xmlStr, _("(domain_snapshot)")))) { + if ((xml = virXMLParse(NULL, xmlStr, _("(domain_snapshot)"), NULL, false))) { xmlKeepBlanksDefault(keepBlanksDefault); ret = virDomainSnapshotDefParseNode(xml, xmlDocGetRootElement(xml), xmlopt, parseOpaque, diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c index ad2eb66417..e72e8d0ade 100644 --- a/src/conf/storage_conf.c +++ b/src/conf/storage_conf.c @@ -1009,7 +1009,7 @@ virStoragePoolDefParse(const char *xmlStr, virStoragePoolDef *ret = NULL; xmlDocPtr xml; - if ((xml = virXMLParse(filename, xmlStr, _("(storage_pool_definition)")))) { + if ((xml = virXMLParse(filename, xmlStr, _("(storage_pool_definition)"), NULL, false))) { ret = virStoragePoolDefParseNode(xml, xmlDocGetRootElement(xml)); xmlFreeDoc(xml); } @@ -1478,7 +1478,7 @@ virStorageVolDefParse(virStoragePoolDef *pool, virStorageVolDef *ret = NULL; xmlDocPtr xml; - if ((xml = virXMLParse(filename, xmlStr, _("(storage_volume_definition)")))) { + if ((xml = virXMLParse(filename, xmlStr, _("(storage_volume_definition)"), NULL, false))) { ret = virStorageVolDefParseNode(pool, xml, xmlDocGetRootElement(xml), flags); xmlFreeDoc(xml); } diff --git a/src/conf/virnetworkportdef.c b/src/conf/virnetworkportdef.c index adda66590a..9c2a6298d2 100644 --- a/src/conf/virnetworkportdef.c +++ b/src/conf/virnetworkportdef.c @@ -297,7 +297,7 @@ virNetworkPortDefParse(const char *xmlStr, virNetworkPortDef *def = NULL; xmlDocPtr xml; - if ((xml = virXMLParse(filename, xmlStr, _("(networkport_definition)")))) { + if ((xml = virXMLParse(filename, xmlStr, _("(networkport_definition)"), NULL, false))) { def = virNetworkPortDefParseNode(xml, xmlDocGetRootElement(xml)); xmlFreeDoc(xml); } diff --git a/src/conf/virnwfilterbindingdef.c b/src/conf/virnwfilterbindingdef.c index 22ecf7b828..4079fcdba4 100644 --- a/src/conf/virnwfilterbindingdef.c +++ b/src/conf/virnwfilterbindingdef.c @@ -188,7 +188,7 @@ virNWFilterBindingDefParse(const char *xmlStr, virNWFilterBindingDef *def = NULL; xmlDocPtr xml; - if ((xml = virXMLParse(filename, xmlStr, _("(nwfilterbinding_definition)")))) { + if ((xml = virXMLParse(filename, xmlStr, _("(nwfilterbinding_definition)"), NULL, false))) { def = virNWFilterBindingDefParseNode(xml, xmlDocGetRootElement(xml)); xmlFreeDoc(xml); } diff --git a/src/conf/virnwfilterbindingobj.c b/src/conf/virnwfilterbindingobj.c index d48c95af88..11426e0a4d 100644 --- a/src/conf/virnwfilterbindingobj.c +++ b/src/conf/virnwfilterbindingobj.c @@ -265,7 +265,7 @@ virNWFilterBindingObjParse(const char *xmlStr, virNWFilterBindingObj *obj = NULL; xmlDocPtr xml; - if ((xml = virXMLParse(filename, xmlStr, _("(nwfilterbinding_status)")))) { + if ((xml = virXMLParse(filename, xmlStr, _("(nwfilterbinding_status)"), NULL, false))) { obj = virNWFilterBindingObjParseNode(xml, xmlDocGetRootElement(xml)); xmlFreeDoc(xml); } diff --git a/src/test/test_driver.c b/src/test/test_driver.c index 7c3bb70be3..00cc13511a 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -832,7 +832,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))) + if (!(doc = virXMLParse(absFile, NULL, type, NULL, false))) goto error; ret = xmlCopyNode(xmlDocGetRootElement(doc), 1); diff --git a/src/util/virxml.c b/src/util/virxml.c index 4360b15486..0220c5906f 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -1096,6 +1096,8 @@ catchXMLError(void *ctx, const char *msg G_GNUC_UNUSED, ...) * @url: URL of XML document for string parser * @rootelement: Optional name of the expected root element * @ctxt: optional pointer to populate with new context pointer + * @schemafile: unused + * @validate: unused * * Parse XML document provided either as a file or a string. The function * guarantees that the XML document contains a root element. @@ -1111,7 +1113,9 @@ virXMLParseHelper(int domcode, const char *xmlStr, const char *url, const char *rootelement, - xmlXPathContextPtr *ctxt) + xmlXPathContextPtr *ctxt, + const char *schemafile G_GNUC_UNUSED, + bool validate G_GNUC_UNUSED) { struct virParserData private; g_autoptr(xmlParserCtxt) pctxt = NULL; diff --git a/src/util/virxml.h b/src/util/virxml.h index 0bb0d1c118..e9359b1ef1 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -170,7 +170,9 @@ virXMLParseHelper(int domcode, const char *xmlStr, const char *url, const char *rootelement, - xmlXPathContextPtr *ctxt); + xmlXPathContextPtr *ctxt, + const char *schemafile, + bool validate); const char * virXMLPickShellSafeComment(const char *str1, @@ -185,8 +187,8 @@ virXMLPickShellSafeComment(const char *str1, * * Return the parsed document object, or NULL on failure. */ -#define virXMLParse(filename, xmlStr, url) \ - virXMLParseHelper(VIR_FROM_THIS, filename, xmlStr, url, NULL, NULL) +#define virXMLParse(filename, xmlStr, url, schemafile, validate) \ + virXMLParseHelper(VIR_FROM_THIS, filename, xmlStr, url, NULL, NULL, schemafile, validate) /** * virXMLParseString: @@ -198,7 +200,7 @@ virXMLPickShellSafeComment(const char *str1, * Return the parsed document object, or NULL on failure. */ #define virXMLParseString(xmlStr, url) \ - virXMLParseHelper(VIR_FROM_THIS, NULL, xmlStr, url, NULL, NULL) + virXMLParseHelper(VIR_FROM_THIS, NULL, xmlStr, url, NULL, NULL, NULL, false) /** * virXMLParseFile: @@ -209,7 +211,7 @@ virXMLPickShellSafeComment(const char *str1, * Return the parsed document object, or NULL on failure. */ #define virXMLParseFile(filename) \ - virXMLParseHelper(VIR_FROM_THIS, filename, NULL, NULL, NULL, NULL) + virXMLParseHelper(VIR_FROM_THIS, filename, NULL, NULL, NULL, NULL, NULL, false) /** * virXMLParseCtxt: @@ -224,7 +226,7 @@ virXMLPickShellSafeComment(const char *str1, * Return the parsed document object, or NULL on failure. */ #define virXMLParseCtxt(filename, xmlStr, url, pctxt) \ - virXMLParseHelper(VIR_FROM_THIS, filename, xmlStr, url, NULL, pctxt) + virXMLParseHelper(VIR_FROM_THIS, filename, xmlStr, url, NULL, pctxt, NULL, false) /** * virXMLParseStringCtxt: @@ -238,11 +240,11 @@ virXMLPickShellSafeComment(const char *str1, * Return the parsed document object, or NULL on failure. */ #define virXMLParseStringCtxt(xmlStr, url, pctxt) \ - virXMLParseHelper(VIR_FROM_THIS, NULL, xmlStr, url, NULL, 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) + virXMLParseHelper(VIR_FROM_THIS, NULL, xmlStr, url, rootnode, pctxt, NULL, false) /** * virXMLParseFileCtxt: @@ -255,7 +257,7 @@ virXMLPickShellSafeComment(const char *str1, * Return the parsed document object, or NULL on failure. */ #define virXMLParseFileCtxt(filename, pctxt) \ - virXMLParseHelper(VIR_FROM_THIS, filename, NULL, NULL, NULL, pctxt) + virXMLParseHelper(VIR_FROM_THIS, filename, NULL, NULL, NULL, pctxt, NULL, false) int virXMLSaveFile(const char *path, diff --git a/src/vbox/vbox_snapshot_conf.c b/src/vbox/vbox_snapshot_conf.c index f7423f60fc..7fb97d49d9 100644 --- a/src/vbox/vbox_snapshot_conf.c +++ b/src/vbox/vbox_snapshot_conf.c @@ -593,7 +593,7 @@ virVBoxSnapshotConfLoadVboxFile(const char *filePath, machineDescription = g_new0(virVBoxSnapshotConfMachine, 1); - xml = virXMLParse(filePath, NULL, NULL); + xml = virXMLParse(filePath, NULL, NULL, NULL, false); if (xml == NULL) { virReportError(VIR_ERR_XML_ERROR, "%s", _("Unable to parse the xml")); @@ -1230,7 +1230,7 @@ virVBoxSnapshotConfGetRWDisksPathsFromLibvirtXML(const char *filePath, _("filePath is null")); goto cleanup; } - xml = virXMLParse(filePath, NULL, NULL); + xml = virXMLParse(filePath, NULL, NULL, NULL, false); if (xml == NULL) { virReportError(VIR_ERR_XML_ERROR, "%s", _("Unable to parse the xml")); @@ -1292,7 +1292,7 @@ virVBoxSnapshotConfGetRODisksPathsFromLibvirtXML(const char *filePath, _("filePath is null")); goto cleanup; } - xml = virXMLParse(filePath, NULL, NULL); + xml = virXMLParse(filePath, 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 26a3acb386..6d3a873158 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -4595,7 +4595,7 @@ prlsdkParseSnapshotTree(const char *treexml) if (*treexml == '\0') return snapshots; - if (!(xml = virXMLParse(NULL, treexml, _("(snapshot_tree)")))) + if (!(xml = virXMLParse(NULL, treexml, _("(snapshot_tree)"), NULL, false))) goto cleanup; root = xmlDocGetRootElement(xml); diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index b552f5deed..2754a8a780 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -678,7 +678,7 @@ testCompareXMLToArgv(const void *data) if (testCheckExclusiveFlags(info->flags) < 0) goto cleanup; - if (!(xml = virXMLParse(info->infile, NULL, "(domain_definition)"))) + if (!(xml = virXMLParse(info->infile, NULL, "(domain_definition)", NULL, false))) goto cleanup; root = xmlDocGetRootElement(xml); -- 2.31.1
We need this in order to validate XML against schema at one place, rather than have the same code for validation in different functions. I will add '--validate' option to more virsh commands soon and this makes it easier as virXMLParse() is called in every one I plan to change. Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- src/util/virxml.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/util/virxml.c b/src/util/virxml.c index 0220c5906f..b896a80460 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -33,6 +33,7 @@ #include "virfile.h" #include "virstring.h" #include "virutil.h" +#include "configmake.h" #define VIR_FROM_THIS VIR_FROM_XML @@ -1096,8 +1097,8 @@ catchXMLError(void *ctx, const char *msg G_GNUC_UNUSED, ...) * @url: URL of XML document for string parser * @rootelement: Optional name of the expected root element * @ctxt: optional pointer to populate with new context pointer - * @schemafile: unused - * @validate: unused + * @schemafile: optional name of the file the parsed XML will be validated against + * @validate: if true, the XML will be validated against schema in @schemafile * * Parse XML document provided either as a file or a string. The function * guarantees that the XML document contains a root element. @@ -1114,8 +1115,8 @@ virXMLParseHelper(int domcode, const char *url, const char *rootelement, xmlXPathContextPtr *ctxt, - const char *schemafile G_GNUC_UNUSED, - bool validate G_GNUC_UNUSED) + const char *schemafile, + bool validate) { struct virParserData private; g_autoptr(xmlParserCtxt) pctxt = NULL; @@ -1181,6 +1182,15 @@ virXMLParseHelper(int domcode, (*ctxt)->node = rootnode; } + if (validate && schemafile != NULL) { + g_autofree char *schema = virFileFindResource(schemafile, + abs_top_srcdir "/docs/schemas", + PKGDATADIR "/schemas"); + if (!schema || + (virXMLValidateAgainstSchema(schema, xml) < 0)) + return NULL; + } + return g_steal_pointer(&xml); } -- 2.31.1
virXMLParse() now allows to validate xml against schema directly, eliminating the need to do it individually. Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- src/conf/domain_conf.c | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b996a0cc3c..492d5524ec 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -19515,7 +19515,7 @@ virDomainDefControllersParse(virDomainDef *def, } static virDomainDef * -virDomainDefParseXML(xmlDocPtr xml, +virDomainDefParseXML(xmlDocPtr xml G_GNUC_UNUSED, xmlXPathContextPtr ctxt, virDomainXMLOption *xmlopt, unsigned int flags) @@ -19529,18 +19529,6 @@ virDomainDefParseXML(xmlDocPtr xml, g_autofree xmlNodePtr *nodes = NULL; g_autofree char *tmp = NULL; - if (flags & VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA) { - g_autofree char *schema = NULL; - - schema = virFileFindResource("domain.rng", - abs_top_srcdir "/docs/schemas", - PKGDATADIR "/schemas"); - if (!schema) - return NULL; - if (virXMLValidateAgainstSchema(schema, xml) < 0) - return NULL; - } - if (!(def = virDomainDefNew())) return NULL; @@ -20400,7 +20388,8 @@ virDomainDefParse(const char *xmlStr, virDomainDef *def = NULL; int keepBlanksDefault = xmlKeepBlanksDefault(0); xmlNodePtr root; - if (!(xml = virXMLParse(filename, xmlStr, _("(domain_definition)"), NULL, false))) + if (!(xml = virXMLParse(filename, xmlStr, _("(domain_definition)"), "domain.rng", + flags & VIR_DOMAIN_DEF_PARSE_VALIDATE_SCHEMA))) goto cleanup; root = xmlDocGetRootElement(xml); -- 2.31.1
xmlDocPtr is no longer needed, because validation against schema was moved to another function. Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- src/conf/domain_conf.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 492d5524ec..16cbb7d0c3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -19515,8 +19515,7 @@ virDomainDefControllersParse(virDomainDef *def, } static virDomainDef * -virDomainDefParseXML(xmlDocPtr xml G_GNUC_UNUSED, - xmlXPathContextPtr ctxt, +virDomainDefParseXML(xmlXPathContextPtr ctxt, virDomainXMLOption *xmlopt, unsigned int flags) { @@ -20281,8 +20280,7 @@ virDomainDefParseXML(xmlDocPtr xml G_GNUC_UNUSED, static virDomainObj * -virDomainObjParseXML(xmlDocPtr xml, - xmlXPathContextPtr ctxt, +virDomainObjParseXML(xmlXPathContextPtr ctxt, virDomainXMLOption *xmlopt, unsigned int flags) { @@ -20310,7 +20308,7 @@ virDomainObjParseXML(xmlDocPtr xml, oldnode = ctxt->node; ctxt->node = config; - obj->def = virDomainDefParseXML(xml, ctxt, xmlopt, flags); + obj->def = virDomainDefParseXML(ctxt, xmlopt, flags); ctxt->node = oldnode; if (!obj->def) return NULL; @@ -20443,7 +20441,7 @@ virDomainDefParseNode(xmlDocPtr xml, ctxt->node = root; - if (!(def = virDomainDefParseXML(xml, ctxt, xmlopt, flags))) + if (!(def = virDomainDefParseXML(ctxt, xmlopt, flags))) return NULL; /* callback to fill driver specific domain aspects */ @@ -20478,7 +20476,7 @@ virDomainObjParseNode(xmlDocPtr xml, return NULL; ctxt->node = root; - return virDomainObjParseXML(xml, ctxt, xmlopt, flags); + return virDomainObjParseXML(ctxt, xmlopt, flags); } -- 2.31.1
virXMLParse() now allows to validate xml against schema directly, eliminating the need to do it individually in each function. Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- src/conf/backup_conf.c | 13 ++----------- src/conf/checkpoint_conf.c | 12 ++---------- src/conf/snapshot_conf.c | 15 ++------------- 3 files changed, 6 insertions(+), 34 deletions(-) diff --git a/src/conf/backup_conf.c b/src/conf/backup_conf.c index 9307357d84..8e378a5d26 100644 --- a/src/conf/backup_conf.c +++ b/src/conf/backup_conf.c @@ -281,7 +281,8 @@ virDomainBackupDefParseString(const char *xmlStr, g_autoptr(xmlDoc) xml = NULL; int keepBlanksDefault = xmlKeepBlanksDefault(0); - if ((xml = virXMLParse(NULL, xmlStr, _("(domain_backup)"), NULL, false))) { + if ((xml = virXMLParse(NULL, xmlStr, _("(domain_backup)"), "domainbackup.rng", + !(flags & VIR_DOMAIN_BACKUP_PARSE_INTERNAL)))) { xmlKeepBlanksDefault(keepBlanksDefault); ret = virDomainBackupDefParseNode(xml, xmlDocGetRootElement(xml), xmlopt, flags); @@ -306,16 +307,6 @@ virDomainBackupDefParseNode(xmlDocPtr xml, return NULL; } - if (!(flags & VIR_DOMAIN_BACKUP_PARSE_INTERNAL)) { - if (!(schema = virFileFindResource("domainbackup.rng", - abs_top_srcdir "/docs/schemas", - PKGDATADIR "/schemas"))) - return NULL; - - if (virXMLValidateAgainstSchema(schema, xml) < 0) - return NULL; - } - if (!(ctxt = virXMLXPathContextNew(xml))) return NULL; diff --git a/src/conf/checkpoint_conf.c b/src/conf/checkpoint_conf.c index dd0e6035fa..ccb01b87f9 100644 --- a/src/conf/checkpoint_conf.c +++ b/src/conf/checkpoint_conf.c @@ -200,15 +200,6 @@ virDomainCheckpointDefParseNode(xmlDocPtr xml, return NULL; } - /* This is a new enough API to make schema validation unconditional */ - schema = virFileFindResource("domaincheckpoint.rng", - abs_top_srcdir "/docs/schemas", - PKGDATADIR "/schemas"); - if (!schema) - return NULL; - if (virXMLValidateAgainstSchema(schema, xml) < 0) - return NULL; - if (!(ctxt = virXMLXPathContextNew(xml))) return NULL; @@ -226,7 +217,8 @@ virDomainCheckpointDefParseString(const char *xmlStr, xmlDocPtr xml; int keepBlanksDefault = xmlKeepBlanksDefault(0); - if ((xml = virXMLParse(NULL, xmlStr, _("(domain_checkpoint)"), NULL, false))) { + if ((xml = virXMLParse(NULL, xmlStr, _("(domain_checkpoint)"), + "domaincheckpoint.rng", true))) { xmlKeepBlanksDefault(keepBlanksDefault); ret = virDomainCheckpointDefParseNode(xml, xmlDocGetRootElement(xml), xmlopt, parseOpaque, flags); diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index 3282627044..6d3c59f98e 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -432,18 +432,6 @@ virDomainSnapshotDefParseNode(xmlDocPtr xml, return NULL; } - if (flags & VIR_DOMAIN_SNAPSHOT_PARSE_VALIDATE) { - g_autofree char *schema = NULL; - - schema = virFileFindResource("domainsnapshot.rng", - abs_top_srcdir "/docs/schemas", - PKGDATADIR "/schemas"); - if (!schema) - return NULL; - if (virXMLValidateAgainstSchema(schema, xml) < 0) - return NULL; - } - if (!(ctxt = virXMLXPathContextNew(xml))) return NULL; @@ -462,7 +450,8 @@ virDomainSnapshotDefParseString(const char *xmlStr, xmlDocPtr xml; int keepBlanksDefault = xmlKeepBlanksDefault(0); - if ((xml = virXMLParse(NULL, xmlStr, _("(domain_snapshot)"), NULL, false))) { + if ((xml = virXMLParse(NULL, xmlStr, _("(domain_snapshot)"), "domainsnapshot.rng", + flags & VIR_DOMAIN_SNAPSHOT_PARSE_VALIDATE))) { xmlKeepBlanksDefault(keepBlanksDefault); ret = virDomainSnapshotDefParseNode(xml, xmlDocGetRootElement(xml), xmlopt, parseOpaque, -- 2.31.1
On a %A in %Y, Kristina Hanicova wrote:
virXMLParse() now allows to validate xml against schema directly,
s/allows to validate/allows validating/
eliminating the need to do it individually in each function.
Signed-off-by: Kristina Hanicova <khanicov@redhat.com> --- src/conf/backup_conf.c | 13 ++----------- src/conf/checkpoint_conf.c | 12 ++---------- src/conf/snapshot_conf.c | 15 ++------------- 3 files changed, 6 insertions(+), 34 deletions(-)
diff --git a/src/conf/backup_conf.c b/src/conf/backup_conf.c index 9307357d84..8e378a5d26 100644 --- a/src/conf/backup_conf.c +++ b/src/conf/backup_conf.c @@ -281,7 +281,8 @@ virDomainBackupDefParseString(const char *xmlStr, g_autoptr(xmlDoc) xml = NULL; int keepBlanksDefault = xmlKeepBlanksDefault(0);
- if ((xml = virXMLParse(NULL, xmlStr, _("(domain_backup)"), NULL, false))) { + if ((xml = virXMLParse(NULL, xmlStr, _("(domain_backup)"), "domainbackup.rng", + !(flags & VIR_DOMAIN_BACKUP_PARSE_INTERNAL)))) { xmlKeepBlanksDefault(keepBlanksDefault); ret = virDomainBackupDefParseNode(xml, xmlDocGetRootElement(xml), xmlopt, flags); @@ -306,16 +307,6 @@ virDomainBackupDefParseNode(xmlDocPtr xml, return NULL; }
- if (!(flags & VIR_DOMAIN_BACKUP_PARSE_INTERNAL)) { - if (!(schema = virFileFindResource("domainbackup.rng", - abs_top_srcdir "/docs/schemas", - PKGDATADIR "/schemas"))) - return NULL; - - if (virXMLValidateAgainstSchema(schema, xml) < 0) - return NULL; - } - if (!(ctxt = virXMLXPathContextNew(xml))) return NULL;
The declaration of 'schema' also needs to go, otherwise clang complains: ../src/conf/backup_conf.c:303:22: error: unused variable 'schema' [-Werror,-Wunused-variable] g_autofree char *schema = NULL; ^ 1 error generated.
diff --git a/src/conf/checkpoint_conf.c b/src/conf/checkpoint_conf.c index dd0e6035fa..ccb01b87f9 100644 --- a/src/conf/checkpoint_conf.c +++ b/src/conf/checkpoint_conf.c @@ -200,15 +200,6 @@ virDomainCheckpointDefParseNode(xmlDocPtr xml, return NULL; }
- /* This is a new enough API to make schema validation unconditional */ - schema = virFileFindResource("domaincheckpoint.rng", - abs_top_srcdir "/docs/schemas", - PKGDATADIR "/schemas"); - if (!schema) - return NULL; - if (virXMLValidateAgainstSchema(schema, xml) < 0) - return NULL; -
same here.
if (!(ctxt = virXMLXPathContextNew(xml))) return NULL;
For the whole series: Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Jano Tomko -
Kristina Hanicova