The XML from libxml2 we use in libvirt has in many cases very bad error messages, which make it very difficult to point to the specific problem with the validated XML. This patch adds infrastructure which will allow users to specify a custom XML validator program which will be used to validate the XML before we invoke libxml's validator. The configuration is done via a global variable as normally the daemon config is not accessible from inside the runtime. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virxml.c | 93 +++++++++++++++++++++++++++++++++++++++- src/util/virxml.h | 3 ++ 3 files changed, 95 insertions(+), 2 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 25794bc2f4..3e90cd4599 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3678,6 +3678,7 @@ virXMLNodeNameEqual; virXMLNodeSanitizeNamespaces; virXMLNodeToString; virXMLParseHelper; +virXMLParseSetCustomValidator; virXMLPickShellSafeComment; virXMLPropEnum; virXMLPropEnumDefault; diff --git a/src/util/virxml.c b/src/util/virxml.c index d6e2e5dd91..aea9b3aac3 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 "vircommand.h" #include "configmake.h" #define VIR_FROM_THIS VIR_FROM_XML @@ -41,6 +42,8 @@ virReportErrorHelper(from, code, __FILE__, \ __FUNCTION__, __LINE__, __VA_ARGS__) +static char *virXMLCustomValidatorPath; + /* Internal data to be passed to SAX parser and used by error handler. */ struct virParserData { int domcode; @@ -1029,6 +1032,68 @@ catchXMLError(void *ctx, const char *msg G_GNUC_UNUSED, ...) } } + +/** + * virXMLParseHelperValidateCustom: + * + * Invokes an external validator program configured in the + * 'virXMLCustomValidatorPath' global variable and captures the validation + * output. The program is invoked as: + * + * /path/to/program /path/to/schema + * + * The XML to validate is fed on stdin of the program. The program is expected + * to return 0 on successful validation and non-zero on failure to validate, + * in which case STDOUT of the program is used in the error message reported + * by libvirt. + */ +static int +virXMLParseHelperValidateCustom(const char *schema, + const char *filename, + const char *xmlStr) +{ + g_autoptr(virCommand) cmd = virCommandNewArgList(virXMLCustomValidatorPath, + schema, NULL); + g_autofree char *filebuf = NULL; + g_autofree char *outbuf = NULL; + int exitstatus = 0; + + if (filename) { + /* virsh uses 10 MiB as max XML size */ + if (virFileReadAll(filename, 10 * 1024 * 1024, &filebuf) < 0) + return -1; + + xmlStr = filebuf; + } + + if (!virFileIsExecutable(virXMLCustomValidatorPath)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("custom XML validator program '%s' is not executable"), + virXMLCustomValidatorPath); + return -1; + } + + virCommandSetInputBuffer(cmd, xmlStr); + virCommandSetOutputBuffer(cmd, &outbuf); + virCommandDoAsyncIO(cmd); + + if (virCommandRunAsync(cmd, NULL) < 0) + return -1; + + if (virCommandWait(cmd, &exitstatus) < 0) + return -1; + + if (exitstatus != 0) { + virReportError(VIR_ERR_XML_INVALID_SCHEMA, + _("Unable to validate doc against %s\n%s"), + schema, outbuf); + return -1; + } + + return 0; +} + + /** * virXMLParseHelper: * @domcode: error domain of the caller, usually VIR_FROM_THIS @@ -1126,8 +1191,14 @@ virXMLParseHelper(int domcode, g_autofree char *schema = virFileFindResource(schemafile, abs_top_srcdir "/src/conf/schemas", PKGDATADIR "/schemas"); - if (!schema || - (virXMLValidateAgainstSchema(schema, xml) < 0)) + if (!schema) + return NULL; + + if (virXMLCustomValidatorPath && + virXMLParseHelperValidateCustom(schema, filename, xmlStr) < 0) + return NULL; + + if (virXMLValidateAgainstSchema(schema, xml) < 0) return NULL; } @@ -1884,3 +1955,21 @@ virXMLNewNode(xmlNsPtr ns, return ret; } + + +/** + * virXMLParseSetCustomValidator: + * @path: path to the custom validator binary + * + * Set the path to the custom XML validator helper program (see + * virXMLParseHelperValidateCustom). This function must be called only + * during daemon initialization. + */ +void +virXMLParseSetCustomValidator(const char *path) +{ + if (virXMLCustomValidatorPath) + return; + + virXMLCustomValidatorPath = g_strdup(path); +} diff --git a/src/util/virxml.h b/src/util/virxml.h index 539228a9ba..41a27a5722 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -165,6 +165,9 @@ virXMLPropEnumDefault(xmlNodePtr node, ATTRIBUTE_NONNULL(5); +void +virXMLParseSetCustomValidator(const char *path); + /* Internal function; prefer the macros below. */ xmlDocPtr virXMLParseHelper(int domcode, -- 2.37.1

