Add a helper method that can validate an XML document against an RNG schema --- include/libvirt/virterror.h | 1 + src/internal.h | 4 +++ src/libvirt_private.syms | 1 + src/util/virerror.c | 6 ++++ src/util/virxml.c | 79 +++++++++++++++++++++++++++++++++++++++++++++ src/util/virxml.h | 5 +++ 6 files changed, 96 insertions(+) diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index 85dd74c..3d3d80a 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -304,6 +304,7 @@ typedef enum { VIR_ERR_STORAGE_VOL_EXIST = 90, /* the storage vol already exists */ VIR_ERR_CPU_INCOMPATIBLE = 91, /* given CPU is incompatible with host CPU*/ + VIR_ERR_XML_INVALID_SCHEMA = 92, /* XML document doens't validate against schema */ } virErrorNumber; /** diff --git a/src/internal.h b/src/internal.h index 30445c1..9855c49 100644 --- a/src/internal.h +++ b/src/internal.h @@ -234,11 +234,15 @@ # define VIR_WARNINGS_NO_CAST_ALIGN \ _Pragma ("GCC diagnostic push") \ _Pragma ("GCC diagnostic ignored \"-Wcast-align\"") +# define VIR_WARNINGS_NO_PRINTF \ + _Pragma ("GCC diagnostic push") \ + _Pragma ("GCC diagnostic ignored \"-Wsuggest-attribute=format\"") # define VIR_WARNINGS_RESET \ _Pragma ("GCC diagnostic pop") # else # define VIR_WARNINGS_NO_CAST_ALIGN +# define VIR_WARNINGS_NO_PRINTF # define VIR_WARNINGS_RESET # endif diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 7ceb54d..7a2c585 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2252,6 +2252,7 @@ virXMLParseHelper; virXMLPickShellSafeComment; virXMLPropString; virXMLSaveFile; +virXMLValidateAgainstSchema; virXPathBoolean; virXPathInt; virXPathLong; diff --git a/src/util/virerror.c b/src/util/virerror.c index 4aa6d04..f5d7f54 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -1285,6 +1285,12 @@ virErrorMsg(virErrorNumber error, const char *info) else errmsg = _("the CPU is incompatible with host CPU: %s"); break; + case VIR_ERR_XML_INVALID_SCHEMA: + if (info == NULL) + errmsg = _("XML document failed to validate against schema"); + else + errmsg = _("XML document failed to validate against schema: %s"); + break; } return errmsg; } diff --git a/src/util/virxml.c b/src/util/virxml.c index 93f8590..489bad8 100644 --- a/src/util/virxml.c +++ b/src/util/virxml.c @@ -1082,3 +1082,82 @@ virXMLInjectNamespace(xmlNodePtr node, return 0; } + +static void catchRNGError(void *ctx, + const char *msg, + ...) +{ + virBufferPtr buf = ctx; + va_list args; + + va_start(args, msg); + VIR_WARNINGS_NO_PRINTF; + virBufferVasprintf(buf, msg, args); + VIR_WARNINGS_RESET; + va_end(args); +} + + +static void ignoreRNGError(void *ctx ATTRIBUTE_UNUSED, + const char *msg ATTRIBUTE_UNUSED, + ...) +{} + + +int +virXMLValidateAgainstSchema(const char *schemafile, + xmlDocPtr doc) +{ + xmlRelaxNGParserCtxtPtr rngParser = NULL; + xmlRelaxNGPtr rng = NULL; + xmlRelaxNGValidCtxtPtr rngValid = NULL; + virBuffer buf = VIR_BUFFER_INITIALIZER; + int ret = -1; + + if (!(rngParser = xmlRelaxNGNewParserCtxt(schemafile))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to create RNG parser for %s"), + schemafile); + goto cleanup; + } + + xmlRelaxNGSetParserErrors(rngParser, + catchRNGError, + ignoreRNGError, + &buf); + + if (!(rng = xmlRelaxNGParse(rngParser))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to parse RNG %s: %s"), + schemafile, virBufferCurrentContent(&buf)); + goto cleanup; + } + + if (!(rngValid = xmlRelaxNGNewValidCtxt(rng))) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to create RNG validation context %s"), + schemafile); + goto cleanup; + } + + xmlRelaxNGSetValidErrors(rngValid, + catchRNGError, + ignoreRNGError, + &buf); + + if (xmlRelaxNGValidateDoc(rngValid, doc) != 0) { + virReportError(VIR_ERR_XML_INVALID_SCHEMA, + _("Unable to validate doc against %s\n%s"), + schemafile, virBufferCurrentContent(&buf)); + goto cleanup; + } + + ret = 0; + + cleanup: + virBufferFreeAndReset(&buf); + xmlRelaxNGFreeParserCtxt(rngParser); + xmlRelaxNGFreeValidCtxt(rngValid); + xmlRelaxNGFree(rng); + return ret; +} diff --git a/src/util/virxml.h b/src/util/virxml.h index 781b3bf..b94de74 100644 --- a/src/util/virxml.h +++ b/src/util/virxml.h @@ -28,6 +28,7 @@ # include <libxml/parser.h> # include <libxml/tree.h> # include <libxml/xpath.h> +# include <libxml/relaxng.h> int virXPathBoolean(const char *xpath, xmlXPathContextPtr ctxt); @@ -176,4 +177,8 @@ int virXMLInjectNamespace(xmlNodePtr node, const char *uri, const char *key); +int +virXMLValidateAgainstSchema(const char *schemafile, + xmlDocPtr xml); + #endif /* __VIR_XML_H__ */ -- 2.1.0

