[libvirt] [PATCHv2 0/5] Speed up schema testing
Instead of spawining a separate xmllint process for every file, introduce virXMLValidator APIs and use them in a C test file. Originally, the shell-based schema tests took about 15s in series (with domainschematest making up for about 13s of that). Now, all the tests run in series take ~.57 ms. This reduces total make check time from ~16s to ~8s, making it more fun to use with git rebase -x. Ján Tomko (5): Introduce virXMLValidator structure Introduce virXMLValidatorFree Introduce virXMLValidatorInit Introduce virXMLValidatorValidate Introduce virschematest .gitignore | 1 - src/libvirt_private.syms | 3 + src/util/virxml.c | 102 ++++++++++++++++------ src/util/virxml.h | 19 +++++ tests/Makefile.am | 28 ++---- tests/capabilityschematest | 9 -- tests/domaincapsschematest | 10 --- tests/domainschematest | 14 --- tests/domainsnapshotschematest | 9 -- tests/interfaceschematest | 9 -- tests/networkschematest | 9 -- tests/nodedevschematest | 9 -- tests/nwfilterschematest | 9 -- tests/schematestutils.sh | 47 ---------- tests/secretschematest | 9 -- tests/storagepoolschematest | 9 -- tests/storagevolschematest | 9 -- tests/virschematest.c | 190 +++++++++++++++++++++++++++++++++++++++++ 18 files changed, 293 insertions(+), 202 deletions(-) delete mode 100755 tests/capabilityschematest delete mode 100755 tests/domaincapsschematest delete mode 100755 tests/domainschematest delete mode 100755 tests/domainsnapshotschematest delete mode 100755 tests/interfaceschematest delete mode 100755 tests/networkschematest delete mode 100755 tests/nodedevschematest delete mode 100755 tests/nwfilterschematest delete mode 100644 tests/schematestutils.sh delete mode 100755 tests/secretschematest delete mode 100755 tests/storagepoolschematest delete mode 100755 tests/storagevolschematest create mode 100644 tests/virschematest.c -- 2.7.3
Store all the data related to RNG validation in one structure to allow splitting virXMLValidateAgainstSchema. --- src/util/virxml.c | 47 +++++++++++++++++++++++++++-------------------- src/util/virxml.h | 10 ++++++++++ 2 files changed, 37 insertions(+), 20 deletions(-) diff --git a/src/util/virxml.c b/src/util/virxml.c index 489bad8..b3e4184 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -1108,56 +1108,63 @@ int virXMLValidateAgainstSchema(const char *schemafile, xmlDocPtr doc) { - xmlRelaxNGParserCtxtPtr rngParser = NULL; - xmlRelaxNGPtr rng = NULL; - xmlRelaxNGValidCtxtPtr rngValid = NULL; - virBuffer buf = VIR_BUFFER_INITIALIZER; + virXMLValidatorPtr validator = NULL; int ret = -1; - if (!(rngParser = xmlRelaxNGNewParserCtxt(schemafile))) { + if (VIR_ALLOC(validator) < 0) + return -1; + + if (VIR_STRDUP(validator->schemafile, schemafile) < 0) + goto cleanup; + + if (!(validator->rngParser = + xmlRelaxNGNewParserCtxt(validator->schemafile))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to create RNG parser for %s"), - schemafile); + validator->schemafile); goto cleanup; } - xmlRelaxNGSetParserErrors(rngParser, + xmlRelaxNGSetParserErrors(validator->rngParser, catchRNGError, ignoreRNGError, - &buf); + &validator->buf); - if (!(rng = xmlRelaxNGParse(rngParser))) { + if (!(validator->rng = xmlRelaxNGParse(validator->rngParser))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to parse RNG %s: %s"), - schemafile, virBufferCurrentContent(&buf)); + validator->schemafile, + virBufferCurrentContent(&validator->buf)); goto cleanup; } - if (!(rngValid = xmlRelaxNGNewValidCtxt(rng))) { + if (!(validator->rngValid = xmlRelaxNGNewValidCtxt(validator->rng))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to create RNG validation context %s"), - schemafile); + validator->schemafile); goto cleanup; } - xmlRelaxNGSetValidErrors(rngValid, + xmlRelaxNGSetValidErrors(validator->rngValid, catchRNGError, ignoreRNGError, - &buf); + &validator->buf); - if (xmlRelaxNGValidateDoc(rngValid, doc) != 0) { + if (xmlRelaxNGValidateDoc(validator->rngValid, doc) != 0) { virReportError(VIR_ERR_XML_INVALID_SCHEMA, _("Unable to validate doc against %s\n%s"), - schemafile, virBufferCurrentContent(&buf)); + validator->schemafile, + virBufferCurrentContent(&validator->buf)); goto cleanup; } ret = 0; cleanup: - virBufferFreeAndReset(&buf); - xmlRelaxNGFreeParserCtxt(rngParser); - xmlRelaxNGFreeValidCtxt(rngValid); - xmlRelaxNGFree(rng); + VIR_FREE(validator->schemafile); + virBufferFreeAndReset(&validator->buf); + xmlRelaxNGFreeParserCtxt(validator->rngParser); + xmlRelaxNGFreeValidCtxt(validator->rngValid); + xmlRelaxNGFree(validator->rng); return ret; } diff --git a/src/util/virxml.h b/src/util/virxml.h index b94de74..b75b109 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -30,6 +30,8 @@ # include <libxml/xpath.h> # include <libxml/relaxng.h> +# include "virbuffer.h" + int virXPathBoolean(const char *xpath, xmlXPathContextPtr ctxt); char * virXPathString(const char *xpath, @@ -177,6 +179,14 @@ int virXMLInjectNamespace(xmlNodePtr node, const char *uri, const char *key); +typedef struct _virXMLValidator { + xmlRelaxNGParserCtxtPtr rngParser; + xmlRelaxNGPtr rng; + xmlRelaxNGValidCtxtPtr rngValid; + virBuffer buf; + char *schemafile; +} virXMLValidator, *virXMLValidatorPtr; + int virXMLValidateAgainstSchema(const char *schemafile, xmlDocPtr xml); -- 2.7.3
On Tue, Jun 07, 2016 at 20:07:28 +0200, Ján Tomko wrote:
Store all the data related to RNG validation in one structure to allow splitting virXMLValidateAgainstSchema. --- src/util/virxml.c | 47 +++++++++++++++++++++++++++-------------------- src/util/virxml.h | 10 ++++++++++ 2 files changed, 37 insertions(+), 20 deletions(-)
[...]
@@ -177,6 +179,14 @@ int virXMLInjectNamespace(xmlNodePtr node, const char *uri, const char *key);
+typedef struct _virXMLValidator { + xmlRelaxNGParserCtxtPtr rngParser; + xmlRelaxNGPtr rng; + xmlRelaxNGValidCtxtPtr rngValid; + virBuffer buf; + char *schemafile; +} virXMLValidator, *virXMLValidatorPtr;
This is too revolutionary. I'd stick with the few extra typedefs as we do (almost) everywhere. ACK with the above changed.
On Wed, Jun 08, 2016 at 08:49:37AM +0200, Peter Krempa wrote:
On Tue, Jun 07, 2016 at 20:07:28 +0200, Ján Tomko wrote:
Store all the data related to RNG validation in one structure to allow splitting virXMLValidateAgainstSchema. --- src/util/virxml.c | 47 +++++++++++++++++++++++++++-------------------- src/util/virxml.h | 10 ++++++++++ 2 files changed, 37 insertions(+), 20 deletions(-)
[...]
@@ -177,6 +179,14 @@ int virXMLInjectNamespace(xmlNodePtr node, const char *uri, const char *key);
+typedef struct _virXMLValidator { + xmlRelaxNGParserCtxtPtr rngParser; + xmlRelaxNGPtr rng; + xmlRelaxNGValidCtxtPtr rngValid; + virBuffer buf; + char *schemafile; +} virXMLValidator, *virXMLValidatorPtr;
This is too revolutionary. I'd stick with the few extra typedefs as we do (almost) everywhere.
The revolution has already been started by commit ff89e9b with a fitting summary: conf: move virDomainDeviceInfo definition from domain_conf.h to device_conf.h
ACK with the above changed.
I have changed it to honour the tradition. Jan
On 06/08/2016 04:10 AM, Ján Tomko wrote:
On Tue, Jun 07, 2016 at 20:07:28 +0200, Ján Tomko wrote:
Store all the data related to RNG validation in one structure to allow splitting virXMLValidateAgainstSchema. --- src/util/virxml.c | 47 +++++++++++++++++++++++++++-------------------- src/util/virxml.h | 10 ++++++++++ 2 files changed, 37 insertions(+), 20 deletions(-) [...]
@@ -177,6 +179,14 @@ int virXMLInjectNamespace(xmlNodePtr node, const char *uri, const char *key);
+typedef struct _virXMLValidator { + xmlRelaxNGParserCtxtPtr rngParser; + xmlRelaxNGPtr rng; + xmlRelaxNGValidCtxtPtr rngValid; + virBuffer buf; + char *schemafile; +} virXMLValidator, *virXMLValidatorPtr; This is too revolutionary. I'd stick with the few extra typedefs as we do (almost) everywhere. The revolution has already been started by commit ff89e9b with a fitting
On Wed, Jun 08, 2016 at 08:49:37AM +0200, Peter Krempa wrote: summary: conf: move virDomainDeviceInfo definition from domain_conf.h to device_conf.h
The revolution started long before that (and it wasn't me who filed the declaration, although I did fire the latest shot): grep "^typedef struct.*{$" src/*/*.[ch] I really see no point in the ancient notation (nor in the extra _virBlah naming of the struct when it's never otherwise used (and could have just as easily been named virBlah to match the typedef - those are different namespaces), although I forgot to remove those when I moved virDomainDeviceInfo). Maybe there was some compiler in days of yore that required it?
ACK with the above changed. I have changed it to honour the tradition.
Death to the redundant oppressors!!
Split out the code cleaning up the validator. --- src/libvirt_private.syms | 1 + src/util/virxml.c | 13 ++++++++++++- src/util/virxml.h | 2 ++ 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index f197f55..53a7a97 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2565,6 +2565,7 @@ virXMLPickShellSafeComment; virXMLPropString; virXMLSaveFile; virXMLValidateAgainstSchema; +virXMLValidatorFree; virXPathBoolean; virXPathInt; virXPathLong; diff --git a/src/util/virxml.c b/src/util/virxml.c index b3e4184..49aed7d 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -1161,10 +1161,21 @@ virXMLValidateAgainstSchema(const char *schemafile, ret = 0; cleanup: + virXMLValidatorFree(validator); + return ret; +} + + +void +virXMLValidatorFree(virXMLValidatorPtr validator) +{ + if (!validator) + return; + VIR_FREE(validator->schemafile); virBufferFreeAndReset(&validator->buf); xmlRelaxNGFreeParserCtxt(validator->rngParser); xmlRelaxNGFreeValidCtxt(validator->rngValid); xmlRelaxNGFree(validator->rng); - return ret; + VIR_FREE(validator); } diff --git a/src/util/virxml.h b/src/util/virxml.h index b75b109..607f063 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -190,5 +190,7 @@ typedef struct _virXMLValidator { int virXMLValidateAgainstSchema(const char *schemafile, xmlDocPtr xml); +void +virXMLValidatorFree(virXMLValidatorPtr validator); #endif /* __VIR_XML_H__ */ -- 2.7.3
On Tue, Jun 07, 2016 at 20:07:29 +0200, Ján Tomko wrote:
Split out the code cleaning up the validator. --- src/libvirt_private.syms | 1 + src/util/virxml.c | 13 ++++++++++++- src/util/virxml.h | 2 ++ 3 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index f197f55..53a7a97 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2565,6 +2565,7 @@ virXMLPickShellSafeComment; virXMLPropString; virXMLSaveFile; virXMLValidateAgainstSchema; +virXMLValidatorFree; virXPathBoolean; virXPathInt; virXPathLong; diff --git a/src/util/virxml.c b/src/util/virxml.c index b3e4184..49aed7d 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -1161,10 +1161,21 @@ virXMLValidateAgainstSchema(const char *schemafile,
[...]
+void +virXMLValidatorFree(virXMLValidatorPtr validator) +{ + if (!validator) + return; + VIR_FREE(validator->schemafile); virBufferFreeAndReset(&validator->buf); xmlRelaxNGFreeParserCtxt(validator->rngParser); xmlRelaxNGFreeValidCtxt(validator->rngValid); xmlRelaxNGFree(validator->rng); - return ret; + VIR_FREE(validator);
This actually belongs to the previous patch. ^^ Otherwise it will be leaking the validator struct in the first patch.
}
ACK with the above fixed.
Split out all the code initializing the validator to a separate function. --- src/libvirt_private.syms | 1 + src/util/virxml.c | 33 ++++++++++++++++++++++++--------- src/util/virxml.h | 3 +++ 3 files changed, 28 insertions(+), 9 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 53a7a97..e631c14 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2566,6 +2566,7 @@ virXMLPropString; virXMLSaveFile; virXMLValidateAgainstSchema; virXMLValidatorFree; +virXMLValidatorInit; virXPathBoolean; virXPathInt; virXPathLong; diff --git a/src/util/virxml.c b/src/util/virxml.c index 49aed7d..19163db 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -1104,25 +1104,23 @@ static void ignoreRNGError(void *ctx ATTRIBUTE_UNUSED, {} -int -virXMLValidateAgainstSchema(const char *schemafile, - xmlDocPtr doc) +virXMLValidatorPtr +virXMLValidatorInit(const char *schemafile) { virXMLValidatorPtr validator = NULL; - int ret = -1; if (VIR_ALLOC(validator) < 0) - return -1; + return NULL; if (VIR_STRDUP(validator->schemafile, schemafile) < 0) - goto cleanup; + goto error; if (!(validator->rngParser = xmlRelaxNGNewParserCtxt(validator->schemafile))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to create RNG parser for %s"), validator->schemafile); - goto cleanup; + goto error; } xmlRelaxNGSetParserErrors(validator->rngParser, @@ -1135,20 +1133,37 @@ virXMLValidateAgainstSchema(const char *schemafile, _("Unable to parse RNG %s: %s"), validator->schemafile, virBufferCurrentContent(&validator->buf)); - goto cleanup; + goto error; } if (!(validator->rngValid = xmlRelaxNGNewValidCtxt(validator->rng))) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to create RNG validation context %s"), validator->schemafile); - goto cleanup; + goto error; } xmlRelaxNGSetValidErrors(validator->rngValid, catchRNGError, ignoreRNGError, &validator->buf); + return validator; + + error: + virXMLValidatorFree(validator); + return NULL; +} + + +int +virXMLValidateAgainstSchema(const char *schemafile, + xmlDocPtr doc) +{ + virXMLValidatorPtr validator = NULL; + int ret = -1; + + if (!(validator = virXMLValidatorInit(schemafile))) + return -1; if (xmlRelaxNGValidateDoc(validator->rngValid, doc) != 0) { virReportError(VIR_ERR_XML_INVALID_SCHEMA, diff --git a/src/util/virxml.h b/src/util/virxml.h index 607f063..6d4c991 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -187,6 +187,9 @@ typedef struct _virXMLValidator { char *schemafile; } virXMLValidator, *virXMLValidatorPtr; +virXMLValidatorPtr +virXMLValidatorInit(const char *schemafile); + int virXMLValidateAgainstSchema(const char *schemafile, xmlDocPtr xml); -- 2.7.3
On Tue, Jun 07, 2016 at 20:07:30 +0200, Ján Tomko wrote:
Split out all the code initializing the validator to a separate function. --- src/libvirt_private.syms | 1 + src/util/virxml.c | 33 ++++++++++++++++++++++++--------- src/util/virxml.h | 3 +++ 3 files changed, 28 insertions(+), 9 deletions(-)
ACK
Split out the code for XML validation into a new function. --- src/libvirt_private.syms | 1 + src/util/virxml.c | 27 +++++++++++++++++++++------ src/util/virxml.h | 4 ++++ 3 files changed, 26 insertions(+), 6 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index e631c14..4d56d1f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2567,6 +2567,7 @@ virXMLSaveFile; virXMLValidateAgainstSchema; virXMLValidatorFree; virXMLValidatorInit; +virXMLValidatorValidate; virXPathBoolean; virXPathInt; virXPathLong; diff --git a/src/util/virxml.c b/src/util/virxml.c index 19163db..aa97940 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -1156,15 +1156,11 @@ virXMLValidatorInit(const char *schemafile) int -virXMLValidateAgainstSchema(const char *schemafile, - xmlDocPtr doc) +virXMLValidatorValidate(virXMLValidatorPtr validator, + xmlDocPtr doc) { - virXMLValidatorPtr validator = NULL; int ret = -1; - if (!(validator = virXMLValidatorInit(schemafile))) - return -1; - if (xmlRelaxNGValidateDoc(validator->rngValid, doc) != 0) { virReportError(VIR_ERR_XML_INVALID_SCHEMA, _("Unable to validate doc against %s\n%s"), @@ -1174,7 +1170,26 @@ virXMLValidateAgainstSchema(const char *schemafile, } ret = 0; + cleanup: + virBufferFreeAndReset(&validator->buf); + return ret; +} + + +int +virXMLValidateAgainstSchema(const char *schemafile, + xmlDocPtr doc) +{ + virXMLValidatorPtr validator = NULL; + int ret = -1; + if (!(validator = virXMLValidatorInit(schemafile))) + return -1; + + if (virXMLValidatorValidate(validator, doc) < 0) + goto cleanup; + + ret = 0; cleanup: virXMLValidatorFree(validator); return ret; diff --git a/src/util/virxml.h b/src/util/virxml.h index 6d4c991..20bad54 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -191,6 +191,10 @@ virXMLValidatorPtr virXMLValidatorInit(const char *schemafile); int +virXMLValidatorValidate(virXMLValidatorPtr validator, + xmlDocPtr doc); + +int virXMLValidateAgainstSchema(const char *schemafile, xmlDocPtr xml); void -- 2.7.3
On Tue, Jun 07, 2016 at 20:07:31 +0200, Ján Tomko wrote:
Split out the code for XML validation into a new function. --- src/libvirt_private.syms | 1 + src/util/virxml.c | 27 +++++++++++++++++++++------ src/util/virxml.h | 4 ++++ 3 files changed, 26 insertions(+), 6 deletions(-)
ACK
Instead of calling xmllint via a shell script, use our virXMLValidator API to do it directly via libxml. --- .gitignore | 1 - tests/Makefile.am | 28 ++---- tests/capabilityschematest | 9 -- tests/domaincapsschematest | 10 --- tests/domainschematest | 14 --- tests/domainsnapshotschematest | 9 -- tests/interfaceschematest | 9 -- tests/networkschematest | 9 -- tests/nodedevschematest | 9 -- tests/nwfilterschematest | 9 -- tests/schematestutils.sh | 47 ---------- tests/secretschematest | 9 -- tests/storagepoolschematest | 9 -- tests/storagevolschematest | 9 -- tests/virschematest.c | 190 +++++++++++++++++++++++++++++++++++++++++ 15 files changed, 196 insertions(+), 175 deletions(-) delete mode 100755 tests/capabilityschematest delete mode 100755 tests/domaincapsschematest delete mode 100755 tests/domainschematest delete mode 100755 tests/domainsnapshotschematest delete mode 100755 tests/interfaceschematest delete mode 100755 tests/networkschematest delete mode 100755 tests/nodedevschematest delete mode 100755 tests/nwfilterschematest delete mode 100644 tests/schematestutils.sh delete mode 100755 tests/secretschematest delete mode 100755 tests/storagepoolschematest delete mode 100755 tests/storagevolschematest create mode 100644 tests/virschematest.c diff --git a/.gitignore b/.gitignore index ee89787..7fd9963 100644 --- a/.gitignore +++ b/.gitignore @@ -162,7 +162,6 @@ /tests/*test /tests/commandhelper /tests/qemucapsprobe -!/tests/*schematest !/tests/virt-aa-helper-test /tests/objectlocking /tests/objectlocking-files.txt diff --git a/tests/Makefile.am b/tests/Makefile.am index 0c4ad3c..3840457 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -88,15 +88,12 @@ EXTRA_DIST = \ bhyvexml2argvdata \ bhyvexml2xmloutdata \ capabilityschemadata \ - capabilityschematest \ commanddata \ cputestdata \ domaincapsschemadata \ - domaincapsschematest \ domainconfdata \ domainschemadata \ domainschematest \ - domainsnapshotschematest \ domainsnapshotxml2xmlin \ domainsnapshotxml2xmlout \ fchostdata \ @@ -106,7 +103,6 @@ EXTRA_DIST = \ lxcconf2xmldata \ lxcxml2xmldata \ lxcxml2xmloutdata \ - networkschematest \ networkxml2confdata \ networkxml2firewalldata \ networkxml2xmlin \ @@ -114,10 +110,8 @@ EXTRA_DIST = \ networkxml2xmlupdatein \ networkxml2xmlupdateout \ nodedevschemadata \ - nodedevschematest \ nodeinfodata \ nssdata \ - nwfilterschematest \ nwfilterxml2firewalldata \ nwfilterxml2xmlin \ nwfilterxml2xmlout \ @@ -131,17 +125,14 @@ EXTRA_DIST = \ qemumonitorjsondata \ qemuxml2argvdata \ qemuxml2xmloutdata \ - schematestutils.sh \ secretxml2xmlin \ securityselinuxhelperdata \ securityselinuxlabeldata \ sexpr2xmldata \ storagepoolschemadata \ - storagepoolschematest \ storagepoolxml2xmlin \ storagepoolxml2xmlout \ storagevolschemadata \ - storagevolschematest \ storagevolxml2argvdata \ storagevolxml2xmlin \ storagevolxml2xmlout \ @@ -190,6 +181,7 @@ test_programs = virshtest sockettest \ virlockspacetest \ virlogtest \ virrotatingfiletest \ + virschematest \ virstringtest \ virportallocatortest \ sysinfotest \ @@ -365,19 +357,7 @@ test_programs += virusbtest \ $(NULL) endif WITH_LINUX -test_scripts = \ - capabilityschematest \ - interfaceschematest \ - networkschematest \ - storagepoolschematest \ - storagevolschematest \ - domaincapsschematest \ - domainschematest \ - nodedevschematest \ - nwfilterschematest \ - domainsnapshotschematest \ - secretschematest - +test_scripts = libvirtd_test_scripts = \ libvirtd-fail \ libvirtd-pool \ @@ -1019,6 +999,10 @@ virtimetest_SOURCES = \ virtimetest.c testutils.h testutils.c virtimetest_LDADD = $(LDADDS) +virschematest_SOURCES = \ + virschematest.c testutils.h testutils.c +virschematest_LDADD = $(LDADDS) + virstringtest_SOURCES = \ virstringtest.c testutils.h testutils.c virstringtest_LDADD = $(LDADDS) diff --git a/tests/capabilityschematest b/tests/capabilityschematest deleted file mode 100755 index 458212e..0000000 --- a/tests/capabilityschematest +++ /dev/null @@ -1,9 +0,0 @@ -#!/bin/sh - -. "$(dirname $0)/test-lib.sh" -. $abs_srcdir/schematestutils.sh - -DIRS="capabilityschemadata xencapsdata" -SCHEMA="capability.rng" - -check_schema "$DIRS" "$SCHEMA" diff --git a/tests/domaincapsschematest b/tests/domaincapsschematest deleted file mode 100755 index 3b2021f..0000000 --- a/tests/domaincapsschematest +++ /dev/null @@ -1,10 +0,0 @@ -#!/bin/sh - -. "$(dirname $0)/test-lib.sh" -. $abs_srcdir/schematestutils.sh - -DIRS="" -DIRS="$DIRS domaincapsschemadata" -SCHEMA="domaincaps.rng" - -check_schema "$DIRS" "$SCHEMA" diff --git a/tests/domainschematest b/tests/domainschematest deleted file mode 100755 index c059c98..0000000 --- a/tests/domainschematest +++ /dev/null @@ -1,14 +0,0 @@ -#!/bin/sh - -. "$(dirname $0)/test-lib.sh" -. $abs_srcdir/schematestutils.sh - -DIRS="" -DIRS="$DIRS domainschemadata qemuargv2xmldata qemuxml2argvdata sexpr2xmldata" -DIRS="$DIRS xmconfigdata xml2sexprdata qemuxml2xmloutdata" -DIRS="$DIRS lxcxml2xmldata lxcxml2xmloutdata" -DIRS="$DIRS bhyvexml2argvdata genericxml2xmlindata genericxml2xmloutdata" -DIRS="$DIRS xlconfigdata" -SCHEMA="domain.rng" - -check_schema "$DIRS" "$SCHEMA" diff --git a/tests/domainsnapshotschematest b/tests/domainsnapshotschematest deleted file mode 100755 index 33b539a..0000000 --- a/tests/domainsnapshotschematest +++ /dev/null @@ -1,9 +0,0 @@ -#!/bin/sh - -. "$(dirname $0)/test-lib.sh" -. $abs_srcdir/schematestutils.sh - -DIRS="domainsnapshotxml2xmlin domainsnapshotxml2xmlout" -SCHEMA="domainsnapshot.rng" - -check_schema "$DIRS" "$SCHEMA" diff --git a/tests/interfaceschematest b/tests/interfaceschematest deleted file mode 100755 index 239b749..0000000 --- a/tests/interfaceschematest +++ /dev/null @@ -1,9 +0,0 @@ -#!/bin/sh - -. "$(dirname $0)/test-lib.sh" -. $abs_srcdir/schematestutils.sh - -DIRS="interfaceschemadata" -SCHEMA="interface.rng" - -check_schema "$DIRS" "$SCHEMA" diff --git a/tests/networkschematest b/tests/networkschematest deleted file mode 100755 index adbc7f4..0000000 --- a/tests/networkschematest +++ /dev/null @@ -1,9 +0,0 @@ -#!/bin/sh - -. "$(dirname $0)/test-lib.sh" -. $abs_srcdir/schematestutils.sh - -DIRS="../src/network networkxml2xmlin networkxml2xmlout" -SCHEMA="network.rng" - -check_schema "$DIRS" "$SCHEMA" diff --git a/tests/nodedevschematest b/tests/nodedevschematest deleted file mode 100755 index 1d85371..0000000 --- a/tests/nodedevschematest +++ /dev/null @@ -1,9 +0,0 @@ -#!/bin/sh - -. "$(dirname $0)/test-lib.sh" -. $abs_srcdir/schematestutils.sh - -DIRS="nodedevschemadata" -SCHEMA="nodedev.rng" - -check_schema "$DIRS" "$SCHEMA" diff --git a/tests/nwfilterschematest b/tests/nwfilterschematest deleted file mode 100755 index 408034c..0000000 --- a/tests/nwfilterschematest +++ /dev/null @@ -1,9 +0,0 @@ -#!/bin/sh - -. "$(dirname $0)/test-lib.sh" -. $abs_srcdir/schematestutils.sh - -DIRS="nwfilterxml2xmlout" -SCHEMA="nwfilter.rng" - -check_schema "$DIRS" "$SCHEMA" diff --git a/tests/schematestutils.sh b/tests/schematestutils.sh deleted file mode 100644 index e07e9b9..0000000 --- a/tests/schematestutils.sh +++ /dev/null @@ -1,47 +0,0 @@ -#!/bin/sh - -(xmllint --version) >/dev/null 2>&1 || skip_test_ 'Missing xmllint' - -check_schema () { - -DIRS=$1 -SCHEMA="$abs_top_srcdir/docs/schemas/$2" - -test_intro $this_test - -n=0 -f=0 -for dir in $DIRS -do - XML=`find $abs_srcdir/$dir -name '*.xml'` || exit 1 - - for xml in `echo "$XML" | sort` - do - n=`expr $n + 1` - cmd="xmllint --relaxng $SCHEMA --noout $xml" - result=`$cmd 2>&1` - ret=$? - - # Alter ret if error was expected. - case $xml:$ret in - *-invalid.xml:[34]) ret=0 ;; - *-invalid.xml:0) ret=3 ;; - esac - - test_result $n $(basename $(dirname $xml))"/"$(basename $xml) $ret - if test "$verbose" = "1" && test $ret != 0 ; then - printf '%s\n' "$cmd" "$result" - fi - if test "$ret" != 0 ; then - f=`expr $f + 1` - fi - done -done - -test_final $n $f - -ret=0 -test $f != 0 && ret=255 -exit $ret - -} diff --git a/tests/secretschematest b/tests/secretschematest deleted file mode 100755 index 9c29021..0000000 --- a/tests/secretschematest +++ /dev/null @@ -1,9 +0,0 @@ -#!/bin/sh - -. "$(dirname $0)/test-lib.sh" -. $abs_srcdir/schematestutils.sh - -DIRS="secretxml2xmlin" -SCHEMA="secret.rng" - -check_schema "$DIRS" "$SCHEMA" diff --git a/tests/storagepoolschematest b/tests/storagepoolschematest deleted file mode 100755 index ebbf4d1..0000000 --- a/tests/storagepoolschematest +++ /dev/null @@ -1,9 +0,0 @@ -#!/bin/sh - -. "$(dirname $0)/test-lib.sh" -. $abs_srcdir/schematestutils.sh - -DIRS="storagepoolxml2xmlin storagepoolxml2xmlout storagepoolschemadata" -SCHEMA="storagepool.rng" - -check_schema "$DIRS" "$SCHEMA" diff --git a/tests/storagevolschematest b/tests/storagevolschematest deleted file mode 100755 index d3ba3a8..0000000 --- a/tests/storagevolschematest +++ /dev/null @@ -1,9 +0,0 @@ -#!/bin/sh - -. "$(dirname $0)/test-lib.sh" -. $abs_srcdir/schematestutils.sh - -DIRS="storagevolxml2xmlin storagevolxml2xmlout storagevolschemadata" -SCHEMA="storagevol.rng" - -check_schema "$DIRS" "$SCHEMA" diff --git a/tests/virschematest.c b/tests/virschematest.c new file mode 100644 index 0000000..11d5375 --- /dev/null +++ b/tests/virschematest.c @@ -0,0 +1,190 @@ +/* + * Copyright (C) 2016 Red Hat, Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + * + * Author: Ján Tomko <jtomko@redhat.com> + */ + +#include <config.h> + +#include <stdlib.h> + +#include "testutils.h" + +#include "virerror.h" +#include "viralloc.h" +#include "virlog.h" +#include "virxml.h" + +#define VIR_FROM_THIS VIR_FROM_NONE + +VIR_LOG_INIT("tests.schematest"); + +struct testSchemaData { + virXMLValidatorPtr validator; + const char *xml_path; +}; + + +static int +testSchemaFile(const void *args) +{ + const struct testSchemaData *data = args; + bool shouldFail = virFileHasSuffix(data->xml_path, "-invalid.xml"); + xmlDocPtr xml = NULL; + int ret = -1; + + if (!(xml = virXMLParseFile(data->xml_path))) + return -1; + + if (virXMLValidatorValidate(data->validator, xml) < 0) { + if (!shouldFail) + goto cleanup; + } else { + if (shouldFail) + goto cleanup; + } + + ret = 0; + cleanup: + xmlFreeDoc(xml); + return ret; +} + + +static int +testSchemaDir(const char *schema, + virXMLValidatorPtr validator, + const char *dir_path) +{ + DIR *dir = NULL; + struct dirent *ent; + int ret = 0; + int rc; + char *test_name; + char *xml_path; + struct testSchemaData data = { + .validator = validator, + }; + + if (!(dir = opendir(dir_path))) { + virReportSystemError(errno, + "Failed to opendir path '%s'", + dir_path); + return -1; + } + + while ((rc = virDirRead(dir, &ent, dir_path)) > 0) { + if (!virFileHasSuffix(ent->d_name, ".xml")) + continue; + + if (virAsprintf(&xml_path, "%s/%s", dir_path, ent->d_name) < 0) + goto cleanup; + + if (virAsprintf(&test_name, "Checking %s against %s", + ent->d_name, schema) < 0) + goto cleanup; + + data.xml_path = xml_path; + if (virtTestRun(test_name, testSchemaFile, &data) < 0) + ret = -1; + + VIR_FREE(test_name); + VIR_FREE(xml_path); + } + + if (rc < 0) + ret = -1; + + cleanup: + VIR_FREE(test_name); + VIR_FREE(xml_path); + closedir(dir); + return ret; +} + + +static int +testSchemaDirs(const char *schema, ...) +{ + virXMLValidatorPtr validator; + va_list args; + int ret = 0; + char *schema_path = NULL; + char *dir_path = NULL; + const char *dir; + + if (virAsprintf(&schema_path, "%s/docs/schemas/%s", abs_topsrcdir, schema) < 0) + return -1; + + if (!(validator = virXMLValidatorInit(schema_path))) + goto cleanup; + + va_start(args, schema); + while ((dir = va_arg(args, char *))) { + if (virAsprintf(&dir_path, "%s/%s", abs_srcdir, dir) < 0) { + ret = -1; + goto cleanup; + } + if (testSchemaDir(schema, validator, dir) < 0) + ret = -1; + VIR_FREE(dir_path); + } + + cleanup: + virXMLValidatorFree(validator); + VIR_FREE(schema_path); + VIR_FREE(dir_path); + va_end(args); + return ret; +} + + +static int +mymain(void) +{ + int ret = 0; + +#define DO_TEST(schema, ...) \ + do { \ + if (testSchemaDirs(schema, __VA_ARGS__) < 0) \ + ret = -1; \ + } while (0) \ + + DO_TEST("capability.rng", "capabilityschemadata", "xencapsdata", NULL); + DO_TEST("domain.rng", "domainschemadata", "qemuargv2xmldata", + "qemuxml2argvdata", "sexpr2xmldata", "xmconfigdata", + "xml2sexprdata", "qemuxml2xmloutdata", "lxcxml2xmldata", + "lxcxml2xmloutdata", "bhyvexml2argvdata", "genericxml2xmlindata", + "genericxml2xmloutdata", "xlconfigdata", NULL); + DO_TEST("domaincaps.rng", "domaincapsschemadata", NULL); + DO_TEST("domainsnapshot.rng", "domainsnapshotxml2xmlin", + "domainsnapshotxml2xmlout", NULL); + DO_TEST("interface.rng", "interfaceschemadata", NULL); + DO_TEST("network.rng", "../src/network", "networkxml2xmlin", + "networkxml2xmlout", NULL); + DO_TEST("nodedev.rng", "nodedevschemadata", NULL); + DO_TEST("nwfilter.rng", "nwfilterxml2xmlout", NULL); + DO_TEST("secret.rng", "secretxml2xmlin", NULL); + DO_TEST("storagepool.rng", "storagepoolxml2xmlin", "storagepoolxml2xmlout", + "storagepoolschemadata", NULL); + DO_TEST("storagevol.rng", "storagevolxml2xmlin", "storagevolxml2xmlout", + "storagevolschemadata", NULL); + + return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; +} + +VIRT_TEST_MAIN(mymain) -- 2.7.3
On Tue, Jun 07, 2016 at 20:07:32 +0200, Ján Tomko wrote:
Instead of calling xmllint via a shell script, use our virXMLValidator API to do it directly via libxml. --- .gitignore | 1 - tests/Makefile.am | 28 ++---- tests/capabilityschematest | 9 -- tests/domaincapsschematest | 10 --- tests/domainschematest | 14 --- tests/domainsnapshotschematest | 9 -- tests/interfaceschematest | 9 -- tests/networkschematest | 9 -- tests/nodedevschematest | 9 -- tests/nwfilterschematest | 9 -- tests/schematestutils.sh | 47 ---------- tests/secretschematest | 9 -- tests/storagepoolschematest | 9 -- tests/storagevolschematest | 9 -- tests/virschematest.c | 190 +++++++++++++++++++++++++++++++++++++++++ 15 files changed, 196 insertions(+), 175 deletions(-) create mode 100644 tests/virschematest.c
[...]
diff --git a/tests/Makefile.am b/tests/Makefile.am index 0c4ad3c..3840457 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -88,15 +88,12 @@ EXTRA_DIST = \ bhyvexml2argvdata \ bhyvexml2xmloutdata \ capabilityschemadata \ - capabilityschematest \ commanddata \ cputestdata \ domaincapsschemadata \ - domaincapsschematest \ domainconfdata \ domainschemadata \ domainschematest \
domainschematest was removed too so that line needs to be deleted
- domainsnapshotschematest \ domainsnapshotxml2xmlin \ domainsnapshotxml2xmlout \ fchostdata \
diff --git a/tests/virschematest.c b/tests/virschematest.c new file mode 100644 index 0000000..11d5375 --- /dev/null +++ b/tests/virschematest.c @@ -0,0 +1,190 @@
[...]
+static int +testSchemaDir(const char *schema, + virXMLValidatorPtr validator, + const char *dir_path) +{ + DIR *dir = NULL; + struct dirent *ent; + int ret = 0; + int rc; + char *test_name;
At least this needs to be initialized as it's possible to jump to cleanup if virAsprintf of xml_path fails in the first iteration.
+ char *xml_path; + struct testSchemaData data = { + .validator = validator, + }; + + if (!(dir = opendir(dir_path))) { + virReportSystemError(errno, + "Failed to opendir path '%s'", + dir_path); + return -1; + } + + while ((rc = virDirRead(dir, &ent, dir_path)) > 0) { + if (!virFileHasSuffix(ent->d_name, ".xml")) + continue; + + if (virAsprintf(&xml_path, "%s/%s", dir_path, ent->d_name) < 0) + goto cleanup; + + if (virAsprintf(&test_name, "Checking %s against %s", + ent->d_name, schema) < 0) + goto cleanup; + + data.xml_path = xml_path; + if (virtTestRun(test_name, testSchemaFile, &data) < 0) + ret = -1; + + VIR_FREE(test_name); + VIR_FREE(xml_path); + } + + if (rc < 0) + ret = -1; + + cleanup: + VIR_FREE(test_name); + VIR_FREE(xml_path); + closedir(dir); + return ret; +} + + +static int +testSchemaDirs(const char *schema, ...) +{ + virXMLValidatorPtr validator; + va_list args; + int ret = 0; + char *schema_path = NULL; + char *dir_path = NULL; + const char *dir; + + if (virAsprintf(&schema_path, "%s/docs/schemas/%s", abs_topsrcdir, schema) < 0) + return -1; + + if (!(validator = virXMLValidatorInit(schema_path))) + goto cleanup;
This may jump to cleanup before, ...
+ + va_start(args, schema);
calling va_start ...
+ while ((dir = va_arg(args, char *))) { + if (virAsprintf(&dir_path, "%s/%s", abs_srcdir, dir) < 0) { + ret = -1; + goto cleanup; + } + if (testSchemaDir(schema, validator, dir) < 0) + ret = -1; + VIR_FREE(dir_path); + } + + cleanup: + virXMLValidatorFree(validator); + VIR_FREE(schema_path); + VIR_FREE(dir_path); + va_end(args);
... and call va_end.
+ return ret; +} + + +static int +mymain(void) +{ + int ret = 0; + +#define DO_TEST(schema, ...) \ + do { \ + if (testSchemaDirs(schema, __VA_ARGS__) < 0) \ + ret = -1; \ + } while (0) \ + + DO_TEST("capability.rng", "capabilityschemadata", "xencapsdata", NULL);
You can hide the 'NULL' sentinel inside the macro after __VA_ARGS__ ACK with the problems fixed.
On Wed, Jun 08, 2016 at 09:58:50AM +0200, Peter Krempa wrote:
On Tue, Jun 07, 2016 at 20:07:32 +0200, Ján Tomko wrote:
Instead of calling xmllint via a shell script, use our virXMLValidator API to do it directly via libxml. --- .gitignore | 1 - tests/Makefile.am | 28 ++---- tests/capabilityschematest | 9 -- tests/domaincapsschematest | 10 --- tests/domainschematest | 14 --- tests/domainsnapshotschematest | 9 -- tests/interfaceschematest | 9 -- tests/networkschematest | 9 -- tests/nodedevschematest | 9 -- tests/nwfilterschematest | 9 -- tests/schematestutils.sh | 47 ---------- tests/secretschematest | 9 -- tests/storagepoolschematest | 9 -- tests/storagevolschematest | 9 -- tests/virschematest.c | 190 +++++++++++++++++++++++++++++++++++++++++ 15 files changed, 196 insertions(+), 175 deletions(-) create mode 100644 tests/virschematest.c
[...]
+#define DO_TEST(schema, ...) \ + do { \ + if (testSchemaDirs(schema, __VA_ARGS__) < 0) \ + ret = -1; \ + } while (0) \ + + DO_TEST("capability.rng", "capabilityschemadata", "xencapsdata", NULL);
You can hide the 'NULL' sentinel inside the macro after __VA_ARGS__
ACK with the problems fixed.
Thanks, I have pushed the series. Jan
I didn't have time to go through all the patches, but did apply them all and found that make rpm fails. It was bedtime before I could investigate any further, but I thought you might want to know the result anyway... On 06/07/2016 02:07 PM, Ján Tomko wrote:
Instead of spawining a separate xmllint process for every file, introduce virXMLValidator APIs and use them in a C test file.
Originally, the shell-based schema tests took about 15s in series (with domainschematest making up for about 13s of that). Now, all the tests run in series take ~.57 ms.
This reduces total make check time from ~16s to ~8s, making it more fun to use with git rebase -x.
Ján Tomko (5): Introduce virXMLValidator structure Introduce virXMLValidatorFree Introduce virXMLValidatorInit Introduce virXMLValidatorValidate Introduce virschematest
.gitignore | 1 - src/libvirt_private.syms | 3 + src/util/virxml.c | 102 ++++++++++++++++------ src/util/virxml.h | 19 +++++ tests/Makefile.am | 28 ++---- tests/capabilityschematest | 9 -- tests/domaincapsschematest | 10 --- tests/domainschematest | 14 --- tests/domainsnapshotschematest | 9 -- tests/interfaceschematest | 9 -- tests/networkschematest | 9 -- tests/nodedevschematest | 9 -- tests/nwfilterschematest | 9 -- tests/schematestutils.sh | 47 ---------- tests/secretschematest | 9 -- tests/storagepoolschematest | 9 -- tests/storagevolschematest | 9 -- tests/virschematest.c | 190 +++++++++++++++++++++++++++++++++++++++++ 18 files changed, 293 insertions(+), 202 deletions(-) delete mode 100755 tests/capabilityschematest delete mode 100755 tests/domaincapsschematest delete mode 100755 tests/domainschematest delete mode 100755 tests/domainsnapshotschematest delete mode 100755 tests/interfaceschematest delete mode 100755 tests/networkschematest delete mode 100755 tests/nodedevschematest delete mode 100755 tests/nwfilterschematest delete mode 100644 tests/schematestutils.sh delete mode 100755 tests/secretschematest delete mode 100755 tests/storagepoolschematest delete mode 100755 tests/storagevolschematest create mode 100644 tests/virschematest.c
On Tue, Jun 07, 2016 at 10:14:15PM -0400, Laine Stump wrote:
I didn't have time to go through all the patches, but did apply them all and found that make rpm fails. It was bedtime before I could investigate any further, but I thought you might want to know the result anyway...
Thanks, I forgot domainschematest in EXTRA_DIST: diff --git a/tests/Makefile.am b/tests/Makefile.am index 3840457..9238a73 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -93,7 +93,6 @@ EXTRA_DIST = \ domaincapsschemadata \ domainconfdata \ domainschemadata \ - domainschematest \ domainsnapshotxml2xmlin \ domainsnapshotxml2xmlout \ fchostdata \ Jan
participants (3)
-
Ján Tomko -
Laine Stump -
Peter Krempa