Introduce a new config option 'xml_validator' into the daemon config file which will allow users to make libvirt daemons use a custom XML validator. The rationale is that validators such as 'jing'[1] provide drastically better error specification when compared to the native libxml2 validator we use. A drawback though is that jing is written in Java and thus unusable by libvirt directly and also not a popular package present in distros. For power users and developers it still is worth having this feature to provide better errors in a native way. An example showing the quality of the errors: XML used: <vcpu placement='static' current='1'>asdf8</vcpu> native validator: error: XML document failed to validate against schema: Unable to validate doc against /home/pipo/libvirt/src/conf/schemas/domain.rng Extra element vcpu in interleave Invalid sequence in interleave Element domain failed to validate content jing: error: XML document failed to validate against schema: Unable to validate doc against /home/pipo/libvirt/src/conf/schemas/domain.rng /dev/stdin:6:52: error: character content of element "vcpu" invalid; must be an integer Example script to make this feature work with jing: #!/bin/bash java -jar /home/pipo/git/jing-trang/build/jing.jar $1 /dev/stdin 2>&1 || exit 1 [1] https://github.com/relaxng/jing-trang Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/remote/libvirtd.aug.in | 1 + src/remote/libvirtd.conf.in | 21 +++++++++++++++++++++ src/remote/remote_daemon.c | 3 +++ src/remote/remote_daemon_config.c | 4 ++++ src/remote/remote_daemon_config.h | 2 ++ src/remote/test_libvirtd.aug.in | 1 + 6 files changed, 32 insertions(+) diff --git a/src/remote/libvirtd.aug.in b/src/remote/libvirtd.aug.in index d744548f41..9a15eb14e5 100644 --- a/src/remote/libvirtd.aug.in +++ b/src/remote/libvirtd.aug.in @@ -91,6 +91,7 @@ module @DAEMON_NAME_UC@ = let misc_entry = str_entry "host_uuid" | str_entry "host_uuid_source" | int_entry "ovs_timeout" + | str_entry "xml_validator" (* Each entry in the config is one of the following three ... *) let entry = sock_acl_entry diff --git a/src/remote/libvirtd.conf.in b/src/remote/libvirtd.conf.in index 80a98b1529..5cfe0c0918 100644 --- a/src/remote/libvirtd.conf.in +++ b/src/remote/libvirtd.conf.in @@ -541,3 +541,24 @@ # potential infinite waits blocking libvirt. # #ovs_timeout = 5 + +################################################################### +# Custom XML validator +# +# The following option instructs @DAEMON_NAME@ to use a custom +# XML validator binary or script before invoking the default +# validator from libxml2. +# +# The custom validator is called using following arguments. +# +# /path/to/validator /path/to/schema.rng +# +# The actual XML file is provided on the standard input of the process. +# If the validation fails the process is expected to return a non-zero exit +# code. The standard output and standard error output are used as error message +# provided back to the user. +# +# Note that the XMLs sent for validation may contain security sensitive +# information. +# +#xml_validator = "/path/to/validator" diff --git a/src/remote/remote_daemon.c b/src/remote/remote_daemon.c index f369d09d35..cbbf81e141 100644 --- a/src/remote/remote_daemon.c +++ b/src/remote/remote_daemon.c @@ -936,6 +936,9 @@ int main(int argc, char **argv) { exit(EXIT_FAILURE); } + if (config->xml_validator) + virXMLParseSetCustomValidator(config->xml_validator); + /* Let's try to initialize global variable that holds the host's boot time. */ if (virHostBootTimeInit() < 0) { /* This is acceptable failure. Maybe we won't need the boot time diff --git a/src/remote/remote_daemon_config.c b/src/remote/remote_daemon_config.c index 3567e337c4..affafa2a86 100644 --- a/src/remote/remote_daemon_config.c +++ b/src/remote/remote_daemon_config.c @@ -219,6 +219,7 @@ daemonConfigFree(struct daemonConfig *data) g_free(data->host_uuid_source); g_free(data->log_filters); g_free(data->log_outputs); + g_free(data->xml_validator); g_free(data); } @@ -380,6 +381,9 @@ daemonConfigLoadOptions(struct daemonConfig *data, if (virConfGetValueUInt(conf, "ovs_timeout", &data->ovs_timeout) < 0) return -1; + if (virConfGetValueString(conf, "xml_validator", &data->xml_validator) < 0) + return -1; + return 0; } diff --git a/src/remote/remote_daemon_config.h b/src/remote/remote_daemon_config.h index 9f9e54e838..4bc8bce90f 100644 --- a/src/remote/remote_daemon_config.h +++ b/src/remote/remote_daemon_config.h @@ -96,6 +96,8 @@ struct daemonConfig { unsigned int admin_keepalive_count; unsigned int ovs_timeout; + + char *xml_validator; }; diff --git a/src/remote/test_libvirtd.aug.in b/src/remote/test_libvirtd.aug.in index c27680e130..eedc3af24d 100644 --- a/src/remote/test_libvirtd.aug.in +++ b/src/remote/test_libvirtd.aug.in @@ -67,3 +67,4 @@ module Test_@DAEMON_NAME@ = { "admin_keepalive_interval" = "5" } { "admin_keepalive_count" = "5" } { "ovs_timeout" = "5" } + { "xml_validator" = "/path/to/validator" } -- 2.37.1