On Fri, 2015-01-16 at 14:19 +0000, Daniel P. Berrange wrote: I'm afraid it doesn't seem to. Specifically: diff --git a/src/internal.h b/src/internal.h index 9855c49..508f8b5 100644 --- a/src/internal.h +++ b/src/internal.h @@ -236,7 +236,7 @@ _Pragma ("GCC diagnostic ignored \"-Wcast-align\"") # define VIR_WARNINGS_NO_PRINTF \ _Pragma ("GCC diagnostic push") \ - _Pragma ("GCC diagnostic ignored \"-Wsuggest-attribute=format\"") + _Pragma ("GCC diagnostic ignored \"-Wsuggest-attribute\"") # define VIR_WARNINGS_RESET \ _Pragma ("GCC diagnostic pop") Didn't help. According to https://gcc.gnu.org/onlinedocs/gcc-4.6.3/gcc/Warning-Options.html#Warning... the valid -Wsuggest-attributes=FOO in that version are pure const and noreturn. =format seems to have arrived in 4.8, FWIW. Ian.

The virDomainDefineXMLFlags and virDomainCreateXML APIs both gain new flags allowing them to be told to validate XML. This updates all the drivers to turn on validation in the XML parser when the flags are set --- include/libvirt/libvirt-domain.h | 5 +++++ src/bhyve/bhyve_driver.c | 16 ++++++++++++---- src/conf/domain_conf.c | 11 +++++++++++ src/conf/domain_conf.h | 1 + src/esx/esx_driver.c | 8 ++++++-- src/libvirt-domain.c | 2 +- src/libxl/libxl_driver.c | 17 +++++++++++++---- src/lxc/lxc_driver.c | 18 ++++++++++++++---- src/openvz/openvz_driver.c | 16 ++++++++++++---- src/parallels/parallels_driver.c | 8 ++++++-- src/phyp/phyp_driver.c | 8 ++++++-- src/qemu/qemu_driver.c | 16 ++++++++++++---- src/test/test_driver.c | 16 ++++++++++++---- src/uml/uml_driver.c | 17 +++++++++++++---- src/vbox/vbox_common.c | 8 ++++++-- src/vmware/vmware_driver.c | 16 ++++++++++++---- src/xen/xen_driver.c | 16 ++++++++++++---- src/xenapi/xenapi_driver.c | 17 +++++++++++++---- 18 files changed, 167 insertions(+), 49 deletions(-) diff --git a/include/libvirt/libvirt-domain.h b/include/libvirt/libvirt-domain.h index 3a83571..4dbd7f5 100644 --- a/include/libvirt/libvirt-domain.h +++ b/include/libvirt/libvirt-domain.h @@ -273,6 +273,7 @@ typedef enum { VIR_DOMAIN_START_AUTODESTROY = 1 << 1, /* Automatically kill guest when virConnectPtr is closed */ VIR_DOMAIN_START_BYPASS_CACHE = 1 << 2, /* Avoid file system cache pollution */ VIR_DOMAIN_START_FORCE_BOOT = 1 << 3, /* Boot, discarding any managed save */ + VIR_DOMAIN_START_VALIDATE = 1 << 4, /* Validate the XML document against schema */ } virDomainCreateFlags; @@ -1417,6 +1418,10 @@ int virDomainMemoryPeek (virDomainPtr dom, void *buffer, unsigned int flags); +typedef enum { + VIR_DOMAIN_DEFINE_VALIDATE = (1 << 0), /* Validate the XML document against schema */ +} virDomainDefineFlags; + /* * defined but not running domains */ diff --git a/src/bhyve/bhyve_driver.c b/src/bhyve/bhyve_driver.c index 9b82598..8264ad8 100644 --- a/src/bhyve/bhyve_driver.c +++ b/src/bhyve/bhyve_driver.c @@ -495,8 +495,12 @@ bhyveDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flag virDomainObjPtr vm = NULL; virObjectEventPtr event = NULL; virCapsPtr caps = NULL; + unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; - virCheckFlags(0, NULL); + virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL); + + if (flags & VIR_DOMAIN_DEFINE_VALIDATE) + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; caps = bhyveDriverGetCapabilities(privconn); if (!caps) @@ -504,7 +508,7 @@ bhyveDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flag if ((def = virDomainDefParseString(xml, caps, privconn->xmlopt, 1 << VIR_DOMAIN_VIRT_BHYVE, - VIR_DOMAIN_DEF_PARSE_INACTIVE)) == NULL) + parse_flags)) == NULL) goto cleanup; if (virDomainDefineXMLFlagsEnsureACL(conn, def) < 0) @@ -891,9 +895,13 @@ bhyveDomainCreateXML(virConnectPtr conn, virObjectEventPtr event = NULL; virCapsPtr caps = NULL; unsigned int start_flags = 0; + unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; - virCheckFlags(VIR_DOMAIN_START_AUTODESTROY, NULL); + virCheckFlags(VIR_DOMAIN_START_AUTODESTROY | + VIR_DOMAIN_START_VALIDATE, NULL); + if (flags & VIR_DOMAIN_START_VALIDATE) + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; if (flags & VIR_DOMAIN_START_AUTODESTROY) start_flags |= VIR_BHYVE_PROCESS_START_AUTODESTROY; @@ -903,7 +911,7 @@ bhyveDomainCreateXML(virConnectPtr conn, if ((def = virDomainDefParseString(xml, caps, privconn->xmlopt, 1 << VIR_DOMAIN_VIRT_BHYVE, - VIR_DOMAIN_DEF_PARSE_INACTIVE)) == NULL) + parse_flags)) == NULL) goto cleanup; if (virDomainCreateXMLEnsureACL(conn, def) < 0) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b316b4b..420f94b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -29,6 +29,7 @@ #include <sys/stat.h> #include <unistd.h> +#include "configmake.h" #include "internal.h" #include "virerror.h" #include "datatypes.h" @@ -12647,6 +12648,16 @@ virDomainDefParseXML(xmlDocPtr xml, bool usb_master = false; bool primaryVideo = false; + if (flags & VIR_DOMAIN_DEF_PARSE_VALIDATE) { + char *schema = virFileFindResource("domain.rng", + "docs/schemas", + PKGDATADIR "/schemas"); + if (!schema) + return NULL; + if (virXMLValidateAgainstSchema(schema, xml) < 0) + return NULL; + } + if (VIR_ALLOC(def) < 0) return NULL; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 1153497..2d5daf8 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2449,6 +2449,7 @@ typedef enum { VIR_DOMAIN_DEF_PARSE_CLOCK_ADJUST = 1 << 6, /* parse only source half of <disk> */ VIR_DOMAIN_DEF_PARSE_DISK_SOURCE = 1 << 7, + VIR_DOMAIN_DEF_PARSE_VALIDATE = 1 << 8, } virDomainDefParseFlags; typedef enum { diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index be2c16c..bfa1d79 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -3045,8 +3045,12 @@ esxDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) char *taskInfoErrorMessage = NULL; virDomainPtr domain = NULL; const char *src; + unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; - virCheckFlags(0, NULL); + virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL); + + if (flags & VIR_DOMAIN_DEFINE_VALIDATE) + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; memset(&data, 0, sizeof(data)); @@ -3056,7 +3060,7 @@ esxDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) /* Parse domain XML */ def = virDomainDefParseString(xml, priv->caps, priv->xmlopt, 1 << VIR_DOMAIN_VIRT_VMWARE, - VIR_DOMAIN_DEF_PARSE_INACTIVE); + parse_flags); if (!def) return NULL; diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 0b4c097..492e90a 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -6479,7 +6479,7 @@ virDomainDefineXML(virConnectPtr conn, const char *xml) * virDomainDefineXMLFlags: * @conn: pointer to the hypervisor connection * @xml: the XML description for the domain, preferably in UTF-8 - * @flags: currently unused, pass 0 + * @flags: bitwise OR of the virDomainDefineFlags constants * * Defines a domain, but does not start it. * This definition is persistent, until explicitly undefined with diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 82e476a..67ecc72 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -638,12 +638,17 @@ libxlDomainCreateXML(virConnectPtr conn, const char *xml, virDomainObjPtr vm = NULL; virDomainPtr dom = NULL; libxlDriverConfigPtr cfg = libxlDriverConfigGet(driver); + unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; - virCheckFlags(VIR_DOMAIN_START_PAUSED, NULL); + virCheckFlags(VIR_DOMAIN_START_PAUSED | + VIR_DOMAIN_START_VALIDATE, NULL); + + if (flags & VIR_DOMAIN_START_VALIDATE) + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; if (!(def = virDomainDefParseString(xml, cfg->caps, driver->xmlopt, 1 << VIR_DOMAIN_VIRT_XEN, - VIR_DOMAIN_DEF_PARSE_INACTIVE))) + parse_flags))) goto cleanup; if (virDomainCreateXMLEnsureACL(conn, def) < 0) @@ -2377,12 +2382,16 @@ libxlDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flag virDomainPtr dom = NULL; virObjectEventPtr event = NULL; virDomainDefPtr oldDef = NULL; + unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; - virCheckFlags(0, NULL); + virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL); + + if (flags & VIR_DOMAIN_DEFINE_VALIDATE) + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; if (!(def = virDomainDefParseString(xml, cfg->caps, driver->xmlopt, 1 << VIR_DOMAIN_VIRT_XEN, - VIR_DOMAIN_DEF_PARSE_INACTIVE))) + parse_flags))) goto cleanup; if (virDomainDefineXMLFlagsEnsureACL(conn, def) < 0) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index f503fcf..e492128 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -455,15 +455,19 @@ lxcDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) virDomainDefPtr oldDef = NULL; virLXCDriverConfigPtr cfg = virLXCDriverGetConfig(driver); virCapsPtr caps = NULL; + unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; - virCheckFlags(0, NULL); + virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL); + + if (flags & VIR_DOMAIN_DEFINE_VALIDATE) + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; if (!(caps = virLXCDriverGetCapabilities(driver, false))) goto cleanup; if (!(def = virDomainDefParseString(xml, caps, driver->xmlopt, 1 << VIR_DOMAIN_VIRT_LXC, - VIR_DOMAIN_DEF_PARSE_INACTIVE))) + parse_flags))) goto cleanup; if (virDomainDefineXMLFlagsEnsureACL(conn, def) < 0) @@ -1196,8 +1200,14 @@ lxcDomainCreateXMLWithFiles(virConnectPtr conn, virObjectEventPtr event = NULL; virLXCDriverConfigPtr cfg = virLXCDriverGetConfig(driver); virCapsPtr caps = NULL; + unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; + + virCheckFlags(VIR_DOMAIN_START_AUTODESTROY | + VIR_DOMAIN_START_VALIDATE, NULL); + - virCheckFlags(VIR_DOMAIN_START_AUTODESTROY, NULL); + if (flags & VIR_DOMAIN_START_VALIDATE) + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; virNWFilterReadLockFilterUpdates(); @@ -1206,7 +1216,7 @@ lxcDomainCreateXMLWithFiles(virConnectPtr conn, if (!(def = virDomainDefParseString(xml, caps, driver->xmlopt, 1 << VIR_DOMAIN_VIRT_LXC, - VIR_DOMAIN_DEF_PARSE_INACTIVE))) + parse_flags))) goto cleanup; if (virDomainCreateXMLWithFilesEnsureACL(conn, def) < 0) diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 53bfbf4..bdab1c3 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -984,13 +984,17 @@ openvzDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int fla virDomainDefPtr vmdef = NULL; virDomainObjPtr vm = NULL; virDomainPtr dom = NULL; + unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; - virCheckFlags(0, NULL); + virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL); + + if (flags & VIR_DOMAIN_DEFINE_VALIDATE) + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; openvzDriverLock(driver); if ((vmdef = virDomainDefParseString(xml, driver->caps, driver->xmlopt, 1 << VIR_DOMAIN_VIRT_OPENVZ, - VIR_DOMAIN_DEF_PARSE_INACTIVE)) == NULL) + parse_flags)) == NULL) goto cleanup; vm = virDomainObjListFindByName(driver->domains, vmdef->name); @@ -1077,13 +1081,17 @@ openvzDomainCreateXML(virConnectPtr conn, const char *xml, virDomainObjPtr vm = NULL; virDomainPtr dom = NULL; const char *progstart[] = {VZCTL, "--quiet", "start", PROGRAM_SENTINEL, NULL}; + unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; - virCheckFlags(0, NULL); + virCheckFlags(VIR_DOMAIN_START_VALIDATE, NULL); + + if (flags & VIR_DOMAIN_START_VALIDATE) + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; openvzDriverLock(driver); if ((vmdef = virDomainDefParseString(xml, driver->caps, driver->xmlopt, 1 << VIR_DOMAIN_VIRT_OPENVZ, - VIR_DOMAIN_DEF_PARSE_INACTIVE)) == NULL) + parse_flags)) == NULL) goto cleanup; vm = virDomainObjListFindByName(driver->domains, vmdef->name); diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c index fab7f24..5531d6d 100644 --- a/src/parallels/parallels_driver.c +++ b/src/parallels/parallels_driver.c @@ -664,13 +664,17 @@ parallelsDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int virDomainPtr retdom = NULL; virDomainDefPtr def; virDomainObjPtr olddom = NULL; + unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; - virCheckFlags(0, NULL); + virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL); + + if (flags & VIR_DOMAIN_DEFINE_VALIDATE) + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; parallelsDriverLock(privconn); if ((def = virDomainDefParseString(xml, privconn->caps, privconn->xmlopt, 1 << VIR_DOMAIN_VIRT_PARALLELS, - VIR_DOMAIN_DEF_PARSE_INACTIVE)) == NULL) + parse_flags)) == NULL) goto cleanup; olddom = virDomainObjListFindByUUID(privconn->domains, def->uuid); diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c index 7d2c849..17859b0 100644 --- a/src/phyp/phyp_driver.c +++ b/src/phyp/phyp_driver.c @@ -3553,13 +3553,17 @@ phypDomainCreateXML(virConnectPtr conn, lparPtr *lpars = uuid_table->lpars; size_t i = 0; char *managed_system = phyp_driver->managed_system; + unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; - virCheckFlags(0, NULL); + virCheckFlags(VIR_DOMAIN_START_VALIDATE, NULL); + + if (flags & VIR_DOMAIN_START_VALIDATE) + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; if (!(def = virDomainDefParseString(xml, phyp_driver->caps, phyp_driver->xmlopt, 1 << VIR_DOMAIN_VIRT_PHYP, - 0))) + parse_flags))) goto err; /* checking if this name already exists on this system */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 89cb33d..cafd96b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1674,10 +1674,14 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr conn, unsigned int start_flags = VIR_QEMU_PROCESS_START_COLD; virQEMUCapsPtr qemuCaps = NULL; virCapsPtr caps = NULL; + unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; virCheckFlags(VIR_DOMAIN_START_PAUSED | - VIR_DOMAIN_START_AUTODESTROY, NULL); + VIR_DOMAIN_START_AUTODESTROY | + VIR_DOMAIN_START_VALIDATE, NULL); + if (flags & VIR_DOMAIN_START_VALIDATE) + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; if (flags & VIR_DOMAIN_START_PAUSED) start_flags |= VIR_QEMU_PROCESS_START_PAUSED; if (flags & VIR_DOMAIN_START_AUTODESTROY) @@ -1690,7 +1694,7 @@ static virDomainPtr qemuDomainCreateXML(virConnectPtr conn, if (!(def = virDomainDefParseString(xml, caps, driver->xmlopt, QEMU_EXPECTED_VIRT_TYPES, - VIR_DOMAIN_DEF_PARSE_INACTIVE))) + parse_flags))) goto cleanup; if (virDomainCreateXMLEnsureACL(conn, def) < 0) @@ -6674,8 +6678,12 @@ static virDomainPtr qemuDomainDefineXMLFlags(virConnectPtr conn, const char *xml virQEMUCapsPtr qemuCaps = NULL; virQEMUDriverConfigPtr cfg; virCapsPtr caps = NULL; + unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; - virCheckFlags(0, NULL); + virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL); + + if (flags & VIR_DOMAIN_DEFINE_VALIDATE) + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; cfg = virQEMUDriverGetConfig(driver); @@ -6684,7 +6692,7 @@ static virDomainPtr qemuDomainDefineXMLFlags(virConnectPtr conn, const char *xml if (!(def = virDomainDefParseString(xml, caps, driver->xmlopt, QEMU_EXPECTED_VIRT_TYPES, - VIR_DOMAIN_DEF_PARSE_INACTIVE))) + parse_flags))) goto cleanup; if (virDomainDefineXMLFlagsEnsureACL(conn, def) < 0) diff --git a/src/test/test_driver.c b/src/test/test_driver.c index e37c07e..777ef3a 100644 --- a/src/test/test_driver.c +++ b/src/test/test_driver.c @@ -1743,13 +1743,17 @@ testDomainCreateXML(virConnectPtr conn, const char *xml, virDomainDefPtr def; virDomainObjPtr dom = NULL; virObjectEventPtr event = NULL; + unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; - virCheckFlags(0, NULL); + virCheckFlags(VIR_DOMAIN_START_VALIDATE, NULL); + + if (flags & VIR_DOMAIN_START_VALIDATE) + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; testDriverLock(privconn); if ((def = virDomainDefParseString(xml, privconn->caps, privconn->xmlopt, 1 << VIR_DOMAIN_VIRT_TEST, - VIR_DOMAIN_DEF_PARSE_INACTIVE)) == NULL) + parse_flags)) == NULL) goto cleanup; if (testDomainGenerateIfnames(def) < 0) @@ -2937,13 +2941,17 @@ static virDomainPtr testDomainDefineXMLFlags(virConnectPtr conn, virDomainObjPtr dom = NULL; virObjectEventPtr event = NULL; virDomainDefPtr oldDef = NULL; + unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; - virCheckFlags(0, NULL); + virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL); + + if (flags & VIR_DOMAIN_DEFINE_VALIDATE) + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; testDriverLock(privconn); if ((def = virDomainDefParseString(xml, privconn->caps, privconn->xmlopt, 1 << VIR_DOMAIN_VIRT_TEST, - VIR_DOMAIN_DEF_PARSE_INACTIVE)) == NULL) + parse_flags)) == NULL) goto cleanup; if (testDomainGenerateIfnames(def) < 0) diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c index 501c5f9..d5bc8ab 100644 --- a/src/uml/uml_driver.c +++ b/src/uml/uml_driver.c @@ -1605,14 +1605,19 @@ static virDomainPtr umlDomainCreateXML(virConnectPtr conn, const char *xml, virDomainObjPtr vm = NULL; virDomainPtr dom = NULL; virObjectEventPtr event = NULL; + unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; - virCheckFlags(VIR_DOMAIN_START_AUTODESTROY, NULL); + virCheckFlags(VIR_DOMAIN_START_AUTODESTROY | + VIR_DOMAIN_START_VALIDATE, NULL); + + if (flags & VIR_DOMAIN_START_VALIDATE) + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; virNWFilterReadLockFilterUpdates(); umlDriverLock(driver); if (!(def = virDomainDefParseString(xml, driver->caps, driver->xmlopt, 1 << VIR_DOMAIN_VIRT_UML, - VIR_DOMAIN_DEF_PARSE_INACTIVE))) + parse_flags))) goto cleanup; if (virDomainCreateXMLEnsureACL(conn, def) < 0) @@ -2082,13 +2087,17 @@ umlDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags) virDomainDefPtr def; virDomainObjPtr vm = NULL; virDomainPtr dom = NULL; + unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; + + virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL); - virCheckFlags(0, NULL); + if (flags & VIR_DOMAIN_DEFINE_VALIDATE) + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; umlDriverLock(driver); if (!(def = virDomainDefParseString(xml, driver->caps, driver->xmlopt, 1 << VIR_DOMAIN_VIRT_UML, - VIR_DOMAIN_DEF_PARSE_INACTIVE))) + parse_flags))) goto cleanup; if (virDomainDefineXMLFlagsEnsureACL(conn, def) < 0) diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 45c0376..e020f83 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -1851,8 +1851,12 @@ vboxDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags nsresult rc; char uuidstr[VIR_UUID_STRING_BUFLEN]; virDomainPtr ret = NULL; + unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; - virCheckFlags(0, NULL); + virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL); + + if (flags & VIR_DOMAIN_DEFINE_VALIDATE) + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; if (!data->vboxObj) return ret; @@ -1860,7 +1864,7 @@ vboxDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int flags VBOX_IID_INITIALIZE(&mchiid); if (!(def = virDomainDefParseString(xml, data->caps, data->xmlopt, 1 << VIR_DOMAIN_VIRT_VBOX, - VIR_DOMAIN_DEF_PARSE_INACTIVE))) { + parse_flags))) { goto cleanup; } diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c index c5378d1..d045993 100644 --- a/src/vmware/vmware_driver.c +++ b/src/vmware/vmware_driver.c @@ -370,15 +370,19 @@ vmwareDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int fla char *vmxPath = NULL; vmwareDomainPtr pDomain = NULL; virVMXContext ctx; + unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; - virCheckFlags(0, NULL); + virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL); + + if (flags & VIR_DOMAIN_DEFINE_VALIDATE) + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; ctx.formatFileName = vmwareCopyVMXFileName; vmwareDriverLock(driver); if ((vmdef = virDomainDefParseString(xml, driver->caps, driver->xmlopt, 1 << VIR_DOMAIN_VIRT_VMWARE, - VIR_DOMAIN_DEF_PARSE_INACTIVE)) == NULL) + parse_flags)) == NULL) goto cleanup; /* generate vmx file */ @@ -657,8 +661,12 @@ vmwareDomainCreateXML(virConnectPtr conn, const char *xml, char *vmxPath = NULL; vmwareDomainPtr pDomain = NULL; virVMXContext ctx; + unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; - virCheckFlags(0, NULL); + virCheckFlags(VIR_DOMAIN_START_VALIDATE, NULL); + + if (flags & VIR_DOMAIN_START_VALIDATE) + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; ctx.formatFileName = vmwareCopyVMXFileName; @@ -666,7 +674,7 @@ vmwareDomainCreateXML(virConnectPtr conn, const char *xml, if ((vmdef = virDomainDefParseString(xml, driver->caps, driver->xmlopt, 1 << VIR_DOMAIN_VIRT_VMWARE, - VIR_DOMAIN_DEF_PARSE_INACTIVE)) == NULL) + parse_flags)) == NULL) goto cleanup; /* generate vmx file */ diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index f3952e6..65dc24f 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -770,12 +770,16 @@ xenUnifiedDomainCreateXML(virConnectPtr conn, xenUnifiedPrivatePtr priv = conn->privateData; virDomainDefPtr def = NULL; virDomainPtr ret = NULL; + unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; - virCheckFlags(0, NULL); + virCheckFlags(VIR_DOMAIN_START_VALIDATE, NULL); + + if (flags & VIR_DOMAIN_START_VALIDATE) + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; if (!(def = virDomainDefParseString(xml, priv->caps, priv->xmlopt, 1 << VIR_DOMAIN_VIRT_XEN, - VIR_DOMAIN_DEF_PARSE_INACTIVE))) + parse_flags))) goto cleanup; if (virDomainCreateXMLEnsureACL(conn, def) < 0) @@ -1888,12 +1892,16 @@ xenUnifiedDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int xenUnifiedPrivatePtr priv = conn->privateData; virDomainDefPtr def = NULL; virDomainPtr ret = NULL; + unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; - virCheckFlags(0, NULL); + virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL); + + if (flags & VIR_DOMAIN_DEFINE_VALIDATE) + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; if (!(def = virDomainDefParseString(xml, priv->caps, priv->xmlopt, 1 << VIR_DOMAIN_VIRT_XEN, - VIR_DOMAIN_DEF_PARSE_INACTIVE))) + parse_flags))) goto cleanup; if (virDomainDefineXMLFlagsEnsureACL(conn, def) < 0) diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c index d078d55..c5118ec 100644 --- a/src/xenapi/xenapi_driver.c +++ b/src/xenapi/xenapi_driver.c @@ -546,15 +546,20 @@ xenapiDomainCreateXML(virConnectPtr conn, xen_vm_record *record = NULL; xen_vm vm = NULL; virDomainPtr domP = NULL; + unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; + if (!priv->caps) return NULL; - virCheckFlags(0, NULL); + virCheckFlags(VIR_DOMAIN_START_VALIDATE, NULL); + + if (flags & VIR_DOMAIN_START_VALIDATE) + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; virDomainDefPtr defPtr = virDomainDefParseString(xmlDesc, priv->caps, priv->xmlopt, 1 << VIR_DOMAIN_VIRT_XEN, - VIR_DOMAIN_DEF_PARSE_INACTIVE); + parse_flags); createVMRecordFromXml(conn, defPtr, &record, &vm); virDomainDefFree(defPtr); if (record) { @@ -1720,15 +1725,19 @@ xenapiDomainDefineXMLFlags(virConnectPtr conn, const char *xml, unsigned int fla xen_vm_record *record = NULL; xen_vm vm = NULL; virDomainPtr domP = NULL; + unsigned int parse_flags = VIR_DOMAIN_DEF_PARSE_INACTIVE; + + virCheckFlags(VIR_DOMAIN_DEFINE_VALIDATE, NULL); - virCheckFlags(0, NULL); + if (flags & VIR_DOMAIN_DEFINE_VALIDATE) + parse_flags |= VIR_DOMAIN_DEF_PARSE_VALIDATE; if (!priv->caps) return NULL; virDomainDefPtr defPtr = virDomainDefParseString(xml, priv->caps, priv->xmlopt, 1 << VIR_DOMAIN_VIRT_XEN, - VIR_DOMAIN_DEF_PARSE_INACTIVE); + parse_flags); if (!defPtr) return NULL; -- 2.1.0

The 'virsh edit' command gets XML validation enabled by default, with a --skip-validate option to disable it. The 'virsh define' and 'virsh create' commands get a --validate option to enable it, to avoid regressions for existing scripts. The quality of error reporting from libxml2 varies depending on the type of XML error made. Sometimes it is quite clear and useful, other times it is obscure & inaccurate. At least the user will see an error now, rather than having their XML modification silently disappear. --- tools/virsh-domain.c | 87 ++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 78 insertions(+), 9 deletions(-) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 6733cfa..fe8f4ba 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -133,6 +133,56 @@ vshCommandOptDomainBy(vshControl *ctl, const vshCmd *cmd, return vshLookupDomainInternal(ctl, cmd->def->name, n, flags); } +static virDomainPtr +vshDomainDefine(virConnectPtr conn, const char *xml, unsigned int flags) +{ + virDomainPtr dom; + if (flags) { + dom = virDomainDefineXMLFlags(conn, xml, flags); + /* If validate is the only flag, just drop it and + * try again. + */ + if (!dom) { + virErrorPtr err = virGetLastError(); + if (err && + (err->code == VIR_ERR_NO_SUPPORT) && + (flags == VIR_DOMAIN_DEFINE_VALIDATE)) + dom = virDomainDefineXML(conn, xml); + } + } else { + dom = virDomainDefineXML(conn, xml); + } + return dom; +} + + +static virDomainPtr +vshDomainCreateXML(virConnectPtr conn, const char *xml, + size_t nfds, int *fds, unsigned int flags) +{ + virDomainPtr dom; + if (nfds) + dom = virDomainCreateXMLWithFiles(conn, xml, + nfds, fds, flags); + else + dom = virDomainCreateXML(conn, xml, flags); + /* If validate is set, just drop it and try again */ + if (!dom) { + virErrorPtr err = virGetLastError(); + if (err && + (err->code == VIR_ERR_INVALID_ARG) && + (flags & VIR_DOMAIN_START_VALIDATE)) { + flags &= ~VIR_DOMAIN_START_VALIDATE; + if (nfds) + dom = virDomainCreateXMLWithFiles(conn, xml, + nfds, fds, flags); + else + dom = virDomainCreateXML(conn, xml, flags); + } + } + return dom; +} + VIR_ENUM_DECL(vshDomainVcpuState) VIR_ENUM_IMPL(vshDomainVcpuState, VIR_VCPU_LAST, @@ -7154,6 +7204,10 @@ static const vshCmdOptDef opts_create[] = { .type = VSH_OT_STRING, .help = N_("pass file descriptors N,M,... to the guest") }, + {.name = "validate", + .type = VSH_OT_BOOL, + .help = N_("validate the XML against the schema") + }, {.name = NULL} }; @@ -7167,7 +7221,7 @@ cmdCreate(vshControl *ctl, const vshCmd *cmd) #ifndef WIN32 bool console = vshCommandOptBool(cmd, "console"); #endif - unsigned int flags = VIR_DOMAIN_NONE; + unsigned int flags = 0; size_t nfds = 0; int *fds = NULL; @@ -7184,11 +7238,10 @@ cmdCreate(vshControl *ctl, const vshCmd *cmd) flags |= VIR_DOMAIN_START_PAUSED; if (vshCommandOptBool(cmd, "autodestroy")) flags |= VIR_DOMAIN_START_AUTODESTROY; + if (vshCommandOptBool(cmd, "validate")) + flags |= VIR_DOMAIN_START_VALIDATE; - if (nfds) - dom = virDomainCreateXMLWithFiles(ctl->conn, buffer, nfds, fds, flags); - else - dom = virDomainCreateXML(ctl->conn, buffer, flags); + dom = vshDomainCreateXML(ctl->conn, buffer, nfds, fds, flags); if (!dom) { vshError(ctl, _("Failed to create domain from %s"), from); @@ -7229,6 +7282,10 @@ static const vshCmdOptDef opts_define[] = { .flags = VSH_OFLAG_REQ, .help = N_("file containing an XML domain description") }, + {.name = "validate", + .type = VSH_OT_BOOL, + .help = N_("validate the XML against the schema") + }, {.name = NULL} }; @@ -7239,14 +7296,18 @@ cmdDefine(vshControl *ctl, const vshCmd *cmd) const char *from = NULL; bool ret = true; char *buffer; + unsigned int flags = 0; if (vshCommandOptStringReq(ctl, cmd, "file", &from) < 0) return false; + if (vshCommandOptBool(cmd, "validate")) + flags |= VIR_DOMAIN_DEFINE_VALIDATE; + if (virFileReadAll(from, VSH_MAX_XML_FILE, &buffer) < 0) return false; - dom = virDomainDefineXML(ctl->conn, buffer); + dom = vshDomainDefine(ctl->conn, buffer, flags); VIR_FREE(buffer); if (dom != NULL) { @@ -11105,6 +11166,10 @@ static const vshCmdOptDef opts_edit[] = { .flags = VSH_OFLAG_REQ, .help = N_("domain name, id or uuid") }, + {.name = "skip-validate", + .type = VSH_OT_BOOL, + .help = N_("skip validation of the XML against the schema") + }, {.name = NULL} }; @@ -11114,13 +11179,17 @@ cmdEdit(vshControl *ctl, const vshCmd *cmd) bool ret = false; virDomainPtr dom = NULL; virDomainPtr dom_edited = NULL; - unsigned int flags = VIR_DOMAIN_XML_SECURE | VIR_DOMAIN_XML_INACTIVE; + unsigned int query_flags = VIR_DOMAIN_XML_SECURE | VIR_DOMAIN_XML_INACTIVE; + unsigned int define_flags = VIR_DOMAIN_DEFINE_VALIDATE; dom = vshCommandOptDomain(ctl, cmd, NULL); if (dom == NULL) goto cleanup; -#define EDIT_GET_XML virDomainGetXMLDesc(dom, flags) + if (vshCommandOptBool(cmd, "skip-validate")) + define_flags &= ~VIR_DOMAIN_DEFINE_VALIDATE; + +#define EDIT_GET_XML virDomainGetXMLDesc(dom, query_flags) #define EDIT_NOT_CHANGED \ do { \ vshPrint(ctl, _("Domain %s XML configuration not changed.\n"), \ @@ -11129,7 +11198,7 @@ cmdEdit(vshControl *ctl, const vshCmd *cmd) goto edit_cleanup; \ } while (0) #define EDIT_DEFINE \ - (dom_edited = virDomainDefineXML(ctl->conn, doc_edited)) + (dom_edited = vshDomainDefine(ctl->conn, doc_edited, define_flags)) #include "virsh-edit.c" vshPrint(ctl, _("Domain %s XML configuration edited.\n"), -- 2.1.0

On Thu, Jan 15, 2015 at 11:14:37AM +0100, Jiri Denemark wrote: True, I forgot about that. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|